Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

wtmsig block production #7404

Merged
merged 49 commits into from
Jul 29, 2019
Merged

wtmsig block production #7404

merged 49 commits into from
Jul 29, 2019

Conversation

b1bart
Copy link
Contributor

@b1bart b1bart commented May 24, 2019

Change Description

This change implements #7403

This now encompasses (and fixes conflicts from) #7421

Remaining work:

  • unit tests for the protocol feature activation
  • producer plugin work to allow BPs to use the new feature

Consensus Changes

  • Consensus Changes

see #7403

API Changes

  • API Changes

new deployments of the API will emit producer_authority 's for all block signing data regardless of feature activation (it will not backport to the legacy structure)

specifically the get_producers API will no longer guarantee the result has a field producer_key. Instead it will return producer_authority. In instances where the producer_authority appears to be a canonical "upsert" of the legacy single key, this API will still emit the producer_key field as expected. In those instances, the producer_authority field will contain a 1-of-1 wtmsig where both the threshold and weight are exactly 1

Documentation Additions

  • Documentation Additions

need to document set_proposed_producers_ex , see TBD PR in eosio.cdt

Other notes

this PR is likely to conflict with #7012 as that will also bump the snapshot version. This PR should give way to a PR regarding that feature Resolved as #7012 was merged to develop and this PR is caught up.

this PR includes #7401 as it introduces an otherwise breaking change to the in-memory format

b1bart added 24 commits May 10, 2019 16:50
…ned authority structure, still fixing bugs and compile errors
…c to solve ABI versioning and contract upgrade issues. TODO: implement intrinsic changes, implement mult-sig signed blocks, fix tests etc
…tester setup to use the interim bios until features are activated
…es for wtmsig block production, augment tester to accomodate
…the activation of the protocol features. Added v2 snapshots based on a deterministic blockchain and added test to make sure that recreateing the blockchain and loading the compatible snapshot produce the same integrity hash
@b1bart b1bart added the CONSENSUS Introduces a change that may modify consensus protocol rules on an existing blockchain. label May 24, 2019
Copy link
Contributor

@arhag arhag left a comment

Choose a reason for hiding this comment

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

Part 1 of review (does not include changes to plugins or tests).

libraries/chain/include/eosio/chain/exceptions.hpp Outdated Show resolved Hide resolved
libraries/chain/protocol_feature_manager.cpp Outdated Show resolved Hide resolved
libraries/chain/protocol_feature_manager.cpp Outdated Show resolved Hide resolved
libraries/chain/protocol_feature_manager.cpp Outdated Show resolved Hide resolved
libraries/chain/protocol_feature_manager.cpp Outdated Show resolved Hide resolved
libraries/chain/block_state.cpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/chain_snapshot.hpp Outdated Show resolved Hide resolved
libraries/chain/block_state.cpp Show resolved Hide resolved
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/types.hpp Outdated Show resolved Hide resolved
libraries/chain/apply_context.cpp Show resolved Hide resolved
libraries/chain/block_state.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@arhag arhag left a comment

Choose a reason for hiding this comment

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

Part 2 of review: covers changes addressing comments from previous review as well as the changes made in this PR to the plugins. That includes changes to the state_history_plugin, but I would like @tbfleming to review that specific part of it as well.

Review of changes related to tests still remain for part 3 of the review.

libraries/chain/block_header_state.cpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/producer_schedule.hpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/producer_schedule.hpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/producer_schedule.hpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/producer_schedule.hpp Outdated Show resolved Hide resolved
plugins/chain_plugin/chain_plugin.cpp Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Outdated Show resolved Hide resolved
plugins/producer_plugin/producer_plugin.cpp Show resolved Hide resolved
Copy link
Contributor

@arhag arhag left a comment

Choose a reason for hiding this comment

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

Part 3 of review: covers tests.

unittests/protocol_feature_tests.cpp Outdated Show resolved Hide resolved
unittests/protocol_feature_tests.cpp Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
unittests/protocol_feature_tests.cpp Outdated Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
unittests/producer_schedule_tests.cpp Outdated Show resolved Hide resolved
refactored the wtmsig tests so that they are only testing that those authorities work.
added a utility to produce blocks (with a limit) until a set of BPs has been seen
@b1bart b1bart requested a review from arhag July 26, 2019 16:09
Copy link

@piqjen-1xofFi-sikxyh piqjen-1xofFi-sikxyh left a comment

Choose a reason for hiding this comment

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CONSENSUS Introduces a change that may modify consensus protocol rules on an existing blockchain.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants