diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index c612870b21..3677d1d48c 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -158,6 +158,8 @@ void finish_next(const block_header_state& prev, // --------------------------------------------------------------------------- next_header_state.finalizer_policies = prev.finalizer_policies; } else { + auto next_block_num = next_header_state.block_num(); + while (it != prev.finalizer_policies.end() && it->first <= lib) { const finalizer_policy_tracker& tracker = it->second; if (tracker.state == finalizer_policy_tracker::state_t::pending) { @@ -166,11 +168,33 @@ void finish_next(const block_header_state& prev, next_header_state.active_finalizer_policy.reset(new finalizer_policy(*tracker.policy)); } else { assert(tracker.state == finalizer_policy_tracker::state_t::proposed); - // block where finalizer_policy was proposed became final. The finalizer policy will - // become active when next block becomes final. + + // The block where finalizer_policy was proposed has became final. The finalizer + // policy will become active when `next_block_num` becomes final. + // + // So `tracker.policy` should become `pending` at `next_block_num`. + // + // Either insert a new `finalizer_policy_tracker` value, or update the `pending` + // policy if there is already one at `next_block_num` (which can happen when + // finality advances multiple block at a time, and more than one policy move from + // proposed to pending. + // + // Since we iterate finalizer_policies which is a multimap sorted by block number, + // the last one we add will be for the highest block number, which is what we want. // --------------------------------------------------------------------------------- - finalizer_policy_tracker t { finalizer_policy_tracker::state_t::pending, tracker.policy }; - next_header_state.finalizer_policies.emplace(next_header_state.block_num(), std::move(t)); + auto range = next_header_state.finalizer_policies.equal_range(next_block_num); + auto itr = range.first; + for (; itr != range.second; ++itr) { + if (itr->second.state == finalizer_policy_tracker::state_t::pending) { + itr->second.policy = tracker.policy; + break; + } + } + if (itr == range.second) { + // there wasn't already a pending one for `next_block_num`, add a new tracker + finalizer_policy_tracker t { finalizer_policy_tracker::state_t::pending, tracker.policy }; + next_header_state.finalizer_policies.emplace(next_block_num, std::move(t)); + } } ++it; } @@ -321,5 +345,37 @@ block_header_state block_header_state::next(const signed_block_header& h, valida return next_header_state; } +// ------------------------------------------------------------------------------- +// do some sanity checks on block_header_state +// ------------------------------------------------------------------------------- +bool block_header_state::sanity_check() const { + // check that we have at most *one* proposed and *one* pending `finalizer_policy` + // for any block number + // ----------------------------------------------------------------------------- + block_num_type block_num(0); + bool pending{false}, proposed{false}; + + for (auto it = finalizer_policies.begin(); it != finalizer_policies.end(); ++it) { + if (block_num != it->first) { + pending = proposed = false; + block_num = it->first; + } + const auto& tracker = it->second; + if (tracker.state == finalizer_policy_tracker::state_t::proposed) { + if (proposed) + return false; + else + proposed = true; + } + if (tracker.state == finalizer_policy_tracker::state_t::pending) { + if (pending) + return false; + else + pending = true; + } + } + return true; +} + } // namespace eosio::chain diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index bb0474b588..3d2b0ee868 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5111,6 +5111,10 @@ block_state_legacy_ptr controller::head_block_state_legacy()const { }); } +bool controller::head_sanity_check()const { + return apply(my->chain_head, [](const auto& head) { return head->sanity_check(); }); +} + const signed_block_ptr& controller::head_block()const { return my->chain_head.block(); } diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 9daf9d7815..d978754dc1 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -131,6 +131,8 @@ struct block_header_state { return qc_claim > core.latest_qc_claim(); } + bool sanity_check() const; // does sanity check of block_header_state, returns true if successful + const vector& get_new_protocol_feature_activations() const; const producer_authority& get_scheduled_producer(block_timestamp_type t) const; diff --git a/libraries/chain/include/eosio/chain/block_header_state_legacy.hpp b/libraries/chain/include/eosio/chain/block_header_state_legacy.hpp index 722125ff26..5c2c6d05e9 100644 --- a/libraries/chain/include/eosio/chain/block_header_state_legacy.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state_legacy.hpp @@ -148,6 +148,8 @@ struct block_header_state_legacy : public detail::block_header_state_legacy_comm void sign( const signer_callback_type& signer ); void verify_signee()const; + bool sanity_check() const { return true; } + const vector& get_new_protocol_feature_activations()const; }; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index fe129b8e9a..44d22ede85 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -240,6 +240,7 @@ namespace eosio::chain { account_name head_block_producer()const; const block_header& head_block_header()const; const signed_block_ptr& head_block()const; + bool head_sanity_check()const; // returns nullptr after instant finality enabled block_state_legacy_ptr head_block_state_legacy()const; // returns finality_data associated with chain head for SHiP when in Savanna, diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 40bacee8a9..3ad61e8c96 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -501,6 +501,7 @@ namespace eosio::testing { // ----------------------------------------------------------------- void check_head_finalizer_policy(uint32_t generation, std::span keys_span) { + BOOST_REQUIRE_EQUAL(control->head_sanity_check(), true); auto finpol = active_finalizer_policy(control->head_block_header().calculate_id()); BOOST_REQUIRE(!!finpol); BOOST_REQUIRE_EQUAL(finpol->generation, generation); diff --git a/unittests/finality_tests.cpp b/unittests/finality_tests.cpp index 5b49d5b505..83e73a0766 100644 --- a/unittests/finality_tests.cpp +++ b/unittests/finality_tests.cpp @@ -522,4 +522,78 @@ BOOST_FIXTURE_TEST_CASE(second_set_finalizers, finality_test_cluster<4>) { try { } FC_LOG_AND_RETHROW() } +// verify issue https://github.com/AntelopeIO/spring/issues/130 is fixed +// --------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(finality_skip, finality_test_cluster<4>) { try { + produce_and_push_block(); + process_votes(1, num_needed_for_quorum); + produce_and_push_block(); + + // when a quorum of nodes vote, LIB should advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), num_nodes); + BOOST_REQUIRE(produce_blocks_and_verify_lib_advancing()); + + auto add_set_finalizers = [&](size_t start_idx) { + assert(fin_policy_0); // current finalizer policy from transition to Savanna + auto indices = fin_policy_indices_0; // start from original set of indices + assert(indices[0] == 0u); // we used index 0 for node0 in original policy + indices[0] = start_idx; // update key used for node0 in policy + auto pubkeys = node0.finkeys.set_finalizer_policy(indices).pubkeys; + produce_and_push_block(); + return pubkeys; + }; + + clear_votes_and_reset_lib(); + + // produce 2 blocks that will be made final after the three `add_set_finalizers` below + // ------------------------------------------------------------------------------------ + for (size_t i=0; i<4; ++i) { + produce_and_push_block(); + process_votes(1, num_nodes - 1); + } + + // run three set_finalizers in 3 blocks without voting + // they will be in `proposed` state with different block numbers. + // ------------------------------------------------------------- + auto pubkeys1 = add_set_finalizers(1); // will be generation == 2 + auto pubkeys2 = add_set_finalizers(2); // will be generation == 3 + auto pubkeys3 = add_set_finalizers(3); // will be generation == 4 + + // produce_and_push 3 blocks. The last one will make finality skip over the three + // `add_set_finalizers` blocks above, so that they all become `pending` on the same block. + // --------------------------------------------------------------------------------------- + for (size_t i=0; i<3; ++i) { + produce_and_push_block(); + process_votes(1, num_nodes - 1); + + // make sure we don't have duplicate finalizer policies for the same block number + // in either `proposed` or `pending` state + // ------------------------------------------------------------------------------ + node0.check_head_finalizer_policy(1u, fin_policy_pubkeys_0); + } + + // now *only* the third `set_finalizers` should be `pending`, the one with + // `generation == 4`. The other policies must have been overwritten since they all + // became `pending` at the same block. + // + // we need another 3-chain to make that block final. + // ------------------------------------------------------------------------------- + for (size_t i=0; i<3; ++i) { + produce_and_push_block(); + process_votes(1, num_nodes - 1); + node0.check_head_finalizer_policy(1u, fin_policy_pubkeys_0); + } + + // when we receive the votes of that last block finishing the 3-chain, the active + // `finalizer_policy` finally changes. + // ------------------------------------------------------------------------------ + produce_and_push_block(); + process_votes(1, num_nodes - 1); + node0.check_head_finalizer_policy(4u, pubkeys3); + +} FC_LOG_AND_RETHROW() } + + + + BOOST_AUTO_TEST_SUITE_END()