diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 2a9f72698e..0e64d61546 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -491,38 +491,27 @@ struct building_block { uint32_t get_block_num() const { return block_num; } - // returns the next proposer schedule version and true if different - // 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 { + // 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& { - // 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 - 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; + 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 return active_proposer_policy->proposer_schedule; }; const producer_authority_schedule& lhs = get_next_sched(); - - if (std::ranges::equal(lhs.producers, producers)) { - return {lhs.version, false}; + auto v = lhs.version; + if (!std::equal(lhs.producers.begin(), lhs.producers.end(), producers.begin(), producers.end())) { + ++v; + return std::optional{v}; } - - return {lhs.version + 1, true}; + return std::nullopt; } }; @@ -613,10 +602,10 @@ struct building_block { v); } - std::tuple get_next_proposer_schedule_version(const 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); } }, @@ -911,13 +900,13 @@ struct pending_state { _block_stage); } - std::tuple get_next_proposer_schedule_version(const 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); } @@ -3142,14 +3131,19 @@ 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) { - 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)); + 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; + 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(); @@ -3157,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), @@ -5232,17 +5227,11 @@ int64_t controller_impl::set_proposed_producers( vector prod assert(pending); - auto [version, diff] = pending->get_next_proposer_schedule_version(producers); - if (!diff) - return version; - 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) ); - - // 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; auto& gpo = db.get(); db.modify( gpo, [&]( auto& gp ) { @@ -5250,7 +5239,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 9347c97b57..7fd43afe32 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -79,6 +79,11 @@ 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 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 flat_map proposer_policies; // track in-flight finalizer policies. This is a `multimap` because the same block number diff --git a/libraries/chain/include/eosio/chain/webassembly/interface.hpp b/libraries/chain/include/eosio/chain/webassembly/interface.hpp index 8d5d00ebcf..39093ddeb2 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); diff --git a/unittests/producer_schedule_if_tests.cpp b/unittests/producer_schedule_if_tests.cpp index ff1afe3b6a..59e3b8667c 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() ) ); + // 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 auto prod = produce_block()->producer; @@ -179,19 +192,125 @@ 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( true, compare_schedules( sch3, control->active_producers() ) ); + 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( true, compare_schedules( sch3, control->active_producers() ) ); + 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} ); + 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( 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 + 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( 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 + 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( 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 + 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( 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( 9u, 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( 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( 12u, 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( 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()