From 6505e0c8b8d3301eed4b585d3a4da3425bde1789 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 29 Apr 2024 18:41:39 -0500 Subject: [PATCH 01/41] Gh-5 Add ordered_diff --- .../include/fc/container/ordered_diff.hpp | 99 +++++++++++++ libraries/libfc/test/CMakeLists.txt | 1 + libraries/libfc/test/test_ordered_diff.cpp | 130 ++++++++++++++++++ 3 files changed, 230 insertions(+) create mode 100644 libraries/libfc/include/fc/container/ordered_diff.hpp create mode 100644 libraries/libfc/test/test_ordered_diff.cpp diff --git a/libraries/libfc/include/fc/container/ordered_diff.hpp b/libraries/libfc/include/fc/container/ordered_diff.hpp new file mode 100644 index 0000000000..cae14fc322 --- /dev/null +++ b/libraries/libfc/include/fc/container/ordered_diff.hpp @@ -0,0 +1,99 @@ +#pragma once + +#include +#include + +namespace fc { + +/** + * @class ordered_diff + * @brief Provides ablity to generate and apply diff of containers of type T + * + * Example use: + * std::vector source = { 'a', 'b', 'f', 'c', 'd' }; + * std::vector target = { 'b', 'f', 'c', 'd', 'e', 'h' }; + * ordered_diff::diff_result diff = ordered_diff::diff(source, target); + * ordered_diff::apply_diff(source, std::move(diff)); + * assert(source == target); + * + * @param T type stored in Container, must provide == + */ +template typename Container = std::vector> +requires std::equality_comparable && std::random_access_iterator::iterator> +class ordered_diff { +public: + struct diff_result { + Container remove_indexes; + Container> insert_indexes; + }; + + /// Generate diff_result that when `apply_diff(source, diff_result)` to source will generate target + static diff_result diff(const Container& source, const Container& target) { + size_t s = 0; + size_t t = 0; + + diff_result result; + while (s < source.size() || t < target.size()) { + if (s < source.size() && t < target.size()) { + if (source[s] == target[t]) { + // nothing to do, skip over + ++s; + ++t; + } else { // not equal + if (s == source.size() - 1 && t == target.size() - 1) { + // both at end, insert target and remove source + result.remove_indexes.push_back(s); + result.insert_indexes.emplace_back(t, target[t]); + ++s; + ++t; + } else if (s + 1 < source.size() && t + 1 < target.size() && source[s + 1] == target[t + 1]) { + // misalignment, but next value equal, insert and remove + result.remove_indexes.push_back(s); + result.insert_indexes.emplace_back(t, target[t]); + ++s; + ++t; + } else if (t + 1 < target.size() && source[s] == target[t + 1]) { + // source equals next target, insert current target + result.insert_indexes.emplace_back(t, target[t]); + ++t; + } else { // source[s + 1] == target[t] + // target matches next source, remove current source + result.remove_indexes.push_back(s); + ++s; + } + } + } else if (s < source.size()) { + // remove extra in source + result.remove_indexes.push_back(s); + ++s; + } else if (t < target.size()) { + // insert extra in target + result.insert_indexes.emplace_back(t, target[t]); + ++t; + } + } + + return result; + } + + template + requires std::same_as, diff_result> + static void apply_diff(Container& source, X&& diff) { + // Remove from the source based on diff.remove_indexes + std::ptrdiff_t offset = 0; + for (size_t index : diff.remove_indexes) { + source.erase(source.begin() + index + offset); + --offset; + } + + // Insert into the source based on diff.insert_indexes + for (auto& t : diff.insert_indexes) { + size_t index = std::get<0>(t); + auto& value = std::get<1>(t); + source.insert(source.begin() + index, std::move(value)); + } + } + +}; // class ordered_diff + +} // namespace fc diff --git a/libraries/libfc/test/CMakeLists.txt b/libraries/libfc/test/CMakeLists.txt index 4b079be425..ee728d2f13 100644 --- a/libraries/libfc/test/CMakeLists.txt +++ b/libraries/libfc/test/CMakeLists.txt @@ -19,6 +19,7 @@ add_executable( test_fc test_base64.cpp test_escape_str.cpp test_bls.cpp + test_ordered_diff.cpp main.cpp ) target_link_libraries( test_fc fc ) diff --git a/libraries/libfc/test/test_ordered_diff.cpp b/libraries/libfc/test/test_ordered_diff.cpp new file mode 100644 index 0000000000..4cf05792b1 --- /dev/null +++ b/libraries/libfc/test/test_ordered_diff.cpp @@ -0,0 +1,130 @@ +#include + +#include +#include + +using namespace fc; + +BOOST_AUTO_TEST_SUITE(ordered_diff_tests) + +BOOST_AUTO_TEST_CASE(ordered_diff_test) try { + using namespace std; + + { // Basic case + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = {'a', 'c', 'e', 'f'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // Basic case, deque + using ordered_deque_char_diff = ordered_diff; + deque source = {'a', 'x', 'c', 'd', 'e'}; + deque target = {'z', 'c', 'y', 'f'}; + auto result = ordered_deque_char_diff::diff(source, target); + ordered_deque_char_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // Empty vectors + vector source; + vector target; + ordered_diff::diff_result result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // All elements removed + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // All elements inserted + vector source; + vector target = {'a', 'b', 'c', 'd', 'e'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // Mix of removals and inserts + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = {'a', 'c', 'e', 'f', 'g', 'h'}; + ordered_diff::diff_result result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // Mix of removals and inserts + vector source = {1, 2, 3, 4, 5}; + vector target = {3, 4, 6, 2, 0}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // Complete change + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = {'f', 'g', 'h', 'i'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // Diff order + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = {'e', 'd', 'c', 'b', 'a'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } +} FC_LOG_AND_RETHROW(); + +BOOST_AUTO_TEST_CASE(ordered_diff_string_test) try { + using namespace std; + { + vector source = {"hello", "how", "are", "you", "today"}; + vector target = {"hi", "are", "you", "here"}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { + vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; + vector target = {"prod2", "prod1", "prod3", "prod4", "prod5"};; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { + vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; + vector target = {"prod5", "prod1", "prod2", "prod3", "prod4"};; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, std::move(result)); + BOOST_TEST(source == target); + } + +} FC_LOG_AND_RETHROW(); + +class count_moves { + std::string s; +public: + inline static size_t num_moves = 0; + count_moves(const count_moves& m) : s(m.s) {}; + count_moves(count_moves&& m) noexcept : s(std::move(m.s)) { ++num_moves; }; + count_moves& operator=(const count_moves& rhs) = default; + explicit count_moves(std::string s) : s(s) {}; + auto operator<=>(const count_moves&) const = default; + bool operator==(const count_moves&) const = default; +}; + +BOOST_AUTO_TEST_CASE(ordered_diff_moveable_test) try { + using namespace std; + { + vector source = {count_moves{"hello"}, count_moves{"there"}}; + vector target = {count_moves{"hi"}, count_moves{"there"}}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, std::move(result)); + BOOST_TEST(source == target); + BOOST_TEST(count_moves::num_moves == 1); + } + +} FC_LOG_AND_RETHROW(); + +BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From 7f4879ebcaea616dffe18a3243027b17407789ba Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 May 2024 14:32:08 -0500 Subject: [PATCH 02/41] GH-5 Add some additional tests cases --- libraries/libfc/test/test_ordered_diff.cpp | 32 ++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/libraries/libfc/test/test_ordered_diff.cpp b/libraries/libfc/test/test_ordered_diff.cpp index 4cf05792b1..2ec54b1650 100644 --- a/libraries/libfc/test/test_ordered_diff.cpp +++ b/libraries/libfc/test/test_ordered_diff.cpp @@ -46,6 +46,13 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { ordered_diff::apply_diff(source, result); BOOST_TEST(source == target); } + { // No change + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = source; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } { // Mix of removals and inserts vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'a', 'c', 'e', 'f', 'g', 'h'}; @@ -74,6 +81,20 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { ordered_diff::apply_diff(source, result); BOOST_TEST(source == target); } + { // shift left + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = {'b', 'c', 'd', 'e', 'f'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // shift right + vector source = {'a', 'b', 'c', 'd', 'e'}; + vector target = {'z', 'a', 'b', 'c', 'd'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(ordered_diff_string_test) try { @@ -87,14 +108,21 @@ BOOST_AUTO_TEST_CASE(ordered_diff_string_test) try { } { vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; - vector target = {"prod2", "prod1", "prod3", "prod4", "prod5"};; + vector target = {"prod2", "prod1", "prod3", "prod4", "prod5"}; auto result = ordered_diff::diff(source, target); ordered_diff::apply_diff(source, result); BOOST_TEST(source == target); } { vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; - vector target = {"prod5", "prod1", "prod2", "prod3", "prod4"};; + vector target = {"prod5", "prod1", "prod2", "prod3", "prod4"}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, std::move(result)); + BOOST_TEST(source == target); + } + { + vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; + vector target = {"prod2", "prod3", "prod4", "prod5", "prod6"}; auto result = ordered_diff::diff(source, target); ordered_diff::apply_diff(source, std::move(result)); BOOST_TEST(source == target); From 1403867d9a10fcf10d20501fc3417193a21ac799 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 May 2024 14:33:21 -0500 Subject: [PATCH 03/41] GH-5 FC pack supports pair not tuple --- libraries/libfc/include/fc/container/ordered_diff.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/libfc/include/fc/container/ordered_diff.hpp b/libraries/libfc/include/fc/container/ordered_diff.hpp index cae14fc322..ed6523d9e8 100644 --- a/libraries/libfc/include/fc/container/ordered_diff.hpp +++ b/libraries/libfc/include/fc/container/ordered_diff.hpp @@ -24,7 +24,7 @@ class ordered_diff { public: struct diff_result { Container remove_indexes; - Container> insert_indexes; + Container> insert_indexes; }; /// Generate diff_result that when `apply_diff(source, diff_result)` to source will generate target From ac49b89d3efb5b5d17f331097b7abaff8f555e7e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 May 2024 14:35:42 -0500 Subject: [PATCH 04/41] GH-5 Provide a diff of finalizers in instant_finality_extension instead of a full list of finalizers --- libraries/chain/block_header_state.cpp | 22 +++++++------ libraries/chain/block_header_state_legacy.cpp | 4 ++- libraries/chain/block_state.cpp | 5 +-- .../eosio/chain/block_header_state.hpp | 24 ++++++++++++-- .../eosio/chain/finality/finalizer_policy.hpp | 29 ++++++++++++++++- .../finality/instant_finality_extension.hpp | 12 +++---- unittests/api_tests.cpp | 18 +++++------ unittests/block_header_tests.cpp | 32 ++++++++----------- unittests/finality_test_cluster.cpp | 8 ++--- unittests/finalizer_update_tests.cpp | 29 ++++++++++++++--- unittests/protocol_feature_tests.cpp | 6 ++-- unittests/svnn_ibc_tests.cpp | 8 ++--- unittests/vote_processor_tests.cpp | 7 ++-- 13 files changed, 136 insertions(+), 68 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index e0f17e9a4a..e89b9adc32 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -22,8 +22,7 @@ digest_type block_header_state::compute_base_digest() const { fc::raw::pack( enc, fp_pair.first ); const finalizer_policy_tracker& tracker = fp_pair.second; fc::raw::pack( enc, tracker.state ); - assert(tracker.policy); - fc::raw::pack( enc, *tracker.policy ); + fc::raw::pack( enc, tracker.policy_diff ); } assert(active_proposer_policy); @@ -136,13 +135,14 @@ void finish_next(const block_header_state& prev, if (tracker.state == finalizer_policy_tracker::state_t::pending) { // new finalizer_policy becones active // ----------------------------------- - next_header_state.active_finalizer_policy.reset(new finalizer_policy(*tracker.policy)); + next_header_state.active_finalizer_policy.reset(new finalizer_policy(*prev.active_finalizer_policy)); + next_header_state.active_finalizer_policy->apply_diff(tracker.policy_diff); } 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. // --------------------------------------------------------------------------------- - finalizer_policy_tracker t { finalizer_policy_tracker::state_t::pending, tracker.policy }; + finalizer_policy_tracker t { finalizer_policy_tracker::state_t::pending, tracker.policy_diff }; next_header_state.finalizer_policies.emplace(next_header_state.block_num(), std::move(t)); } ++it; @@ -156,19 +156,19 @@ void finish_next(const block_header_state& prev, } } - if (if_ext.new_finalizer_policy) { + if (if_ext.new_finalizer_policy_diff) { // a new `finalizer_policy` was proposed in the previous block, and is present in the previous // block's header extensions. // Add this new proposal to the `finalizer_policies` multimap which tracks the in-flight proposals, // increment the generation number, and log that proposal (debug level). // ------------------------------------------------------------------------------------------------ dlog("New finalizer policy proposed in block ${id}: ${pol}", - ("id", prev.block_id)("pol", *if_ext.new_finalizer_policy)); - next_header_state.finalizer_policy_generation = if_ext.new_finalizer_policy->generation; + ("id", prev.block_id)("pol", *if_ext.new_finalizer_policy_diff)); + next_header_state.finalizer_policy_generation = if_ext.new_finalizer_policy_diff->generation; next_header_state.finalizer_policies.emplace( next_header_state.block_num(), finalizer_policy_tracker{finalizer_policy_tracker::state_t::proposed, - std::make_shared(std::move(*if_ext.new_finalizer_policy))}); + std::move(*if_ext.new_finalizer_policy_diff)}); } else { next_header_state.finalizer_policy_generation = prev.finalizer_policy_generation; @@ -205,8 +205,12 @@ block_header_state block_header_state::next(block_header_state_input& input) con // finality extension // ------------------ + std::optional new_finalizer_policy_diff; + if (input.new_finalizer_policy) { + new_finalizer_policy_diff = calculate_finalizer_policy_diff(*input.new_finalizer_policy); + } instant_finality_extension new_if_ext { input.most_recent_ancestor_with_qc, - std::move(input.new_finalizer_policy), + std::move(new_finalizer_policy_diff), std::move(input.new_proposer_policy) }; uint16_t if_ext_id = instant_finality_extension::extension_id(); diff --git a/libraries/chain/block_header_state_legacy.cpp b/libraries/chain/block_header_state_legacy.cpp index a47ca2282f..01cbe4113e 100644 --- a/libraries/chain/block_header_state_legacy.cpp +++ b/libraries/chain/block_header_state_legacy.cpp @@ -220,8 +220,10 @@ namespace eosio::chain { // set current block_num as qc_claim.last_qc_block_num in the IF extension qc_claim_t initial_if_claim { .block_num = block_num, .is_strong_qc = false }; + finalizer_policy no_policy; + auto new_fin_policy_diff = no_policy.create_diff(*new_finalizer_policy); emplace_extension(h.header_extensions, instant_finality_extension::extension_id(), - fc::raw::pack(instant_finality_extension{ initial_if_claim, std::move(new_finalizer_policy), {} })); + fc::raw::pack(instant_finality_extension{ initial_if_claim, std::move(new_fin_policy_diff), {} })); } else if (qc_claim) { emplace_extension(h.header_extensions, instant_finality_extension::extension_id(), fc::raw::pack(instant_finality_extension{ *qc_claim, {}, {} })); diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 5d84d2f60a..690ba7408c 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -72,8 +72,9 @@ block_state_ptr block_state::create_if_genesis_block(const block_state_legacy& b assert(bsp.block->contains_header_extension(instant_finality_extension::extension_id())); // required by transition mechanism instant_finality_extension if_ext = bsp.block->extract_header_extension(); - assert(if_ext.new_finalizer_policy); // required by transition mechanism - result.active_finalizer_policy = std::make_shared(*if_ext.new_finalizer_policy); + assert(if_ext.new_finalizer_policy_diff); // required by transition mechanism + result.active_finalizer_policy = std::make_shared(); + result.active_finalizer_policy->apply_diff(std::move(*if_ext.new_finalizer_policy_diff)); result.active_proposer_policy = std::make_shared(); result.active_proposer_policy->active_time = bsp.timestamp(); result.active_proposer_policy->proposer_schedule = bsp.active_schedule; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 7fd43afe32..cd89d3338b 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -46,8 +46,8 @@ struct finality_digest_data_v1 { // ------------------------------------------------------------------------------------------ struct finalizer_policy_tracker { enum class state_t { proposed = 0, pending }; - state_t state; - finalizer_policy_ptr policy; + state_t state; + finalizer_policy_diff policy_diff; }; struct building_block_input { @@ -129,6 +129,24 @@ struct block_header_state { const vector& get_new_protocol_feature_activations() const; const producer_authority& get_scheduled_producer(block_timestamp_type t) const; + + finalizer_policy_diff calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const { + if (finalizer_policies.empty()) { + return active_finalizer_policy->create_diff(new_policy); + } + finalizer_policy_ptr fin_policy_ptr = std::make_shared(*active_finalizer_policy); + for (const auto& e : finalizer_policies) { + if (e.second.state == finalizer_policy_tracker::state_t::pending) { + fin_policy_ptr->apply_diff(e.second.policy_diff); + } + } + for (const auto& e : finalizer_policies) { + if (e.second.state == finalizer_policy_tracker::state_t::proposed) { + fin_policy_ptr->apply_diff(e.second.policy_diff); + } + } + return fin_policy_ptr->create_diff(new_policy); + } }; using block_header_state_ptr = std::shared_ptr; @@ -137,7 +155,7 @@ using block_header_state_ptr = std::shared_ptr; FC_REFLECT_ENUM( eosio::chain::finalizer_policy_tracker::state_t, (proposed)(pending)) -FC_REFLECT( eosio::chain::finalizer_policy_tracker, (state)(policy)) +FC_REFLECT( eosio::chain::finalizer_policy_tracker, (state)(policy_diff)) FC_REFLECT( eosio::chain::block_header_state, (block_id)(header) (activated_protocol_features)(core)(active_finalizer_policy) diff --git a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp index 1fa27d14ac..64089fa950 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp @@ -2,13 +2,37 @@ #include #include +#include namespace eosio::chain { + using finalizers_differ = fc::ordered_diff; + using finalizers_diff_t = fc::ordered_diff::diff_result; + + struct finalizer_policy_diff { + uint32_t generation = 0; ///< sequentially incrementing version number + uint64_t threshold = 0; ///< vote weight threshold to finalize blocks + finalizers_diff_t finalizers_diff; + }; + struct finalizer_policy { uint32_t generation = 0; ///< sequentially incrementing version number uint64_t threshold = 0; ///< vote weight threshold to finalize blocks - std::vector finalizers; ///< Instant Finality voter set + std::vector finalizers; ///< Instant Finality voter set + + finalizer_policy_diff create_diff(const finalizer_policy& target) const { + return {.generation = target.generation, + .threshold = target.threshold, + .finalizers_diff = finalizers_differ::diff(finalizers, target.finalizers)}; + } + + template + requires std::same_as, finalizer_policy_diff> + void apply_diff(X&& diff) { + generation = diff.generation; + threshold = diff.threshold; + finalizers_differ::apply_diff(finalizers, std::move(diff.finalizers_diff)); + } // max accumulated weak weight before becoming weak_final uint64_t max_weak_sum_before_weak_final() const { @@ -23,7 +47,10 @@ namespace eosio::chain { }; using finalizer_policy_ptr = std::shared_ptr; + using finalizer_policy_diff_ptr = std::shared_ptr; } /// eosio::chain FC_REFLECT( eosio::chain::finalizer_policy, (generation)(threshold)(finalizers) ) +FC_REFLECT( eosio::chain::finalizers_diff_t, (remove_indexes)(insert_indexes) ) +FC_REFLECT( eosio::chain::finalizer_policy_diff, (generation)(threshold)(finalizers_diff) ) diff --git a/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp b/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp index 3b78770a5b..ff2cb53e82 100644 --- a/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp +++ b/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp @@ -12,10 +12,10 @@ struct instant_finality_extension : fc::reflect_init { instant_finality_extension() = default; instant_finality_extension(qc_claim_t qc_claim, - std::optional new_finalizer_policy, + std::optional new_finalizer_policy_diff, std::shared_ptr new_proposer_policy) : qc_claim(qc_claim), - new_finalizer_policy(std::move(new_finalizer_policy)), + new_finalizer_policy_diff(std::move(new_finalizer_policy_diff)), new_proposer_policy(std::move(new_proposer_policy)) {} @@ -25,11 +25,11 @@ struct instant_finality_extension : fc::reflect_init { static_assert( extension_id() == 2, "instant_finality_extension extension id must be 2" ); } - qc_claim_t qc_claim; - std::optional new_finalizer_policy; - std::shared_ptr new_proposer_policy; + qc_claim_t qc_claim; + std::optional new_finalizer_policy_diff; + std::shared_ptr new_proposer_policy; }; } /// eosio::chain -FC_REFLECT( eosio::chain::instant_finality_extension, (qc_claim)(new_finalizer_policy)(new_proposer_policy) ) +FC_REFLECT( eosio::chain::instant_finality_extension, (qc_claim)(new_finalizer_policy_diff)(new_proposer_policy) ) diff --git a/unittests/api_tests.cpp b/unittests/api_tests.cpp index c344f4d0a9..0796d4333f 100644 --- a/unittests/api_tests.cpp +++ b/unittests/api_tests.cpp @@ -3890,11 +3890,11 @@ BOOST_AUTO_TEST_CASE(initial_set_finalizer_test) { try { std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); BOOST_TEST(!!ext); - std::optional fin_policy = std::get(*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); + std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; + BOOST_TEST(!!fin_policy_diff); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == finalizers.size()); + BOOST_TEST(fin_policy_diff->generation == 1); + BOOST_TEST(fin_policy_diff->threshold == finalizers.size() / 3 * 2 + 1); block_id_type if_genesis_block_id = block->calculate_id(); for (block_num_type active_block_num = block->block_num(); active_block_num > lib; t.produce_block()) { @@ -3943,10 +3943,10 @@ void test_finality_transition(const vector& accounts, const base_t std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); BOOST_TEST(!!ext); - std::optional fin_policy = std::get(*ext).new_finalizer_policy; - BOOST_TEST(!!fin_policy); - BOOST_TEST(fin_policy->finalizers.size() == accounts.size()); - BOOST_TEST(fin_policy->generation == 1); + std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; + BOOST_TEST(!!fin_policy_diff); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == accounts.size()); + BOOST_TEST(fin_policy_diff->generation == 1); block_id_type if_genesis_block_id = block->calculate_id(); block_num_type active_block_num = block->block_num(); diff --git a/unittests/block_header_tests.cpp b/unittests/block_header_tests.cpp index 22b68345c9..67fdd24a8e 100644 --- a/unittests/block_header_tests.cpp +++ b/unittests/block_header_tests.cpp @@ -24,7 +24,7 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_empty_values_test) header.header_extensions, instant_finality_extension::extension_id(), fc::raw::pack( instant_finality_extension{qc_claim_t{last_qc_block_num, is_last_strong_qc}, - std::optional{}, std::shared_ptr{}} ) + std::optional{}, std::shared_ptr{}} ) ); std::optional ext = header.extract_header_extension(instant_finality_extension::extension_id()); @@ -33,7 +33,7 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_empty_values_test) const auto& if_extension = std::get(*ext); BOOST_REQUIRE_EQUAL( if_extension.qc_claim.block_num, last_qc_block_num ); BOOST_REQUIRE_EQUAL( if_extension.qc_claim.is_strong_qc, is_last_strong_qc ); - BOOST_REQUIRE( !if_extension.new_finalizer_policy ); + BOOST_REQUIRE( !if_extension.new_finalizer_policy_diff ); BOOST_REQUIRE( !if_extension.new_proposer_policy ); } @@ -50,17 +50,15 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_uniqueness_test) ); std::vector finalizers { {"test description", 50, fc::crypto::blslib::bls_public_key{"PUB_BLS_qVbh4IjYZpRGo8U_0spBUM-u-r_G0fMo4MzLZRsKWmm5uyeQTp74YFaMN9IDWPoVVT5rj_Tw1gvps6K9_OZ6sabkJJzug3uGfjA6qiaLbLh5Fnafwv-nVgzzzBlU2kwRrcHc8Q" }} }; - finalizer_policy new_finalizer_policy; - new_finalizer_policy.generation = 1; - new_finalizer_policy.threshold = 100; - new_finalizer_policy.finalizers = finalizers; + auto fin_policy = std::make_shared(); + finalizer_policy_diff new_finalizer_policy_diff = fin_policy->create_diff(finalizer_policy{.generation = 1, .threshold = 100, .finalizers = finalizers}); proposer_policy_ptr new_proposer_policy = std::make_shared(1, block_timestamp_type{200}, producer_authority_schedule{} ); emplace_extension( header.header_extensions, instant_finality_extension::extension_id(), - fc::raw::pack( instant_finality_extension{qc_claim_t{100, true}, new_finalizer_policy, new_proposer_policy} ) + fc::raw::pack( instant_finality_extension{qc_claim_t{100, true}, new_finalizer_policy_diff, new_proposer_policy} ) ); BOOST_CHECK_THROW(header.validate_and_extract_header_extensions(), invalid_block_header_extension); @@ -74,17 +72,15 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_values_test) constexpr bool is_strong_qc {true}; std::vector finalizers { {"test description", 50, fc::crypto::blslib::bls_public_key{"PUB_BLS_qVbh4IjYZpRGo8U_0spBUM-u-r_G0fMo4MzLZRsKWmm5uyeQTp74YFaMN9IDWPoVVT5rj_Tw1gvps6K9_OZ6sabkJJzug3uGfjA6qiaLbLh5Fnafwv-nVgzzzBlU2kwRrcHc8Q" }} }; - finalizer_policy new_finalizer_policy; - new_finalizer_policy.generation = 1; - new_finalizer_policy.threshold = 100; - new_finalizer_policy.finalizers = finalizers; + auto fin_policy = std::make_shared(); + finalizer_policy_diff new_finalizer_policy_diff = fin_policy->create_diff(finalizer_policy{.generation = 1, .threshold = 100, .finalizers = finalizers}); proposer_policy_ptr new_proposer_policy = std::make_shared(1, block_timestamp_type{200}, producer_authority_schedule{} ); emplace_extension( header.header_extensions, instant_finality_extension::extension_id(), - fc::raw::pack( instant_finality_extension{qc_claim_t{last_qc_block_num, is_strong_qc}, new_finalizer_policy, new_proposer_policy} ) + fc::raw::pack( instant_finality_extension{qc_claim_t{last_qc_block_num, is_strong_qc}, new_finalizer_policy_diff, new_proposer_policy} ) ); std::optional ext = header.extract_header_extension(instant_finality_extension::extension_id()); @@ -95,12 +91,12 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_values_test) BOOST_REQUIRE_EQUAL( if_extension.qc_claim.block_num, last_qc_block_num ); BOOST_REQUIRE_EQUAL( if_extension.qc_claim.is_strong_qc, is_strong_qc ); - 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_qVbh4IjYZpRGo8U_0spBUM-u-r_G0fMo4MzLZRsKWmm5uyeQTp74YFaMN9IDWPoVVT5rj_Tw1gvps6K9_OZ6sabkJJzug3uGfjA6qiaLbLh5Fnafwv-nVgzzzBlU2kwRrcHc8Q"); + BOOST_REQUIRE( !!if_extension.new_finalizer_policy_diff ); + BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->generation, 1u); + BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->threshold, 100u); + BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->finalizers_diff.insert_indexes[0].second.description, "test description"); + BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->finalizers_diff.insert_indexes[0].second.weight, 50u); + BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->finalizers_diff.insert_indexes[0].second.public_key.to_string(), "PUB_BLS_qVbh4IjYZpRGo8U_0spBUM-u-r_G0fMo4MzLZRsKWmm5uyeQTp74YFaMN9IDWPoVVT5rj_Tw1gvps6K9_OZ6sabkJJzug3uGfjA6qiaLbLh5Fnafwv-nVgzzzBlU2kwRrcHc8Q"); BOOST_REQUIRE( !!if_extension.new_proposer_policy ); BOOST_REQUIRE_EQUAL(if_extension.new_proposer_policy->schema_version, 1u); diff --git a/unittests/finality_test_cluster.cpp b/unittests/finality_test_cluster.cpp index 0a49e0fba3..c1a9ce2064 100644 --- a/unittests/finality_test_cluster.cpp +++ b/unittests/finality_test_cluster.cpp @@ -35,10 +35,10 @@ void finality_test_cluster::initial_tests(){ // this block contains the header exten/sion for the instant finality std::optional ext = block_1_n0->extract_header_extension(eosio::chain::instant_finality_extension::extension_id()); BOOST_TEST(!!ext); - std::optional fin_policy = std::get(*ext).new_finalizer_policy; - BOOST_TEST(!!fin_policy); - BOOST_TEST(fin_policy->finalizers.size() == 3); - BOOST_TEST(fin_policy->generation == 1); + std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; + BOOST_TEST(!!fin_policy_diff); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == 3); + BOOST_TEST(fin_policy_diff->generation == 1); produce_and_push_block(); // make setfinalizer irreversible diff --git a/unittests/finalizer_update_tests.cpp b/unittests/finalizer_update_tests.cpp index 17cdd1666c..6d52b4dbd8 100644 --- a/unittests/finalizer_update_tests.cpp +++ b/unittests/finalizer_update_tests.cpp @@ -179,24 +179,43 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { auto b2 = t.produce_block(); auto pubkeys5 = t.set_active_finalizers({&finalizers[5], finset_size}); t.produce_blocks(2); + auto pubkeys6 = t.set_active_finalizers({&finalizers[6], finset_size}); b5 = t.produce_block(); + auto pubkeys7 = t.set_active_finalizers({&finalizers[7], finset_size}); check_finalizer_policy(t, b5, 2, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active b6 = t.produce_block(); + auto pubkeys8 = t.set_active_finalizers({&finalizers[8], finset_size}); check_finalizer_policy(t, b6, 3, pubkeys3); // 6 blocks after pubkeys3 (b6 - b0), pubkeys3 should be active auto b7 = t.produce_block(); + auto pubkeys9 = t.set_active_finalizers({&finalizers[9], finset_size}); check_finalizer_policy(t, b7, 4, pubkeys4); // 6 blocks after pubkeys4 (b7 - b1), pubkeys4 should be active auto b8 = t.produce_block(); + auto pubkeys10 = t.set_active_finalizers({&finalizers[10], finset_size}); check_finalizer_policy(t, b8, 4, pubkeys4); // 7 blocks after pubkeys4, pubkeys4 should still be active auto b9 = t.produce_block(); + auto pubkeys11 = t.set_active_finalizers({&finalizers[11], finset_size}); check_finalizer_policy(t, b9, 5, pubkeys5); // 6 blocks after pubkeys5 (b9 - b3), pubkeys5 should be active + auto b10 = t.produce_block(); + auto b11 = t.produce_block(); // two blocks between 5 & 6 proposals + check_finalizer_policy(t, b11, 6, pubkeys6); // the rest are all one block apart, tests pending with propsed + auto b12 = t.produce_block(); + check_finalizer_policy(t, b12, 7, pubkeys7); + auto b13 = t.produce_block(); + check_finalizer_policy(t, b13, 8, pubkeys8); + auto b14 = t.produce_block(); + check_finalizer_policy(t, b14, 9, pubkeys9); + auto b15 = t.produce_block(); + check_finalizer_policy(t, b15, 10, pubkeys10); + auto b16 = t.produce_block(); + check_finalizer_policy(t, b16, 11, pubkeys11); // and no further change - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); + ensure_next_block_finalizer_policy(t, 11, pubkeys11); + ensure_next_block_finalizer_policy(t, 11, pubkeys11); + ensure_next_block_finalizer_policy(t, 11, pubkeys11); + ensure_next_block_finalizer_policy(t, 11, pubkeys11); + ensure_next_block_finalizer_policy(t, 11, pubkeys11); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file diff --git a/unittests/protocol_feature_tests.cpp b/unittests/protocol_feature_tests.cpp index 7557c163ae..3893e46477 100644 --- a/unittests/protocol_feature_tests.cpp +++ b/unittests/protocol_feature_tests.cpp @@ -1110,9 +1110,9 @@ BOOST_AUTO_TEST_CASE( protocol_activatation_works_after_transition_to_savanna ) std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); BOOST_TEST(!!ext); - std::optional fin_policy = std::get(*ext).new_finalizer_policy; - BOOST_TEST(!!fin_policy); - BOOST_TEST(fin_policy->finalizers.size() == accounts.size()); + std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; + BOOST_TEST(!!fin_policy_diff); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == accounts.size()); block = c.produce_block(); // savanna now active auto fb = c.control->fetch_block_by_id(block->calculate_id()); diff --git a/unittests/svnn_ibc_tests.cpp b/unittests/svnn_ibc_tests.cpp index b243166f02..d879e88a06 100644 --- a/unittests/svnn_ibc_tests.cpp +++ b/unittests/svnn_ibc_tests.cpp @@ -95,10 +95,10 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) BOOST_CHECK(std::holds_alternative(genesis_if_ext)); // and that it has the expected initial finalizer_policy - std::optional maybe_active_finalizer_policy = std::get(genesis_if_ext).new_finalizer_policy; - - BOOST_CHECK(maybe_active_finalizer_policy.has_value()); + std::optional maybe_active_finalizer_policy_diff = std::get(genesis_if_ext).new_finalizer_policy_diff; + BOOST_CHECK(maybe_active_finalizer_policy_diff.has_value()); +/** TODO diff or full active policy ? eosio::chain::finalizer_policy active_finalizer_policy = maybe_active_finalizer_policy.value(); BOOST_CHECK(active_finalizer_policy.finalizers.size() == 3); @@ -352,7 +352,7 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) // verify action has failed, as expected BOOST_CHECK(failed); - +*/ } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp index 85c2ca0598..99f3cec753 100644 --- a/unittests/vote_processor_tests.cpp +++ b/unittests/vote_processor_tests.cpp @@ -42,13 +42,14 @@ auto create_genesis_block_state() { // block 2 std::vector finalizers; finalizers.push_back(finalizer_authority{.description = "first", .weight = 1, .public_key = bls_priv_keys.at(0).get_public_key()}); - finalizers.push_back(finalizer_authority{.description = "first", .weight = 1, .public_key = bls_priv_keys.at(1).get_public_key()}); - finalizers.push_back(finalizer_authority{.description = "first", .weight = 1, .public_key = bls_priv_keys.at(2).get_public_key()}); + finalizers.push_back(finalizer_authority{.description = "second", .weight = 1, .public_key = bls_priv_keys.at(1).get_public_key()}); + finalizers.push_back(finalizer_authority{.description = "third", .weight = 1, .public_key = bls_priv_keys.at(2).get_public_key()}); finalizer_policy new_finalizer_policy{.finalizers = finalizers}; + finalizer_policy_diff new_finalizer_policy_diff = finalizer_policy{}.create_diff(new_finalizer_policy); qc_claim_t initial_if_claim { .block_num = 2, .is_strong_qc = false }; emplace_extension(block->header_extensions, instant_finality_extension::extension_id(), - fc::raw::pack(instant_finality_extension{ initial_if_claim, new_finalizer_policy, {} })); + fc::raw::pack(instant_finality_extension{ initial_if_claim, new_finalizer_policy_diff, {} })); producer_authority_schedule schedule = { 0, { producer_authority{block->producer, block_signing_authority_v0{ 1, {{pub_key, 1}} } } } }; auto genesis = std::make_shared(); From 8000be2e2d245812f98b6be2fb7f400ae16c0ca8 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 May 2024 16:25:40 -0500 Subject: [PATCH 05/41] GH-5 Fix include --- libraries/libfc/include/fc/container/ordered_diff.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/libfc/include/fc/container/ordered_diff.hpp b/libraries/libfc/include/fc/container/ordered_diff.hpp index ed6523d9e8..5d0ac7ed07 100644 --- a/libraries/libfc/include/fc/container/ordered_diff.hpp +++ b/libraries/libfc/include/fc/container/ordered_diff.hpp @@ -1,7 +1,7 @@ #pragma once #include -#include +#include namespace fc { From 6397cbd8466c403d5449ff642228bcc8c2c1f3d6 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 May 2024 19:23:16 -0500 Subject: [PATCH 06/41] GH-5 shared_ptr not needed --- .../chain/include/eosio/chain/block_header_state.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index cd89d3338b..174356e3da 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -134,18 +134,18 @@ struct block_header_state { if (finalizer_policies.empty()) { return active_finalizer_policy->create_diff(new_policy); } - finalizer_policy_ptr fin_policy_ptr = std::make_shared(*active_finalizer_policy); + finalizer_policy fin_policy = *active_finalizer_policy; for (const auto& e : finalizer_policies) { if (e.second.state == finalizer_policy_tracker::state_t::pending) { - fin_policy_ptr->apply_diff(e.second.policy_diff); + fin_policy.apply_diff(e.second.policy_diff); } } for (const auto& e : finalizer_policies) { if (e.second.state == finalizer_policy_tracker::state_t::proposed) { - fin_policy_ptr->apply_diff(e.second.policy_diff); + fin_policy.apply_diff(e.second.policy_diff); } } - return fin_policy_ptr->create_diff(new_policy); + return fin_policy.create_diff(new_policy); } }; From 16628b8692e04fb32eb9e44fd40ca4746d2e850e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 May 2024 19:23:36 -0500 Subject: [PATCH 07/41] GH-5 Fix merge issue --- libraries/testing/include/eosio/testing/tester.hpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 6a26539cfb..1bcc1e9c2a 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -817,11 +817,11 @@ namespace eosio::testing { // Do some sanity checks on the genesis block // ------------------------------------------ const auto& ext = genesis_block->template extract_header_extension(); - const auto& fin_policy = ext.new_finalizer_policy; - BOOST_TEST(!!fin_policy); - BOOST_TEST(fin_policy->finalizers.size() == fin_policy_size); - BOOST_TEST(fin_policy->generation == 1); - BOOST_TEST(fin_policy->threshold == (fin_policy_size * 2) / 3 + 1); + const auto& fin_policy_diff = ext.new_finalizer_policy_diff; + BOOST_TEST(!!fin_policy_diff); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == fin_policy_size); + BOOST_TEST(fin_policy_diff->generation == 1); + BOOST_TEST(fin_policy_diff->threshold == (fin_policy_size * 2) / 3 + 1); // wait till the genesis_block becomes irreversible. // The critical block is the block that makes the genesis_block irreversible @@ -850,7 +850,9 @@ namespace eosio::testing { auto b = produce_block(); BOOST_REQUIRE_EQUAL(t.lib_block->block_num(), pt_block->block_num()); - return *fin_policy; + finalizer_policy fin_policy; + fin_policy.apply_diff(*fin_policy_diff); + return fin_policy; } Tester& t; From 44006962c2bbdf0e4a574838e05bd4080ce73c6c Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 10:54:56 -0400 Subject: [PATCH 08/41] Update libtest `set_finalizers` to also return the trace. --- libraries/chain/controller.cpp | 2 + .../testing/include/eosio/testing/tester.hpp | 22 +++++----- libraries/testing/tester.cpp | 40 +++++++++---------- unittests/finality_test_cluster.hpp | 2 +- unittests/finality_tests.cpp | 2 +- unittests/finalizer_update_tests.cpp | 14 +++---- unittests/forked_tests.cpp | 2 +- 7 files changed, 43 insertions(+), 41 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 56f892e6cc..777743bbaf 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -2942,6 +2942,8 @@ struct controller_impl { handle_exception(wrapper); } + // this code is hit if an exception was thrown, and handled by `handle_exception` + // ------------------------------------------------------------------------------ if (!trx->is_transient()) { dmlog_applied_transaction(trace); emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 6a26539cfb..7f154f1b54 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -147,7 +147,7 @@ namespace eosio::testing { struct produce_block_result_t { signed_block_ptr block; transaction_trace_ptr onblock_trace; - std::vector traces; // transaction traces + std::vector unapplied_transaction_traces; // unapplied from previous `push_block` not on head branch }; /** @@ -271,19 +271,23 @@ namespace eosio::testing { transaction_trace_ptr set_producer_schedule(const vector& schedule); transaction_trace_ptr set_producers_legacy(const vector& producer_names); + struct set_finalizers_output_t { + transaction_trace_ptr setfinalizer_trace; + std::vector privkeys; // private keys of **local** finalizers + std::vector pubkeys; // public keys of all finalizers in the policy + }; + // libtester uses 1 as weight of each of the finalizer, sets (2/3 finalizers + 1) // as threshold, and makes all finalizers vote QC - std::pair> - set_finalizers(std::span finalizer_names); + set_finalizers_output_t set_finalizers(std::span finalizer_names); - std::pair> - set_finalizers(const std::vector& names) { + set_finalizers_output_t set_finalizers(const std::vector& names) { return set_finalizers(std::span{names.begin(), names.end()}); } void set_node_finalizers(std::span finalizer_names); - std::vector set_active_finalizers(std::span finalizer_names); + set_finalizers_output_t set_active_finalizers(std::span finalizer_names); // Finalizer policy input to set up a test: weights, threshold and local finalizers // which participate voting. @@ -297,7 +301,7 @@ namespace eosio::testing { uint64_t threshold {0}; std::vector local_finalizers; }; - std::pair> set_finalizers(const finalizer_policy_input& input); + set_finalizers_output_t set_finalizers(const finalizer_policy_input& input); std::optional active_finalizer_policy(const block_id_type& id) const { return control->active_finalizer_policy(id); @@ -782,11 +786,11 @@ namespace eosio::testing { // updates the finalizer_policy to the `fin_policy_size` keys starting at `first_key` // ---------------------------------------------------------------------------------- - std::vector set_finalizer_policy(size_t first_key) { + base_tester::set_finalizers_output_t set_finalizer_policy(size_t first_key) { return t.set_active_finalizers({&key_names.at(first_key), fin_policy_size}); } - std::vector set_finalizer_policy(std::span indices) { + base_tester::set_finalizers_output_t set_finalizer_policy(std::span indices) { assert(indices.size() == fin_policy_size); vector names; names.reserve(fin_policy_size); diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 32f5cd8790..c5c3944a31 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -413,14 +413,14 @@ namespace eosio::testing { trace->except->dynamic_rethrow_exception(); } itr = unapplied_transactions.erase( itr ); - res.traces.emplace_back( std::move(trace) ); + res.unapplied_transaction_traces.emplace_back( std::move(trace) ); } vector scheduled_trxs; while ((scheduled_trxs = get_scheduled_transactions()).size() > 0 ) { for( const auto& trx : scheduled_trxs ) { auto trace = control->push_scheduled_transaction( trx, fc::time_point::maximum(), fc::microseconds::maximum(), DEFAULT_BILLED_CPU_TIME_US, true ); - res.traces.emplace_back( trace ); + res.unapplied_transaction_traces.emplace_back( trace ); if( !no_throw && trace->except ) { // this always throws an fc::exception, since the original exception is copied into an fc::exception trace->except->dynamic_rethrow_exception(); @@ -1203,8 +1203,7 @@ namespace eosio::testing { } - std::pair> - base_tester::set_finalizers(std::span finalizer_names) { + base_tester::set_finalizers_output_t base_tester::set_finalizers(std::span finalizer_names) { auto num_finalizers = finalizer_names.size(); std::vector finalizers_info; finalizers_info.reserve(num_finalizers); @@ -1221,11 +1220,11 @@ namespace eosio::testing { return set_finalizers(policy_input); } - std::pair> - base_tester::set_finalizers(const finalizer_policy_input& input) { + base_tester::set_finalizers_output_t base_tester::set_finalizers(const finalizer_policy_input& input) { + set_finalizers_output_t res; + chain::bls_pub_priv_key_map_t local_finalizer_keys; fc::variants finalizer_auths; - std::vector priv_keys; for (const auto& f: input.finalizers) { auto [privkey, pubkey, pop] = get_bls_key( f.name ); @@ -1234,9 +1233,11 @@ namespace eosio::testing { if( auto it = std::ranges::find_if(input.local_finalizers, [&](const auto& name) { return name == f.name; }); it != input.local_finalizers.end()) { local_finalizer_keys[pubkey.to_string()] = privkey.to_string(); - priv_keys.emplace_back(privkey); + res.privkeys.emplace_back(privkey); }; + res.pubkeys.emplace_back(pubkey); + finalizer_auths.emplace_back( fc::mutable_variant_object() ("description", f.name.to_string() + " description") @@ -1251,14 +1252,14 @@ namespace eosio::testing { fin_policy_variant("threshold", input.threshold); fin_policy_variant("finalizers", std::move(finalizer_auths)); - return { push_action( config::system_account_name, "setfinalizer"_n, config::system_account_name, - fc::mutable_variant_object()("finalizer_policy", std::move(fin_policy_variant))), - priv_keys }; + res.setfinalizer_trace = + push_action( config::system_account_name, "setfinalizer"_n, config::system_account_name, + fc::mutable_variant_object()("finalizer_policy", std::move(fin_policy_variant))); + return res; } void base_tester::set_node_finalizers(std::span names) { - - chain::bls_pub_priv_key_map_t local_finalizer_keys; + bls_pub_priv_key_map_t local_finalizer_keys; for (auto name: names) { auto [privkey, pubkey, pop] = get_bls_key(name); local_finalizer_keys[pubkey.to_string()] = privkey.to_string(); @@ -1266,20 +1267,15 @@ namespace eosio::testing { control->set_node_finalizer_keys(local_finalizer_keys); } - std::vector base_tester::set_active_finalizers(std::span names) { - std::vector pubkeys; - pubkeys.reserve(names.size()); + base_tester::set_finalizers_output_t base_tester::set_active_finalizers(std::span names) { finalizer_policy_input input; input.finalizers.reserve(names.size()); - for (auto name : names) { - auto [privkey, pubkey, pop] = get_bls_key(name); - pubkeys.push_back(pubkey); + for (auto name : names) input.finalizers.emplace_back(name, 1); - } + // same as reference-contracts/.../contracts/eosio.system/src/finalizer_key.cpp#L73 input.threshold = (names.size() * 2) / 3 + 1; - set_finalizers(input); - return pubkeys; + return set_finalizers(input); } const table_id_object* base_tester::find_table( name code, name scope, name table ) { diff --git a/unittests/finality_test_cluster.hpp b/unittests/finality_test_cluster.hpp index 0491189d75..dbb8613977 100644 --- a/unittests/finality_test_cluster.hpp +++ b/unittests/finality_test_cluster.hpp @@ -155,7 +155,7 @@ class finality_test_cluster { // ---------------------------- for (size_t i=0; i) { try { auto indices1 = fin_policy_indices_0; // start from original set of indices assert(indices1[0] == 0); // we used index 0 for node0 in original policy indices1[0] = 1; // update key used for node0 in policy - auto pubkeys1 = node0.finkeys.set_finalizer_policy(indices1); + auto pubkeys1 = node0.finkeys.set_finalizer_policy(indices1).pubkeys; // we need two 3-chains for the new finalizer policy to be activated for (size_t i=0; i<6; ++i) { diff --git a/unittests/finalizer_update_tests.cpp b/unittests/finalizer_update_tests.cpp index 2dd81d613f..44daf1728a 100644 --- a/unittests/finalizer_update_tests.cpp +++ b/unittests/finalizer_update_tests.cpp @@ -42,12 +42,12 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_single_test) { try { fin_keys.set_node_finalizers(0, num_keys); // run initial set_finalizer_policy() and waits until transition is complete - auto pubkeys0 = fin_keys.set_finalizer_policy(0); + auto pubkeys0 = fin_keys.set_finalizer_policy(0).pubkeys; fin_keys.transition_to_savanna(); // run set_finalizers(), verify it becomes active after exactly two 3-chains // ------------------------------------------------------------------------- - auto pubkeys1 = fin_keys.set_finalizer_policy(1); + auto pubkeys1 = fin_keys.set_finalizer_policy(1).pubkeys; t.produce_block(); t.check_head_finalizer_policy(1, pubkeys0); // new policy should only be active until after two 3-chains @@ -78,13 +78,13 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { fin_keys.set_node_finalizers(0, num_keys); // run initial set_finalizer_policy() and waits until transition is complete - auto pubkeys0 = fin_keys.set_finalizer_policy(0); + auto pubkeys0 = fin_keys.set_finalizer_policy(0).pubkeys; fin_keys.transition_to_savanna(); // run set_finalizers() twice in same block, verify only latest one becomes active // ------------------------------------------------------------------------------- (void)fin_keys.set_finalizer_policy(1); - auto pubkeys2 = fin_keys.set_finalizer_policy(2); + auto pubkeys2 = fin_keys.set_finalizer_policy(2).pubkeys; t.produce_block(); t.check_head_finalizer_policy(1, pubkeys0); // new policy should only be active until after two 3-chains t.produce_blocks(5); @@ -95,12 +95,12 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { // run a test with multiple set_finlizers in-flight during the two 3-chains they // take to become active // ----------------------------------------------------------------------------- - auto pubkeys3 = fin_keys.set_finalizer_policy(3); + auto pubkeys3 = fin_keys.set_finalizer_policy(3).pubkeys; t.produce_block(); - auto pubkeys4 = fin_keys.set_finalizer_policy(4); + auto pubkeys4 = fin_keys.set_finalizer_policy(4).pubkeys; t.produce_block(); t.produce_block(); - auto pubkeys5 = fin_keys.set_finalizer_policy(5); + auto pubkeys5 = fin_keys.set_finalizer_policy(5).pubkeys; t.produce_blocks(3); t.check_head_finalizer_policy(2, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active t.produce_block(); diff --git a/unittests/forked_tests.cpp b/unittests/forked_tests.cpp index 846f84ec93..b9f19a25a0 100644 --- a/unittests/forked_tests.cpp +++ b/unittests/forked_tests.cpp @@ -751,7 +751,7 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { // produce block which will apply the unapplied transactions produce_block_result_t produce_block_result = c.produce_block_ex(fc::milliseconds(config::block_interval_ms), true); - std::vector& traces = produce_block_result.traces; + std::vector& traces = produce_block_result.unapplied_transaction_traces; BOOST_REQUIRE_EQUAL( 4u, traces.size() ); BOOST_CHECK_EQUAL( trace1->id, traces.at(0)->id ); From 92a21962db0144c2174e1f66d308f72123382f08 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 11:14:17 -0400 Subject: [PATCH 09/41] Minor code reorg. --- .../testing/include/eosio/testing/tester.hpp | 43 +++++++++++-------- libraries/testing/tester.cpp | 3 +- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 7f154f1b54..01ec744c74 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -271,24 +271,6 @@ namespace eosio::testing { transaction_trace_ptr set_producer_schedule(const vector& schedule); transaction_trace_ptr set_producers_legacy(const vector& producer_names); - struct set_finalizers_output_t { - transaction_trace_ptr setfinalizer_trace; - std::vector privkeys; // private keys of **local** finalizers - std::vector pubkeys; // public keys of all finalizers in the policy - }; - - // libtester uses 1 as weight of each of the finalizer, sets (2/3 finalizers + 1) - // as threshold, and makes all finalizers vote QC - set_finalizers_output_t set_finalizers(std::span finalizer_names); - - set_finalizers_output_t set_finalizers(const std::vector& names) { - return set_finalizers(std::span{names.begin(), names.end()}); - } - - void set_node_finalizers(std::span finalizer_names); - - set_finalizers_output_t set_active_finalizers(std::span finalizer_names); - // Finalizer policy input to set up a test: weights, threshold and local finalizers // which participate voting. struct finalizer_policy_input { @@ -301,8 +283,33 @@ namespace eosio::testing { uint64_t threshold {0}; std::vector local_finalizers; }; + + struct set_finalizers_output_t { + transaction_trace_ptr setfinalizer_trace; + std::vector privkeys; // private keys of **local** finalizers + std::vector pubkeys; // public keys of all finalizers in the policy + }; + set_finalizers_output_t set_finalizers(const finalizer_policy_input& input); + void set_node_finalizers(std::span finalizer_names); + + set_finalizers_output_t set_active_finalizers(std::span finalizer_names); + + // Useful when using a single node. + // Set a finalizer policy with a few finalizers, all local to the current node. + // All have weight == 1, threshold is `num_finalizers * 2 / 3 + 1` + // ----------------------------------------------------------------------------- + set_finalizers_output_t set_finalizers(std::span finalizer_names); + + // Useful when using a single node. + // Set a finalizer policy with a few finalizers, all local to the current node. + // All have weight == 1, threshold is `num_finalizers * 2 / 3 + 1` + // ----------------------------------------------------------------------------- + set_finalizers_output_t set_finalizers(const std::vector& names) { + return set_finalizers(std::span{names.begin(), names.end()}); + } + std::optional active_finalizer_policy(const block_id_type& id) const { return control->active_finalizer_policy(id); } diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index c5c3944a31..270456c7c1 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -1161,6 +1161,7 @@ namespace eosio::testing { vector base_tester::get_producer_authorities( const vector& producer_names )const { // Create producer schedule vector schedule; + schedule.reserve(producer_names.size()); for (auto& producer_name: producer_names) { schedule.emplace_back(producer_authority{ producer_name, block_signing_authority_v0{1, {{ get_public_key( producer_name, "active" ), 1}} } }); } @@ -1183,7 +1184,6 @@ namespace eosio::testing { return push_action( config::system_account_name, "setprods"_n, config::system_account_name, fc::mutable_variant_object()("schedule", schedule_variant)); - } transaction_trace_ptr base_tester::set_producers_legacy(const vector& producer_names) { @@ -1200,7 +1200,6 @@ namespace eosio::testing { return push_action( config::system_account_name, "setprods"_n, config::system_account_name, fc::mutable_variant_object()("schedule", legacy_keys)); - } base_tester::set_finalizers_output_t base_tester::set_finalizers(std::span finalizer_names) { From bc78d46ad401d92de0ea57ea9ad485bf9a2e16ee Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 May 2024 12:47:50 -0500 Subject: [PATCH 10/41] GH-5 Reduce max by one to equal uint16_t max --- libraries/chain/include/eosio/chain/config.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index 74af7b59ca..c82d6c8c05 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -134,7 +134,7 @@ static_assert(maximum_tracked_dpos_confirmations >= ((max_producers * 2 / 3) + 1 /** * Maximum number of finalizers in the finalizer set */ -const static size_t max_finalizers = 64*1024; +const static size_t max_finalizers = 65535; // largest allowed finalizer policy diff const static size_t max_finalizer_description_size = 256; /** From ad7e9a7d4b711bc5df2a863cda1fcfe6a541f6e3 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 May 2024 12:49:28 -0500 Subject: [PATCH 11/41] GH-5 Make the index type templated to save space in packed diff_result. Added asserts to check for overflow. --- .../include/fc/container/ordered_diff.hpp | 36 ++++++++----- libraries/libfc/test/test_ordered_diff.cpp | 52 +++++++++++++++++-- 2 files changed, 71 insertions(+), 17 deletions(-) diff --git a/libraries/libfc/include/fc/container/ordered_diff.hpp b/libraries/libfc/include/fc/container/ordered_diff.hpp index 5d0ac7ed07..31437261c3 100644 --- a/libraries/libfc/include/fc/container/ordered_diff.hpp +++ b/libraries/libfc/include/fc/container/ordered_diff.hpp @@ -7,7 +7,7 @@ namespace fc { /** * @class ordered_diff - * @brief Provides ablity to generate and apply diff of containers of type T + * @brief Provides ability to generate and apply diff of containers of type T * * Example use: * std::vector source = { 'a', 'b', 'f', 'c', 'd' }; @@ -17,17 +17,19 @@ namespace fc { * assert(source == target); * * @param T type stored in Container, must provide == + * @param SizeType numeric type used for index into diff_result, for non-unique Containers a larger type may be required + * @param Container container type for ordered diff and for diff_result */ -template typename Container = std::vector> +template typename Container = std::vector> requires std::equality_comparable && std::random_access_iterator::iterator> class ordered_diff { public: struct diff_result { - Container remove_indexes; - Container> insert_indexes; + Container remove_indexes; + Container> insert_indexes; }; - /// Generate diff_result that when `apply_diff(source, diff_result)` to source will generate target + /// Generate diff_result that when `apply_diff(source, diff_result)` will modify source to be equal to target. static diff_result diff(const Container& source, const Container& target) { size_t s = 0; size_t t = 0; @@ -37,37 +39,47 @@ class ordered_diff { if (s < source.size() && t < target.size()) { if (source[s] == target[t]) { // nothing to do, skip over + assert(s <= std::numeric_limits::max()); + assert(t <= std::numeric_limits::max()); ++s; ++t; } else { // not equal if (s == source.size() - 1 && t == target.size() - 1) { // both at end, insert target and remove source + assert(s <= std::numeric_limits::max()); + assert(t <= std::numeric_limits::max()); result.remove_indexes.push_back(s); result.insert_indexes.emplace_back(t, target[t]); ++s; ++t; } else if (s + 1 < source.size() && t + 1 < target.size() && source[s + 1] == target[t + 1]) { // misalignment, but next value equal, insert and remove + assert(s <= std::numeric_limits::max()); + assert(t <= std::numeric_limits::max()); result.remove_indexes.push_back(s); result.insert_indexes.emplace_back(t, target[t]); ++s; ++t; } else if (t + 1 < target.size() && source[s] == target[t + 1]) { // source equals next target, insert current target + assert(t <= std::numeric_limits::max()); result.insert_indexes.emplace_back(t, target[t]); ++t; } else { // source[s + 1] == target[t] // target matches next source, remove current source + assert(s <= std::numeric_limits::max()); result.remove_indexes.push_back(s); ++s; } } } else if (s < source.size()) { // remove extra in source + assert(s <= std::numeric_limits::max()); result.remove_indexes.push_back(s); ++s; } else if (t < target.size()) { // insert extra in target + assert(t <= std::numeric_limits::max()); result.insert_indexes.emplace_back(t, target[t]); ++t; } @@ -76,21 +88,21 @@ class ordered_diff { return result; } + /// @param diff the diff_result created from diff(source, target), apply_diff(source, diff_result) => target + /// @param container the source of diff(source, target) to modify using the diff_result to produce original target template requires std::same_as, diff_result> - static void apply_diff(Container& source, X&& diff) { + static void apply_diff(Container& container, X&& diff) { // Remove from the source based on diff.remove_indexes std::ptrdiff_t offset = 0; - for (size_t index : diff.remove_indexes) { - source.erase(source.begin() + index + offset); + for (SizeType index : diff.remove_indexes) { + container.erase(container.begin() + index + offset); --offset; } // Insert into the source based on diff.insert_indexes - for (auto& t : diff.insert_indexes) { - size_t index = std::get<0>(t); - auto& value = std::get<1>(t); - source.insert(source.begin() + index, std::move(value)); + for (auto& [index, value] : diff.insert_indexes) { + container.insert(container.begin() + index, std::move(value)); } } diff --git a/libraries/libfc/test/test_ordered_diff.cpp b/libraries/libfc/test/test_ordered_diff.cpp index 2ec54b1650..23b9c2035f 100644 --- a/libraries/libfc/test/test_ordered_diff.cpp +++ b/libraries/libfc/test/test_ordered_diff.cpp @@ -18,7 +18,7 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { BOOST_TEST(source == target); } { // Basic case, deque - using ordered_deque_char_diff = ordered_diff; + using ordered_deque_char_diff = ordered_diff; deque source = {'a', 'x', 'c', 'd', 'e'}; deque target = {'z', 'c', 'y', 'f'}; auto result = ordered_deque_char_diff::diff(source, target); @@ -28,15 +28,15 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { { // Empty vectors vector source; vector target; - ordered_diff::diff_result result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + ordered_diff::diff_result result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); BOOST_TEST(source == target); } { // All elements removed vector source = {'a', 'b', 'c', 'd', 'e'}; vector target; - auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); BOOST_TEST(source == target); } { // All elements inserted @@ -95,6 +95,48 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { ordered_diff::apply_diff(source, result); BOOST_TEST(source == target); } + { // non-unique + vector source = {'a', 'b', 'c', 'd', 'e', 'c', 'a', 'q'}; + vector target = {'z', 'a', 'b', 'c', 'd', 'a'}; + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // full + vector source(std::numeric_limits::max()+1); + std::iota(source.begin(), source.end(), 0); + vector target(source.size()); + std::reverse_copy(source.begin(), source.end(), target.begin()); + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + target.clear(); + result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + source.clear(); + result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } + { // non-unique full + vector source(std::numeric_limits::max()*2); + std::iota(source.begin(), source.begin()+std::numeric_limits::max(), 0); + std::iota(source.begin()+std::numeric_limits::max(), source.end(), 0); + vector target(source.size()); + std::reverse_copy(source.begin(), source.end(), target.begin()); + auto result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + target.clear(); + result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + source.clear(); + result = ordered_diff::diff(source, target); + ordered_diff::apply_diff(source, result); + BOOST_TEST(source == target); + } } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(ordered_diff_string_test) try { From 7495f2aa759c144fdfc3cd86a912fc860539a49d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 May 2024 12:50:08 -0500 Subject: [PATCH 12/41] GH-5 Use uint16_t for index type of diff since max_finalizers fit --- .../chain/include/eosio/chain/finality/finalizer_policy.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp index 64089fa950..d0df6383dd 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp @@ -6,8 +6,9 @@ namespace eosio::chain { - using finalizers_differ = fc::ordered_diff; - using finalizers_diff_t = fc::ordered_diff::diff_result; + static_assert(std::numeric_limits::max() <= config::max_finalizers); + using finalizers_differ = fc::ordered_diff; + using finalizers_diff_t = finalizers_differ::diff_result; struct finalizer_policy_diff { uint32_t generation = 0; ///< sequentially incrementing version number From 8456ea5ac12fdbc9a3d3433d59414f4e4dd586ee Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 May 2024 12:50:30 -0500 Subject: [PATCH 13/41] GH-5 Add assert, update log message --- libraries/chain/block_header_state.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index e89b9adc32..494e668a92 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -135,6 +135,7 @@ void finish_next(const block_header_state& prev, if (tracker.state == finalizer_policy_tracker::state_t::pending) { // new finalizer_policy becones active // ----------------------------------- + assert(prev.active_finalizer_policy); next_header_state.active_finalizer_policy.reset(new finalizer_policy(*prev.active_finalizer_policy)); next_header_state.active_finalizer_policy->apply_diff(tracker.policy_diff); } else { @@ -162,7 +163,7 @@ void finish_next(const block_header_state& prev, // Add this new proposal to the `finalizer_policies` multimap which tracks the in-flight proposals, // increment the generation number, and log that proposal (debug level). // ------------------------------------------------------------------------------------------------ - dlog("New finalizer policy proposed in block ${id}: ${pol}", + dlog("New finalizer policy proposed in block ${id}, diff: ${pol}", ("id", prev.block_id)("pol", *if_ext.new_finalizer_policy_diff)); next_header_state.finalizer_policy_generation = if_ext.new_finalizer_policy_diff->generation; next_header_state.finalizer_policies.emplace( From 0fd8e5cc278a47815391623f488316b79afcabef Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 8 May 2024 14:22:54 -0400 Subject: [PATCH 14/41] fix a couple different signedness comparison warnings --- libraries/testing/include/eosio/testing/tester.hpp | 2 +- unittests/svnn_ibc_tests.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 6a26539cfb..e152b05a13 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -820,7 +820,7 @@ namespace eosio::testing { const auto& fin_policy = ext.new_finalizer_policy; BOOST_TEST(!!fin_policy); BOOST_TEST(fin_policy->finalizers.size() == fin_policy_size); - BOOST_TEST(fin_policy->generation == 1); + BOOST_TEST(fin_policy->generation == 1u); BOOST_TEST(fin_policy->threshold == (fin_policy_size * 2) / 3 + 1); // wait till the genesis_block becomes irreversible. diff --git a/unittests/svnn_ibc_tests.cpp b/unittests/svnn_ibc_tests.cpp index 60ee4062ca..27b61a8185 100644 --- a/unittests/svnn_ibc_tests.cpp +++ b/unittests/svnn_ibc_tests.cpp @@ -84,7 +84,7 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) auto genesis_block = cluster.produce_and_push_block(); // ensure out of scope setup and initial cluster wiring is consistent - BOOST_CHECK_EQUAL(genesis_block->block_num(), 4); + BOOST_CHECK_EQUAL(genesis_block->block_num(), 4u); // check if IF Genesis block contains an IF extension std::optional maybe_genesis_if_ext = @@ -105,7 +105,7 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) eosio::chain::finalizer_policy active_finalizer_policy = maybe_active_finalizer_policy.value(); BOOST_CHECK_EQUAL(active_finalizer_policy.finalizers.size(), cluster.num_nodes); - BOOST_CHECK_EQUAL(active_finalizer_policy.generation, 1); + BOOST_CHECK_EQUAL(active_finalizer_policy.generation, 1u); // compute the digest of the finalizer policy auto active_finalizer_policy_digest = fc::sha256::hash(active_finalizer_policy); From 622d2317a7900ef4e4eb736f3699a2e4678dfc09 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 14:39:33 -0400 Subject: [PATCH 15/41] Remove incorrect line. --- libraries/testing/tester.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 270456c7c1..2f3cd42636 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -420,7 +420,6 @@ namespace eosio::testing { while ((scheduled_trxs = get_scheduled_transactions()).size() > 0 ) { for( const auto& trx : scheduled_trxs ) { auto trace = control->push_scheduled_transaction( trx, fc::time_point::maximum(), fc::microseconds::maximum(), DEFAULT_BILLED_CPU_TIME_US, true ); - res.unapplied_transaction_traces.emplace_back( trace ); if( !no_throw && trace->except ) { // this always throws an fc::exception, since the original exception is copied into an fc::exception trace->except->dynamic_rethrow_exception(); From 6a5d0ab5da82dcc584e6d616829d776414dfdc18 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 14:59:10 -0400 Subject: [PATCH 16/41] Use unsigned litterals. --- unittests/finalizer_update_tests.cpp | 64 ++++++++++++++-------------- 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/unittests/finalizer_update_tests.cpp b/unittests/finalizer_update_tests.cpp index 44daf1728a..0f5d009221 100644 --- a/unittests/finalizer_update_tests.cpp +++ b/unittests/finalizer_update_tests.cpp @@ -32,33 +32,33 @@ static void ensure_next_block_finalizer_policy(validating_tester& t, // --------------------------------------------------------------------- BOOST_AUTO_TEST_CASE(savanna_set_finalizer_single_test) { try { validating_tester t; - size_t num_keys = 22; - size_t finset_size = 21; + size_t num_keys = 22u; + size_t finset_size = 21u; // Create finalizer keys finalizer_keys fin_keys(t, num_keys, finset_size); // set finalizers on current node - fin_keys.set_node_finalizers(0, num_keys); + fin_keys.set_node_finalizers(0u, num_keys); // run initial set_finalizer_policy() and waits until transition is complete - auto pubkeys0 = fin_keys.set_finalizer_policy(0).pubkeys; + auto pubkeys0 = fin_keys.set_finalizer_policy(0u).pubkeys; fin_keys.transition_to_savanna(); // run set_finalizers(), verify it becomes active after exactly two 3-chains // ------------------------------------------------------------------------- - auto pubkeys1 = fin_keys.set_finalizer_policy(1).pubkeys; + auto pubkeys1 = fin_keys.set_finalizer_policy(1u).pubkeys; t.produce_block(); - t.check_head_finalizer_policy(1, pubkeys0); // new policy should only be active until after two 3-chains + t.check_head_finalizer_policy(1u, pubkeys0); // new policy should only be active until after two 3-chains t.produce_blocks(3); - t.check_head_finalizer_policy(1, pubkeys0); // one 3-chain - new policy still should not be active + t.check_head_finalizer_policy(1u, pubkeys0); // one 3-chain - new policy still should not be active t.produce_blocks(2); - t.check_head_finalizer_policy(1, pubkeys0); // one 3-chain + 2 blocks - new policy still should not be active + t.check_head_finalizer_policy(1u, pubkeys0); // one 3-chain + 2 blocks - new policy still should not be active t.produce_block(); - t.check_head_finalizer_policy(2, pubkeys1); // two 3-chain - new policy *should* be active + t.check_head_finalizer_policy(2u, pubkeys1); // two 3-chain - new policy *should* be active } FC_LOG_AND_RETHROW() } @@ -68,57 +68,55 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_single_test) { try { // --------------------------------------------------------------------------- BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { validating_tester t; - size_t num_keys = 50; - size_t finset_size = 21; + size_t num_keys = 50u; + size_t finset_size = 21u; // Create finalizer keys finalizer_keys fin_keys(t, num_keys, finset_size); // set finalizers on current node - fin_keys.set_node_finalizers(0, num_keys); + fin_keys.set_node_finalizers(0u, num_keys); // run initial set_finalizer_policy() and waits until transition is complete - auto pubkeys0 = fin_keys.set_finalizer_policy(0).pubkeys; + auto pubkeys0 = fin_keys.set_finalizer_policy(0u).pubkeys; fin_keys.transition_to_savanna(); // run set_finalizers() twice in same block, verify only latest one becomes active // ------------------------------------------------------------------------------- - (void)fin_keys.set_finalizer_policy(1); - auto pubkeys2 = fin_keys.set_finalizer_policy(2).pubkeys; + (void)fin_keys.set_finalizer_policy(1u); + auto pubkeys2 = fin_keys.set_finalizer_policy(2u).pubkeys; t.produce_block(); - t.check_head_finalizer_policy(1, pubkeys0); // new policy should only be active until after two 3-chains + t.check_head_finalizer_policy(1u, pubkeys0); // new policy should only be active until after two 3-chains t.produce_blocks(5); - t.check_head_finalizer_policy(1, pubkeys0); // new policy should only be active until after two 3-chains + t.check_head_finalizer_policy(1u, pubkeys0); // new policy should only be active until after two 3-chains t.produce_block(); - t.check_head_finalizer_policy(2, pubkeys2); // two 3-chain - new policy pubkeys2 *should* be active + t.check_head_finalizer_policy(2u, pubkeys2); // two 3-chain - new policy pubkeys2 *should* be active - // run a test with multiple set_finlizers in-flight during the two 3-chains they + // run a test with multiple set_finalizers in-flight during the two 3-chains they // take to become active - // ----------------------------------------------------------------------------- - auto pubkeys3 = fin_keys.set_finalizer_policy(3).pubkeys; + // ------------------------------------------------------------------------------ + auto pubkeys3 = fin_keys.set_finalizer_policy(3u).pubkeys; t.produce_block(); - auto pubkeys4 = fin_keys.set_finalizer_policy(4).pubkeys; + auto pubkeys4 = fin_keys.set_finalizer_policy(4u).pubkeys; t.produce_block(); t.produce_block(); - auto pubkeys5 = fin_keys.set_finalizer_policy(5).pubkeys; + auto pubkeys5 = fin_keys.set_finalizer_policy(5u).pubkeys; t.produce_blocks(3); - t.check_head_finalizer_policy(2, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active + t.check_head_finalizer_policy(2u, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active t.produce_block(); - t.check_head_finalizer_policy(3, pubkeys3); // 6 blocks after pubkeys3 (b6 - b0), pubkeys3 should be active + t.check_head_finalizer_policy(3u, pubkeys3); // 6 blocks after pubkeys3 (b6 - b0), pubkeys3 should be active t.produce_block(); - t.check_head_finalizer_policy(4, pubkeys4); // 6 blocks after pubkeys4 (b7 - b1), pubkeys4 should be active + t.check_head_finalizer_policy(4u, pubkeys4); // 6 blocks after pubkeys4 (b7 - b1), pubkeys4 should be active t.produce_block(); - t.check_head_finalizer_policy(4, pubkeys4); // 7 blocks after pubkeys4, pubkeys4 should still be active + t.check_head_finalizer_policy(4u, pubkeys4); // 7 blocks after pubkeys4, pubkeys4 should still be active t.produce_block(); - t.check_head_finalizer_policy(5, pubkeys5); // 6 blocks after pubkeys5 (b9 - b3), pubkeys5 should be active + t.check_head_finalizer_policy(5u, pubkeys5); // 6 blocks after pubkeys5 (b9 - b3), pubkeys5 should be active // and no further change - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); - ensure_next_block_finalizer_policy(t, 5, pubkeys5); + // --------------------- + for (size_t i=0; i<10; ++i) + ensure_next_block_finalizer_policy(t, 5u, pubkeys5); } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From f0901247166f4d3d4926d5187a50d94e0f094a66 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 15:10:58 -0400 Subject: [PATCH 17/41] Fix some sign comparison warnings with gcc --- unittests/finality_tests.cpp | 64 ++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/unittests/finality_tests.cpp b/unittests/finality_tests.cpp index d35dbbf6aa..5b49d5b505 100644 --- a/unittests/finality_tests.cpp +++ b/unittests/finality_tests.cpp @@ -23,14 +23,14 @@ BOOST_FIXTURE_TEST_CASE(quorum_of_votes, finality_test_cluster<4>) { try { // verify LIB does not advances with finalizers not voting. // -------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(no_votes, finality_test_cluster<4>) { try { - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); produce_and_push_block(); for (auto i = 0; i < 3; ++i) { produce_and_push_block(); // don't process votes // when only node0 votes, LIB shouldn't advance - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); } } FC_LOG_AND_RETHROW() } @@ -38,14 +38,14 @@ BOOST_FIXTURE_TEST_CASE(no_votes, finality_test_cluster<4>) { try { // verify LIB does not advances when one less than the quorum votes // ---------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(quorum_minus_one, finality_test_cluster<4>) { try { - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); produce_and_push_block(); for (auto i = 0; i < 3; ++i) { produce_and_push_block(); process_votes(1, num_needed_for_quorum - 1); // when one less than required vote, LIB shouldn't advance - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); } } FC_LOG_AND_RETHROW() } @@ -111,7 +111,7 @@ BOOST_FIXTURE_TEST_CASE(one_delayed_votes, finality_test_cluster<4>) { try { // block 1 (index 1) has the same QC claim as block 0. It cannot move LIB process_votes(1, num_needed_for_quorum, 1); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // producing, pushing, and voting a new block makes LIB moving process_votes(1, num_needed_for_quorum); @@ -131,7 +131,7 @@ BOOST_FIXTURE_TEST_CASE(three_delayed_votes, finality_test_cluster<4>) { try { produce_and_push_block(); // LIB did not advance - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // vote block 0 (index 0) to make it have a strong QC, // prompting LIB advacing on nodes @@ -143,7 +143,7 @@ BOOST_FIXTURE_TEST_CASE(three_delayed_votes, finality_test_cluster<4>) { try { for (auto i=1; i < 4; ++i) { process_votes(1, num_needed_for_quorum, i); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); } // Now send votes for the last block that node0 produced (block 8). It will be @@ -175,12 +175,12 @@ BOOST_FIXTURE_TEST_CASE(out_of_order_votes, finality_test_cluster<4>) { try { // block 1 (index 1) has the same QC claim as block 2. It will not move LIB process_votes(1, num_needed_for_quorum, 1); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // block 0 (index 0) has the same QC claim as block 2. It will not move LIB process_votes(1, num_needed_for_quorum, 0); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // producing, pushing, and voting a new block makes LIB moving process_votes(1, num_needed_for_quorum); @@ -229,7 +229,7 @@ BOOST_FIXTURE_TEST_CASE(lost_votes, finality_test_cluster<4>) { try { clear_votes_and_reset_lib(); produce_and_push_block(); // Produce another block - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // LIB doesn't advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // LIB doesn't advance process_votes(1, num_needed_for_quorum); // and propagate the votes for this new block to node0 produce_and_push_block(); @@ -247,7 +247,7 @@ BOOST_FIXTURE_TEST_CASE(one_weak_vote, finality_test_cluster<4>) { try { auto next_idx = process_votes(1, num_needed_for_quorum -1); // one less strong vote than needed for quorum process_vote(next_idx, -1, vote_mode::weak); // and one weak vote produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // weak QC (1 shy of strong) => LIB does not advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // weak QC (1 shy of strong) => LIB does not advance process_votes(1, num_needed_for_quorum); // now this provides enough strong votes for quorum produce_and_push_block(); @@ -263,7 +263,7 @@ BOOST_FIXTURE_TEST_CASE(quorum_minus_one_weak_vote, finality_test_cluster<4>) { process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // weak QC => LIB does not advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // weak QC => LIB does not advance process_votes(1, num_needed_for_quorum); produce_and_push_block(); @@ -279,7 +279,7 @@ BOOST_FIXTURE_TEST_CASE(weak_strong_weak_strong, finality_test_cluster<4>) { try process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // weak QC => LIB does not advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // weak QC => LIB does not advance process_votes(1, num_needed_for_quorum); produce_and_push_block(); @@ -287,7 +287,7 @@ BOOST_FIXTURE_TEST_CASE(weak_strong_weak_strong, finality_test_cluster<4>) { try process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // weak QC => LIB does not advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // weak QC => LIB does not advance process_votes(1, num_needed_for_quorum); produce_and_push_block(); @@ -303,11 +303,11 @@ BOOST_FIXTURE_TEST_CASE(weak_weak_strong_strong, finality_test_cluster<4>) { try process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // weak QC => LIB does not advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // weak QC => LIB does not advance process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // weak QC => LIB does not advance + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // weak QC => LIB does not advance process_votes(1, num_needed_for_quorum); produce_and_push_block(); @@ -329,12 +329,12 @@ BOOST_FIXTURE_TEST_CASE(weak_delayed_lost_vote, finality_test_cluster<4>) { try // quorum of weak votes process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // delay votes at index 1 constexpr uint32_t delayed_index = 1; produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // quorum of strong votes process_votes(1, num_needed_for_quorum); @@ -343,12 +343,12 @@ BOOST_FIXTURE_TEST_CASE(weak_delayed_lost_vote, finality_test_cluster<4>) { try // A lost vote produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // The delayed vote arrives, does not advance lib process_votes(1, num_needed_for_quorum, delayed_index); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // strong vote advances lib process_votes(1, num_needed_for_quorum); @@ -366,7 +366,7 @@ BOOST_FIXTURE_TEST_CASE(delayed_strong_weak_lost_vote, finality_test_cluster<4>) // delay votes at index 1 constexpr uint32_t delayed_index = 0; produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // quorum of strong votes process_votes(1, num_needed_for_quorum); @@ -376,7 +376,7 @@ BOOST_FIXTURE_TEST_CASE(delayed_strong_weak_lost_vote, finality_test_cluster<4>) // quorum of weak votes process_votes(1, num_needed_for_quorum, -1, vote_mode::weak); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // quorum of strong votes process_votes(1, num_needed_for_quorum); @@ -385,12 +385,12 @@ BOOST_FIXTURE_TEST_CASE(delayed_strong_weak_lost_vote, finality_test_cluster<4>) // A lost vote produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // The delayed vote arrives, does not advance lib process_votes(1, num_needed_for_quorum, delayed_index); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // strong vote advances lib process_votes(1, num_needed_for_quorum); @@ -432,9 +432,9 @@ BOOST_FIXTURE_TEST_CASE(unknown_proposal_votes, finality_test_cluster<4>) { try process_votes(2, num_needed_for_quorum - 1); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); - node1.restore_to_original_vote(0); // restore node1's vote at index 0 to original vote + node1.restore_to_original_vote(0u); // restore node1's vote at index 0 to original vote process_votes(1, 1, 0, vote_mode::strong); // send restored vote to node0 produce_and_push_block(); // produce a block so the new QC can propagate BOOST_REQUIRE_EQUAL(num_lib_advancing(), num_nodes); @@ -457,7 +457,7 @@ BOOST_FIXTURE_TEST_CASE(unknown_finalizer_key_votes, finality_test_cluster<4>) { BOOST_REQUIRE(process_vote(1, 0) == eosio::chain::vote_status::unknown_public_key); // restore to original vote - node1.restore_to_original_vote(0); + node1.restore_to_original_vote(0u); // process the original vote. LIB should advance process_vote(1, 0); @@ -478,9 +478,9 @@ BOOST_FIXTURE_TEST_CASE(corrupted_signature_votes, finality_test_cluster<4>) { t process_votes(2, num_needed_for_quorum - 1); produce_and_push_block(); - BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0); // because of the one corrupted vote, quorum is not reached + BOOST_REQUIRE_EQUAL(num_lib_advancing(), 0u); // because of the one corrupted vote, quorum is not reached - node1.restore_to_original_vote(0); // restore node1's vote at index 0 to original vote + node1.restore_to_original_vote(0u); // restore node1's vote at index 0 to original vote process_votes(1, 1, 0, vote_mode::strong); // send restored vote to node0 produce_and_push_block(); // produce a block so the new QC can propagate BOOST_REQUIRE_EQUAL(num_lib_advancing(), num_nodes); @@ -504,7 +504,7 @@ BOOST_FIXTURE_TEST_CASE(second_set_finalizers, finality_test_cluster<4>) { try { assert(fin_policy_0); // current finalizer policy from transition to Savanna auto indices1 = fin_policy_indices_0; // start from original set of indices - assert(indices1[0] == 0); // we used index 0 for node0 in original policy + assert(indices1[0] == 0u); // we used index 0 for node0 in original policy indices1[0] = 1; // update key used for node0 in policy auto pubkeys1 = node0.finkeys.set_finalizer_policy(indices1).pubkeys; @@ -517,8 +517,8 @@ BOOST_FIXTURE_TEST_CASE(second_set_finalizers, finality_test_cluster<4>) { try { // we just completed the two 3-chains, so the next block we produce will have the new finalizer policy activated produce_and_push_block(); - node0.check_head_finalizer_policy(2, pubkeys1); - node1.check_head_finalizer_policy(2, pubkeys1); + node0.check_head_finalizer_policy(2u, pubkeys1); + node1.check_head_finalizer_policy(2u, pubkeys1); } FC_LOG_AND_RETHROW() } From ad4ced333ce3492d9f2ec9dee16eccfb643ca26b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 May 2024 16:25:25 -0500 Subject: [PATCH 18/41] GH-5 Take rvalue reference and return the container from apply_diff to make usage more obvious. --- .../eosio/chain/finality/finalizer_policy.hpp | 2 +- .../include/fc/container/ordered_diff.hpp | 6 ++- libraries/libfc/test/test_ordered_diff.cpp | 48 +++++++++---------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp index d0df6383dd..2637cd90c3 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp @@ -32,7 +32,7 @@ namespace eosio::chain { void apply_diff(X&& diff) { generation = diff.generation; threshold = diff.threshold; - finalizers_differ::apply_diff(finalizers, std::move(diff.finalizers_diff)); + finalizers = finalizers_differ::apply_diff(std::move(finalizers), std::forward(diff).finalizers_diff); } // max accumulated weak weight before becoming weak_final diff --git a/libraries/libfc/include/fc/container/ordered_diff.hpp b/libraries/libfc/include/fc/container/ordered_diff.hpp index 31437261c3..e2440d6ab9 100644 --- a/libraries/libfc/include/fc/container/ordered_diff.hpp +++ b/libraries/libfc/include/fc/container/ordered_diff.hpp @@ -88,11 +88,12 @@ class ordered_diff { return result; } - /// @param diff the diff_result created from diff(source, target), apply_diff(source, diff_result) => target + /// @param diff the diff_result created from diff(source, target), apply_diff(std::move(source), diff_result) => target /// @param container the source of diff(source, target) to modify using the diff_result to produce original target + /// @return the modified container now equal to original target template requires std::same_as, diff_result> - static void apply_diff(Container& container, X&& diff) { + static Container apply_diff(Container&& container, X&& diff) { // Remove from the source based on diff.remove_indexes std::ptrdiff_t offset = 0; for (SizeType index : diff.remove_indexes) { @@ -104,6 +105,7 @@ class ordered_diff { for (auto& [index, value] : diff.insert_indexes) { container.insert(container.begin() + index, std::move(value)); } + return container; } }; // class ordered_diff diff --git a/libraries/libfc/test/test_ordered_diff.cpp b/libraries/libfc/test/test_ordered_diff.cpp index 23b9c2035f..e7ffcfebea 100644 --- a/libraries/libfc/test/test_ordered_diff.cpp +++ b/libraries/libfc/test/test_ordered_diff.cpp @@ -14,7 +14,7 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'a', 'c', 'e', 'f'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // Basic case, deque @@ -22,84 +22,84 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { deque source = {'a', 'x', 'c', 'd', 'e'}; deque target = {'z', 'c', 'y', 'f'}; auto result = ordered_deque_char_diff::diff(source, target); - ordered_deque_char_diff::apply_diff(source, result); + source = ordered_deque_char_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // Empty vectors vector source; vector target; ordered_diff::diff_result result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // All elements removed vector source = {'a', 'b', 'c', 'd', 'e'}; vector target; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // All elements inserted vector source; vector target = {'a', 'b', 'c', 'd', 'e'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // No change vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = source; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // Mix of removals and inserts vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'a', 'c', 'e', 'f', 'g', 'h'}; ordered_diff::diff_result result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // Mix of removals and inserts vector source = {1, 2, 3, 4, 5}; vector target = {3, 4, 6, 2, 0}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // Complete change vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'f', 'g', 'h', 'i'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // Diff order vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'e', 'd', 'c', 'b', 'a'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // shift left vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'b', 'c', 'd', 'e', 'f'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // shift right vector source = {'a', 'b', 'c', 'd', 'e'}; vector target = {'z', 'a', 'b', 'c', 'd'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // non-unique vector source = {'a', 'b', 'c', 'd', 'e', 'c', 'a', 'q'}; vector target = {'z', 'a', 'b', 'c', 'd', 'a'}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // full @@ -108,15 +108,15 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { vector target(source.size()); std::reverse_copy(source.begin(), source.end(), target.begin()); auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); target.clear(); result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); source.clear(); result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { // non-unique full @@ -126,15 +126,15 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try { vector target(source.size()); std::reverse_copy(source.begin(), source.end(), target.begin()); auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); target.clear(); result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); source.clear(); result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } } FC_LOG_AND_RETHROW(); @@ -145,28 +145,28 @@ BOOST_AUTO_TEST_CASE(ordered_diff_string_test) try { vector source = {"hello", "how", "are", "you", "today"}; vector target = {"hi", "are", "you", "here"}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; vector target = {"prod2", "prod1", "prod3", "prod4", "prod5"}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, result); + source = ordered_diff::apply_diff(std::move(source), result); BOOST_TEST(source == target); } { vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; vector target = {"prod5", "prod1", "prod2", "prod3", "prod4"}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, std::move(result)); + source = ordered_diff::apply_diff(std::move(source), std::move(result)); BOOST_TEST(source == target); } { vector source = {"prod1", "prod2", "prod3", "prod4", "prod5"}; vector target = {"prod2", "prod3", "prod4", "prod5", "prod6"}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, std::move(result)); + source = ordered_diff::apply_diff(std::move(source), std::move(result)); BOOST_TEST(source == target); } @@ -190,7 +190,7 @@ BOOST_AUTO_TEST_CASE(ordered_diff_moveable_test) try { vector source = {count_moves{"hello"}, count_moves{"there"}}; vector target = {count_moves{"hi"}, count_moves{"there"}}; auto result = ordered_diff::diff(source, target); - ordered_diff::apply_diff(source, std::move(result)); + source = ordered_diff::apply_diff(std::move(source), std::move(result)); BOOST_TEST(source == target); BOOST_TEST(count_moves::num_moves == 1); } From e5223867de1493d3e996bc470700cd7dc8f00eae Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 17:42:05 -0400 Subject: [PATCH 19/41] Make variable name consistent with comments. --- unittests/forked_tests.cpp | 102 ++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/unittests/forked_tests.cpp b/unittests/forked_tests.cpp index b9f19a25a0..ce4c83c2be 100644 --- a/unittests/forked_tests.cpp +++ b/unittests/forked_tests.cpp @@ -568,47 +568,47 @@ BOOST_AUTO_TEST_CASE( reopen_forkdb ) try { } FC_LOG_AND_RETHROW() BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { - tester c; - while (c.control->head_block_num() < 3) { - c.produce_block(); + tester c1; + while (c1.control->head_block_num() < 3) { + c1.produce_block(); } - auto r = c.create_accounts( {"dan"_n,"sam"_n,"pam"_n} ); - c.produce_block(); - auto res = c.set_producers( {"dan"_n,"sam"_n,"pam"_n} ); + auto r = c1.create_accounts( {"dan"_n,"sam"_n,"pam"_n} ); + c1.produce_block(); + auto res = c1.set_producers( {"dan"_n,"sam"_n,"pam"_n} ); wlog("set producer schedule to [dan,sam,pam]"); - c.produce_blocks(40); + c1.produce_blocks(40); tester c2(setup_policy::none); wlog( "push c1 blocks to c2" ); - push_blocks(c, c2); + push_blocks(c1, c2); wlog( "c1 blocks:" ); signed_block_ptr cb; - c.produce_blocks(3); + c1.produce_blocks(3); signed_block_ptr b; - cb = b = c.produce_block(); + cb = b = c1.produce_block(); account_name expected_producer = "dan"_n; BOOST_REQUIRE_EQUAL( b->producer.to_string(), expected_producer.to_string() ); - b = c.produce_block(); + b = c1.produce_block(); expected_producer = "sam"_n; BOOST_REQUIRE_EQUAL( b->producer.to_string(), expected_producer.to_string() ); - c.produce_blocks(10); - c.create_accounts( {"cam"_n} ); - c.set_producers( {"dan"_n,"sam"_n,"pam"_n,"cam"_n} ); + c1.produce_blocks(10); + c1.create_accounts( {"cam"_n} ); + c1.set_producers( {"dan"_n,"sam"_n,"pam"_n,"cam"_n} ); wlog("set producer schedule to [dan,sam,pam,cam]"); - c.produce_block(); + c1.produce_block(); // The next block should be produced by pam. // Sync second chain with first chain. wlog( "push c1 blocks to c2" ); - push_blocks(c, c2); + push_blocks(c1, c2); wlog( "end push c1 blocks to c2" ); // Now sam and pam go on their own fork while dan is producing blocks by himself. wlog( "sam and pam go off on their own fork on c2 while dan produces blocks by himself in c1" ); - auto fork_block_num = c.control->head_block_num(); + auto fork_block_num = c1.control->head_block_num(); signed_block_ptr c2b; wlog( "c2 blocks:" ); @@ -624,11 +624,11 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { wlog( "c1 blocks:" ); - b = c.produce_block( fc::milliseconds(config::block_interval_ms * 13) ); // dan skips over pam's blocks + b = c1.produce_block( fc::milliseconds(config::block_interval_ms * 13) ); // dan skips over pam's blocks expected_producer = "dan"_n; BOOST_REQUIRE_EQUAL( b->producer.to_string(), expected_producer.to_string() ); // create accounts on c1 which will be forked out - c.produce_block(); + c1.produce_block(); transaction_trace_ptr trace1, trace2, trace3, trace4; { // create account the hard way so we can set reference block and expiration @@ -642,12 +642,12 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { .owner = owner_auth, .active = active_auth, }); - trx.expiration = fc::time_point_sec{c.control->head_block_time() + fc::seconds( 60 )}; + trx.expiration = fc::time_point_sec{c1.control->head_block_time() + fc::seconds( 60 )}; trx.set_reference_block( cb->calculate_id() ); - trx.sign( get_private_key( config::system_account_name, "active" ), c.control->get_chain_id() ); - trace1 = c.push_transaction( trx ); + trx.sign( get_private_key( config::system_account_name, "active" ), c1.control->get_chain_id() ); + trace1 = c1.push_transaction( trx ); } - c.produce_block(); + c1.produce_block(); { signed_transaction trx; authority active_auth( get_public_key( "test2"_n, "active" ) ); @@ -659,10 +659,10 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { .owner = owner_auth, .active = active_auth, }); - trx.expiration = fc::time_point_sec{c.control->head_block_time() + fc::seconds( 60 )}; + trx.expiration = fc::time_point_sec{c1.control->head_block_time() + fc::seconds( 60 )}; trx.set_reference_block( cb->calculate_id() ); - trx.sign( get_private_key( config::system_account_name, "active" ), c.control->get_chain_id() ); - trace2 = c.push_transaction( trx ); + trx.sign( get_private_key( config::system_account_name, "active" ), c1.control->get_chain_id() ); + trace2 = c1.push_transaction( trx ); } { signed_transaction trx; @@ -675,10 +675,10 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { .owner = owner_auth, .active = active_auth, }); - trx.expiration = fc::time_point_sec{c.control->head_block_time() + fc::seconds( 60 )}; + trx.expiration = fc::time_point_sec{c1.control->head_block_time() + fc::seconds( 60 )}; trx.set_reference_block( cb->calculate_id() ); - trx.sign( get_private_key( config::system_account_name, "active" ), c.control->get_chain_id() ); - trace3 = c.push_transaction( trx ); + trx.sign( get_private_key( config::system_account_name, "active" ), c1.control->get_chain_id() ); + trace3 = c1.push_transaction( trx ); } { signed_transaction trx; @@ -691,18 +691,18 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { .owner = owner_auth, .active = active_auth, }); - trx.expiration = fc::time_point_sec{c.control->head_block_time() + fc::seconds( 60 )}; + trx.expiration = fc::time_point_sec{c1.control->head_block_time() + fc::seconds( 60 )}; trx.set_reference_block( b->calculate_id() ); // tapos to dan's block should be rejected on fork switch - trx.sign( get_private_key( config::system_account_name, "active" ), c.control->get_chain_id() ); - trace4 = c.push_transaction( trx ); + trx.sign( get_private_key( config::system_account_name, "active" ), c1.control->get_chain_id() ); + trace4 = c1.push_transaction( trx ); BOOST_CHECK( trace4->receipt->status == transaction_receipt_header::executed ); } - c.produce_block(); - c.produce_blocks(9); + c1.produce_block(); + c1.produce_blocks(9); // test forked blocks signal accepted_block in order, required by trace_api_plugin std::vector accepted_blocks; - auto conn = c.control->accepted_block().connect( [&]( block_signal_params t ) { + auto conn = c1.control->accepted_block().connect( [&]( block_signal_params t ) { const auto& [ block, id ] = t; accepted_blocks.emplace_back( block ); } ); @@ -711,10 +711,10 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { wlog( "push c2 blocks to c1" ); for( uint32_t start = fork_block_num + 1, end = c2.control->head_block_num(); start <= end; ++start ) { auto fb = c2.control->fetch_block_by_number( start ); - c.push_block( fb ); + c1.push_block( fb ); } - { // verify forked blocks where signaled in order + { // verify forked blocks were signaled in order auto itr = std::find( accepted_blocks.begin(), accepted_blocks.end(), c2b ); BOOST_CHECK( itr != accepted_blocks.end() ); ++itr; @@ -726,31 +726,31 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { BOOST_CHECK( i == 11 + 12 ); } // verify transaction on fork is reported by push_block in order - BOOST_REQUIRE_EQUAL( 4u, c.get_unapplied_transaction_queue().size() ); - BOOST_REQUIRE_EQUAL( trace1->id, c.get_unapplied_transaction_queue().begin()->id() ); - BOOST_REQUIRE_EQUAL( trace2->id, (++c.get_unapplied_transaction_queue().begin())->id() ); - BOOST_REQUIRE_EQUAL( trace3->id, (++(++c.get_unapplied_transaction_queue().begin()))->id() ); - BOOST_REQUIRE_EQUAL( trace4->id, (++(++(++c.get_unapplied_transaction_queue().begin())))->id() ); + BOOST_REQUIRE_EQUAL( 4u, c1.get_unapplied_transaction_queue().size() ); + BOOST_REQUIRE_EQUAL( trace1->id, c1.get_unapplied_transaction_queue().begin()->id() ); + BOOST_REQUIRE_EQUAL( trace2->id, (++c1.get_unapplied_transaction_queue().begin())->id() ); + BOOST_REQUIRE_EQUAL( trace3->id, (++(++c1.get_unapplied_transaction_queue().begin()))->id() ); + BOOST_REQUIRE_EQUAL( trace4->id, (++(++(++c1.get_unapplied_transaction_queue().begin())))->id() ); - BOOST_REQUIRE_EXCEPTION(c.control->get_account( "test1"_n ), fc::exception, + BOOST_REQUIRE_EXCEPTION(c1.control->get_account( "test1"_n ), fc::exception, [a="test1"_n] (const fc::exception& e)->bool { return std::string( e.what() ).find( a.to_string() ) != std::string::npos; }) ; - BOOST_REQUIRE_EXCEPTION(c.control->get_account( "test2"_n ), fc::exception, + BOOST_REQUIRE_EXCEPTION(c1.control->get_account( "test2"_n ), fc::exception, [a="test2"_n] (const fc::exception& e)->bool { return std::string( e.what() ).find( a.to_string() ) != std::string::npos; }) ; - BOOST_REQUIRE_EXCEPTION(c.control->get_account( "test3"_n ), fc::exception, + BOOST_REQUIRE_EXCEPTION(c1.control->get_account( "test3"_n ), fc::exception, [a="test3"_n] (const fc::exception& e)->bool { return std::string( e.what() ).find( a.to_string() ) != std::string::npos; }) ; - BOOST_REQUIRE_EXCEPTION(c.control->get_account( "test4"_n ), fc::exception, + BOOST_REQUIRE_EXCEPTION(c1.control->get_account( "test4"_n ), fc::exception, [a="test4"_n] (const fc::exception& e)->bool { return std::string( e.what() ).find( a.to_string() ) != std::string::npos; }) ; // produce block which will apply the unapplied transactions - produce_block_result_t produce_block_result = c.produce_block_ex(fc::milliseconds(config::block_interval_ms), true); + produce_block_result_t produce_block_result = c1.produce_block_ex(fc::milliseconds(config::block_interval_ms), true); std::vector& traces = produce_block_result.unapplied_transaction_traces; BOOST_REQUIRE_EQUAL( 4u, traces.size() ); @@ -766,12 +766,12 @@ BOOST_AUTO_TEST_CASE( push_block_returns_forked_transactions ) try { BOOST_CHECK( traces.at(3)->except ); // verify unapplied transactions ran - BOOST_REQUIRE_EQUAL( c.control->get_account( "test1"_n ).name, "test1"_n ); - BOOST_REQUIRE_EQUAL( c.control->get_account( "test2"_n ).name, "test2"_n ); - BOOST_REQUIRE_EQUAL( c.control->get_account( "test3"_n ).name, "test3"_n ); + BOOST_REQUIRE_EQUAL( c1.control->get_account( "test1"_n ).name, "test1"_n ); + BOOST_REQUIRE_EQUAL( c1.control->get_account( "test2"_n ).name, "test2"_n ); + BOOST_REQUIRE_EQUAL( c1.control->get_account( "test3"_n ).name, "test3"_n ); // failed because of tapos to forked out block - BOOST_REQUIRE_EXCEPTION(c.control->get_account( "test4"_n ), fc::exception, + BOOST_REQUIRE_EXCEPTION(c1.control->get_account( "test4"_n ), fc::exception, [a="test4"_n] (const fc::exception& e)->bool { return std::string( e.what() ).find( a.to_string() ) != std::string::npos; }) ; From 65e9730391f8170d034a6b23e5b170ee43fb5a25 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 May 2024 18:04:49 -0400 Subject: [PATCH 20/41] Update comment. --- libraries/testing/include/eosio/testing/tester.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 49f93fe6b1..8d975d4e79 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -147,7 +147,7 @@ namespace eosio::testing { struct produce_block_result_t { signed_block_ptr block; transaction_trace_ptr onblock_trace; - std::vector unapplied_transaction_traces; // unapplied from previous `push_block` not on head branch + std::vector unapplied_transaction_traces; // only traces of any unapplied transactions }; /** From 6fe5ec2f66c7ae85371f0b12f7fca235b46b22c0 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 May 2024 18:40:26 -0500 Subject: [PATCH 21/41] GH-5 Track full finalizer policy instead of diffs --- libraries/chain/block_header_state.cpp | 76 +++++++++++++++---- .../eosio/chain/block_header_state.hpp | 23 +----- unittests/svnn_ibc_tests.cpp | 9 ++- 3 files changed, 71 insertions(+), 37 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 494e668a92..528947b393 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -22,7 +22,8 @@ digest_type block_header_state::compute_base_digest() const { fc::raw::pack( enc, fp_pair.first ); const finalizer_policy_tracker& tracker = fp_pair.second; fc::raw::pack( enc, tracker.state ); - fc::raw::pack( enc, tracker.policy_diff ); + assert(tracker.policy); + fc::raw::pack( enc, *tracker.policy ); } assert(active_proposer_policy); @@ -66,6 +67,48 @@ const vector& block_header_state::get_new_protocol_feature_activati return detail::get_new_protocol_feature_activations(header_exts); } +finalizer_policy_diff block_header_state::calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const { + if (finalizer_policies.empty()) { + return active_finalizer_policy->create_diff(new_policy); + } + for (const auto& e : finalizer_policies) { + if (e.second.state == finalizer_policy_tracker::state_t::pending) { + return e.second.policy->create_diff(new_policy); + } + } + for (const auto& e : finalizer_policies) { + if (e.second.state == finalizer_policy_tracker::state_t::proposed) { + return e.second.policy->create_diff(new_policy); + } + } + assert(false); +} + +finalizer_policy block_header_state::calculate_finalizer_policy(const finalizer_policy_diff& diff) const { + finalizer_policy result; + if (finalizer_policies.empty()) { + assert(active_finalizer_policy); + result = *active_finalizer_policy; + result.apply_diff(diff); + return result; + } + for (const auto& e : finalizer_policies) { + if (e.second.state == finalizer_policy_tracker::state_t::pending) { + result = *e.second.policy; + result.apply_diff(diff); + return result; + } + } + for (const auto& e : finalizer_policies) { + if (e.second.state == finalizer_policy_tracker::state_t::proposed) { + result = *e.second.policy; + result.apply_diff(diff); + return result; + } + } + assert(false); +} + // ------------------------------------------------------------------------------------------------- // `finish_next` updates the next `block_header_state` according to the contents of the // header extensions (either new protocol_features or instant_finality_extension) applicable to this @@ -77,7 +120,8 @@ const vector& block_header_state::get_new_protocol_feature_activati void finish_next(const block_header_state& prev, block_header_state& next_header_state, vector new_protocol_feature_activations, - instant_finality_extension if_ext) { + instant_finality_extension if_ext, + std::optional new_finalizer_policy) { // activated protocol features // --------------------------- if (!new_protocol_feature_activations.empty()) { @@ -135,15 +179,13 @@ void finish_next(const block_header_state& prev, if (tracker.state == finalizer_policy_tracker::state_t::pending) { // new finalizer_policy becones active // ----------------------------------- - assert(prev.active_finalizer_policy); - next_header_state.active_finalizer_policy.reset(new finalizer_policy(*prev.active_finalizer_policy)); - next_header_state.active_finalizer_policy->apply_diff(tracker.policy_diff); + 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. // --------------------------------------------------------------------------------- - finalizer_policy_tracker t { finalizer_policy_tracker::state_t::pending, tracker.policy_diff }; + 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)); } ++it; @@ -157,20 +199,19 @@ void finish_next(const block_header_state& prev, } } - if (if_ext.new_finalizer_policy_diff) { + if (new_finalizer_policy) { // a new `finalizer_policy` was proposed in the previous block, and is present in the previous // block's header extensions. // Add this new proposal to the `finalizer_policies` multimap which tracks the in-flight proposals, // increment the generation number, and log that proposal (debug level). // ------------------------------------------------------------------------------------------------ - dlog("New finalizer policy proposed in block ${id}, diff: ${pol}", - ("id", prev.block_id)("pol", *if_ext.new_finalizer_policy_diff)); - next_header_state.finalizer_policy_generation = if_ext.new_finalizer_policy_diff->generation; + dlog("New finalizer policy proposed in block ${id}..: ${pol}", + ("id", prev.block_id.str().substr(8,16))("pol", *new_finalizer_policy)); + next_header_state.finalizer_policy_generation = new_finalizer_policy->generation; next_header_state.finalizer_policies.emplace( next_header_state.block_num(), finalizer_policy_tracker{finalizer_policy_tracker::state_t::proposed, - std::move(*if_ext.new_finalizer_policy_diff)}); - + std::make_shared(std::move(*new_finalizer_policy))}); } else { next_header_state.finalizer_policy_generation = prev.finalizer_policy_generation; } @@ -228,7 +269,8 @@ block_header_state block_header_state::next(block_header_state_input& input) con next_header_state.header_exts.emplace(ext_id, std::move(pfa_ext)); } - finish_next(*this, next_header_state, std::move(input.new_protocol_feature_activations), std::move(new_if_ext)); + finish_next(*this, next_header_state, std::move(input.new_protocol_feature_activations), std::move(new_if_ext), + std::move(input.new_finalizer_policy)); return next_header_state; } @@ -285,7 +327,13 @@ block_header_state block_header_state::next(const signed_block_header& h, valida ("f", next_core_metadata.final_on_strong_qc_block_num)); }; - finish_next(*this, next_header_state, std::move(new_protocol_feature_activations), if_ext); + std::optional new_finalizer_policy; + if (if_ext.new_finalizer_policy_diff) { + new_finalizer_policy = calculate_finalizer_policy(*if_ext.new_finalizer_policy_diff); + } + + finish_next(*this, next_header_state, std::move(new_protocol_feature_activations), if_ext, + std::move(new_finalizer_policy)); return next_header_state; } diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 174356e3da..7e82b874da 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -47,7 +47,7 @@ struct finality_digest_data_v1 { struct finalizer_policy_tracker { enum class state_t { proposed = 0, pending }; state_t state; - finalizer_policy_diff policy_diff; + finalizer_policy_ptr policy; }; struct building_block_input { @@ -130,23 +130,8 @@ struct block_header_state { const vector& get_new_protocol_feature_activations() const; const producer_authority& get_scheduled_producer(block_timestamp_type t) const; - finalizer_policy_diff calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const { - if (finalizer_policies.empty()) { - return active_finalizer_policy->create_diff(new_policy); - } - finalizer_policy fin_policy = *active_finalizer_policy; - for (const auto& e : finalizer_policies) { - if (e.second.state == finalizer_policy_tracker::state_t::pending) { - fin_policy.apply_diff(e.second.policy_diff); - } - } - for (const auto& e : finalizer_policies) { - if (e.second.state == finalizer_policy_tracker::state_t::proposed) { - fin_policy.apply_diff(e.second.policy_diff); - } - } - return fin_policy.create_diff(new_policy); - } + finalizer_policy_diff calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const; + finalizer_policy calculate_finalizer_policy(const finalizer_policy_diff& diff) const; }; using block_header_state_ptr = std::shared_ptr; @@ -155,7 +140,7 @@ using block_header_state_ptr = std::shared_ptr; FC_REFLECT_ENUM( eosio::chain::finalizer_policy_tracker::state_t, (proposed)(pending)) -FC_REFLECT( eosio::chain::finalizer_policy_tracker, (state)(policy_diff)) +FC_REFLECT( eosio::chain::finalizer_policy_tracker, (state)(policy)) FC_REFLECT( eosio::chain::block_header_state, (block_id)(header) (activated_protocol_features)(core)(active_finalizer_policy) diff --git a/unittests/svnn_ibc_tests.cpp b/unittests/svnn_ibc_tests.cpp index 74d06f587d..ab70b0eac0 100644 --- a/unittests/svnn_ibc_tests.cpp +++ b/unittests/svnn_ibc_tests.cpp @@ -101,10 +101,11 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) std::get(genesis_if_ext).new_finalizer_policy_diff; BOOST_CHECK(maybe_active_finalizer_policy_diff.has_value()); -/** TODO diff or full active policy ? - eosio::chain::finalizer_policy active_finalizer_policy = maybe_active_finalizer_policy.value(); - BOOST_CHECK_EQUAL(active_finalizer_policy.finalizers.size(), cluster.num_nodes); + eosio::chain::finalizer_policy_diff active_finalizer_policy_diff = maybe_active_finalizer_policy_diff.value(); + eosio::chain::finalizer_policy active_finalizer_policy; + active_finalizer_policy.apply_diff(active_finalizer_policy_diff); + BOOST_CHECK_EQUAL(active_finalizer_policy.generation, 1u); // compute the digest of the finalizer policy @@ -374,7 +375,7 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) // verify action has failed, as expected BOOST_CHECK(failed); -*/ + } FC_LOG_AND_RETHROW() } BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From 59967253596822f21c997c55d728be468c4003ba Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 8 May 2024 20:15:24 -0400 Subject: [PATCH 22/41] Add proposed finalizer policy to SHiP finality_data stream --- libraries/chain/block_state.cpp | 16 ++++++++++++++-- .../chain/include/eosio/chain/block_state.hpp | 3 ++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 5d84d2f60a..b6b70c5d38 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -337,11 +337,23 @@ finality_data_t block_state::get_finality_data() { if (!base_digest) { base_digest = compute_base_digest(); // cache it } + + // Check if there is any proposed finalizer policy in the block + std::optional proposed_finalizer_policy = std::nullopt; + auto range = finalizer_policies.equal_range(block_num()); + for (auto itr = range.first; itr != range.second; ++itr) { + if (itr->second.state == finalizer_policy_tracker::state_t::proposed) { + proposed_finalizer_policy = *itr->second.policy; + break; + } + } + return { // other fields take the default values set by finality_data_t definition .active_finalizer_policy_generation = active_finalizer_policy->generation, - .action_mroot = action_mroot, - .base_digest = *base_digest + .action_mroot = action_mroot, + .base_digest = *base_digest, + .proposed_finalizer_policy = proposed_finalizer_policy }; } diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp index 35cfcf098e..cb13675756 100644 --- a/libraries/chain/include/eosio/chain/block_state.hpp +++ b/libraries/chain/include/eosio/chain/block_state.hpp @@ -64,6 +64,7 @@ struct finality_data_t { uint32_t active_finalizer_policy_generation{0}; digest_type action_mroot{}; digest_type base_digest{}; + std::optional proposed_finalizer_policy{std::nullopt}; // finalizer policy, if proposed in the block }; struct block_state : public block_header_state { // block_header_state provides parent link @@ -176,5 +177,5 @@ using block_state_pair = std::pair, blo // not exporting pending_qc or valid_qc FC_REFLECT( eosio::chain::valid_t::finality_leaf_node_t, (major_version)(minor_version)(block_num)(finality_digest)(action_mroot) ) FC_REFLECT( eosio::chain::valid_t, (validation_tree)(validation_mroots)) -FC_REFLECT( eosio::chain::finality_data_t, (major_version)(minor_version)(active_finalizer_policy_generation)(action_mroot)(base_digest)) +FC_REFLECT( eosio::chain::finality_data_t, (major_version)(minor_version)(active_finalizer_policy_generation)(action_mroot)(base_digest)(proposed_finalizer_policy) ) FC_REFLECT_DERIVED( eosio::chain::block_state, (eosio::chain::block_header_state), (block)(strong_digest)(weak_digest)(pending_qc)(valid)(validated) ) From 01177018891387bb736151021ad7f1e52b9d7f46 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 12:20:45 -0500 Subject: [PATCH 23/41] GH-5 Verify instant_finality_extension finalizes diff --- unittests/finalizer_update_tests.cpp | 40 +++++++++++++++++++++------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/unittests/finalizer_update_tests.cpp b/unittests/finalizer_update_tests.cpp index 1c9a2d8bbf..b96d9888ec 100644 --- a/unittests/finalizer_update_tests.cpp +++ b/unittests/finalizer_update_tests.cpp @@ -71,6 +71,17 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { size_t num_keys = 50u; size_t finset_size = 21u; + auto verify_block_finality_generation = [](const signed_block_ptr& block, uint32_t gen) { + std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); + BOOST_TEST(!!ext); + std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; + BOOST_TEST(!!fin_policy_diff); + BOOST_TEST(fin_policy_diff->generation == gen); + // each set_finalizer_policy in this test removes one and adds one + BOOST_TEST(fin_policy_diff->finalizers_diff.remove_indexes.size() == 1); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == 1); + }; + // Create finalizer keys finalizer_keys fin_keys(t, num_keys, finset_size); @@ -96,30 +107,39 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { // take to become active // ------------------------------------------------------------------------------ auto pubkeys3 = fin_keys.set_finalizer_policy(3u).pubkeys; - t.produce_block(); + auto b = t.produce_block(); + verify_block_finality_generation(b, 3); auto pubkeys4 = fin_keys.set_finalizer_policy(4u).pubkeys; - t.produce_block(); + b = t.produce_block(); + verify_block_finality_generation(b, 4); t.produce_block(); auto pubkeys5 = fin_keys.set_finalizer_policy(5u).pubkeys; - t.produce_blocks(2); - auto pubkeys6 = fin_keys.set_finalizer_policy(6u).pubkeys; + b = t.produce_block(); + verify_block_finality_generation(b, 5); t.produce_block(); + auto pubkeys6 = fin_keys.set_finalizer_policy(6u).pubkeys; + b = t.produce_block(); + verify_block_finality_generation(b, 6); auto pubkeys7 = fin_keys.set_finalizer_policy(7u).pubkeys; t.check_head_finalizer_policy(2u, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active - t.produce_block(); + b = t.produce_block(); + verify_block_finality_generation(b, 7); auto pubkeys8 = fin_keys.set_finalizer_policy(8u).pubkeys; t.check_head_finalizer_policy(3u, pubkeys3); // 6 blocks after pubkeys3 (b6 - b0), pubkeys3 should be active - t.produce_block(); + b = t.produce_block(); + verify_block_finality_generation(b, 8); auto pubkeys9 = fin_keys.set_finalizer_policy(9u).pubkeys; t.check_head_finalizer_policy(4u, pubkeys4); // 6 blocks after pubkeys4 (b7 - b1), pubkeys4 should be active - - t.produce_block(); + b = t.produce_block(); + verify_block_finality_generation(b, 9); auto pubkeys10 = fin_keys.set_finalizer_policy(10u).pubkeys; t.check_head_finalizer_policy(4u, pubkeys4); // 7 blocks after pubkeys4, pubkeys4 should still be active - t.produce_block(); + b = t.produce_block(); + verify_block_finality_generation(b, 10); auto pubkeys11 = fin_keys.set_finalizer_policy(11u).pubkeys; t.check_head_finalizer_policy(5u, pubkeys5); // 6 blocks after pubkeys5 (b9 - b3), pubkeys5 should be active - t.produce_block(); + b = t.produce_block(); + verify_block_finality_generation(b, 11); t.produce_block(); // two blocks between 5 & 6 proposals t.check_head_finalizer_policy(6u, pubkeys6); // the rest are all one block apart, tests pending with propsed auto b12 = t.produce_block(); From 822ac388422c51d5b355caffe1c217bfc275be7b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 12:21:07 -0500 Subject: [PATCH 24/41] GH-5 Calculate policy instead of passing it --- libraries/chain/block_header_state.cpp | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 528947b393..7835f633cb 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -120,8 +120,7 @@ finalizer_policy block_header_state::calculate_finalizer_policy(const finalizer_ void finish_next(const block_header_state& prev, block_header_state& next_header_state, vector new_protocol_feature_activations, - instant_finality_extension if_ext, - std::optional new_finalizer_policy) { + instant_finality_extension if_ext) { // activated protocol features // --------------------------- if (!new_protocol_feature_activations.empty()) { @@ -199,19 +198,21 @@ void finish_next(const block_header_state& prev, } } - if (new_finalizer_policy) { + if (if_ext.new_finalizer_policy_diff) { + finalizer_policy new_finalizer_policy = prev.calculate_finalizer_policy(*if_ext.new_finalizer_policy_diff); + // a new `finalizer_policy` was proposed in the previous block, and is present in the previous // block's header extensions. // Add this new proposal to the `finalizer_policies` multimap which tracks the in-flight proposals, // increment the generation number, and log that proposal (debug level). // ------------------------------------------------------------------------------------------------ dlog("New finalizer policy proposed in block ${id}..: ${pol}", - ("id", prev.block_id.str().substr(8,16))("pol", *new_finalizer_policy)); - next_header_state.finalizer_policy_generation = new_finalizer_policy->generation; + ("id", prev.block_id.str().substr(8,16))("pol", new_finalizer_policy)); + next_header_state.finalizer_policy_generation = new_finalizer_policy.generation; next_header_state.finalizer_policies.emplace( next_header_state.block_num(), finalizer_policy_tracker{finalizer_policy_tracker::state_t::proposed, - std::make_shared(std::move(*new_finalizer_policy))}); + std::make_shared(std::move(new_finalizer_policy))}); } else { next_header_state.finalizer_policy_generation = prev.finalizer_policy_generation; } @@ -269,8 +270,7 @@ block_header_state block_header_state::next(block_header_state_input& input) con next_header_state.header_exts.emplace(ext_id, std::move(pfa_ext)); } - finish_next(*this, next_header_state, std::move(input.new_protocol_feature_activations), std::move(new_if_ext), - std::move(input.new_finalizer_policy)); + finish_next(*this, next_header_state, std::move(input.new_protocol_feature_activations), std::move(new_if_ext)); return next_header_state; } @@ -327,13 +327,7 @@ block_header_state block_header_state::next(const signed_block_header& h, valida ("f", next_core_metadata.final_on_strong_qc_block_num)); }; - std::optional new_finalizer_policy; - if (if_ext.new_finalizer_policy_diff) { - new_finalizer_policy = calculate_finalizer_policy(*if_ext.new_finalizer_policy_diff); - } - - finish_next(*this, next_header_state, std::move(new_protocol_feature_activations), if_ext, - std::move(new_finalizer_policy)); + finish_next(*this, next_header_state, std::move(new_protocol_feature_activations), if_ext); return next_header_state; } From c4d4700dba2fdab12995256a5ad3664de01d958f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 12:34:17 -0500 Subject: [PATCH 25/41] GH-5 Verify instant_finality_extension diff --- unittests/finalizer_update_tests.cpp | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/unittests/finalizer_update_tests.cpp b/unittests/finalizer_update_tests.cpp index b96d9888ec..bd043170a8 100644 --- a/unittests/finalizer_update_tests.cpp +++ b/unittests/finalizer_update_tests.cpp @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { size_t num_keys = 50u; size_t finset_size = 21u; - auto verify_block_finality_generation = [](const signed_block_ptr& block, uint32_t gen) { + auto verify_block_finality_generation = [](const signed_block_ptr& block, uint32_t gen, const bls_public_key& key) { std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); BOOST_TEST(!!ext); std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; @@ -79,7 +79,8 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { BOOST_TEST(fin_policy_diff->generation == gen); // each set_finalizer_policy in this test removes one and adds one BOOST_TEST(fin_policy_diff->finalizers_diff.remove_indexes.size() == 1); - BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes.size() == 1); + BOOST_TEST_REQUIRE(fin_policy_diff->finalizers_diff.insert_indexes.size() == 1); + BOOST_TEST(fin_policy_diff->finalizers_diff.insert_indexes[0].second.public_key == key); }; // Create finalizer keys @@ -108,38 +109,38 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { // ------------------------------------------------------------------------------ auto pubkeys3 = fin_keys.set_finalizer_policy(3u).pubkeys; auto b = t.produce_block(); - verify_block_finality_generation(b, 3); + verify_block_finality_generation(b, 3, pubkeys3.back()); auto pubkeys4 = fin_keys.set_finalizer_policy(4u).pubkeys; b = t.produce_block(); - verify_block_finality_generation(b, 4); + verify_block_finality_generation(b, 4, pubkeys4.back()); t.produce_block(); auto pubkeys5 = fin_keys.set_finalizer_policy(5u).pubkeys; b = t.produce_block(); - verify_block_finality_generation(b, 5); + verify_block_finality_generation(b, 5, pubkeys5.back()); t.produce_block(); auto pubkeys6 = fin_keys.set_finalizer_policy(6u).pubkeys; b = t.produce_block(); - verify_block_finality_generation(b, 6); + verify_block_finality_generation(b, 6, pubkeys6.back()); auto pubkeys7 = fin_keys.set_finalizer_policy(7u).pubkeys; t.check_head_finalizer_policy(2u, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active b = t.produce_block(); - verify_block_finality_generation(b, 7); + verify_block_finality_generation(b, 7, pubkeys7.back()); auto pubkeys8 = fin_keys.set_finalizer_policy(8u).pubkeys; t.check_head_finalizer_policy(3u, pubkeys3); // 6 blocks after pubkeys3 (b6 - b0), pubkeys3 should be active b = t.produce_block(); - verify_block_finality_generation(b, 8); + verify_block_finality_generation(b, 8, pubkeys8.back()); auto pubkeys9 = fin_keys.set_finalizer_policy(9u).pubkeys; t.check_head_finalizer_policy(4u, pubkeys4); // 6 blocks after pubkeys4 (b7 - b1), pubkeys4 should be active b = t.produce_block(); - verify_block_finality_generation(b, 9); + verify_block_finality_generation(b, 9, pubkeys9.back()); auto pubkeys10 = fin_keys.set_finalizer_policy(10u).pubkeys; t.check_head_finalizer_policy(4u, pubkeys4); // 7 blocks after pubkeys4, pubkeys4 should still be active b = t.produce_block(); - verify_block_finality_generation(b, 10); + verify_block_finality_generation(b, 10, pubkeys10.back()); auto pubkeys11 = fin_keys.set_finalizer_policy(11u).pubkeys; t.check_head_finalizer_policy(5u, pubkeys5); // 6 blocks after pubkeys5 (b9 - b3), pubkeys5 should be active b = t.produce_block(); - verify_block_finality_generation(b, 11); + verify_block_finality_generation(b, 11, pubkeys11.back()); t.produce_block(); // two blocks between 5 & 6 proposals t.check_head_finalizer_policy(6u, pubkeys6); // the rest are all one block apart, tests pending with propsed auto b12 = t.produce_block(); From 30c56b7999e3974f01e19d66cc0897c6915f2232 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 12:36:16 -0500 Subject: [PATCH 26/41] GH-5 Compare with correct in-flight finalizer policy --- libraries/chain/block_header_state.cpp | 50 ++++++------------- .../eosio/chain/block_header_state.hpp | 1 + 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 7835f633cb..1f5bf538bf 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -67,46 +67,26 @@ const vector& block_header_state::get_new_protocol_feature_activati return detail::get_new_protocol_feature_activations(header_exts); } -finalizer_policy_diff block_header_state::calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const { - if (finalizer_policies.empty()) { - return active_finalizer_policy->create_diff(new_policy); - } - for (const auto& e : finalizer_policies) { - if (e.second.state == finalizer_policy_tracker::state_t::pending) { - return e.second.policy->create_diff(new_policy); - } - } - for (const auto& e : finalizer_policies) { - if (e.second.state == finalizer_policy_tracker::state_t::proposed) { - return e.second.policy->create_diff(new_policy); +// The last proposed finalizer policy if none proposed or pending is the active finalizer policy +const finalizer_policy& block_header_state::get_last_proposed_finalizer_policy() const { + if (!finalizer_policies.empty()) { + for (auto ritr = finalizer_policies.rbegin(); ritr != finalizer_policies.rend(); ++ritr) { + if (ritr->second.state == finalizer_policy_tracker::state_t::proposed) + return *ritr->second.policy; } + return *finalizer_policies.rbegin()->second.policy; } - assert(false); + return *active_finalizer_policy; +} + +finalizer_policy_diff block_header_state::calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const { + return get_last_proposed_finalizer_policy().create_diff(new_policy); } finalizer_policy block_header_state::calculate_finalizer_policy(const finalizer_policy_diff& diff) const { - finalizer_policy result; - if (finalizer_policies.empty()) { - assert(active_finalizer_policy); - result = *active_finalizer_policy; - result.apply_diff(diff); - return result; - } - for (const auto& e : finalizer_policies) { - if (e.second.state == finalizer_policy_tracker::state_t::pending) { - result = *e.second.policy; - result.apply_diff(diff); - return result; - } - } - for (const auto& e : finalizer_policies) { - if (e.second.state == finalizer_policy_tracker::state_t::proposed) { - result = *e.second.policy; - result.apply_diff(diff); - return result; - } - } - assert(false); + finalizer_policy result = get_last_proposed_finalizer_policy(); + result.apply_diff(diff); + return result; } // ------------------------------------------------------------------------------------------------- diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 7e82b874da..c775de9d66 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -130,6 +130,7 @@ struct block_header_state { const vector& get_new_protocol_feature_activations() const; const producer_authority& get_scheduled_producer(block_timestamp_type t) const; + const finalizer_policy& get_last_proposed_finalizer_policy() const; finalizer_policy_diff calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const; finalizer_policy calculate_finalizer_policy(const finalizer_policy_diff& diff) const; }; From 17ddf2385b9d0515f5cbbc1431dbcedce3835409 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 9 May 2024 14:23:57 -0400 Subject: [PATCH 27/41] Use std::move to assign proposed_finalizer_policy --- libraries/chain/block_state.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index b6b70c5d38..eecb41f9a7 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -353,7 +353,7 @@ finality_data_t block_state::get_finality_data() { .active_finalizer_policy_generation = active_finalizer_policy->generation, .action_mroot = action_mroot, .base_digest = *base_digest, - .proposed_finalizer_policy = proposed_finalizer_policy + .proposed_finalizer_policy = std::move(proposed_finalizer_policy) }; } From f1d4d2a2fe2bc8f63cb6398a72a6566413331ed5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 14:37:35 -0500 Subject: [PATCH 28/41] GH-5 Place proposer policy diff into instant_finality_extension instead of full proposer policy --- libraries/chain/block_header_state.cpp | 40 +++++++++- libraries/chain/controller.cpp | 10 +-- .../eosio/chain/block_header_state.hpp | 4 +- .../finality/instant_finality_extension.hpp | 10 +-- .../eosio/chain/finality/proposer_policy.hpp | 31 ++++++- unittests/block_header_tests.cpp | 19 +++-- unittests/producer_schedule_if_tests.cpp | 80 ++++++++++++++----- 7 files changed, 148 insertions(+), 46 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 1f5bf538bf..5206326f44 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -89,6 +89,30 @@ finalizer_policy block_header_state::calculate_finalizer_policy(const finalizer_ return result; } +proposer_policy_diff block_header_state::calculate_proposer_policy_diff(const proposer_policy& new_policy) const { + if (proposer_policies.empty()) { + assert(active_proposer_policy); + return active_proposer_policy->create_diff(new_policy); + } + auto it = proposer_policies.rbegin(); + assert(it != proposer_policies.rend()); + return it->second->create_diff(new_policy); +} + +proposer_policy block_header_state::calculate_proposer_policy(const proposer_policy_diff& diff) const { + if (proposer_policies.empty()) { + assert(active_proposer_policy); + proposer_policy result = *active_proposer_policy; + result.apply_diff(diff); + return result; + } + auto it = proposer_policies.rbegin(); + assert(it != proposer_policies.rend()); + proposer_policy result = *it->second; + result.apply_diff(diff); + return result; +} + // ------------------------------------------------------------------------------------------------- // `finish_next` updates the next `block_header_state` according to the contents of the // header extensions (either new protocol_features or instant_finality_extension) applicable to this @@ -125,10 +149,14 @@ void finish_next(const block_header_state& prev, } } - if (if_ext.new_proposer_policy) { + std::optional new_proposer_policy; + if (if_ext.new_proposer_policy_diff) { + new_proposer_policy = prev.calculate_proposer_policy(*if_ext.new_proposer_policy_diff); + } + if (new_proposer_policy) { // called when assembling the block - next_header_state.proposer_policies[if_ext.new_proposer_policy->active_time] = - std::move(if_ext.new_proposer_policy); + next_header_state.proposer_policies[new_proposer_policy->active_time] = + std::make_shared(std::move(*new_proposer_policy)); } // finality_core @@ -232,9 +260,13 @@ block_header_state block_header_state::next(block_header_state_input& input) con if (input.new_finalizer_policy) { new_finalizer_policy_diff = calculate_finalizer_policy_diff(*input.new_finalizer_policy); } + std::optional new_proposer_policy_diff; + if (input.new_proposer_policy) { + new_proposer_policy_diff = calculate_proposer_policy_diff(*input.new_proposer_policy); + } instant_finality_extension new_if_ext { input.most_recent_ancestor_with_qc, std::move(new_finalizer_policy_diff), - std::move(input.new_proposer_policy) }; + std::move(new_proposer_policy_diff) }; uint16_t if_ext_id = instant_finality_extension::extension_id(); emplace_extension(next_header_state.header.header_extensions, if_ext_id, fc::raw::pack(new_if_ext)); diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 777743bbaf..9086bc5c56 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -706,7 +706,7 @@ struct building_block { assembled_block assemble_block(boost::asio::io_context& ioc, const protocol_feature_set& pfs, fork_database& fork_db, - std::unique_ptr new_proposer_policy, + std::optional new_proposer_policy, std::optional new_finalizer_policy, bool validating, std::optional validating_qc_data, @@ -3164,13 +3164,13 @@ struct controller_impl { resource_limits.process_block_usage(bb.block_num()); // Any proposer policy? - auto process_new_proposer_policy = [&](auto&) -> std::unique_ptr { - std::unique_ptr new_proposer_policy; + auto process_new_proposer_policy = [&](auto&) -> std::optional { + std::optional new_proposer_policy; const auto& gpo = db.get(); if (gpo.proposed_schedule_block_num) { std::optional version = pending->get_next_proposer_schedule_version(gpo.proposed_schedule.producers); if (version) { - new_proposer_policy = std::make_unique(); + new_proposer_policy.emplace(); new_proposer_policy->active_time = detail::get_next_next_round_block_time(bb.timestamp()); new_proposer_policy->proposer_schedule = producer_authority_schedule::from_shared(gpo.proposed_schedule); new_proposer_policy->proposer_schedule.version = *version; @@ -3186,7 +3186,7 @@ struct controller_impl { } return new_proposer_policy; }; - auto new_proposer_policy = apply_s>(chain_head, process_new_proposer_policy); + auto new_proposer_policy = apply_s>(chain_head, process_new_proposer_policy); // Any finalizer policy? std::optional new_finalizer_policy = std::nullopt; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index c775de9d66..f9c6600e89 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -61,7 +61,7 @@ struct building_block_input { // this struct can be extracted from a building block struct block_header_state_input : public building_block_input { digest_type transaction_mroot; // Comes from std::get(building_block::trx_mroot_or_receipt_digests) - std::shared_ptr new_proposer_policy; // Comes from building_block::new_proposer_policy + std::optional new_proposer_policy; // Comes from building_block::new_proposer_policy std::optional new_finalizer_policy; // Comes from building_block::new_finalizer_policy qc_claim_t most_recent_ancestor_with_qc; // Comes from traversing branch from parent and calling get_best_qc() digest_type finality_mroot_claim; @@ -133,6 +133,8 @@ struct block_header_state { const finalizer_policy& get_last_proposed_finalizer_policy() const; finalizer_policy_diff calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const; finalizer_policy calculate_finalizer_policy(const finalizer_policy_diff& diff) const; + proposer_policy_diff calculate_proposer_policy_diff(const proposer_policy& new_policy) const; + proposer_policy calculate_proposer_policy(const proposer_policy_diff& diff) const; }; using block_header_state_ptr = std::shared_ptr; diff --git a/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp b/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp index ff2cb53e82..e3c893dc9c 100644 --- a/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp +++ b/libraries/chain/include/eosio/chain/finality/instant_finality_extension.hpp @@ -12,11 +12,11 @@ struct instant_finality_extension : fc::reflect_init { instant_finality_extension() = default; instant_finality_extension(qc_claim_t qc_claim, - std::optional new_finalizer_policy_diff, - std::shared_ptr new_proposer_policy) : + std::optional&& new_finalizer_policy_diff, + std::optional&& new_proposer_policy_diff) : qc_claim(qc_claim), new_finalizer_policy_diff(std::move(new_finalizer_policy_diff)), - new_proposer_policy(std::move(new_proposer_policy)) + new_proposer_policy_diff(std::move(new_proposer_policy_diff)) {} void reflector_init() const { @@ -27,9 +27,9 @@ struct instant_finality_extension : fc::reflect_init { qc_claim_t qc_claim; std::optional new_finalizer_policy_diff; - std::shared_ptr new_proposer_policy; + std::optional new_proposer_policy_diff; }; } /// eosio::chain -FC_REFLECT( eosio::chain::instant_finality_extension, (qc_claim)(new_finalizer_policy_diff)(new_proposer_policy) ) +FC_REFLECT( eosio::chain::instant_finality_extension, (qc_claim)(new_finalizer_policy_diff)(new_proposer_policy_diff) ) diff --git a/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp b/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp index 9c0a10dd5e..6ce10c23fb 100644 --- a/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp @@ -5,16 +5,41 @@ namespace eosio::chain { +static_assert(std::numeric_limits::max() >= config::max_producers - 1); +using producer_auth_differ = fc::ordered_diff; +using producer_auth_diff_t = producer_auth_differ::diff_result; + +struct proposer_policy_diff { + uint32_t version = 0; ///< sequentially incrementing version number of producer_authority_schedule + block_timestamp_type active_time; // block when schedule will become active + producer_auth_diff_t producer_auth_diff; +}; + 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; + + proposer_policy_diff create_diff(const proposer_policy& target) const { + return {.version = target.proposer_schedule.version, + .active_time = target.active_time, + .producer_auth_diff = producer_auth_differ::diff(proposer_schedule.producers, target.proposer_schedule.producers)}; + } + + template + requires std::same_as, proposer_policy_diff> + void apply_diff(X&& diff) { + proposer_schedule.version = diff.version; + active_time = diff.active_time; + proposer_schedule.producers = producer_auth_differ::apply_diff(std::move(proposer_schedule.producers), + std::forward(diff).producer_auth_diff); + } }; using proposer_policy_ptr = std::shared_ptr; } /// eosio::chain -FC_REFLECT( eosio::chain::proposer_policy, (schema_version)(active_time)(proposer_schedule) ) +FC_REFLECT( eosio::chain::proposer_policy, (active_time)(proposer_schedule) ) +FC_REFLECT( eosio::chain::producer_auth_diff_t, (remove_indexes)(insert_indexes) ) +FC_REFLECT( eosio::chain::proposer_policy_diff, (version)(active_time)(producer_auth_diff) ) diff --git a/unittests/block_header_tests.cpp b/unittests/block_header_tests.cpp index 67fdd24a8e..05551a7bc3 100644 --- a/unittests/block_header_tests.cpp +++ b/unittests/block_header_tests.cpp @@ -24,7 +24,7 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_empty_values_test) header.header_extensions, instant_finality_extension::extension_id(), fc::raw::pack( instant_finality_extension{qc_claim_t{last_qc_block_num, is_last_strong_qc}, - std::optional{}, std::shared_ptr{}} ) + std::optional{}, std::optional{}} ) ); std::optional ext = header.extract_header_extension(instant_finality_extension::extension_id()); @@ -34,7 +34,7 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_empty_values_test) BOOST_REQUIRE_EQUAL( if_extension.qc_claim.block_num, last_qc_block_num ); BOOST_REQUIRE_EQUAL( if_extension.qc_claim.is_strong_qc, is_last_strong_qc ); BOOST_REQUIRE( !if_extension.new_finalizer_policy_diff ); - BOOST_REQUIRE( !if_extension.new_proposer_policy ); + BOOST_REQUIRE( !if_extension.new_proposer_policy_diff ); } // test for instant_finality_extension uniqueness @@ -46,19 +46,19 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_uniqueness_test) header.header_extensions, instant_finality_extension::extension_id(), fc::raw::pack( instant_finality_extension{qc_claim_t{0, false}, {std::nullopt}, - std::shared_ptr{}} ) + std::optional{}} ) ); std::vector finalizers { {"test description", 50, fc::crypto::blslib::bls_public_key{"PUB_BLS_qVbh4IjYZpRGo8U_0spBUM-u-r_G0fMo4MzLZRsKWmm5uyeQTp74YFaMN9IDWPoVVT5rj_Tw1gvps6K9_OZ6sabkJJzug3uGfjA6qiaLbLh5Fnafwv-nVgzzzBlU2kwRrcHc8Q" }} }; auto fin_policy = std::make_shared(); finalizer_policy_diff new_finalizer_policy_diff = fin_policy->create_diff(finalizer_policy{.generation = 1, .threshold = 100, .finalizers = finalizers}); - proposer_policy_ptr new_proposer_policy = std::make_shared(1, block_timestamp_type{200}, producer_authority_schedule{} ); + proposer_policy_diff new_proposer_policy_diff = proposer_policy_diff{.version = 1, .active_time = block_timestamp_type{200}, .producer_auth_diff = {}}; emplace_extension( header.header_extensions, instant_finality_extension::extension_id(), - fc::raw::pack( instant_finality_extension{qc_claim_t{100, true}, new_finalizer_policy_diff, new_proposer_policy} ) + fc::raw::pack( instant_finality_extension{qc_claim_t{100, true}, new_finalizer_policy_diff, new_proposer_policy_diff} ) ); BOOST_CHECK_THROW(header.validate_and_extract_header_extensions(), invalid_block_header_extension); @@ -75,12 +75,12 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_values_test) auto fin_policy = std::make_shared(); finalizer_policy_diff new_finalizer_policy_diff = fin_policy->create_diff(finalizer_policy{.generation = 1, .threshold = 100, .finalizers = finalizers}); - proposer_policy_ptr new_proposer_policy = std::make_shared(1, block_timestamp_type{200}, producer_authority_schedule{} ); + proposer_policy_diff new_proposer_policy_diff = proposer_policy_diff{.version = 1, .active_time = block_timestamp_type{200}, .producer_auth_diff = {}}; emplace_extension( header.header_extensions, instant_finality_extension::extension_id(), - fc::raw::pack( instant_finality_extension{qc_claim_t{last_qc_block_num, is_strong_qc}, new_finalizer_policy_diff, new_proposer_policy} ) + fc::raw::pack( instant_finality_extension{qc_claim_t{last_qc_block_num, is_strong_qc}, new_finalizer_policy_diff, new_proposer_policy_diff} ) ); std::optional ext = header.extract_header_extension(instant_finality_extension::extension_id()); @@ -98,9 +98,8 @@ BOOST_AUTO_TEST_CASE(instant_finality_extension_with_values_test) BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->finalizers_diff.insert_indexes[0].second.weight, 50u); BOOST_REQUIRE_EQUAL(if_extension.new_finalizer_policy_diff->finalizers_diff.insert_indexes[0].second.public_key.to_string(), "PUB_BLS_qVbh4IjYZpRGo8U_0spBUM-u-r_G0fMo4MzLZRsKWmm5uyeQTp74YFaMN9IDWPoVVT5rj_Tw1gvps6K9_OZ6sabkJJzug3uGfjA6qiaLbLh5Fnafwv-nVgzzzBlU2kwRrcHc8Q"); - 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( !!if_extension.new_proposer_policy_diff ); + fc::time_point t = (fc::time_point)(if_extension.new_proposer_policy_diff->active_time); BOOST_REQUIRE_EQUAL(t.time_since_epoch().to_seconds(), 946684900ll); } diff --git a/unittests/producer_schedule_if_tests.cpp b/unittests/producer_schedule_if_tests.cpp index 6f83e19992..5a1c4ef391 100644 --- a/unittests/producer_schedule_if_tests.cpp +++ b/unittests/producer_schedule_if_tests.cpp @@ -122,6 +122,19 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t fc::mutable_variant_object()("schedule", schedule_variant), DEFAULT_EXPIRATION_DELTA + (++unique)); }; + auto verify_block_if_ext_producer = [](const signed_block_ptr& block, uint32_t version, account_name new_producer) { + std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); + BOOST_TEST(!!ext); + std::optional policy_diff = std::get(*ext).new_proposer_policy_diff; + BOOST_TEST_REQUIRE(!!policy_diff); + BOOST_TEST(policy_diff->version == version); + bool new_producer_in_insert = std::ranges::find_if(policy_diff->producer_auth_diff.insert_indexes, + [&](const auto& e) { + return e.second.producer_name == new_producer; + }) != policy_diff->producer_auth_diff.insert_indexes.end(); + BOOST_TEST(new_producer_in_insert); + }; + while (control->head_block_num() < 3) { produce_block(); } @@ -139,12 +152,14 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // set a new proposer policy sch1 set_producers( {"alice"_n} ); + auto b = produce_block(); + verify_block_if_ext_producer(b, 1, "alice"_n); vector alice_sch = { producer_authority{"alice"_n, block_signing_authority_v0{1, {{get_public_key("alice"_n, "active"), 1}}}} }; // start a round of production - produce_blocks(config::producer_repetitions); + produce_blocks(config::producer_repetitions-1); // sch1 cannot become active before one round of production BOOST_CHECK_EQUAL( 0u, control->active_producers().version ); @@ -156,7 +171,8 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t producer_authority{"bob"_n, block_signing_authority_v0{ 1, {{get_public_key("bob"_n, "active"),1}}}}, producer_authority{"carol"_n, block_signing_authority_v0{ 1, {{get_public_key("carol"_n, "active"),1}}}} }; - produce_block(); + b = produce_block(); + verify_block_if_ext_producer(b, 2u, "bob"_n); // set another ploicy should replace sch2 set_producers( {"bob"_n,"alice"_n} ); @@ -164,9 +180,11 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t producer_authority{"bob"_n, block_signing_authority_v0{ 1, {{get_public_key("bob"_n, "active"),1}}}}, producer_authority{"alice"_n, block_signing_authority_v0{ 1, {{get_public_key("alice"_n, "active"),1}}}} }; + b = produce_block(); + verify_block_if_ext_producer(b, 3u, "alice"_n); // another round - produce_blocks(config::producer_repetitions-1); // -1, already produced one of the round above + produce_blocks(config::producer_repetitions-2); // -2, already produced tow of the round above // sch1 must become active no later than 2 rounds but sch2 cannot become active yet BOOST_CHECK_EQUAL( control->active_producers().version, 1u ); @@ -184,7 +202,13 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // test no change to active schedule set_producers( {"bob"_n,"alice"_n} ); // same as before, so no change - produce_blocks(config::producer_repetitions); + b = produce_block(); + std::optional ext = b->extract_header_extension(instant_finality_extension::extension_id()); + BOOST_TEST(!!ext); + std::optional policy_diff = std::get(*ext).new_proposer_policy_diff; + BOOST_TEST_REQUIRE(!policy_diff); // no diff + + produce_blocks(config::producer_repetitions-1); produce_blocks(config::producer_repetitions); BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 3 as not different so no change BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); @@ -208,10 +232,13 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // test change in same block where there is an existing proposed that is the same set_producers( {"bob"_n,"alice"_n} ); - produce_block(); + b = produce_block(); + verify_block_if_ext_producer(b, 5u, "alice"_n); set_producers( {"bob"_n,"carol"_n} ); set_producers_force({"bob"_n,"carol"_n} ); - produce_blocks(config::producer_repetitions-1); + b = produce_block(); + verify_block_if_ext_producer(b, 6u, "carol"_n); + produce_blocks(config::producer_repetitions-2); produce_blocks(config::producer_repetitions); BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); // should be 6 now as bob,carol now active BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); @@ -238,9 +265,12 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t produce_blocks(config::producer_repetitions-1); // A12 produce_block(); set_producers({"bob"_n,"carol"_n}); // B2 - produce_block(); + b = produce_block(); + verify_block_if_ext_producer(b, 8u, "bob"_n); set_producers({"bob"_n, "alice"_n} ); // P3 - produce_blocks(config::producer_repetitions-2); // B12 + b = produce_block(); + verify_block_if_ext_producer(b, 9u, "alice"_n); + produce_blocks(config::producer_repetitions-3); // B12 produce_block(); // C1 BOOST_CHECK_EQUAL( 7u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( alice_sch, control->active_producers() ) ); @@ -259,12 +289,17 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // propose P3 in B3, active in D1, replaces P2 produce_block(); set_producers({"bob"_n,"carol"_n}); // A2, P1 - produce_blocks(config::producer_repetitions-1); // A12 + b = produce_block(); + verify_block_if_ext_producer(b, 10u, "carol"_n); + produce_blocks(config::producer_repetitions-2); // A12 produce_block(); set_producers({"alice"_n}); // B2 - produce_block(); + b = produce_block(); + verify_block_if_ext_producer(b, 11u, "alice"_n); set_producers({"bob"_n,"carol"_n}); // P3 == P1 - produce_blocks(config::producer_repetitions-2); // B12 + b = produce_block(); + verify_block_if_ext_producer(b, 12u, "bob"_n); + produce_blocks(config::producer_repetitions-3); // B12 produce_block(); // C1 BOOST_CHECK_EQUAL( 10u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); @@ -281,25 +316,34 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers({"bob"_n,"carol"_n}); produce_block(); // 2 set_producers({"alice"_n}); - produce_block(); // 3 + b = produce_block(); // 3 + verify_block_if_ext_producer(b, 13u, "alice"_n); set_producers({"carol"_n,"alice"_n}); - produce_block(); // 4 + b = produce_block(); // 4 + verify_block_if_ext_producer(b, 14u, "carol"_n); set_producers({"carol"_n}); produce_block(); // 5 set_producers({"alice"_n}); - produce_block(); // 6 + b = produce_block(); // 6 + verify_block_if_ext_producer(b, 16u, "alice"_n); set_producers({"bob"_n,"carol"_n}); - produce_blocks(config::producer_repetitions-6); + b = produce_block(); + verify_block_if_ext_producer(b, 17u, "bob"_n); + produce_blocks(config::producer_repetitions-7); set_producers({"bob"_n}); produce_block(); // 2 set_producers({"bob"_n,"carol"_n}); - produce_block(); // 3 + b = produce_block(); // 3 + verify_block_if_ext_producer(b, 19u, "carol"_n); set_producers({"carol"_n,"bob"_n}); produce_block(); // 4 set_producers({"alice"_n} ); - produce_block(); // 5 + b = produce_block(); // 5 + verify_block_if_ext_producer(b, 21u, "alice"_n); set_producers({"bob"_n,"carol"_n}); - produce_blocks(config::producer_repetitions-5); // B12 + b = produce_block(); + verify_block_if_ext_producer(b, 22u, "bob"_n); + produce_blocks(config::producer_repetitions-6); // B12 BOOST_CHECK_EQUAL( 17u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); produce_blocks(config::producer_repetitions); From b9d2d479abfd5004d1ce28cd2d4afe500c3b8c41 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 14:43:30 -0500 Subject: [PATCH 29/41] GH-5 Fix static_assert --- libraries/chain/include/eosio/chain/config.hpp | 2 +- .../chain/include/eosio/chain/finality/finalizer_policy.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index c82d6c8c05..1712ef7685 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -134,7 +134,7 @@ static_assert(maximum_tracked_dpos_confirmations >= ((max_producers * 2 / 3) + 1 /** * Maximum number of finalizers in the finalizer set */ -const static size_t max_finalizers = 65535; // largest allowed finalizer policy diff +const static size_t max_finalizers = 64*1024; // largest allowed finalizer policy diff const static size_t max_finalizer_description_size = 256; /** diff --git a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp index 2637cd90c3..9a616ba1d0 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp @@ -6,7 +6,7 @@ namespace eosio::chain { - static_assert(std::numeric_limits::max() <= config::max_finalizers); + static_assert(std::numeric_limits::max() >= config::max_finalizers - 1); using finalizers_differ = fc::ordered_diff; using finalizers_diff_t = finalizers_differ::diff_result; From f69c2b7eaf9185879945696875cc135f56a0c1c7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 14:43:59 -0500 Subject: [PATCH 30/41] GH-5 rename lambda --- unittests/finalizer_update_tests.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/unittests/finalizer_update_tests.cpp b/unittests/finalizer_update_tests.cpp index bd043170a8..db71dfa21c 100644 --- a/unittests/finalizer_update_tests.cpp +++ b/unittests/finalizer_update_tests.cpp @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { size_t num_keys = 50u; size_t finset_size = 21u; - auto verify_block_finality_generation = [](const signed_block_ptr& block, uint32_t gen, const bls_public_key& key) { + auto verify_block_finality_policy_diff = [](const signed_block_ptr& block, uint32_t gen, const bls_public_key& key) { std::optional ext = block->extract_header_extension(instant_finality_extension::extension_id()); BOOST_TEST(!!ext); std::optional fin_policy_diff = std::get(*ext).new_finalizer_policy_diff; @@ -109,38 +109,38 @@ BOOST_AUTO_TEST_CASE(savanna_set_finalizer_multiple_test) { try { // ------------------------------------------------------------------------------ auto pubkeys3 = fin_keys.set_finalizer_policy(3u).pubkeys; auto b = t.produce_block(); - verify_block_finality_generation(b, 3, pubkeys3.back()); + verify_block_finality_policy_diff(b, 3, pubkeys3.back()); auto pubkeys4 = fin_keys.set_finalizer_policy(4u).pubkeys; b = t.produce_block(); - verify_block_finality_generation(b, 4, pubkeys4.back()); + verify_block_finality_policy_diff(b, 4, pubkeys4.back()); t.produce_block(); auto pubkeys5 = fin_keys.set_finalizer_policy(5u).pubkeys; b = t.produce_block(); - verify_block_finality_generation(b, 5, pubkeys5.back()); + verify_block_finality_policy_diff(b, 5, pubkeys5.back()); t.produce_block(); auto pubkeys6 = fin_keys.set_finalizer_policy(6u).pubkeys; b = t.produce_block(); - verify_block_finality_generation(b, 6, pubkeys6.back()); + verify_block_finality_policy_diff(b, 6, pubkeys6.back()); auto pubkeys7 = fin_keys.set_finalizer_policy(7u).pubkeys; t.check_head_finalizer_policy(2u, pubkeys2); // 5 blocks after pubkeys3 (b5 - b0), pubkeys2 should still be active b = t.produce_block(); - verify_block_finality_generation(b, 7, pubkeys7.back()); + verify_block_finality_policy_diff(b, 7, pubkeys7.back()); auto pubkeys8 = fin_keys.set_finalizer_policy(8u).pubkeys; t.check_head_finalizer_policy(3u, pubkeys3); // 6 blocks after pubkeys3 (b6 - b0), pubkeys3 should be active b = t.produce_block(); - verify_block_finality_generation(b, 8, pubkeys8.back()); + verify_block_finality_policy_diff(b, 8, pubkeys8.back()); auto pubkeys9 = fin_keys.set_finalizer_policy(9u).pubkeys; t.check_head_finalizer_policy(4u, pubkeys4); // 6 blocks after pubkeys4 (b7 - b1), pubkeys4 should be active b = t.produce_block(); - verify_block_finality_generation(b, 9, pubkeys9.back()); + verify_block_finality_policy_diff(b, 9, pubkeys9.back()); auto pubkeys10 = fin_keys.set_finalizer_policy(10u).pubkeys; t.check_head_finalizer_policy(4u, pubkeys4); // 7 blocks after pubkeys4, pubkeys4 should still be active b = t.produce_block(); - verify_block_finality_generation(b, 10, pubkeys10.back()); + verify_block_finality_policy_diff(b, 10, pubkeys10.back()); auto pubkeys11 = fin_keys.set_finalizer_policy(11u).pubkeys; t.check_head_finalizer_policy(5u, pubkeys5); // 6 blocks after pubkeys5 (b9 - b3), pubkeys5 should be active b = t.produce_block(); - verify_block_finality_generation(b, 11, pubkeys11.back()); + verify_block_finality_policy_diff(b, 11, pubkeys11.back()); t.produce_block(); // two blocks between 5 & 6 proposals t.check_head_finalizer_policy(6u, pubkeys6); // the rest are all one block apart, tests pending with propsed auto b12 = t.produce_block(); From 1be5d61fa6e917ad555a4ad3a8c2029b6102562c Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 9 May 2024 17:28:06 -0400 Subject: [PATCH 31/41] Remove redundant initialization of proposed_finalizer_policy to std::nullopt --- libraries/chain/block_state.cpp | 2 +- libraries/chain/include/eosio/chain/block_state.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index eecb41f9a7..968ade4300 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -339,7 +339,7 @@ finality_data_t block_state::get_finality_data() { } // Check if there is any proposed finalizer policy in the block - std::optional proposed_finalizer_policy = std::nullopt; + std::optional proposed_finalizer_policy; auto range = finalizer_policies.equal_range(block_num()); for (auto itr = range.first; itr != range.second; ++itr) { if (itr->second.state == finalizer_policy_tracker::state_t::proposed) { diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp index cb13675756..04039e864a 100644 --- a/libraries/chain/include/eosio/chain/block_state.hpp +++ b/libraries/chain/include/eosio/chain/block_state.hpp @@ -64,7 +64,7 @@ struct finality_data_t { uint32_t active_finalizer_policy_generation{0}; digest_type action_mroot{}; digest_type base_digest{}; - std::optional proposed_finalizer_policy{std::nullopt}; // finalizer policy, if proposed in the block + std::optional proposed_finalizer_policy; // finalizer policy, if proposed in the block }; struct block_state : public block_header_state { // block_header_state provides parent link From 8c77c797cebb090e62b733bd8d6ea3eac31750c7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 16:36:13 -0500 Subject: [PATCH 32/41] Increase max proposers to match max finalizers --- libraries/chain/controller.cpp | 4 ++++ libraries/chain/include/eosio/chain/config.hpp | 3 ++- .../chain/include/eosio/chain/finality/proposer_policy.hpp | 4 ++-- libraries/chain/webassembly/privileged.cpp | 1 - 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 9086bc5c56..bb0474b588 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5283,6 +5283,8 @@ int64_t controller_impl::set_proposed_producers( vector prod if (producers.empty()) return -1; // INSTANT_FINALITY depends on DISALLOW_EMPTY_PRODUCER_SCHEDULE + EOS_ASSERT(producers.size() <= config::max_proposers, wasm_execution_error, + "Producer schedule exceeds the maximum proposer count for this chain"); assert(pending); producer_authority_schedule sch; @@ -5301,6 +5303,8 @@ int64_t controller_impl::set_proposed_producers( vector prod } int64_t controller_impl::set_proposed_producers_legacy( vector producers ) { + EOS_ASSERT(producers.size() <= config::max_producers, wasm_execution_error, + "Producer schedule exceeds the maximum producer count for this chain"); const auto& gpo = db.get(); auto cur_block_num = chain_head.block_num() + 1; diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index 1712ef7685..0baaed9870 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -126,7 +126,8 @@ const static uint32_t default_abi_serializer_max_time_us = 15*1000; ///< defau * The number of sequential blocks produced by a single producer */ const static int producer_repetitions = 12; -const static int max_producers = 125; +const static int max_producers = 125; // pre-savanna producer (proposer) limit +const static int max_proposers = 64*1024; // savanna max proposer (producer) limit const static size_t maximum_tracked_dpos_confirmations = 1024; ///< static_assert(maximum_tracked_dpos_confirmations >= ((max_producers * 2 / 3) + 1) * producer_repetitions, "Settings never allow for DPOS irreversibility" ); diff --git a/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp b/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp index 6ce10c23fb..0ecc04fde1 100644 --- a/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp @@ -5,8 +5,8 @@ namespace eosio::chain { -static_assert(std::numeric_limits::max() >= config::max_producers - 1); -using producer_auth_differ = fc::ordered_diff; +static_assert(std::numeric_limits::max() >= config::max_proposers - 1); +using producer_auth_differ = fc::ordered_diff; using producer_auth_diff_t = producer_auth_differ::diff_result; struct proposer_policy_diff { diff --git a/libraries/chain/webassembly/privileged.cpp b/libraries/chain/webassembly/privileged.cpp index 5149421d39..88c5c92540 100644 --- a/libraries/chain/webassembly/privileged.cpp +++ b/libraries/chain/webassembly/privileged.cpp @@ -44,7 +44,6 @@ namespace eosio { namespace chain { namespace webassembly { } int64_t set_proposed_producers_common( apply_context& context, vector && producers, bool validate_keys ) { - EOS_ASSERT(producers.size() <= config::max_producers, wasm_execution_error, "Producer schedule exceeds the maximum producer count for this chain"); EOS_ASSERT( producers.size() > 0 || !context.control.is_builtin_activated( builtin_protocol_feature_t::disallow_empty_producer_schedule ), wasm_execution_error, From 0321b45154403564d2613bd52a5db0af21ecac60 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 16:38:30 -0500 Subject: [PATCH 33/41] Update comment --- libraries/chain/include/eosio/chain/config.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index 0baaed9870..41d663f2eb 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -127,7 +127,7 @@ const static uint32_t default_abi_serializer_max_time_us = 15*1000; ///< defau */ const static int producer_repetitions = 12; const static int max_producers = 125; // pre-savanna producer (proposer) limit -const static int max_proposers = 64*1024; // savanna max proposer (producer) limit +const static int max_proposers = 64*1024; // savanna proposer (producer) limit const static size_t maximum_tracked_dpos_confirmations = 1024; ///< static_assert(maximum_tracked_dpos_confirmations >= ((max_producers * 2 / 3) + 1) * producer_repetitions, "Settings never allow for DPOS irreversibility" ); From 35910b60c06f1cb3f50bc921e14e8ef77efe8bb2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 9 May 2024 17:49:22 -0500 Subject: [PATCH 34/41] GH-5 Simplify implementation --- libraries/chain/block_header_state.cpp | 39 ++++--------------- libraries/chain/block_state.cpp | 3 +- .../eosio/chain/block_header_state.hpp | 5 +-- .../eosio/chain/finality/finalizer_policy.hpp | 11 ++++-- .../eosio/chain/finality/proposer_policy.hpp | 13 ++++--- .../testing/include/eosio/testing/tester.hpp | 4 +- unittests/svnn_ibc_tests.cpp | 4 +- 7 files changed, 28 insertions(+), 51 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 5206326f44..c612870b21 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -79,38 +79,15 @@ const finalizer_policy& block_header_state::get_last_proposed_finalizer_policy() return *active_finalizer_policy; } -finalizer_policy_diff block_header_state::calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const { - return get_last_proposed_finalizer_policy().create_diff(new_policy); -} - -finalizer_policy block_header_state::calculate_finalizer_policy(const finalizer_policy_diff& diff) const { - finalizer_policy result = get_last_proposed_finalizer_policy(); - result.apply_diff(diff); - return result; -} - -proposer_policy_diff block_header_state::calculate_proposer_policy_diff(const proposer_policy& new_policy) const { - if (proposer_policies.empty()) { - assert(active_proposer_policy); - return active_proposer_policy->create_diff(new_policy); - } - auto it = proposer_policies.rbegin(); - assert(it != proposer_policies.rend()); - return it->second->create_diff(new_policy); -} - -proposer_policy block_header_state::calculate_proposer_policy(const proposer_policy_diff& diff) const { +// The last proposed proposer policy, if none proposed then the active proposer policy +const proposer_policy& block_header_state::get_last_proposed_proposer_policy() const { if (proposer_policies.empty()) { assert(active_proposer_policy); - proposer_policy result = *active_proposer_policy; - result.apply_diff(diff); - return result; + return *active_proposer_policy; } auto it = proposer_policies.rbegin(); assert(it != proposer_policies.rend()); - proposer_policy result = *it->second; - result.apply_diff(diff); - return result; + return *it->second; } // ------------------------------------------------------------------------------------------------- @@ -151,7 +128,7 @@ void finish_next(const block_header_state& prev, std::optional new_proposer_policy; if (if_ext.new_proposer_policy_diff) { - new_proposer_policy = prev.calculate_proposer_policy(*if_ext.new_proposer_policy_diff); + new_proposer_policy = prev.get_last_proposed_proposer_policy().apply_diff(*if_ext.new_proposer_policy_diff); } if (new_proposer_policy) { // called when assembling the block @@ -207,7 +184,7 @@ void finish_next(const block_header_state& prev, } if (if_ext.new_finalizer_policy_diff) { - finalizer_policy new_finalizer_policy = prev.calculate_finalizer_policy(*if_ext.new_finalizer_policy_diff); + finalizer_policy new_finalizer_policy = prev.get_last_proposed_finalizer_policy().apply_diff(*if_ext.new_finalizer_policy_diff); // a new `finalizer_policy` was proposed in the previous block, and is present in the previous // block's header extensions. @@ -258,11 +235,11 @@ block_header_state block_header_state::next(block_header_state_input& input) con // ------------------ std::optional new_finalizer_policy_diff; if (input.new_finalizer_policy) { - new_finalizer_policy_diff = calculate_finalizer_policy_diff(*input.new_finalizer_policy); + new_finalizer_policy_diff = get_last_proposed_finalizer_policy().create_diff(*input.new_finalizer_policy); } std::optional new_proposer_policy_diff; if (input.new_proposer_policy) { - new_proposer_policy_diff = calculate_proposer_policy_diff(*input.new_proposer_policy); + new_proposer_policy_diff = get_last_proposed_proposer_policy().create_diff(*input.new_proposer_policy); } instant_finality_extension new_if_ext { input.most_recent_ancestor_with_qc, std::move(new_finalizer_policy_diff), diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 690ba7408c..e287e4f8f3 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -73,8 +73,7 @@ block_state_ptr block_state::create_if_genesis_block(const block_state_legacy& b assert(bsp.block->contains_header_extension(instant_finality_extension::extension_id())); // required by transition mechanism instant_finality_extension if_ext = bsp.block->extract_header_extension(); assert(if_ext.new_finalizer_policy_diff); // required by transition mechanism - result.active_finalizer_policy = std::make_shared(); - result.active_finalizer_policy->apply_diff(std::move(*if_ext.new_finalizer_policy_diff)); + result.active_finalizer_policy = std::make_shared(finalizer_policy{}.apply_diff(std::move(*if_ext.new_finalizer_policy_diff))); result.active_proposer_policy = std::make_shared(); result.active_proposer_policy->active_time = bsp.timestamp(); result.active_proposer_policy->proposer_schedule = bsp.active_schedule; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index f9c6600e89..e6c6c6fb6e 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -131,10 +131,7 @@ struct block_header_state { const producer_authority& get_scheduled_producer(block_timestamp_type t) const; const finalizer_policy& get_last_proposed_finalizer_policy() const; - finalizer_policy_diff calculate_finalizer_policy_diff(const finalizer_policy& new_policy) const; - finalizer_policy calculate_finalizer_policy(const finalizer_policy_diff& diff) const; - proposer_policy_diff calculate_proposer_policy_diff(const proposer_policy& new_policy) const; - proposer_policy calculate_proposer_policy(const proposer_policy_diff& diff) const; + const proposer_policy& get_last_proposed_proposer_policy() const; }; using block_header_state_ptr = std::shared_ptr; diff --git a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp index 9a616ba1d0..dc3d03c869 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer_policy.hpp @@ -29,10 +29,13 @@ namespace eosio::chain { template requires std::same_as, finalizer_policy_diff> - void apply_diff(X&& diff) { - generation = diff.generation; - threshold = diff.threshold; - finalizers = finalizers_differ::apply_diff(std::move(finalizers), std::forward(diff).finalizers_diff); + [[nodiscard]] finalizer_policy apply_diff(X&& diff) const { + finalizer_policy result; + result.generation = diff.generation; + result.threshold = diff.threshold; + auto copy = finalizers; + result.finalizers = finalizers_differ::apply_diff(std::move(copy), std::forward(diff).finalizers_diff); + return result; } // max accumulated weak weight before becoming weak_final diff --git a/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp b/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp index 6ce10c23fb..01be9f476f 100644 --- a/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp +++ b/libraries/chain/include/eosio/chain/finality/proposer_policy.hpp @@ -28,11 +28,14 @@ struct proposer_policy { template requires std::same_as, proposer_policy_diff> - void apply_diff(X&& diff) { - proposer_schedule.version = diff.version; - active_time = diff.active_time; - proposer_schedule.producers = producer_auth_differ::apply_diff(std::move(proposer_schedule.producers), - std::forward(diff).producer_auth_diff); + [[nodiscard]] proposer_policy apply_diff(X&& diff) const { + proposer_policy result; + result.proposer_schedule.version = diff.version; + result.active_time = diff.active_time; + auto copy = proposer_schedule.producers; + result.proposer_schedule.producers = producer_auth_differ::apply_diff(std::move(copy), + std::forward(diff).producer_auth_diff); + return result; } }; diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index fc82431b0e..40bacee8a9 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -861,9 +861,7 @@ namespace eosio::testing { auto b = produce_block(); BOOST_REQUIRE_EQUAL(t.lib_block->block_num(), pt_block->block_num()); - finalizer_policy fin_policy; - fin_policy.apply_diff(*fin_policy_diff); - return fin_policy; + return finalizer_policy{}.apply_diff(*fin_policy_diff); } Tester& t; diff --git a/unittests/svnn_ibc_tests.cpp b/unittests/svnn_ibc_tests.cpp index ab70b0eac0..477c945d33 100644 --- a/unittests/svnn_ibc_tests.cpp +++ b/unittests/svnn_ibc_tests.cpp @@ -103,8 +103,8 @@ BOOST_AUTO_TEST_SUITE(svnn_ibc) BOOST_CHECK(maybe_active_finalizer_policy_diff.has_value()); eosio::chain::finalizer_policy_diff active_finalizer_policy_diff = maybe_active_finalizer_policy_diff.value(); - eosio::chain::finalizer_policy active_finalizer_policy; - active_finalizer_policy.apply_diff(active_finalizer_policy_diff); + eosio::chain::finalizer_policy active_finalizer_policy = + eosio::chain::finalizer_policy{}.apply_diff(active_finalizer_policy_diff); BOOST_CHECK_EQUAL(active_finalizer_policy.generation, 1u); From d13759ed61a5a5b05a9de00aaa67b284c991e38e Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 10 May 2024 10:06:47 -0400 Subject: [PATCH 35/41] Add is_savanna_genesis_block() to block_header_state --- libraries/chain/include/eosio/chain/block_header_state.hpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 7fd43afe32..22c11318f8 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -119,6 +119,9 @@ struct block_header_state { digest_type compute_base_digest() const; digest_type compute_finality_digest() const; + // Returns true if the block is a Savanna Genesis Block + bool is_savanna_genesis_block() const { return core.is_genesis_block_num(block_num()); } + // Returns true if the block is a Proper Savanna Block bool is_proper_svnn_block() const { return header.is_proper_svnn_block(); } From 874879ed9d4808522e7e1280aa62a39211b28cec Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 10 May 2024 10:08:14 -0400 Subject: [PATCH 36/41] Use active_finalizer_policy for finality_data if it is the Savanna Genesis Block --- libraries/chain/block_state.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 968ade4300..f2aa9d872c 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -340,11 +340,16 @@ finality_data_t block_state::get_finality_data() { // Check if there is any proposed finalizer policy in the block std::optional proposed_finalizer_policy; - auto range = finalizer_policies.equal_range(block_num()); - for (auto itr = range.first; itr != range.second; ++itr) { - if (itr->second.state == finalizer_policy_tracker::state_t::proposed) { - proposed_finalizer_policy = *itr->second.policy; - break; + if (is_savanna_genesis_block()) { + // For Genesis Block, use the acttive finalizer policy which was proposed in the block. + proposed_finalizer_policy = *active_finalizer_policy; + } else { + auto range = finalizer_policies.equal_range(block_num()); + for (auto itr = range.first; itr != range.second; ++itr) { + if (itr->second.state == finalizer_policy_tracker::state_t::proposed) { + proposed_finalizer_policy = *itr->second.policy; + break; + } } } From 60ea6b984f4defe023f9c2a58461d13def42ab95 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 10 May 2024 10:59:40 -0400 Subject: [PATCH 37/41] update comments --- libraries/chain/block_state.cpp | 2 +- libraries/chain/include/eosio/chain/block_header_state.hpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 9273264e0a..f33d5345f1 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -341,7 +341,7 @@ finality_data_t block_state::get_finality_data() { // Check if there is any proposed finalizer policy in the block std::optional proposed_finalizer_policy; if (is_savanna_genesis_block()) { - // For Genesis Block, use the acttive finalizer policy which was proposed in the block. + // For Genesis Block, use the active finalizer policy which was proposed in the block. proposed_finalizer_policy = *active_finalizer_policy; } else { auto range = finalizer_policies.equal_range(block_num()); diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 1c4f2718fc..9daf9d7815 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -119,7 +119,8 @@ struct block_header_state { digest_type compute_base_digest() const; digest_type compute_finality_digest() const; - // Returns true if the block is a Savanna Genesis Block + // Returns true if the block is a Savanna Genesis Block. + // This method is applicable to any transition block which is re-classified as a Savanna block. bool is_savanna_genesis_block() const { return core.is_genesis_block_num(block_num()); } // Returns true if the block is a Proper Savanna Block From 035adf2429e9bcf88560742e40845e4a5846aac7 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 10 May 2024 11:10:37 -0400 Subject: [PATCH 38/41] Fix a compile warning in ordered_diff_moveable_test --- libraries/libfc/test/test_ordered_diff.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/libfc/test/test_ordered_diff.cpp b/libraries/libfc/test/test_ordered_diff.cpp index e7ffcfebea..6477c3f8a3 100644 --- a/libraries/libfc/test/test_ordered_diff.cpp +++ b/libraries/libfc/test/test_ordered_diff.cpp @@ -192,9 +192,9 @@ BOOST_AUTO_TEST_CASE(ordered_diff_moveable_test) try { auto result = ordered_diff::diff(source, target); source = ordered_diff::apply_diff(std::move(source), std::move(result)); BOOST_TEST(source == target); - BOOST_TEST(count_moves::num_moves == 1); + BOOST_TEST(count_moves::num_moves == 1u); } } FC_LOG_AND_RETHROW(); -BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file +BOOST_AUTO_TEST_SUITE_END() From 0c8be97451c085efe7175ddff07ca4c0072515b4 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 10 May 2024 18:13:51 -0400 Subject: [PATCH 39/41] Correctly support when finality advances multiple blocks at a time. In that case, duplicate policies can become pending for the same block. --- libraries/chain/block_header_state.cpp | 32 ++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index c612870b21..a70909b869 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; } From 79dd55e68b8629306066f612b7319c57e74ba8ab Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 13 May 2024 11:15:00 -0400 Subject: [PATCH 40/41] Add test for issue #130 (correct `finalizer_policies` update when finality skips blocks). --- libraries/chain/block_header_state.cpp | 32 +++++++ libraries/chain/controller.cpp | 4 + .../eosio/chain/block_header_state.hpp | 2 + .../eosio/chain/block_header_state_legacy.hpp | 2 + .../chain/include/eosio/chain/controller.hpp | 1 + .../testing/include/eosio/testing/tester.hpp | 1 + unittests/finality_tests.cpp | 96 +++++++++++++++++++ 7 files changed, 138 insertions(+) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index a70909b869..3677d1d48c 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -345,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..bb3ca56a7a 100644 --- a/unittests/finality_tests.cpp +++ b/unittests/finality_tests.cpp @@ -522,4 +522,100 @@ 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<2; ++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); + } + + produce_and_push_block(); + process_votes(1, num_nodes - 1); + node0.check_head_finalizer_policy(4u, pubkeys3); + +#if 0 + + // Now process the votes for these last 3 blocks and send the QC in new block. + // After the `push_block()`, all 3 set_finalizers should become pending. + // -------------------------------------------------------------------------- + for (size_t i=3; i<6; ++i) + process_votes(1, num_nodes - 1, i); + produce_and_push_block(); + + // 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); + + // After another 3-chain, pubkeys3 should become active. + // The two `set_finalizers` for pubkeys1 and pubkeys2 should have been skipped + // so generation should be 2. + // ----------------------------------------------------- + for (size_t i=0; i<6; ++i) { + produce_and_push_block(); + process_votes(1, num_nodes - 1); + } + node0.check_head_finalizer_policy(2u, pubkeys3); +#endif + +} FC_LOG_AND_RETHROW() } + + + + BOOST_AUTO_TEST_SUITE_END() From 9852aff081119de67555f2a03b54bab44d2732aa Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 13 May 2024 11:24:42 -0400 Subject: [PATCH 41/41] Small test update. --- unittests/finality_tests.cpp | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/unittests/finality_tests.cpp b/unittests/finality_tests.cpp index bb3ca56a7a..83e73a0766 100644 --- a/unittests/finality_tests.cpp +++ b/unittests/finality_tests.cpp @@ -547,7 +547,7 @@ BOOST_FIXTURE_TEST_CASE(finality_skip, finality_test_cluster<4>) { try { // produce 2 blocks that will be made final after the three `add_set_finalizers` below // ------------------------------------------------------------------------------------ - for (size_t i=0; i<2; ++i) { + for (size_t i=0; i<4; ++i) { produce_and_push_block(); process_votes(1, num_nodes - 1); } @@ -584,35 +584,13 @@ BOOST_FIXTURE_TEST_CASE(finality_skip, finality_test_cluster<4>) { try { 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); -#if 0 - - // Now process the votes for these last 3 blocks and send the QC in new block. - // After the `push_block()`, all 3 set_finalizers should become pending. - // -------------------------------------------------------------------------- - for (size_t i=3; i<6; ++i) - process_votes(1, num_nodes - 1, i); - produce_and_push_block(); - - // 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); - - // After another 3-chain, pubkeys3 should become active. - // The two `set_finalizers` for pubkeys1 and pubkeys2 should have been skipped - // so generation should be 2. - // ----------------------------------------------------- - for (size_t i=0; i<6; ++i) { - produce_and_push_block(); - process_votes(1, num_nodes - 1); - } - node0.check_head_finalizer_policy(2u, pubkeys3); -#endif - } FC_LOG_AND_RETHROW() }