Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protocol-spec votes - Closes #4182 #4215

Merged
merged 12 commits into from Sep 9, 2019
Merged

Conversation

pablitovicente
Copy link
Contributor

What was the problem?

Protocol-specs generators for votes did not exist

How did I solve it?

  • by adding helper methods for generating vote transactions
  • by adding the generator for votes (see issue for list of expected scenarios)

How to manually test it?

Review checklist

  • The PR resolves Protocol-spec votes #4182
  • All new code is covered with unit tests
  • Relevant integration / functional tests are added
  • Commit messages follow the commit guidelines
  • Documentation has been added/updated

@pablitovicente pablitovicente self-assigned this Sep 5, 2019
@pablitovicente pablitovicente changed the title Protocol-spec multi-signature registration - Closes #4182 Protocol-spec votes - Closes #4182 Sep 5, 2019
protocol-specs/generators/block_processing_votes/index.js Outdated Show resolved Hide resolved
} = require('@liskhq/lisk-cryptography');

const BaseGenerator = require('../base_generator');
const defaultConfig = require('../../config/devnet');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we only support devnet config for now? Or make this configurable in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to discuss how much we want this tool to grow and allot time based on that as this issue is only for generating the JSON I'm trying to avoid big involvements in the library itself as that is just a tool and not the objective but I do agree that we might want to check if we want to develop the tools further.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, thanks! Let's just keep track of these possible improvements.

protocol-specs/generators/block_processing_votes/index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@michielmulders michielmulders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@shuse2 shuse2 merged commit caf82af into development Sep 9, 2019
@shuse2 shuse2 deleted the 4182-procol-specs-votes branch September 9, 2019 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Protocol-spec votes
4 participants