From c4bbe578cc47d0817bd5aa42132eebd629af0c62 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 26 Apr 2024 06:33:56 -0500 Subject: [PATCH 01/11] GH-6 Fix issue with setting proposed proposer schedule more than once in a block --- libraries/chain/controller.cpp | 16 ++++++- unittests/producer_schedule_if_tests.cpp | 53 ++++++++++++++++++++---- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 750015fee5..72d4b1f429 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5216,6 +5216,21 @@ int64_t controller_impl::set_proposed_producers( vector prod assert(pending); + // see if one already proposed in this block + auto& gpo = db.get(); + if (gpo.proposed_schedule_block_num) { + if (std::equal(producers.begin(), producers.end(), + gpo.proposed_schedule.producers.begin(), gpo.proposed_schedule.producers.end())) { + return gpo.proposed_schedule.version; // the proposed producer schedule does not change + } + // clear gpo proposed_schedule as we may determine no diff between proposed producers and next proposer schedule + db.modify( gpo, [&]( auto& gp ) { + gp.proposed_schedule_block_num.reset(); + gp.proposed_schedule.version = 0; + gp.proposed_schedule.producers.clear(); + }); + } + auto [version, diff] = pending->get_next_proposer_schedule_version(producers); if (!diff) return version; @@ -5228,7 +5243,6 @@ int64_t controller_impl::set_proposed_producers( vector prod // overwrite any existing proposed_schedule set earlier in this block auto cur_block_num = chain_head.block_num() + 1; - auto& gpo = db.get(); db.modify( gpo, [&]( auto& gp ) { gp.proposed_schedule_block_num = cur_block_num; gp.proposed_schedule = sch; diff --git a/unittests/producer_schedule_if_tests.cpp b/unittests/producer_schedule_if_tests.cpp index ff1afe3b6a..0e3968dc2d 100644 --- a/unittests/producer_schedule_if_tests.cpp +++ b/unittests/producer_schedule_if_tests.cpp @@ -115,6 +115,19 @@ bool compare_schedules( const vector& a, const producer_auth BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) try { create_accounts( {"alice"_n,"bob"_n,"carol"_n} ); + // set_producers in same block, do it the explicit way to use a diff expiration and avoid duplicate trx + auto set_producers_force = [&](const std::vector& producers) { + static int unique = 0; // used to force uniqueness of transaction + auto schedule = get_producer_authorities( producers ); + fc::variants schedule_variant; + schedule_variant.reserve(schedule.size()); + for( const auto& e: schedule ) { + schedule_variant.emplace_back(e.get_abi_variant()); + } + push_action( config::system_account_name, "setprods"_n, config::system_account_name, + fc::mutable_variant_object()("schedule", schedule_variant), DEFAULT_EXPIRATION_DELTA + (++unique)); + }; + while (control->head_block_num() < 3) { produce_block(); } @@ -132,7 +145,7 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // set a new proposer policy sch1 set_producers( {"alice"_n} ); - vector sch1 = { + vector alice_sch = { producer_authority{"alice"_n, block_signing_authority_v0{1, {{get_public_key("alice"_n, "active"), 1}}}} }; @@ -145,7 +158,7 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // set another ploicy to have multiple pending different active time policies set_producers( {"bob"_n,"carol"_n} ); - vector sch2 = { + vector bob_carol_sch = { 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}}}} }; @@ -153,7 +166,7 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // set another ploicy should replace sch2 set_producers( {"bob"_n,"alice"_n} ); - vector sch3 = { + vector bob_alice_sch = { 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}}}} }; @@ -163,13 +176,13 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t // sch1 must become active no later than 2 rounds but sch2 cannot become active yet BOOST_CHECK_EQUAL( control->active_producers().version, 1u ); - BOOST_CHECK_EQUAL( true, compare_schedules( sch1, control->active_producers() ) ); + BOOST_CHECK_EQUAL( true, compare_schedules( alice_sch, control->active_producers() ) ); produce_blocks(config::producer_repetitions); // sch3 becomes active BOOST_CHECK_EQUAL( 2u, control->active_producers().version ); // should be 2 as sch2 was replaced by sch3 - BOOST_CHECK_EQUAL( true, compare_schedules( sch3, control->active_producers() ) ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); // get to next producer round auto prod = produce_block()->producer; @@ -180,18 +193,42 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t produce_blocks(config::producer_repetitions); produce_blocks(config::producer_repetitions); BOOST_CHECK_EQUAL( 2u, control->active_producers().version ); // should be 2 as not different so no change - BOOST_CHECK_EQUAL( true, compare_schedules( sch3, control->active_producers() ) ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); // test no change to proposed schedule, only the first one will take affect for (size_t i = 0; i < config::producer_repetitions*2-1; ++i) { BOOST_CHECK_EQUAL( 2u, control->active_producers().version ); // should be 2 as not taken affect yet - BOOST_CHECK_EQUAL( true, compare_schedules( sch3, control->active_producers() ) ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); set_producers( {"bob"_n,"carol"_n} ); + set_producers_force({"bob"_n,"carol"_n} ); + set_producers_force({"bob"_n,"carol"_n} ); produce_block(); } produce_block(); BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 3 now as bob,carol now active - BOOST_CHECK_EQUAL( true, compare_schedules( sch2, control->active_producers() ) ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); + + // get to next producer round + prod = produce_block()->producer; + for (auto b = produce_block(); b->producer == prod; b = produce_block()); + + // test change in same block where there is an existing proposed that is the same + set_producers( {"bob"_n,"alice"_n} ); + produce_block(); + set_producers( {"bob"_n,"carol"_n} ); + set_producers_force({"bob"_n,"carol"_n} ); + produce_blocks(config::producer_repetitions-1); + produce_blocks(config::producer_repetitions); + BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); // should be 4 now as bob,alice now active + BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); + + // test change in same block where there is no change + set_producers( {"bob"_n,"carol"_n} ); + set_producers_force({"bob"_n,"alice"_n} ); // put back, no change expected + produce_blocks(config::producer_repetitions); + produce_blocks(config::producer_repetitions); + BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); // should be 4 now as bob,alice is still active + BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); } FC_LOG_AND_RETHROW() From 4c0ff25a72654df7f4408f7a9c4fc610c2f2805d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 30 Apr 2024 06:49:19 -0500 Subject: [PATCH 02/11] GH-6 Add comment --- libraries/chain/include/eosio/chain/block_header_state.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index c8ae9e9762..c20201ae49 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -69,6 +69,11 @@ struct block_header_state { proposer_policy_ptr active_proposer_policy; // producer authority schedule, supports `digest()` // block time when proposer_policy will become active + // The active time is the next,next producer round. For example, + // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] + // If proposed in A1, A2, .. A12 becomes active in C1 + // If proposed in B1, B2, .. B12 becomes active in D1 + // If proposed in A2 and another proposed in B3 then there can be 2 in the map flat_map proposer_policies; // track in-flight finalizer policies. This is a `multimap` because the same block number From 5adf5f00b84163dbe9ef5ec3585b44d6066497f1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 30 Apr 2024 08:55:57 -0500 Subject: [PATCH 03/11] GH-6 Additional fixes and test cases --- libraries/chain/controller.cpp | 17 +++-- unittests/producer_schedule_if_tests.cpp | 89 ++++++++++++++++++++++-- 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 1fdcfbf0e7..9807a89a31 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -491,17 +491,19 @@ struct building_block { uint32_t get_block_num() const { return block_num; } - // returns the next proposer schedule version and true if different + // returns the next proposer schedule version and true if producers should be proposed in block // if producers is not different then returns the current schedule version (or next schedule version) std::tuple get_next_proposer_schedule_version(const vector& producers) const { assert(active_proposer_policy); + bool should_propose = false; auto get_next_sched = [&]() -> const producer_authority_schedule& { // if there are any policies already proposed but not active yet then they are what needs to be compared if (!parent.proposer_policies.empty()) { block_timestamp_type active_time = detail::get_next_next_round_block_time(timestamp); if (auto itr = parent.proposer_policies.find(active_time); itr != parent.proposer_policies.cend()) { // Same active time, a new proposer schedule will replace this entry, `next` therefore is the previous + should_propose = true; if (itr != parent.proposer_policies.begin()) { return (--itr)->second->proposer_schedule; } @@ -518,11 +520,14 @@ struct building_block { const producer_authority_schedule& lhs = get_next_sched(); - if (std::ranges::equal(lhs.producers, producers)) { - return {lhs.version, false}; + decltype(lhs.version) v = lhs.version; + + if (!std::ranges::equal(lhs.producers, producers)) { + ++v; + should_propose = true; } - return {lhs.version + 1, true}; + return {v, should_propose}; } }; @@ -5231,8 +5236,8 @@ int64_t controller_impl::set_proposed_producers( vector prod }); } - auto [version, diff] = pending->get_next_proposer_schedule_version(producers); - if (!diff) + auto [version, should_propose] = pending->get_next_proposer_schedule_version(producers); + if (!should_propose) return version; producer_authority_schedule sch; diff --git a/unittests/producer_schedule_if_tests.cpp b/unittests/producer_schedule_if_tests.cpp index 0e3968dc2d..2050d18e91 100644 --- a/unittests/producer_schedule_if_tests.cpp +++ b/unittests/producer_schedule_if_tests.cpp @@ -219,17 +219,96 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers_force({"bob"_n,"carol"_n} ); produce_blocks(config::producer_repetitions-1); produce_blocks(config::producer_repetitions); - BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); // should be 4 now as bob,alice now active - BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); + BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 4 now as bob,alice now active + BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); // test change in same block where there is no change - set_producers( {"bob"_n,"carol"_n} ); - set_producers_force({"bob"_n,"alice"_n} ); // put back, no change expected + set_producers( {"bob"_n,"alice"_n} ); + set_producers_force({"bob"_n,"carol"_n} ); // put back, no change expected produce_blocks(config::producer_repetitions); produce_blocks(config::producer_repetitions); - BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); // should be 4 now as bob,alice is still active + BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 4 now as bob,alice is still active + BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); + + // get to next producer round + prod = produce_block()->producer; + for (auto b = produce_block(); b->producer == prod; b = produce_block()); + + // test two in-flight + // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] + // propose P1 in A2, active in C1 + // propose P2 in B2, active in D1 + // propose P3 in B3, active in D1, replaces P2 + produce_block(); + set_producers({"alice"_n}); // A2, P1 + produce_blocks(config::producer_repetitions-1); // A12 + produce_block(); + set_producers({"bob"_n,"carol"_n}); // B2 + produce_block(); + set_producers({"bob"_n, "alice"_n} ); // P3 + produce_blocks(config::producer_repetitions-2); // B12 + produce_block(); // C1 + BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); + BOOST_CHECK_EQUAL( true, compare_schedules( alice_sch, control->active_producers() ) ); + produce_blocks(config::producer_repetitions); // D1 + BOOST_CHECK_EQUAL( 5u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); + // get to next producer round + prod = produce_block()->producer; + for (auto b = produce_block(); b->producer == prod; b = produce_block()); + + // test two in-flight, P1 == P3, so no change + // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] + // propose P1 in A2, active in C1 + // propose P2 in B2, active in D1 + // 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 + produce_block(); + set_producers({"alice"_n}); // B2 + produce_block(); + set_producers({"bob"_n,"carol"_n}); // P3 == P1 + produce_blocks(config::producer_repetitions-2); // B12 + produce_block(); // C1 + BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); + produce_blocks(config::producer_repetitions); // D1 + BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); + + // get to next producer round + prod = produce_block()->producer; + for (auto b = produce_block(); b->producer == prod; b = produce_block()); + + // test two in-flight, ultimately no change + produce_block(); // 1 + set_producers({"bob"_n,"carol"_n}); + produce_block(); // 2 + set_producers({"alice"_n}); + produce_block(); // 3 + set_producers({"carol"_n,"alice"_n}); + produce_block(); // 4 + set_producers({"carol"_n}); + produce_block(); // 5 + set_producers({"alice"_n}); + produce_block(); // 6 + set_producers({"bob"_n,"carol"_n}); + produce_blocks(config::producer_repetitions-6); + set_producers({"bob"_n}); + produce_block(); // 2 + set_producers({"bob"_n,"carol"_n}); + produce_block(); // 3 + set_producers({"carol"_n,"bob"_n}); + produce_block(); // 4 + set_producers({"alice"_n} ); + produce_block(); // 5 + set_producers({"bob"_n,"carol"_n}); + produce_blocks(config::producer_repetitions-5); // B12 + BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); + BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); + } FC_LOG_AND_RETHROW() From 51d448868c326bad701d5a8723ad98b7fea11296 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 30 Apr 2024 13:03:31 -0500 Subject: [PATCH 04/11] GH-6 Update comment --- libraries/chain/controller.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 9807a89a31..bbf0193bbe 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5246,7 +5246,7 @@ int64_t controller_impl::set_proposed_producers( vector prod ilog( "proposed producer schedule with version ${v}", ("v", sch.version) ); - // overwrite any existing proposed_schedule set earlier in this block + // store schedule in gpo so it will be rolledback if transaction fails auto cur_block_num = chain_head.block_num() + 1; db.modify( gpo, [&]( auto& gp ) { gp.proposed_schedule_block_num = cur_block_num; From 7e10458b249d02c65c7c81ee40da2ba95b4af417 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 May 2024 07:15:20 -0500 Subject: [PATCH 05/11] GH-6 Update logic to increment version on every new proposal schedule placed in a block header extension --- libraries/chain/block_header_state.cpp | 15 +++++++---- libraries/chain/controller.cpp | 25 +++++-------------- .../eosio/chain/block_header_state.hpp | 17 ++++++++----- .../include/eosio/chain/snapshot_detail.hpp | 2 +- .../eosio/chain/webassembly/interface.hpp | 6 +++-- 5 files changed, 32 insertions(+), 33 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index f3786aaad9..774788938d 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -106,18 +106,23 @@ void finish_next(const block_header_state& prev, if (!prev.proposer_policies.empty()) { auto it = prev.proposer_policies.begin(); // +1 since this is called after the block is built, this will be the active schedule for the next block - if (it->first.slot <= next_header_state.header.timestamp.slot + 1) { + while (it != prev.proposer_policies.end() && it->first.slot <= next_header_state.header.timestamp.slot + 1) { next_header_state.active_proposer_policy = it->second; - next_header_state.proposer_policies = { ++it, prev.proposer_policies.end() }; - } else { + ++it; + } + if (it == prev.proposer_policies.begin()) { // none made active next_header_state.proposer_policies = prev.proposer_policies; + } else if (it != prev.proposer_policies.end()) { // some made active + next_header_state.proposer_policies = { it, prev.proposer_policies.end() }; + } else { // all made active + // next_header_state.proposer_policies will be emtpy } } if (if_ext.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.emplace_back(if_ext.new_proposer_policy->active_time, + std::move(if_ext.new_proposer_policy)); } // finality_core diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index bbf0193bbe..1fa819b471 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -496,24 +496,11 @@ struct building_block { std::tuple get_next_proposer_schedule_version(const vector& producers) const { assert(active_proposer_policy); - bool should_propose = false; auto get_next_sched = [&]() -> const producer_authority_schedule& { // if there are any policies already proposed but not active yet then they are what needs to be compared if (!parent.proposer_policies.empty()) { - block_timestamp_type active_time = detail::get_next_next_round_block_time(timestamp); - if (auto itr = parent.proposer_policies.find(active_time); itr != parent.proposer_policies.cend()) { - // Same active time, a new proposer schedule will replace this entry, `next` therefore is the previous - should_propose = true; - if (itr != parent.proposer_policies.begin()) { - return (--itr)->second->proposer_schedule; - } - // no previous to what will be replaced, use active - return active_proposer_policy->proposer_schedule; - } - // will not replace any proposed policies, use next to become active - return parent.proposer_policies.begin()->second->proposer_schedule; + return (--parent.proposer_policies.end())->second->proposer_schedule; } - // none currently in-flight, use active return active_proposer_policy->proposer_schedule; }; @@ -524,10 +511,10 @@ struct building_block { if (!std::ranges::equal(lhs.producers, producers)) { ++v; - should_propose = true; + return {v, true}; } - return {v, should_propose}; + return {v, false}; } }; @@ -5226,7 +5213,7 @@ int64_t controller_impl::set_proposed_producers( vector prod if (gpo.proposed_schedule_block_num) { if (std::equal(producers.begin(), producers.end(), gpo.proposed_schedule.producers.begin(), gpo.proposed_schedule.producers.end())) { - return gpo.proposed_schedule.version; // the proposed producer schedule does not change + return std::numeric_limits::max(); // the proposed producer schedule does not change } // clear gpo proposed_schedule as we may determine no diff between proposed producers and next proposer schedule db.modify( gpo, [&]( auto& gp ) { @@ -5238,7 +5225,7 @@ int64_t controller_impl::set_proposed_producers( vector prod auto [version, should_propose] = pending->get_next_proposer_schedule_version(producers); if (!should_propose) - return version; + return std::numeric_limits::max(); producer_authority_schedule sch; sch.version = version; @@ -5253,7 +5240,7 @@ int64_t controller_impl::set_proposed_producers( vector prod gp.proposed_schedule = sch; }); - return sch.version; + return std::numeric_limits::max(); } int64_t controller_impl::set_proposed_producers_legacy( vector producers ) { diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 618c1e731e..95683213d0 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -68,13 +68,18 @@ struct block_header_state { finalizer_policy_ptr active_finalizer_policy; // finalizer set + threshold + generation, supports `digest()` proposer_policy_ptr active_proposer_policy; // producer authority schedule, supports `digest()` - // block time when proposer_policy will become active + // // The active time is the next,next producer round. For example, - // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] - // If proposed in A1, A2, .. A12 becomes active in C1 - // If proposed in B1, B2, .. B12 becomes active in D1 - // If proposed in A2 and another proposed in B3 then there can be 2 in the map - flat_map proposer_policies; + // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] + // If proposed in A1, A2, .. A12 becomes active in C1 + // If proposed in B1, B2, .. B12 becomes active in D1 + // This is a `deque` because the same active block time can contain up to 12 entries (one per block) + // for the entry proposed in a block: + // For example, proposer policy proposed: P1 in A1, P2 in A7, P3 in A12, P4 in B2, P5 in B12 then the map contains: + // [C1 -> P1],[C1 -> P2],[C1 -> P3],[D1 -> P4],[D1 -> P5] + // At C1 P1,P2,P3 are applied causing an active policy of P3 + // At D1 P4,P5 are appliced causing an acive policy of P5 + std::deque> proposer_policies; // track in-flight finalizer policies. This is a `multimap` because the same block number // can hold a `proposed` and a `pending` finalizer_policy. When that block becomes final, the diff --git a/libraries/chain/include/eosio/chain/snapshot_detail.hpp b/libraries/chain/include/eosio/chain/snapshot_detail.hpp index 58e760d03d..78f00e3e5b 100644 --- a/libraries/chain/include/eosio/chain/snapshot_detail.hpp +++ b/libraries/chain/include/eosio/chain/snapshot_detail.hpp @@ -116,7 +116,7 @@ namespace eosio::chain::snapshot_detail { finality_core core; finalizer_policy_ptr active_finalizer_policy; proposer_policy_ptr active_proposer_policy; - flat_map proposer_policies; + std::deque> proposer_policies; flat_multimap finalizer_policies; uint32_t finalizer_policy_generation; diff --git a/libraries/chain/include/eosio/chain/webassembly/interface.hpp b/libraries/chain/include/eosio/chain/webassembly/interface.hpp index f2094c32c8..68f3763249 100644 --- a/libraries/chain/include/eosio/chain/webassembly/interface.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/interface.hpp @@ -153,7 +153,8 @@ namespace webassembly { * * @param packed_producer_schedule - vector of producer keys * - * @return -1 if proposing a new producer schedule was unsuccessful, otherwise returns the version of the new proposed schedule. + * @return pre-savanna: -1 if proposing a new producer schedule was unsuccessful, otherwise returns the version of the new proposed schedule. + * post-savanna: -1 if proposing a new producer schedule was unsuccessful, otherwise returns max uint32_t */ int64_t set_proposed_producers(legacy_span packed_producer_schedule); @@ -169,7 +170,8 @@ namespace webassembly { * @param packed_producer_format - format of the producer data blob. * @param packed_producer_schedule - packed data of representing the producer schedule in the format indicated. * - * @return -1 if proposing a new producer schedule was unsuccessful, otherwise returns the version of the new proposed schedule. + * @return pre-savanna: -1 if proposing a new producer schedule was unsuccessful, otherwise returns the version of the new proposed schedule. + * post-savanna: -1 if proposing a new producer schedule was unsuccessful, otherwise returns max uint32_t */ int64_t set_proposed_producers_ex(uint64_t packed_producer_format, legacy_span packed_producer_schedule); From 5c310b51d87ca6a1f53dddbe6f03e69d3d8ef0bf Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 May 2024 07:15:41 -0500 Subject: [PATCH 06/11] GH-6 Update tests for new version change scheme --- unittests/producer_schedule_if_tests.cpp | 27 +++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/unittests/producer_schedule_if_tests.cpp b/unittests/producer_schedule_if_tests.cpp index 2050d18e91..59e3b8667c 100644 --- a/unittests/producer_schedule_if_tests.cpp +++ b/unittests/producer_schedule_if_tests.cpp @@ -180,8 +180,8 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t produce_blocks(config::producer_repetitions); - // sch3 becomes active - BOOST_CHECK_EQUAL( 2u, control->active_producers().version ); // should be 2 as sch2 was replaced by sch3 + // sch3 becomes active, version should be 3 even though sch2 was replaced by sch3 + BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); // get to next producer round @@ -192,12 +192,12 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers( {"bob"_n,"alice"_n} ); // same as before, so no change produce_blocks(config::producer_repetitions); produce_blocks(config::producer_repetitions); - BOOST_CHECK_EQUAL( 2u, control->active_producers().version ); // should be 2 as not different so no change + 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() ) ); // test no change to proposed schedule, only the first one will take affect for (size_t i = 0; i < config::producer_repetitions*2-1; ++i) { - BOOST_CHECK_EQUAL( 2u, control->active_producers().version ); // should be 2 as not taken affect yet + BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 3 as not taken affect yet BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); set_producers( {"bob"_n,"carol"_n} ); set_producers_force({"bob"_n,"carol"_n} ); @@ -205,7 +205,7 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t produce_block(); } produce_block(); - BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 3 now as bob,carol now active + BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); // should be 4 now as bob,carol now active BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); // get to next producer round @@ -219,7 +219,7 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers_force({"bob"_n,"carol"_n} ); produce_blocks(config::producer_repetitions-1); produce_blocks(config::producer_repetitions); - BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 4 now as bob,alice now active + 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() ) ); // test change in same block where there is no change @@ -227,7 +227,7 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers_force({"bob"_n,"carol"_n} ); // put back, no change expected produce_blocks(config::producer_repetitions); produce_blocks(config::producer_repetitions); - BOOST_CHECK_EQUAL( 3u, control->active_producers().version ); // should be 4 now as bob,alice is still active + BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); // should be 6 now as bob,carol is still active BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); // get to next producer round @@ -248,10 +248,10 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers({"bob"_n, "alice"_n} ); // P3 produce_blocks(config::producer_repetitions-2); // B12 produce_block(); // C1 - BOOST_CHECK_EQUAL( 4u, control->active_producers().version ); + BOOST_CHECK_EQUAL( 7u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( alice_sch, control->active_producers() ) ); produce_blocks(config::producer_repetitions); // D1 - BOOST_CHECK_EQUAL( 5u, control->active_producers().version ); + BOOST_CHECK_EQUAL( 9u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_alice_sch, control->active_producers() ) ); // get to next producer round @@ -272,10 +272,10 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t set_producers({"bob"_n,"carol"_n}); // P3 == P1 produce_blocks(config::producer_repetitions-2); // B12 produce_block(); // C1 - BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); + BOOST_CHECK_EQUAL( 10u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); produce_blocks(config::producer_repetitions); // D1 - BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); + BOOST_CHECK_EQUAL( 12u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); // get to next producer round @@ -306,7 +306,10 @@ BOOST_FIXTURE_TEST_CASE( proposer_policy_progression_test, validating_tester ) t produce_block(); // 5 set_producers({"bob"_n,"carol"_n}); produce_blocks(config::producer_repetitions-5); // B12 - BOOST_CHECK_EQUAL( 6u, control->active_producers().version ); + 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); + BOOST_CHECK_EQUAL( 22u, control->active_producers().version ); BOOST_CHECK_EQUAL( true, compare_schedules( bob_carol_sch, control->active_producers() ) ); } FC_LOG_AND_RETHROW() From 887b0575d1cdd2b256821aab304592b9e4c29bea Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 May 2024 11:45:36 -0500 Subject: [PATCH 07/11] GH-6 Revert back to flat_map for simpler and not storing unneeded implementation --- libraries/chain/block_header_state.cpp | 15 +++++---------- libraries/chain/controller.cpp | 2 +- .../include/eosio/chain/block_header_state.hpp | 11 +++-------- .../chain/include/eosio/chain/snapshot_detail.hpp | 2 +- 4 files changed, 10 insertions(+), 20 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 774788938d..f3786aaad9 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -106,23 +106,18 @@ void finish_next(const block_header_state& prev, if (!prev.proposer_policies.empty()) { auto it = prev.proposer_policies.begin(); // +1 since this is called after the block is built, this will be the active schedule for the next block - while (it != prev.proposer_policies.end() && it->first.slot <= next_header_state.header.timestamp.slot + 1) { + if (it->first.slot <= next_header_state.header.timestamp.slot + 1) { next_header_state.active_proposer_policy = it->second; - ++it; - } - if (it == prev.proposer_policies.begin()) { // none made active + next_header_state.proposer_policies = { ++it, prev.proposer_policies.end() }; + } else { next_header_state.proposer_policies = prev.proposer_policies; - } else if (it != prev.proposer_policies.end()) { // some made active - next_header_state.proposer_policies = { it, prev.proposer_policies.end() }; - } else { // all made active - // next_header_state.proposer_policies will be emtpy } } if (if_ext.new_proposer_policy) { // called when assembling the block - next_header_state.proposer_policies.emplace_back(if_ext.new_proposer_policy->active_time, - std::move(if_ext.new_proposer_policy)); + next_header_state.proposer_policies[if_ext.new_proposer_policy->active_time] = + std::move(if_ext.new_proposer_policy); } // finality_core diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 1fa819b471..3a3c85b580 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -507,7 +507,7 @@ struct building_block { const producer_authority_schedule& lhs = get_next_sched(); - decltype(lhs.version) v = lhs.version; + auto v = lhs.version; if (!std::ranges::equal(lhs.producers, producers)) { ++v; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 95683213d0..c7ad66309d 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -68,18 +68,13 @@ struct block_header_state { finalizer_policy_ptr active_finalizer_policy; // finalizer set + threshold + generation, supports `digest()` proposer_policy_ptr active_proposer_policy; // producer authority schedule, supports `digest()` - // + // block time when proposer_policy will become active + // current algorithm only two entries possible, for for the next,next round and one for block round after that // The active time is the next,next producer round. For example, // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] // If proposed in A1, A2, .. A12 becomes active in C1 // If proposed in B1, B2, .. B12 becomes active in D1 - // This is a `deque` because the same active block time can contain up to 12 entries (one per block) - // for the entry proposed in a block: - // For example, proposer policy proposed: P1 in A1, P2 in A7, P3 in A12, P4 in B2, P5 in B12 then the map contains: - // [C1 -> P1],[C1 -> P2],[C1 -> P3],[D1 -> P4],[D1 -> P5] - // At C1 P1,P2,P3 are applied causing an active policy of P3 - // At D1 P4,P5 are appliced causing an acive policy of P5 - std::deque> proposer_policies; + flat_map proposer_policies; // track in-flight finalizer policies. This is a `multimap` because the same block number // can hold a `proposed` and a `pending` finalizer_policy. When that block becomes final, the diff --git a/libraries/chain/include/eosio/chain/snapshot_detail.hpp b/libraries/chain/include/eosio/chain/snapshot_detail.hpp index 78f00e3e5b..58e760d03d 100644 --- a/libraries/chain/include/eosio/chain/snapshot_detail.hpp +++ b/libraries/chain/include/eosio/chain/snapshot_detail.hpp @@ -116,7 +116,7 @@ namespace eosio::chain::snapshot_detail { finality_core core; finalizer_policy_ptr active_finalizer_policy; proposer_policy_ptr active_proposer_policy; - std::deque> proposer_policies; + flat_map proposer_policies; flat_multimap finalizer_policies; uint32_t finalizer_policy_generation; From ce2cb2ef93a357e776d88a091bdae0c76e492968 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 May 2024 06:30:31 -0500 Subject: [PATCH 08/11] GH-6 Simplify by doing diff during assemble_block --- libraries/chain/controller.cpp | 53 +++++++------------ .../eosio/chain/block_header_state.hpp | 2 +- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 1404ac84e7..8020ce5a8e 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -493,12 +493,12 @@ struct building_block { // returns the next proposer schedule version and true if producers should be proposed in block // if producers is not different then returns the current schedule version (or next schedule version) - std::tuple get_next_proposer_schedule_version(const vector& producers) const { + std::tuple get_next_proposer_schedule_version(const shared_vector& producers) const { assert(active_proposer_policy); auto get_next_sched = [&]() -> const producer_authority_schedule& { - // if there are any policies already proposed but not active yet then they are what needs to be compared - if (!parent.proposer_policies.empty()) { + if (!parent.proposer_policies.empty()) { // proposed in-flight + // return the last proposed policy to use for comparison return (--parent.proposer_policies.end())->second->proposer_schedule; } // none currently in-flight, use active @@ -506,14 +506,11 @@ struct building_block { }; const producer_authority_schedule& lhs = get_next_sched(); - auto v = lhs.version; - - if (!std::ranges::equal(lhs.producers, producers)) { + if (!std::equal(lhs.producers.begin(), lhs.producers.end(), producers.begin(), producers.end())) { ++v; return {v, true}; } - return {v, false}; } @@ -605,7 +602,7 @@ struct building_block { v); } - std::tuple get_next_proposer_schedule_version(const vector& producers) const { + std::tuple get_next_proposer_schedule_version(const shared_vector& producers) const { return std::visit( overloaded{[](const building_block_legacy&) -> std::tuple { return {-1, false}; }, [&](const building_block_if& bb) -> std::tuple { @@ -903,7 +900,7 @@ struct pending_state { _block_stage); } - std::tuple get_next_proposer_schedule_version(const vector& producers) const { + std::tuple get_next_proposer_schedule_version(const shared_vector& producers) const { return std::visit(overloaded{ [&](const building_block& stage) -> std::tuple { return stage.get_next_proposer_schedule_version(producers); @@ -3138,10 +3135,16 @@ struct controller_impl { auto process_new_proposer_policy = [&](auto&) -> void { const auto& gpo = db.get(); if (gpo.proposed_schedule_block_num) { - new_proposer_policy = std::make_unique(); - 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); - ilog("Scheduling proposer schedule change at ${t}: ${s}", ("t", new_proposer_policy->active_time)("s", new_proposer_policy->proposer_schedule)); + + auto [version, should_propose] = pending->get_next_proposer_schedule_version(gpo.proposed_schedule.producers); + if (should_propose) { + new_proposer_policy = std::make_unique(); + 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; + ilog("Scheduling proposer schedule change at ${t}: ${s}", + ("t", new_proposer_policy->active_time)("s", new_proposer_policy->proposer_schedule)); + } db.modify( gpo, [&]( auto& gp ) { gp.proposed_schedule_block_num = std::optional(); @@ -5224,33 +5227,13 @@ int64_t controller_impl::set_proposed_producers( vector prod assert(pending); - // see if one already proposed in this block - auto& gpo = db.get(); - if (gpo.proposed_schedule_block_num) { - if (std::equal(producers.begin(), producers.end(), - gpo.proposed_schedule.producers.begin(), gpo.proposed_schedule.producers.end())) { - return std::numeric_limits::max(); // the proposed producer schedule does not change - } - // clear gpo proposed_schedule as we may determine no diff between proposed producers and next proposer schedule - db.modify( gpo, [&]( auto& gp ) { - gp.proposed_schedule_block_num.reset(); - gp.proposed_schedule.version = 0; - gp.proposed_schedule.producers.clear(); - }); - } - - auto [version, should_propose] = pending->get_next_proposer_schedule_version(producers); - if (!should_propose) - return std::numeric_limits::max(); - producer_authority_schedule sch; - sch.version = version; + // sch.version is set in assemble_block sch.producers = std::move(producers); - ilog( "proposed producer schedule with version ${v}", ("v", sch.version) ); - // store schedule in gpo so it will be rolledback if transaction fails auto cur_block_num = chain_head.block_num() + 1; + auto& gpo = db.get(); db.modify( gpo, [&]( auto& gp ) { gp.proposed_schedule_block_num = cur_block_num; gp.proposed_schedule = sch; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index c7ad66309d..c671f05bbc 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -69,7 +69,7 @@ struct block_header_state { proposer_policy_ptr active_proposer_policy; // producer authority schedule, supports `digest()` // block time when proposer_policy will become active - // current algorithm only two entries possible, for for the next,next round and one for block round after that + // current algorithm only two entries possible, for the next,next round and one for block round after that // The active time is the next,next producer round. For example, // round A [1,2,..12], next_round B [1,2,..12], next_next_round C [1,2,..12], D [1,2,..12] // If proposed in A1, A2, .. A12 becomes active in C1 From 78c6c19876eef98be786ea967153bae4511d81fe Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 May 2024 12:17:56 -0500 Subject: [PATCH 09/11] GH-97 Change some additional leap references to spring --- .github/workflows/build.yaml | 4 ++-- libraries/chain/finality/finalizer.cpp | 2 +- libraries/chain/include/eosio/chain/finality/finalizer.hpp | 4 ++-- libraries/libfc/include/fc/crypto/base64.hpp | 6 +++--- unittests/snapshot_tests.cpp | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 7e06f61147..cb74a40886 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -285,9 +285,9 @@ jobs: run: | zstdcat build.tar.zst | tar x - if: ${{ matrix.test == 'build-tree' }} - name: Set leap_DIR env var + name: Set spring_DIR env var run: | - echo "leap_DIR=$PWD/build/lib/cmake/leap" >> "$GITHUB_ENV" + echo "spring_DIR=$PWD/build/lib/cmake/spring" >> "$GITHUB_ENV" - if: ${{ matrix.test == 'make-dev-install' }} name: spring dev-install run: | diff --git a/libraries/chain/finality/finalizer.cpp b/libraries/chain/finality/finalizer.cpp index bb3a4fbc2c..35100dd805 100644 --- a/libraries/chain/finality/finalizer.cpp +++ b/libraries/chain/finality/finalizer.cpp @@ -232,7 +232,7 @@ void my_finalizers_t::set_keys(const std::map& finaliz // -------------------------------------------------------------------------------------------- // Can be called either: // - when transitioning to IF (before any votes are to be sent) -// - at leap startup, if we start at a block which is either within or past the IF transition. +// - at spring startup, if we start at a block which is either within or past the IF transition. // In either case, we are never updating existing finalizer safety information. This is only // to ensure that the safety information will have defaults that ensure safety as much as // possible, and allow for liveness which will allow the finalizers to eventually vote. diff --git a/libraries/chain/include/eosio/chain/finality/finalizer.hpp b/libraries/chain/include/eosio/chain/finality/finalizer.hpp index fec83366be..eb965b2ddc 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer.hpp @@ -18,7 +18,7 @@ // are an essential part of the Savanna consensus algorithm. // - every time a finalizer votes, it may update its own safety info in memory // - finalizer safety info is appropriately initialized (iff not already present -// in the persistent file) at Leap startup. +// in the persistent file) at Spring startup. // // my_finalizers_t: // --------------- @@ -77,7 +77,7 @@ namespace eosio::chain { mutable fc::datastream persist_file; // we want to keep the file open for speed std::map finalizers; // the active finalizers for this node, loaded at startup, not mutated afterwards fsi_map inactive_safety_info; // loaded at startup, not mutated afterwards - fsi_t default_fsi = fsi_t::unset_fsi(); // default provided at leap startup + fsi_t default_fsi = fsi_t::unset_fsi(); // default provided at spring startup mutable bool inactive_safety_info_written{false}; public: diff --git a/libraries/libfc/include/fc/crypto/base64.hpp b/libraries/libfc/include/fc/crypto/base64.hpp index 5e025fdc0f..3edee59033 100644 --- a/libraries/libfc/include/fc/crypto/base64.hpp +++ b/libraries/libfc/include/fc/crypto/base64.hpp @@ -75,7 +75,7 @@ RetString base64_decode(const String& s, bool remove_linebreaks = false, bool ur template RetString base64_encode(const unsigned char* s, size_t len, bool url = false); -// Convenient methods for existing Leap uses +// Convenient methods for existing Spring uses std::string base64_encode(char const* s, unsigned int len); std::vector base64_decode( const std::string& s); @@ -150,7 +150,7 @@ inline unsigned int pos_of_char(const unsigned char chr, bool url) { //(Pablo Martin-Gomez, https://github.com/Bouska) // // Original version throw std::runtime_error("Input is not valid base64-encoded data."); - // Throw FC assert and the same error text to match existing Leap usages. + // Throw FC assert and the same error text to match existing Spring usages. FC_ASSERT(false, "encountered non-base64 character"); } @@ -349,7 +349,7 @@ inline RetString base64_encode_mime(const String& s) { return detail::encode_mime(s); } -// Convenient methods for existing Leap uses +// Convenient methods for existing Spring uses inline std::string base64_encode(char const* s, unsigned int len) { return base64_encode((unsigned char const*)s, len, false); } diff --git a/unittests/snapshot_tests.cpp b/unittests/snapshot_tests.cpp index d4ac1646bb..ac2654b851 100644 --- a/unittests/snapshot_tests.cpp +++ b/unittests/snapshot_tests.cpp @@ -449,7 +449,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(test_compatible_versions, SNAPSHOT_SUITE, snapshot // ---------------------------------------------------------- // 1. update `current_version` and the list of versions in `for` loop // 2. run: `unittests/unit_test -t "snapshot_tests/test_com*" -- --save-snapshot` to generate new snapshot files - // 3. copy the newly generated files (see `ls -lrth ./unittests/snapshots/snap_*` to `leap/unittests/snapshots` + // 3. copy the newly generated files (see `ls -lrth ./unittests/snapshots/snap_*` to `spring/unittests/snapshots` // for example `cp ./unittests/snapshots/snap_v7.* ../unittests/snapshots` // 4. edit `unittests/snapshots/CMakeLists.txt` and add the `configure_file` commands for the 3 new files. // now the test should pass. From 091ea815f52ce4738aa05424b5e454bc4f645fea Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 May 2024 12:33:48 -0500 Subject: [PATCH 10/11] GH-6 Use optional instead of tuple --- libraries/chain/controller.cpp | 38 +++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 8020ce5a8e..0e64d61546 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -491,9 +491,9 @@ struct building_block { uint32_t get_block_num() const { return block_num; } - // returns the next proposer schedule version and true if producers should be proposed in block - // if producers is not different then returns the current schedule version (or next schedule version) - std::tuple get_next_proposer_schedule_version(const shared_vector& producers) const { + // returns the next proposer schedule version if producers should be proposed in block + // if producers is not different then returns empty optional + std::optional get_next_proposer_schedule_version(const shared_vector& producers) const { assert(active_proposer_policy); auto get_next_sched = [&]() -> const producer_authority_schedule& { @@ -509,9 +509,9 @@ struct building_block { auto v = lhs.version; if (!std::equal(lhs.producers.begin(), lhs.producers.end(), producers.begin(), producers.end())) { ++v; - return {v, true}; + return std::optional{v}; } - return {v, false}; + return std::nullopt; } }; @@ -602,10 +602,10 @@ struct building_block { v); } - std::tuple get_next_proposer_schedule_version(const shared_vector& producers) const { + std::optional get_next_proposer_schedule_version(const shared_vector& producers) const { return std::visit( - overloaded{[](const building_block_legacy&) -> std::tuple { return {-1, false}; }, - [&](const building_block_if& bb) -> std::tuple { + overloaded{[](const building_block_legacy&) -> std::optional { return std::nullopt; }, + [&](const building_block_if& bb) -> std::optional { return bb.get_next_proposer_schedule_version(producers); } }, @@ -900,13 +900,13 @@ struct pending_state { _block_stage); } - std::tuple get_next_proposer_schedule_version(const shared_vector& producers) const { + std::optional get_next_proposer_schedule_version(const shared_vector& producers) const { return std::visit(overloaded{ - [&](const building_block& stage) -> std::tuple { + [&](const building_block& stage) -> std::optional { return stage.get_next_proposer_schedule_version(producers); }, - [](const assembled_block&) -> std::tuple { assert(false); return {-1, false}; }, - [](const completed_block&) -> std::tuple { assert(false); return {-1, false}; } + [](const assembled_block&) -> std::optional { assert(false); return std::nullopt; }, + [](const completed_block&) -> std::optional { assert(false); return std::nullopt; } }, _block_stage); } @@ -3131,17 +3131,16 @@ struct controller_impl { resource_limits.process_block_usage(bb.block_num()); // Any proposer policy? - std::unique_ptr new_proposer_policy; - auto process_new_proposer_policy = [&](auto&) -> void { + auto process_new_proposer_policy = [&](auto&) -> std::unique_ptr { + std::unique_ptr new_proposer_policy; const auto& gpo = db.get(); if (gpo.proposed_schedule_block_num) { - - auto [version, should_propose] = pending->get_next_proposer_schedule_version(gpo.proposed_schedule.producers); - if (should_propose) { + std::optional version = pending->get_next_proposer_schedule_version(gpo.proposed_schedule.producers); + if (version) { new_proposer_policy = std::make_unique(); 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; + new_proposer_policy->proposer_schedule.version = *version; ilog("Scheduling proposer schedule change at ${t}: ${s}", ("t", new_proposer_policy->active_time)("s", new_proposer_policy->proposer_schedule)); } @@ -3152,8 +3151,9 @@ struct controller_impl { gp.proposed_schedule.producers.clear(); }); } + return new_proposer_policy; }; - apply_s(chain_head, process_new_proposer_policy); + auto new_proposer_policy = apply_s>(chain_head, process_new_proposer_policy); auto assembled_block = bb.assemble_block(thread_pool.get_executor(), protocol_features.get_protocol_feature_set(), fork_db, std::move(new_proposer_policy), From 59861d97340d4da1c4319578debdac3a41c95aa7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 May 2024 15:07:20 -0500 Subject: [PATCH 11/11] GH-97 Reference contracts updated to use Spring --- .github/workflows/build.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index cb74a40886..65014cfbf7 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -336,7 +336,7 @@ jobs: apt-get -y install cmake build-essential - name: Build & Test reference-contracts run: | - cmake -S reference-contracts -B reference-contracts/build -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=On -DSYSTEM_ENABLE_LEAP_VERSION_CHECK=Off -DSYSTEM_ENABLE_CDT_VERSION_CHECK=Off + cmake -S reference-contracts -B reference-contracts/build -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=On -DSYSTEM_ENABLE_SPRING_VERSION_CHECK=Off -DSYSTEM_ENABLE_CDT_VERSION_CHECK=Off cmake --build reference-contracts/build -- -j $(nproc) cd reference-contracts/build/tests ctest --output-on-failure -j $(nproc)