From caa0e8ae74ae826a82aae5e1972d9796cb252c61 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 21 Nov 2023 15:42:21 -0500 Subject: [PATCH 1/6] implement block_header_state_core and its state transition --- libraries/chain/block_header_state.cpp | 46 ++++++++ .../eosio/chain/block_header_state.hpp | 26 +++++ unittests/block_header_state_tests.cpp | 109 ++++++++++++++++++ 3 files changed, 181 insertions(+) create mode 100644 unittests/block_header_state_tests.cpp diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index c1b8992776..dd97d29b29 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -23,6 +23,52 @@ namespace eosio { namespace chain { } } + block_header_state_core::block_header_state_core( uint32_t last_final_blk_ht, + std::optional final_on_strong_qc_blk_ht, + std::optional last_qc_blk_ht ) : + last_final_block_height(last_final_blk_ht), + final_on_strong_qc_block_height(final_on_strong_qc_blk_ht), + last_qc_block_height(last_qc_blk_ht) {} + + block_header_state_core block_header_state_core::next( uint32_t last_qc_block_height, + bool is_last_qc_strong) { + // no state change if last_qc_block_height is the same + if( last_qc_block_height == this->last_qc_block_height ) { + return std::move(*this); + } + + EOS_ASSERT( last_qc_block_height > this->last_qc_block_height, block_validate_exception, "new last_qc_block_height must be greater than old last_qc_block_height" ); + + auto old_last_qc_block_height = this->last_qc_block_height; + auto old_final_on_strong_qc_block_height = this->final_on_strong_qc_block_height; + + block_header_state_core result(std::move(*this)); + + if( is_last_qc_strong ) { + // last QC is strong. We can progress forward. + + // block with old final_on_strong_qc_block_height becomes irreversible + if( old_final_on_strong_qc_block_height.has_value() ) { + result.last_final_block_height = *old_final_on_strong_qc_block_height; + } + + // next block which can become irreversible is the block with + // old last_qc_block_height + if( old_last_qc_block_height.has_value() ) { + result.final_on_strong_qc_block_height = *old_last_qc_block_height; + } + } else { + // new final_on_strong_qc_block_height should not be present + result.final_on_strong_qc_block_height.reset(); + + // new last_final_block_height should be the same as the old last_final_block_height + } + + // new last_qc_block_height is always the input last_qc_block_height. + result.last_qc_block_height = last_qc_block_height; + + return result; + } producer_authority block_header_state::get_scheduled_producer( block_timestamp_type t )const { auto index = t.slot % (active_schedule.producers.size() * config::producer_repetitions); index /= config::producer_repetitions; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 8fbdd6e57a..dec9b2a23c 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -120,6 +120,32 @@ struct pending_block_header_state : public detail::block_header_state_common { const vector& )>& validator )&&; }; +/** + * @struct block_header_state_core + * + * A data structure holding hotstuff core information + */ +struct block_header_state_core { + uint32_t last_final_block_height; // the block height of the last irreversible + // (final) block. + std::optional final_on_strong_qc_block_height; // the block height of + // the block that would + // become irreversible + // if the associated block + // header was to achieve a + // strong QC. + std::optional last_qc_block_height; // the block height of the block that + // is referenced as the last QC block + + block_header_state_core() = default; + + explicit block_header_state_core( uint32_t last_final_block_height, + std::optional final_on_strong_qc_block_height, + std::optional last_qc_block_height ); + + block_header_state_core next( uint32_t last_qc_block_height, + bool is_last_qc_strong); +}; /** * @struct block_header_state * diff --git a/unittests/block_header_state_tests.cpp b/unittests/block_header_state_tests.cpp new file mode 100644 index 0000000000..82ff0acb34 --- /dev/null +++ b/unittests/block_header_state_tests.cpp @@ -0,0 +1,109 @@ +#include + +#include + +using namespace eosio::chain; + +BOOST_AUTO_TEST_SUITE(block_header_state_tests) + +// test for block_header_state_core constructor +BOOST_AUTO_TEST_CASE(block_header_state_core_constructor_test) +{ + // verifies members are constructed correctly + block_header_state_core bhs_core1(1, 2, 3); + BOOST_REQUIRE_EQUAL(bhs_core1.last_final_block_height, 1u); + BOOST_REQUIRE_EQUAL(*bhs_core1.final_on_strong_qc_block_height, 2u); + BOOST_REQUIRE_EQUAL(*bhs_core1.last_qc_block_height, 3u); + + // verifies optional arguments work as expected + block_header_state_core bhs_core2(10, std::nullopt, {}); + BOOST_REQUIRE_EQUAL(bhs_core2.last_final_block_height, 10u); + BOOST_REQUIRE(!bhs_core2.final_on_strong_qc_block_height.has_value()); + BOOST_REQUIRE(!bhs_core2.last_qc_block_height.has_value()); +} + +// comprehensive state transition test +BOOST_AUTO_TEST_CASE(block_header_state_core_state_transition_test) +{ + constexpr auto old_last_final_block_height = 1u; + constexpr auto old_final_on_strong_qc_block_height = 2u; + constexpr auto old_last_qc_block_height = 3u; + block_header_state_core old_bhs_core(old_last_final_block_height, old_final_on_strong_qc_block_height, old_last_qc_block_height); + + // verifies the state is kept the same when old last_final_block_height + // and new last_final_block_height are the same + for (bool is_last_qc_strong: { true, false }) { + auto new_bhs_core = old_bhs_core.next(old_last_qc_block_height, is_last_qc_strong); + BOOST_REQUIRE_EQUAL(new_bhs_core.last_final_block_height, old_bhs_core.last_final_block_height); + BOOST_REQUIRE_EQUAL(*new_bhs_core.final_on_strong_qc_block_height, *old_bhs_core.final_on_strong_qc_block_height); + BOOST_REQUIRE_EQUAL(*new_bhs_core.last_qc_block_height, *old_bhs_core.last_qc_block_height); + } + + // verifies state cannot be transitioned to a smaller last_qc_block_height + for (bool is_last_qc_strong: { true, false }) { + BOOST_REQUIRE_THROW(old_bhs_core.next(old_last_qc_block_height - 1, is_last_qc_strong), block_validate_exception); + } + + // verifies state transition works when is_last_qc_strong is true + constexpr auto input_last_qc_block_height = 4u; + auto new_bhs_core = old_bhs_core.next(input_last_qc_block_height, true); + // old final_on_strong_qc block became final + BOOST_REQUIRE_EQUAL(new_bhs_core.last_final_block_height, old_final_on_strong_qc_block_height); + // old last_qc block became final_on_strong_qc block + BOOST_REQUIRE_EQUAL(*new_bhs_core.final_on_strong_qc_block_height, old_last_qc_block_height); + // new last_qc_block_height is the same as input + BOOST_REQUIRE_EQUAL(*new_bhs_core.last_qc_block_height, input_last_qc_block_height); + + // verifies state transition works when is_last_qc_strong is false + new_bhs_core = old_bhs_core.next(input_last_qc_block_height, false); + // last_final_block_height should not change + BOOST_REQUIRE_EQUAL(new_bhs_core.last_final_block_height, old_last_final_block_height); + // new final_on_strong_qc_block_height should not be present + BOOST_REQUIRE(!new_bhs_core.final_on_strong_qc_block_height.has_value()); + // new last_qc_block_height is the same as input + BOOST_REQUIRE_EQUAL(*new_bhs_core.last_qc_block_height, input_last_qc_block_height); +} + +// A test to demonstrate 3-chain state transitions from the first +// block after hotstuff activation +BOOST_AUTO_TEST_CASE(block_header_state_core_3_chain_transition_test) +{ + // block2: initial setup + constexpr auto block2_last_final_block_height = 1u; + block_header_state_core block2_bhs_core(block2_last_final_block_height, {}, {}); + + // block2 --> block3 + constexpr auto block3_input_last_qc_block_height = 2u; + auto block3_bhs_core = block2_bhs_core.next(block3_input_last_qc_block_height, true); + // last_final_block_height should be the same as old one + BOOST_REQUIRE_EQUAL(block3_bhs_core.last_final_block_height, block2_last_final_block_height); + // final_on_strong_qc_block_height should be same as old one + BOOST_REQUIRE(!block3_bhs_core.final_on_strong_qc_block_height.has_value()); + // new last_qc_block_height is the same as input + BOOST_REQUIRE_EQUAL(*block3_bhs_core.last_qc_block_height, block3_input_last_qc_block_height); + auto block3_last_qc_block_height = *block3_bhs_core.last_qc_block_height; + + // block3 --> block4 + constexpr auto block4_input_last_qc_block_height = 3u; + auto block4_bhs_core = block3_bhs_core.next(block4_input_last_qc_block_height, true); + // last_final_block_height should not change + BOOST_REQUIRE_EQUAL(block4_bhs_core.last_final_block_height, block2_last_final_block_height); + // final_on_strong_qc_block_height should be block3's last_qc_block_height + BOOST_REQUIRE_EQUAL(*block4_bhs_core.final_on_strong_qc_block_height, block3_last_qc_block_height); + // new last_qc_block_height is the same as input + BOOST_REQUIRE_EQUAL(*block4_bhs_core.last_qc_block_height, block4_input_last_qc_block_height); + auto block4_final_on_strong_qc_block_height = *block4_bhs_core.final_on_strong_qc_block_height; + auto block4_last_qc_block_height = *block4_bhs_core.last_qc_block_height; + + // block4 --> block5 + constexpr auto block5_input_last_qc_block_height = 4u; + auto block5_bhs_core = block4_bhs_core.next(block5_input_last_qc_block_height, true); + // last_final_block_height should have a new value + BOOST_REQUIRE_EQUAL(block5_bhs_core.last_final_block_height, block4_final_on_strong_qc_block_height); + // final_on_strong_qc_block_height should be block4's last_qc_block_height + BOOST_REQUIRE_EQUAL(*block5_bhs_core.final_on_strong_qc_block_height, block4_last_qc_block_height); + // new last_qc_block_height is the same as input + BOOST_REQUIRE_EQUAL(*block5_bhs_core.last_qc_block_height, block5_input_last_qc_block_height); +} + +BOOST_AUTO_TEST_SUITE_END() From b99cf30f0c3272e98f8d1113a1084927bc6a53ad Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 21 Nov 2023 17:40:28 -0500 Subject: [PATCH 2/6] initialize member last_final_block_height to 0 in class definition --- libraries/chain/include/eosio/chain/block_header_state.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index dec9b2a23c..3f03dac768 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -126,8 +126,8 @@ struct pending_block_header_state : public detail::block_header_state_common { * A data structure holding hotstuff core information */ struct block_header_state_core { - uint32_t last_final_block_height; // the block height of the last irreversible - // (final) block. + uint32_t last_final_block_height = 0; // the block height of the last irreversible + // (final) block. std::optional final_on_strong_qc_block_height; // the block height of // the block that would // become irreversible From 6d1c138600385d1f25816bc7809663b1c93ea403 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 21 Nov 2023 17:55:06 -0500 Subject: [PATCH 3/6] do not move the old block_header_state_core object when doing state transition --- libraries/chain/block_header_state.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index dd97d29b29..d4e7be5e7f 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -24,17 +24,19 @@ namespace eosio { namespace chain { } block_header_state_core::block_header_state_core( uint32_t last_final_blk_ht, - std::optional final_on_strong_qc_blk_ht, - std::optional last_qc_blk_ht ) : + std::optional final_on_strong_qc_blk_ht, + std::optional last_qc_blk_ht ) + : last_final_block_height(last_final_blk_ht), final_on_strong_qc_block_height(final_on_strong_qc_blk_ht), last_qc_block_height(last_qc_blk_ht) {} block_header_state_core block_header_state_core::next( uint32_t last_qc_block_height, - bool is_last_qc_strong) { + bool is_last_qc_strong) { // no state change if last_qc_block_height is the same if( last_qc_block_height == this->last_qc_block_height ) { - return std::move(*this); + block_header_state_core result{*this}; + return result; } EOS_ASSERT( last_qc_block_height > this->last_qc_block_height, block_validate_exception, "new last_qc_block_height must be greater than old last_qc_block_height" ); @@ -42,7 +44,7 @@ namespace eosio { namespace chain { auto old_last_qc_block_height = this->last_qc_block_height; auto old_final_on_strong_qc_block_height = this->final_on_strong_qc_block_height; - block_header_state_core result(std::move(*this)); + block_header_state_core result{*this}; if( is_last_qc_strong ) { // last QC is strong. We can progress forward. From 1863301b2431f09e2ffef0f7da9878fcfedcc44d Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 22 Nov 2023 09:47:40 -0500 Subject: [PATCH 4/6] spell out blk_ht to block_height so it is easier to read --- libraries/chain/block_header_state.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index d4e7be5e7f..fbbce6d84a 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -23,13 +23,13 @@ namespace eosio { namespace chain { } } - block_header_state_core::block_header_state_core( uint32_t last_final_blk_ht, - std::optional final_on_strong_qc_blk_ht, - std::optional last_qc_blk_ht ) + block_header_state_core::block_header_state_core( uint32_t last_final_block_height, + std::optional final_on_strong_qc_block_height, + std::optional last_qc_block_height ) : - last_final_block_height(last_final_blk_ht), - final_on_strong_qc_block_height(final_on_strong_qc_blk_ht), - last_qc_block_height(last_qc_blk_ht) {} + last_final_block_height(last_final_block_height), + final_on_strong_qc_block_height(final_on_strong_qc_block_height), + last_qc_block_height(last_qc_block_height) {} block_header_state_core block_header_state_core::next( uint32_t last_qc_block_height, bool is_last_qc_strong) { From 1290c249654a487b517d2f61e671e5f2871fd6f2 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 22 Nov 2023 10:04:06 -0500 Subject: [PATCH 5/6] return *this directly instead of constructing it --- libraries/chain/block_header_state.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index fbbce6d84a..07a27d8330 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -35,8 +35,7 @@ namespace eosio { namespace chain { bool is_last_qc_strong) { // no state change if last_qc_block_height is the same if( last_qc_block_height == this->last_qc_block_height ) { - block_header_state_core result{*this}; - return result; + return {*this}; } EOS_ASSERT( last_qc_block_height > this->last_qc_block_height, block_validate_exception, "new last_qc_block_height must be greater than old last_qc_block_height" ); From 87c7cbd59928b6c3930e1953ef811a540545fb66 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 22 Nov 2023 10:05:38 -0500 Subject: [PATCH 6/6] make comments in longer lines --- .../eosio/chain/block_header_state.hpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 3f03dac768..996aa6b2e4 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -126,16 +126,15 @@ struct pending_block_header_state : public detail::block_header_state_common { * A data structure holding hotstuff core information */ struct block_header_state_core { - uint32_t last_final_block_height = 0; // the block height of the last irreversible - // (final) block. - std::optional final_on_strong_qc_block_height; // the block height of - // the block that would - // become irreversible - // if the associated block - // header was to achieve a - // strong QC. - std::optional last_qc_block_height; // the block height of the block that - // is referenced as the last QC block + // the block height of the last irreversible (final) block. + uint32_t last_final_block_height = 0; + + // the block height of the block that would become irreversible (final) if the + // associated block header was to achieve a strong QC. + std::optional final_on_strong_qc_block_height; + + // the block height of the block that is referenced as the last QC block + std::optional last_qc_block_height; block_header_state_core() = default;