Skip to content

Commit

Permalink
GH-2125 Make last_qc_block_num non-optional
Browse files Browse the repository at this point in the history
  • Loading branch information
@heifner
heifner committed Feb 14, 2024
1 parent 8101af0 commit 5d2a839
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 24 deletions.
4 changes: 1 addition & 3 deletions libraries/chain/block_header_state.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ block_header_state_core block_header_state_core::next(qc_claim_t incoming) const

// next block which can become irreversible is the block with
// old last_qc_block_num
if (old_last_qc_block_num.has_value()) {
result.final_on_strong_qc_block_num = *old_last_qc_block_num;
}
result.final_on_strong_qc_block_num = old_last_qc_block_num;
} else {
// new final_on_strong_qc_block_num should not be present
result.final_on_strong_qc_block_num.reset();
Expand Down
4 changes: 2 additions & 2 deletions libraries/chain/hotstuff/block_construction_data_flow.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ The new storage for IF is:
struct block_header_state_core {
uint32_t last_final_block_num = 0; // last irreversible (final) block.
std::optional<uint32_t> final_on_strong_qc_block_num; // will become final if this header achives a strong QC.
std::optional<uint32_t> last_qc_block_num; //
uint32_t last_qc_block_num; //
uint32_t finalizer_policy_generation;
block_header_state_core next(uint32_t last_qc_block_num, bool is_last_qc_strong) const;
Expand Down Expand Up @@ -110,7 +110,7 @@ struct block_header_state {
// block descending from this need the provided qc in the block extension
bool is_needed(const quorum_certificate& qc) const {
return !core.last_qc_block_num || qc.block_num > *core.last_qc_block_num;
return qc.block_num > core.last_qc_block_num;
}
block_header_state next(const block_header_state_input& data) const;
Expand Down
6 changes: 3 additions & 3 deletions libraries/chain/include/eosio/chain/block_header_state.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ struct block_header_state_input : public building_block_input {
struct block_header_state_core {
uint32_t last_final_block_num = 0; // last irreversible (final) block.
std::optional<uint32_t> final_on_strong_qc_block_num; // will become final if this header achives a strong QC.
std::optional<uint32_t> last_qc_block_num; //
uint32_t finalizer_policy_generation; //
uint32_t last_qc_block_num = 0;
uint32_t finalizer_policy_generation = 0;

block_header_state_core next(qc_claim_t incoming) const;
};
Expand Down Expand Up @@ -81,7 +81,7 @@ struct block_header_state {

// block descending from this need the provided qc in the block extension
bool is_needed(const quorum_certificate& qc) const {
return !core.last_qc_block_num || qc.block_num > *core.last_qc_block_num;
return qc.block_num > core.last_qc_block_num;
}

flat_set<digest_type> get_activated_protocol_features() const { return activated_protocol_features->protocol_features; }
Expand Down
2 changes: 1 addition & 1 deletion libraries/chain/include/eosio/chain/fork_database.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace eosio::chain {
}
// only safe to call while holding fork_database lock
uint32_t last_qc_block_num() const {
return current_core.last_qc_block_num.value_or(final_on_strong_qc_block_num());
return current_core.last_qc_block_num;
}

// thread safe
Expand Down
4 changes: 2 additions & 2 deletions libraries/chain/include/eosio/chain/hotstuff/instant_finality_extension.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
namespace eosio::chain {

struct qc_claim_t {
uint32_t last_qc_block_num; // The block height of the most recent ancestor block that has a QC justification
bool is_last_qc_strong; // Whether the QC for the block referenced by last_qc_block_height is strong or weak.
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.
};

struct instant_finality_extension : fc::reflect_init {
Expand Down
27 changes: 14 additions & 13 deletions unittests/block_header_state_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_constructor_test)
block_header_state_core bhs_core1(1, 2, 3);
BOOST_REQUIRE_EQUAL(bhs_core1.last_final_block_num, 1u);
BOOST_REQUIRE_EQUAL(*bhs_core1.final_on_strong_qc_block_num, 2u);
BOOST_REQUIRE_EQUAL(*bhs_core1.last_qc_block_num, 3u);
BOOST_REQUIRE_EQUAL(bhs_core1.last_qc_block_num, 3u);

// verifies optional arguments work as expected
block_header_state_core bhs_core2(10, std::nullopt, {});
BOOST_REQUIRE_EQUAL(bhs_core2.last_final_block_num, 10u);
BOOST_REQUIRE(!bhs_core2.final_on_strong_qc_block_num.has_value());
BOOST_REQUIRE(!bhs_core2.last_qc_block_num.has_value());
BOOST_REQUIRE_EQUAL(bhs_core2.last_qc_block_num, 0);
}

// comprehensive state transition test
Expand All @@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_state_transition_test)
auto new_bhs_core = old_bhs_core.next({old_last_qc_block_num, is_last_qc_strong});
BOOST_REQUIRE_EQUAL(new_bhs_core.last_final_block_num, old_bhs_core.last_final_block_num);
BOOST_REQUIRE_EQUAL(*new_bhs_core.final_on_strong_qc_block_num, *old_bhs_core.final_on_strong_qc_block_num);
BOOST_REQUIRE_EQUAL(*new_bhs_core.last_qc_block_num, *old_bhs_core.last_qc_block_num);
BOOST_REQUIRE_EQUAL(new_bhs_core.last_qc_block_num, old_bhs_core.last_qc_block_num);
}

// verifies state cannot be transitioned to a smaller last_qc_block_num
Expand All @@ -53,7 +53,7 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_state_transition_test)
// old last_qc block became final_on_strong_qc block
BOOST_REQUIRE_EQUAL(*new_bhs_core.final_on_strong_qc_block_num, old_last_qc_block_num);
// new last_qc_block_num is the same as input
BOOST_REQUIRE_EQUAL(*new_bhs_core.last_qc_block_num, input_last_qc_block_num);
BOOST_REQUIRE_EQUAL(new_bhs_core.last_qc_block_num, input_last_qc_block_num);

// verifies state transition works when is_last_qc_strong is false
new_bhs_core = old_bhs_core.next({input_last_qc_block_num, false});
Expand All @@ -62,7 +62,7 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_state_transition_test)
// new final_on_strong_qc_block_num should not be present
BOOST_REQUIRE(!new_bhs_core.final_on_strong_qc_block_num.has_value());
// new last_qc_block_num is the same as input
BOOST_REQUIRE_EQUAL(*new_bhs_core.last_qc_block_num, input_last_qc_block_num);
BOOST_REQUIRE_EQUAL(new_bhs_core.last_qc_block_num, input_last_qc_block_num);
}

