IF: Block header extension -- replace finalizer_policy_extension and proposal_info_extension with instant_finality_extension #2024
Conversation
|
Note:start |
libraries/chain/controller.cpp
Outdated
| finalizer_policy_extension::extension_id(), | ||
| fc::raw::pack( finalizer_policy_extension{ std::move(fin_pol) } ) | ||
| instant_finality_extension::extension_id(), | ||
| fc::raw::pack( instant_finality_extension{ last_qc_block_num, is_last_qc_strong, std::move(fin_pol), new_proposer_policy } ) |
| _active_finalizer_policy = std::move(std::get<finalizer_policy_extension>(*ext)); | ||
| const auto& if_extension = std::get<instant_finality_extension>(*ext); | ||
| if (if_extension.new_finalizer_policy) { | ||
| _active_finalizer_policy = *if_extension.new_finalizer_policy; |
There was a problem hiding this comment.
Use non-const and use std::move here.
There was a problem hiding this comment.
Also add a TODO for changing when finalizer policy change design is complete as this is not necessarily when we will change active finalizer policy.
| static_assert( fc::raw::has_feature_reflector_init_on_unpacked_reflected_types, | ||
| "instant_finality_extension expects FC to support reflector_init" ); | ||
| static_assert( extension_id() == 2, "instant_finality_extension extension id must be 2" ); | ||
| static_assert( enforce_unique(), "instant_finality_extension must enforce uniqueness"); |
There was a problem hiding this comment.
Seems odd to me to have a reflector_init that only has static_asserts. These static_assert could be anywhere. Also they are only asserting what is explicitly defined.
There was a problem hiding this comment.
This was intended as a placeholder for any additional validations which would be discovered during design. exension_id definitions are scattered around the code base; wanted to make sure it is not changed by accident. enforce_unique is not necessary.
|
|
||
| instant_finality_extension() = default; | ||
| instant_finality_extension( uint32_t last_qc_block_num, bool is_last_qc_strong, std::optional<finalizer_policy> new_finalizer_policy, std::optional<proposer_policy> new_proposer_policy) | ||
| :last_qc_block_num( last_qc_block_num ), is_last_qc_strong( is_last_qc_strong ), new_finalizer_policy( new_finalizer_policy ), new_proposer_policy( new_proposer_policy ) |
| std::optional<finalizer_policy> new_finalizer_policy {std::nullopt}; | ||
| std::optional<proposer_policy> new_proposer_policy {std::nullopt}; |
There was a problem hiding this comment.
No point adding the {std::nullopt} initializer, it is already the default for std::optional.
unittests/api_tests.cpp
Outdated
| @@ -3876,13 +3876,15 @@ BOOST_AUTO_TEST_CASE(set_finalizer_test) { try { | |||
|
|
|||
| // activate hotstuff | |||
| t.set_finalizers(finalizers); | |||
| auto block = t.produce_block(); // this block contains the header extension of the finalizer set | |||
| auto block = t.produce_block(); // this block contains the header extension of the instant finality | |||
There was a problem hiding this comment.
| auto block = t.produce_block(); // this block contains the header extension of the instant finality | |
| auto block = t.produce_block(); // this block contains the header extension for instant finality |
…y in instant_finality_extension_with_empty_values_test
unittests/block_header_tests.cpp
Outdated
| constexpr uint32_t last_qc_block_num {0}; | ||
| constexpr bool is_last_qc_strong {false}; |
There was a problem hiding this comment.
No big deal, but now indentation looks excessive :-).
finalizer_policy_extensionandproposal_info_extensionwithinstant_finality_extensioninstant_finality_extensionResolved #1911