-
Notifications
You must be signed in to change notification settings - Fork 68
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
IF: Block header extension -- replace finalizer_policy_extension and proposal_info_extension with instant_finality_extension #2024
Changes from 7 commits
ce87ab3
fb84206
282ccd7
7eb3c80
78ca2b4
33fae7d
8432f06
1d0a41e
2634c47
cc9cbca
3a12919
c948e5f
3e9534a
ee5c187
4e7f413
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -167,7 +167,7 @@ namespace eosio::chain { | |
// called from main thread | ||
void chain_pacemaker::on_irreversible_block( const signed_block_ptr& block ) { | ||
if (!block->header_extensions.empty()) { | ||
std::optional<block_header_extension> ext = block->extract_header_extension(finalizer_policy_extension::extension_id()); | ||
std::optional<block_header_extension> ext = block->extract_header_extension(instant_finality_extension::extension_id()); | ||
if (ext) { | ||
std::scoped_lock g( _chain_state_mutex ); | ||
if (_active_finalizer_policy.generation == 0) { | ||
|
@@ -176,7 +176,10 @@ namespace eosio::chain { | |
// block header extension is set in finalize_block to value set by host function set_finalizers | ||
_chain->set_hs_irreversible_block_num(block->block_num()); // can be any value <= dpos lib | ||
} | ||
_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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use non-const and use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
} | ||
} | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
#include <eosio/chain/hotstuff/instant_finality_extension.hpp> | ||
#include <eosio/chain/exceptions.hpp> | ||
|
||
namespace eosio::chain { | ||
|
||
void instant_finality_extension::reflector_init() { | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems odd to me to have a reflector_init that only has There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was intended as a placeholder for any additional validations which would be discovered during design. |
||
} | ||
|
||
} // eosio::chain |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
#pragma once | ||
|
||
#include <eosio/chain/hotstuff/finalizer_policy.hpp> | ||
#include <eosio/chain/hotstuff/proposer_policy.hpp> | ||
|
||
namespace eosio::chain { | ||
|
||
struct instant_finality_extension : fc::reflect_init { | ||
static constexpr uint16_t extension_id() { return 2; } | ||
static constexpr bool enforce_unique() { return true; } | ||
|
||
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 ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{} | ||
|
||
void reflector_init(); | ||
|
||
uint32_t last_qc_block_num {0}; // The block height of the most recent ancestor block that has a QC justification | ||
bool is_last_qc_strong {false}; // Whether the QC for the block referenced by last_qc_block_height is strong or weak. | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No point adding the |
||
}; | ||
|
||
} /// eosio::chain | ||
|
||
FC_REFLECT( eosio::chain::instant_finality_extension, (last_qc_block_num)(is_last_qc_strong)(new_finalizer_policy)(new_proposer_policy) ) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#pragma once | ||
|
||
#include <eosio/chain/block_timestamp.hpp> | ||
#include <eosio/chain/producer_schedule.hpp> | ||
|
||
namespace eosio::chain { | ||
|
||
struct proposer_policy { | ||
constexpr static uint8_t current_schema_version = 1; | ||
uint8_t schema_version {current_schema_version}; | ||
// Useful for light clients, not necessary for nodeos | ||
block_timestamp_type active_time; // block when schedule will become active | ||
producer_authority_schedule proposer_schedule; | ||
}; | ||
|
||
} /// eosio::chain | ||
heifner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
FC_REFLECT( eosio::chain::proposer_policy, (schema_version)(active_time)(proposer_schedule) ) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
std::optional<block_header_extension> ext = block->extract_header_extension(finalizer_policy_extension::extension_id()); | ||||||
std::optional<block_header_extension> ext = block->extract_header_extension(instant_finality_extension::extension_id()); | ||||||
BOOST_TEST(!!ext); | ||||||
BOOST_TEST(std::get<finalizer_policy_extension>(*ext).finalizers.size() == finalizers.size()); | ||||||
BOOST_TEST(std::get<finalizer_policy_extension>(*ext).generation == 1); | ||||||
BOOST_TEST(std::get<finalizer_policy_extension>(*ext).threshold == finalizers.size() / 3 * 2 + 1); | ||||||
std::optional<finalizer_policy> fin_policy = std::get<instant_finality_extension>(*ext).new_finalizer_policy; | ||||||
BOOST_TEST(!!fin_policy); | ||||||
BOOST_TEST(fin_policy->finalizers.size() == finalizers.size()); | ||||||
BOOST_TEST(fin_policy->generation == 1); | ||||||
BOOST_TEST(fin_policy->threshold == finalizers.size() / 3 * 2 + 1); | ||||||
|
||||||
// old dpos still in affect until block is irreversible | ||||||
BOOST_TEST(block->confirmed == 0); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
#include <eosio/chain/block_header.hpp> | ||
|
||
#include <boost/test/unit_test.hpp> | ||
|
||
using namespace eosio::chain; | ||
|
||
BOOST_AUTO_TEST_SUITE(block_header_tests) | ||
|
||
// test for block header without extension | ||
BOOST_AUTO_TEST_CASE(block_header_without_extension_test) | ||
{ | ||
block_header header; | ||
std::optional<block_header_extension> ext = header.extract_header_extension(instant_finality_extension::extension_id()); | ||
BOOST_REQUIRE(!ext); | ||
} | ||
|
||
// test for empty instant_finality_extension | ||
BOOST_AUTO_TEST_CASE(instant_finality_extension_with_empty_values_test) | ||
{ | ||
block_header header; | ||
constexpr uint32_t last_qc_block_num {0}; | ||
constexpr bool is_last_qc_strong {false}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No big deal, but now indentation looks excessive :-). |
||
const std::optional<finalizer_policy> new_finalizer_policy {std::nullopt}; | ||
const std::optional<proposer_policy> new_proposer_policy {std::nullopt}; | ||
|
||
greg7mdp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
emplace_extension( | ||
header.header_extensions, | ||
instant_finality_extension::extension_id(), | ||
fc::raw::pack( instant_finality_extension{last_qc_block_num, is_last_qc_strong, new_finalizer_policy, new_proposer_policy} ) | ||
); | ||
|
||
std::optional<block_header_extension> ext = header.extract_header_extension(instant_finality_extension::extension_id()); | ||
BOOST_REQUIRE( !!ext ); | ||
|
||
const auto& if_extension = std::get<instant_finality_extension>(*ext); | ||
BOOST_REQUIRE_EQUAL( if_extension.last_qc_block_num, last_qc_block_num ); | ||
BOOST_REQUIRE_EQUAL( if_extension.is_last_qc_strong, is_last_qc_strong ); | ||
BOOST_REQUIRE( !if_extension.new_finalizer_policy ); | ||
BOOST_REQUIRE( !if_extension.new_proposer_policy ); | ||
} | ||
|
||
// test for instant_finality_extension uniqueness | ||
BOOST_AUTO_TEST_CASE(instant_finality_extension_uniqueness_test) | ||
{ | ||
block_header header; | ||
|
||
emplace_extension( | ||
header.header_extensions, | ||
instant_finality_extension::extension_id(), | ||
fc::raw::pack( instant_finality_extension{0, false, {std::nullopt}, {std::nullopt}} ) | ||
); | ||
|
||
std::vector<finalizer_authority> finalizers { {"test description", 50, fc::crypto::blslib::bls_public_key{"PUB_BLS_MPPeebAPxt/ibL2XPuZVGpADjGn+YEVPPoYmTZeBD6Ok2E19M8SnmDGSdZBf2qwSuJim+8H83EsTpEn3OiStWBiFeJYfVRLlEsZuSF0SYYwtVteY48n+KeE1IWzlSAkSyBqiGA==" }} }; | ||
finalizer_policy new_finalizer_policy; | ||
new_finalizer_policy.generation = 1; | ||
new_finalizer_policy.threshold = 100; | ||
new_finalizer_policy.finalizers = finalizers; | ||
|
||
proposer_policy new_proposer_policy {1, block_timestamp_type{200}, {} }; | ||
|
||
emplace_extension( | ||
header.header_extensions, | ||
instant_finality_extension::extension_id(), | ||
fc::raw::pack( instant_finality_extension{100, true, new_finalizer_policy, new_proposer_policy} ) | ||
); | ||
|
||
BOOST_CHECK_THROW(header.validate_and_extract_header_extensions(), invalid_block_header_extension); | ||
} | ||
|
||
// test for instant_finality_extension with values | ||
BOOST_AUTO_TEST_CASE(instant_finality_extension_with_values_test) | ||
{ | ||
block_header header; | ||
constexpr uint32_t last_qc_block_num {10}; | ||
constexpr bool is_last_qc_strong {true}; | ||
|
||
std::vector<finalizer_authority> finalizers { {"test description", 50, fc::crypto::blslib::bls_public_key{"PUB_BLS_MPPeebAPxt/ibL2XPuZVGpADjGn+YEVPPoYmTZeBD6Ok2E19M8SnmDGSdZBf2qwSuJim+8H83EsTpEn3OiStWBiFeJYfVRLlEsZuSF0SYYwtVteY48n+KeE1IWzlSAkSyBqiGA==" }} }; | ||
finalizer_policy new_finalizer_policy; | ||
new_finalizer_policy.generation = 1; | ||
new_finalizer_policy.threshold = 100; | ||
new_finalizer_policy.finalizers = finalizers; | ||
|
||
proposer_policy new_proposer_policy {1, block_timestamp_type{200}, {} }; | ||
|
||
emplace_extension( | ||
header.header_extensions, | ||
instant_finality_extension::extension_id(), | ||
fc::raw::pack( instant_finality_extension{last_qc_block_num, is_last_qc_strong, new_finalizer_policy, new_proposer_policy} ) | ||
); | ||
|
||
std::optional<block_header_extension> ext = header.extract_header_extension(instant_finality_extension::extension_id()); | ||
BOOST_REQUIRE( !!ext ); | ||
|
||
const auto& if_extension = std::get<instant_finality_extension>(*ext); | ||
|
||
BOOST_REQUIRE_EQUAL( if_extension.last_qc_block_num, last_qc_block_num ); | ||
BOOST_REQUIRE_EQUAL( if_extension.is_last_qc_strong, is_last_qc_strong ); | ||
|
||
BOOST_REQUIRE( !!if_extension.new_finalizer_policy ); | ||
BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy->generation, 1u); | ||
BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy->threshold, 100u); | ||
BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy->finalizers[0].description, "test description"); | ||
BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy->finalizers[0].weight, 50u); | ||
BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy->finalizers[0].public_key.to_string(), "PUB_BLS_MPPeebAPxt/ibL2XPuZVGpADjGn+YEVPPoYmTZeBD6Ok2E19M8SnmDGSdZBf2qwSuJim+8H83EsTpEn3OiStWBiFeJYfVRLlEsZuSF0SYYwtVteY48n+KeE1IWzlSAkSyBqiGA=="); | ||
greg7mdp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
BOOST_REQUIRE( !!if_extension.new_proposer_policy ); | ||
BOOST_REQUIRE_EQUAL(if_extension.new_proposer_policy->schema_version, 1u); | ||
fc::time_point t = (fc::time_point)(if_extension.new_proposer_policy->active_time); | ||
BOOST_REQUIRE_EQUAL(t.time_since_epoch().to_seconds(), 946684900ll); | ||
} | ||
|
||
BOOST_AUTO_TEST_SUITE_END() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::move(new_proposer_policy)