// A test to demonstrate 3-chain state transitions from the first
Expand All @@ -71,18 +71,19 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_3_chain_transition_test)
{
// block2: initial setup
constexpr auto block2_last_final_block_num = 1u;
block_header_state_core block2_bhs_core(block2_last_final_block_num, {}, {});
block_header_state_core block2_bhs_core(block2_last_final_block_num, {}, block2_last_final_block_num);

// block2 --> block3
constexpr auto block3_input_last_qc_block_num = 2u;
auto block3_bhs_core = block2_bhs_core.next({block3_input_last_qc_block_num, true});
// last_final_block_num should be the same as old one
BOOST_REQUIRE_EQUAL(block3_bhs_core.last_final_block_num, block2_last_final_block_num);
// final_on_strong_qc_block_num should be same as old one
BOOST_REQUIRE(!block3_bhs_core.final_on_strong_qc_block_num.has_value());
// final_on_strong_qc_block_num should be last_qc_block_num
BOOST_REQUIRE(block3_bhs_core.final_on_strong_qc_block_num.has_value());
BOOST_REQUIRE_EQUAL(*block3_bhs_core.final_on_strong_qc_block_num, block2_last_final_block_num);
// new last_qc_block_num is the same as input
BOOST_REQUIRE_EQUAL(*block3_bhs_core.last_qc_block_num, block3_input_last_qc_block_num);
auto block3_last_qc_block_num = *block3_bhs_core.last_qc_block_num;
BOOST_REQUIRE_EQUAL(block3_bhs_core.last_qc_block_num, block3_input_last_qc_block_num);
auto block3_last_qc_block_num = block3_bhs_core.last_qc_block_num;

// block3 --> block4
constexpr auto block4_input_last_qc_block_num = 3u;
Expand All @@ -92,9 +93,9 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_3_chain_transition_test)
// final_on_strong_qc_block_num should be block3's last_qc_block_num
BOOST_REQUIRE_EQUAL(*block4_bhs_core.final_on_strong_qc_block_num, block3_last_qc_block_num);
// new last_qc_block_num is the same as input
BOOST_REQUIRE_EQUAL(*block4_bhs_core.last_qc_block_num, block4_input_last_qc_block_num);
BOOST_REQUIRE_EQUAL(block4_bhs_core.last_qc_block_num, block4_input_last_qc_block_num);
auto block4_final_on_strong_qc_block_num = *block4_bhs_core.final_on_strong_qc_block_num;
auto block4_last_qc_block_num = *block4_bhs_core.last_qc_block_num;
auto block4_last_qc_block_num = block4_bhs_core.last_qc_block_num;

// block4 --> block5
constexpr auto block5_input_last_qc_block_num = 4u;
Expand All @@ -104,7 +105,7 @@ BOOST_AUTO_TEST_CASE(block_header_state_core_3_chain_transition_test)
// final_on_strong_qc_block_num should be block4's last_qc_block_num
BOOST_REQUIRE_EQUAL(*block5_bhs_core.final_on_strong_qc_block_num, block4_last_qc_block_num);
// new last_qc_block_num is the same as input
BOOST_REQUIRE_EQUAL(*block5_bhs_core.last_qc_block_num, block5_input_last_qc_block_num);
BOOST_REQUIRE_EQUAL(block5_bhs_core.last_qc_block_num, block5_input_last_qc_block_num);
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 5d2a839

Please sign in to comment.