From 3211b33e05288109ba0bbd156aa82f6f53644b16 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 20 Aug 2024 17:52:38 -0400 Subject: [PATCH 1/8] Add proposer policy tests for Savanna. --- unittests/savanna_proposer_policy_tests.cpp | 157 ++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 unittests/savanna_proposer_policy_tests.cpp diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp new file mode 100644 index 0000000000..5e6fdc45e2 --- /dev/null +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -0,0 +1,157 @@ +#include "savanna_cluster.hpp" + +using namespace eosio::chain; +using namespace eosio::testing; + +BOOST_AUTO_TEST_SUITE(savanna_proposer_policy) + +// --------------------------------------------------------------------------------------------------- +// Proposer Policy change - check expected delay when policy change initiated on +// the first block of a round +// --------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(policy_change_first_block_delay_check, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; + + auto start_num = A.head().block_num(); + while(start_num % 12 != 0) { + A.produce_block(); + start_num = A.head().block_num(); + } + + const vector producers { "p0"_n, "p1"_n }; + A.create_accounts(producers); + A.set_producers(producers); // set new producers and produce blocks until the switch is pending + auto end_num = A.head().block_num(); + + // under Savanna, a new policy becomes active on the first block of a 12 block round after: + // 1. finishing the current round + // 2. a full round + // ---------------------------------------------------------------------------------------- + uint32_t expected_gap = 12 - (start_num % 12); // 1. finishing the current round + expected_gap += 12; // 2. a full round + BOOST_REQUIRE_EQUAL(end_num, start_num + expected_gap); +} FC_LOG_AND_RETHROW() + + +// --------------------------------------------------------------------------------------------------- +// Proposer Policy change - check expected delay when policy change initiated on +// the 6th block of a round +// --------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; + + auto start_num = A.head().block_num(); + while(start_num % 12 != 6) { + A.produce_block(); + start_num = A.head().block_num(); + } + + const vector producers { "p0"_n, "p1"_n }; + A.create_accounts(producers); + A.set_producers(producers); // set new producers and produce blocks until the switch is pending + auto end_num = A.head().block_num(); + + // under Savanna, a new policy becomes active on the first block of a 12 block round after: + // 1. finishing the current round + // 2. a full round + // ---------------------------------------------------------------------------------------- + uint32_t expected_gap = 12 - (start_num % 12); // 1. finishing the current round + expected_gap += 12; // 2. a full round + BOOST_REQUIRE_EQUAL(end_num, start_num + expected_gap); +} FC_LOG_AND_RETHROW() + + +// --------------------------------------------------------------------------------------------------- +// Proposer Policy change - check expected delay when policy change initiated on +// the last block of a round +// --------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(policy_change_last_block_delay_check, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; + + auto start_num = A.head().block_num(); + while(start_num % 12 != 11) { + A.produce_block(); + start_num = A.head().block_num(); + } + + const vector producers { "p0"_n, "p1"_n }; + A.create_accounts(producers); + A.set_producers(producers); // set new producers and produce blocks until the switch is pending + auto end_num = A.head().block_num(); + + // under Savanna, a new policy becomes active on the first block of a 12 block round after: + // 1. finishing the current round + // 2. a full round + // ---------------------------------------------------------------------------------------- + uint32_t expected_gap = 12 - (start_num % 12); // 1. finishing the current round + expected_gap += 12; // 2. a full round + BOOST_REQUIRE_EQUAL(end_num, start_num + expected_gap); +} FC_LOG_AND_RETHROW() + + +// --------------------------------------------------------------------------------------------------- +// Verify that a proposer policy does not become active when finality has stalled +// --------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; + + // split network { A, B } and { C, D } + // Regardless of how many blocks A produces, finality will not advance + // by more than one (1 QC in flight) + // ------------------------------------------------------------------- + const std::vector partition {2, 3}; + set_partition(partition); + A.produce_block(); // take care of the in-flight QC + + const vector producers { "p0"_n, "p1"_n }; + auto orig_version = A.control->active_producers().version; + auto orig_lib_num = A.lib_number; + + A.create_accounts(producers); + A.tester::set_producers(producers); // push the action to update the producer schedule + + // produce 24 more blocks. If finality was advancing, the new proposer policy would be active, + // but with a split network finality will stall, and the new proposer policy should *not* become active. + // ----------------------------------------------------------------------------------------------------- + A.produce_blocks(24); + BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); + BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); +} FC_LOG_AND_RETHROW() + +// --------------------------------------------------------------------------------------------------- +// Verify that a proposer policy becomes active when finality has advanced enough to make it pending +// --------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; + + const vector producers { "p0"_n, "p1"_n }; + auto orig_version = A.control->active_producers().version; + + while (A.head().block_num() % 12 >= 10) + A.produce_block(); // make sure we are not on the last two blocks of a round + + A.create_accounts(producers); + A.tester::set_producers(producers); // push the action to update the producer schedule + A.produce_blocks(12); // produce 12 blocks which guarantees that the proposer policy is pending + + BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); + auto orig_lib_num = A.lib_number; + + // split network { A, B } and { C, D } + // Regardless of how many blocks A produces, finality will not advance + // by more than one (1 QC in flight) + // ------------------------------------------------------------------- + const std::vector partition {2, 3}; + set_partition(partition); + + // produce 24 more blocks. If finality was advancing, the new proposer policy would be active, + // but with a split network finality will stall, and the new proposer policy should *not* become active. + // ----------------------------------------------------------------------------------------------------- + A.produce_blocks(24); + BOOST_REQUIRE_LE(A.lib_number, orig_lib_num + 1); + BOOST_REQUIRE_GT(A.control->active_producers().version, orig_version); +} FC_LOG_AND_RETHROW() + + + +BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From b05c72e5bd267165cb053b58f43a9742060f6dce Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 20 Aug 2024 18:04:54 -0400 Subject: [PATCH 2/8] Add check validating that the proposer policy is pending after producing 13 blocks. --- unittests/savanna_proposer_policy_tests.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index 5e6fdc45e2..8b584edb84 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -132,9 +132,11 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, A.create_accounts(producers); A.tester::set_producers(producers); // push the action to update the producer schedule - A.produce_blocks(12); // produce 12 blocks which guarantees that the proposer policy is pending + A.produce_blocks(13); // produce 13 blocks which guarantees that the proposer policy is pending BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); + BOOST_REQUIRE(!!A.control->pending_producers()); + BOOST_REQUIRE_GT(A.control->pending_producers()->version, orig_version); auto orig_lib_num = A.lib_number; // split network { A, B } and { C, D } From 2606f4ad250f6a5589a05834849ba9011d4d81e6 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 21 Aug 2024 11:31:59 -0400 Subject: [PATCH 3/8] Address PR comments - part 1 --- unittests/savanna_cluster.hpp | 17 ++- unittests/savanna_proposer_policy_tests.cpp | 134 ++++++++++++++------ 2 files changed, 107 insertions(+), 44 deletions(-) diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 64ac1e8323..b135a1ca7e 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -113,12 +113,10 @@ namespace savanna_cluster { BOOST_REQUIRE_EQUAL(lib_num(), pt_block->block_num()); } - // updates producers (producer updates will be propagated to connected nodes), and - // wait until one of the new producers is pending. + // wait until one of `producers` is the producer that will be used for the next block produced. // return the index of the pending new producer (we assume no duplicates in producer list) - // ----------------------------------------------------------------------------------- - size_t set_producers(const std::vector& producers) { - tester::set_producers(producers); + // -------------------------------------------------------------------------------------------- + size_t wait_for_producer(const std::vector& producers) { account_name pending; size_t max_blocks_produced = 400; while (--max_blocks_produced) { @@ -131,6 +129,15 @@ namespace savanna_cluster { return ranges::find(producers, pending) - producers.begin(); } + // updates producers (producer updates will be propagated to connected nodes), and + // wait until one of `producers` is the producer that will be used for the next block produced. + // return the index of the pending new producer (we assume no duplicates in producer list) + // -------------------------------------------------------------------------------------------- + size_t set_producers(const std::vector& producers) { + tester::set_producers(producers); + return wait_for_producer(producers); + } + uint32_t lib_num() const { return lib_number; } template diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index 8b584edb84..0fd1c76889 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -3,6 +3,8 @@ using namespace eosio::chain; using namespace eosio::testing; +static const uint32_t prod_rep = static_cast(config::producer_repetitions); + BOOST_AUTO_TEST_SUITE(savanna_proposer_policy) // --------------------------------------------------------------------------------------------------- @@ -12,52 +14,72 @@ BOOST_AUTO_TEST_SUITE(savanna_proposer_policy) BOOST_FIXTURE_TEST_CASE(policy_change_first_block_delay_check, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; - auto start_num = A.head().block_num(); - while(start_num % 12 != 0) { + while(A.head().timestamp().slot % prod_rep != prod_rep - 1) A.produce_block(); - start_num = A.head().block_num(); - } const vector producers { "p0"_n, "p1"_n }; A.create_accounts(producers); - A.set_producers(producers); // set new producers and produce blocks until the switch is pending - auto end_num = A.head().block_num(); - - // under Savanna, a new policy becomes active on the first block of a 12 block round after: + A.tester::set_producers(producers); // push the action to update the producer schedule + auto sb = A.produce_block(); // produce a block that will include the policy change transaction + auto orig_producer = sb->producer; // save producer before transition + auto start_slot = sb->timestamp.slot; + BOOST_REQUIRE_EQUAL(start_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep + + A.wait_for_producer(producers); // produce blocks until the new schedule will be active on next block produced + BOOST_REQUIRE_EQUAL(A.head().block()->producer, // head block should still have been produced using + orig_producer); // the original producer + sb = A.produce_block(); + bool using_new_sched = std::ranges::find(producers, sb->producer) != producers.end(); + BOOST_REQUIRE(using_new_sched); // verify that we have just switched to new schedule + auto end_slot = sb->timestamp.slot; + BOOST_REQUIRE_EQUAL(end_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep + + // under Savanna, a new policy becomes active on the first block of a prod_rep block round after: // 1. finishing the current round // 2. a full round // ---------------------------------------------------------------------------------------- - uint32_t expected_gap = 12 - (start_num % 12); // 1. finishing the current round - expected_gap += 12; // 2. a full round - BOOST_REQUIRE_EQUAL(end_num, start_num + expected_gap); + uint32_t expected_gap = prod_rep - (start_slot % prod_rep); // 1. finishing the current round + expected_gap += prod_rep; // 2. a full round + BOOST_REQUIRE_EQUAL(end_slot, start_slot + expected_gap); } FC_LOG_AND_RETHROW() // --------------------------------------------------------------------------------------------------- // Proposer Policy change - check expected delay when policy change initiated on -// the 6th block of a round +// the middle block of a round // --------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; + uint32_t middle = prod_rep / 2; + assert(middle > 0); - auto start_num = A.head().block_num(); - while(start_num % 12 != 6) { + while(A.head().timestamp().slot % prod_rep != middle - 1) A.produce_block(); - start_num = A.head().block_num(); - } const vector producers { "p0"_n, "p1"_n }; A.create_accounts(producers); - A.set_producers(producers); // set new producers and produce blocks until the switch is pending - auto end_num = A.head().block_num(); - - // under Savanna, a new policy becomes active on the first block of a 12 block round after: + A.tester::set_producers(producers); // push the action to update the producer schedule + auto sb = A.produce_block(); // produce a block that will include the policy change transaction + auto orig_producer = sb->producer; // save producer before transition + auto start_slot = sb->timestamp.slot; + BOOST_REQUIRE_EQUAL(start_slot % prod_rep, middle); // validate that the policy change occurs on the middle block of prod_rep + + A.wait_for_producer(producers); // produce blocks until the new schedule will be active on next block produced + BOOST_REQUIRE_EQUAL(A.head().block()->producer, // head block should still have been produced using + orig_producer); // the original producer + sb = A.produce_block(); + bool using_new_sched = std::ranges::find(producers, sb->producer) != producers.end(); + BOOST_REQUIRE(using_new_sched); // verify that we have just switched to new schedule + auto end_slot = sb->timestamp.slot; + BOOST_REQUIRE_EQUAL(end_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep + + // under Savanna, a new policy becomes active on the first block of a prod_rep block round after: // 1. finishing the current round // 2. a full round // ---------------------------------------------------------------------------------------- - uint32_t expected_gap = 12 - (start_num % 12); // 1. finishing the current round - expected_gap += 12; // 2. a full round - BOOST_REQUIRE_EQUAL(end_num, start_num + expected_gap); + uint32_t expected_gap = prod_rep - (start_slot % prod_rep); // 1. finishing the current round + expected_gap += prod_rep; // 2. a full round + BOOST_REQUIRE_EQUAL(end_slot, start_slot + expected_gap); } FC_LOG_AND_RETHROW() @@ -67,28 +89,40 @@ BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster:: // --------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(policy_change_last_block_delay_check, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; + uint32_t last = prod_rep - 1; + assert(last > 0); - auto start_num = A.head().block_num(); - while(start_num % 12 != 11) { + while(A.head().timestamp().slot % prod_rep != last - 1) A.produce_block(); - start_num = A.head().block_num(); - } const vector producers { "p0"_n, "p1"_n }; A.create_accounts(producers); - A.set_producers(producers); // set new producers and produce blocks until the switch is pending - auto end_num = A.head().block_num(); - - // under Savanna, a new policy becomes active on the first block of a 12 block round after: + A.tester::set_producers(producers); // push the action to update the producer schedule + auto sb = A.produce_block(); // produce a block that will include the policy change transaction + auto orig_producer = sb->producer; // save producer before transition + auto start_slot = sb->timestamp.slot; + BOOST_REQUIRE_EQUAL(start_slot % prod_rep, last); // validate that the policy change occurs on the last block of prod_rep + + A.wait_for_producer(producers); // produce blocks until the new schedule will be active on next block produced + BOOST_REQUIRE_EQUAL(A.head().block()->producer, // head block should still have been produced using + orig_producer); // the original producer + sb = A.produce_block(); + bool using_new_sched = std::ranges::find(producers, sb->producer) != producers.end(); + BOOST_REQUIRE(using_new_sched); // verify that we have just switched to new schedule + auto end_slot = sb->timestamp.slot; + BOOST_REQUIRE_EQUAL(end_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep + + // under Savanna, a new policy becomes active on the first block of a prod_rep block round after: // 1. finishing the current round // 2. a full round // ---------------------------------------------------------------------------------------- - uint32_t expected_gap = 12 - (start_num % 12); // 1. finishing the current round - expected_gap += 12; // 2. a full round - BOOST_REQUIRE_EQUAL(end_num, start_num + expected_gap); + uint32_t expected_gap = prod_rep - (start_slot % prod_rep); // 1. finishing the current round + expected_gap += prod_rep; // 2. a full round + BOOST_REQUIRE_EQUAL(end_slot, start_slot + expected_gap); } FC_LOG_AND_RETHROW() + // --------------------------------------------------------------------------------------------------- // Verify that a proposer policy does not become active when finality has stalled // --------------------------------------------------------------------------------------------------- @@ -101,21 +135,43 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_clus // ------------------------------------------------------------------- const std::vector partition {2, 3}; set_partition(partition); - A.produce_block(); // take care of the in-flight QC + auto sb = A.produce_block(); // take care of the in-flight QC const vector producers { "p0"_n, "p1"_n }; auto orig_version = A.control->active_producers().version; auto orig_lib_num = A.lib_number; + auto orig_producer = sb->producer; A.create_accounts(producers); - A.tester::set_producers(producers); // push the action to update the producer schedule + A.tester::set_producers(producers); // push the action to update the producer schedule - // produce 24 more blocks. If finality was advancing, the new proposer policy would be active, + // produce `2 * prod_rep` more blocks. If finality was advancing, the new proposer policy would be active, // but with a split network finality will stall, and the new proposer policy should *not* become active. // ----------------------------------------------------------------------------------------------------- - A.produce_blocks(24); + A.produce_blocks(2 * prod_rep); BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); + + // remove network split + // -------------------- + set_partition({}); + propagate_heads(); + + // Now that the network is not split anymore, finality will start advancing again on the third + // block produced, and we expect the new proposer policy to become active on the next first block of a round + // --------------------------------------------------------------------------------------------------------- + sb = A.produce_blocks(3); // allow two blocks to be voted on. + BOOST_REQUIRE_EQUAL(sb->producer, orig_producer); // should still use orig producer + + // now switch should happen within the next `prod_rep` blocks + for (uint32_t i=0; iproducer != orig_producer) { + BOOST_REQUIRE_EQUAL(sb->timestamp.slot % prod_rep, 0); + break; + } + } + BOOST_REQUIRE_NE(sb->producer, orig_producer); } FC_LOG_AND_RETHROW() // --------------------------------------------------------------------------------------------------- @@ -127,7 +183,7 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, const vector producers { "p0"_n, "p1"_n }; auto orig_version = A.control->active_producers().version; - while (A.head().block_num() % 12 >= 10) + while (A.head().timestamp().slot % prod_rep >= prod_rep - 2) A.produce_block(); // make sure we are not on the last two blocks of a round A.create_accounts(producers); From 310264cd097da3285abd2ad03d059e3ffc4860eb Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 21 Aug 2024 12:07:40 -0400 Subject: [PATCH 4/8] Add `no_proposer_policy_change_without_finality_2` test --- unittests/savanna_proposer_policy_tests.cpp | 64 +++++++++++++++++++-- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index 0fd1c76889..055c8bf002 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -121,8 +121,6 @@ BOOST_FIXTURE_TEST_CASE(policy_change_last_block_delay_check, savanna_cluster::c BOOST_REQUIRE_EQUAL(end_slot, start_slot + expected_gap); } FC_LOG_AND_RETHROW() - - // --------------------------------------------------------------------------------------------------- // Verify that a proposer policy does not become active when finality has stalled // --------------------------------------------------------------------------------------------------- @@ -148,7 +146,7 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_clus // produce `2 * prod_rep` more blocks. If finality was advancing, the new proposer policy would be active, // but with a split network finality will stall, and the new proposer policy should *not* become active. // ----------------------------------------------------------------------------------------------------- - A.produce_blocks(2 * prod_rep); + A.produce_blocks(2 * prod_rep); // make sure finality stalls long enough for new policy to be eligible BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); @@ -160,7 +158,7 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_clus // Now that the network is not split anymore, finality will start advancing again on the third // block produced, and we expect the new proposer policy to become active on the next first block of a round // --------------------------------------------------------------------------------------------------------- - sb = A.produce_blocks(3); // allow two blocks to be voted on. + sb = A.produce_blocks(2); // allow two blocks to be voted on. BOOST_REQUIRE_EQUAL(sb->producer, orig_producer); // should still use orig producer // now switch should happen within the next `prod_rep` blocks @@ -174,6 +172,60 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_clus BOOST_REQUIRE_NE(sb->producer, orig_producer); } FC_LOG_AND_RETHROW() +// --------------------------------------------------------------------------------------------------- +// Verify that a proposer policy does not become active when finality has stalled +// AND +// if finality starts advancing again while there are only two blocks left to produce in the round, +// the proposer schedule change will happen exactly on the first block of the next round (provided +// finality stalled long enough) +// --------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality_2, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; + + // split network { A, B } and { C, D } + // Regardless of how many blocks A produces, finality will not advance + // by more than one (1 QC in flight) + // ------------------------------------------------------------------- + const std::vector partition {2, 3}; + set_partition(partition); + auto sb = A.produce_block(); // take care of the in-flight QC + + const vector producers { "p0"_n, "p1"_n }; + auto orig_version = A.control->active_producers().version; + auto orig_lib_num = A.lib_number; + auto orig_producer = sb->producer; + + A.create_accounts(producers); + A.tester::set_producers(producers); // push the action to update the producer schedule + + // produce `2 * prod_rep` more blocks. If finality was advancing, the new proposer policy would be active, + // but with a split network finality will stall, and the new proposer policy should *not* become active. + // ----------------------------------------------------------------------------------------------------- + sb = A.produce_blocks(2 * prod_rep); // make sure finality stalls long enough for new policy to be eligible + while(sb->timestamp.slot % prod_rep != prod_rep - 3) // produce blocks until there are only two more left in the round + sb = A.produce_block(); + + BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); + BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); + + // remove network split + // -------------------- + set_partition({}); + propagate_heads(); + + // Now that the network is not split anymore, finality will start advancing again on the third + // block produced, and we expect the new proposer policy to become active on the next first block of a round + // --------------------------------------------------------------------------------------------------------- + sb = A.produce_blocks(2); // allow two blocks to be voted on (the last two blocks of a round) + BOOST_REQUIRE_EQUAL(sb->producer, orig_producer); // should still use orig producer + + // now switch should happen in the next block, as it is the first block of a round + // ------------------------------------------------------------------------------- + sb = A.produce_block(); + BOOST_REQUIRE_NE(sb->producer, orig_producer); // verify switch has happened + BOOST_REQUIRE_EQUAL(sb->timestamp.slot % prod_rep, 0); // verify first block of a round +} FC_LOG_AND_RETHROW() + // --------------------------------------------------------------------------------------------------- // Verify that a proposer policy becomes active when finality has advanced enough to make it pending // --------------------------------------------------------------------------------------------------- @@ -202,10 +254,10 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, const std::vector partition {2, 3}; set_partition(partition); - // produce 24 more blocks. If finality was advancing, the new proposer policy would be active, + // produce `2 * prod_rep` more blocks. If finality was advancing, the new proposer policy would be active, // but with a split network finality will stall, and the new proposer policy should *not* become active. // ----------------------------------------------------------------------------------------------------- - A.produce_blocks(24); + A.produce_blocks(2 * prod_rep); BOOST_REQUIRE_LE(A.lib_number, orig_lib_num + 1); BOOST_REQUIRE_GT(A.control->active_producers().version, orig_version); } FC_LOG_AND_RETHROW() From 677eb03399ffede09c4703cb4510c7e6d5f36552 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 21 Aug 2024 15:03:38 -0400 Subject: [PATCH 5/8] Update `pending_proposer_policy_becomes_active_without_finality` test. --- unittests/savanna_proposer_policy_tests.cpp | 55 ++++++++++++--------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index 055c8bf002..74bf2531d8 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -17,7 +17,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change_first_block_delay_check, savanna_cluster:: while(A.head().timestamp().slot % prod_rep != prod_rep - 1) A.produce_block(); - const vector producers { "p0"_n, "p1"_n }; + const vector producers { "pa"_n, "pb"_n }; A.create_accounts(producers); A.tester::set_producers(producers); // push the action to update the producer schedule auto sb = A.produce_block(); // produce a block that will include the policy change transaction @@ -56,7 +56,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster:: while(A.head().timestamp().slot % prod_rep != middle - 1) A.produce_block(); - const vector producers { "p0"_n, "p1"_n }; + const vector producers { "pa"_n, "pb"_n }; A.create_accounts(producers); A.tester::set_producers(producers); // push the action to update the producer schedule auto sb = A.produce_block(); // produce a block that will include the policy change transaction @@ -95,7 +95,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change_last_block_delay_check, savanna_cluster::c while(A.head().timestamp().slot % prod_rep != last - 1) A.produce_block(); - const vector producers { "p0"_n, "p1"_n }; + const vector producers { "pa"_n, "pb"_n }; A.create_accounts(producers); A.tester::set_producers(producers); // push the action to update the producer schedule auto sb = A.produce_block(); // produce a block that will include the policy change transaction @@ -135,7 +135,7 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_clus set_partition(partition); auto sb = A.produce_block(); // take care of the in-flight QC - const vector producers { "p0"_n, "p1"_n }; + const vector producers { "pa"_n, "pb"_n }; auto orig_version = A.control->active_producers().version; auto orig_lib_num = A.lib_number; auto orig_producer = sb->producer; @@ -150,8 +150,8 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality, savanna_clus BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); - // remove network split - // -------------------- + // remove network split. verify that proposer policy becomes active + // ---------------------------------------------------------------- set_partition({}); propagate_heads(); @@ -190,7 +190,7 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality_2, savanna_cl set_partition(partition); auto sb = A.produce_block(); // take care of the in-flight QC - const vector producers { "p0"_n, "p1"_n }; + const vector producers { "pa"_n, "pb"_n }; auto orig_version = A.control->active_producers().version; auto orig_lib_num = A.lib_number; auto orig_producer = sb->producer; @@ -208,8 +208,8 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality_2, savanna_cl BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); - // remove network split - // -------------------- + // remove network split. Verify that proposer policy becomes active + // ---------------------------------------------------------------- set_partition({}); propagate_heads(); @@ -232,20 +232,19 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality_2, savanna_cl BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; - const vector producers { "p0"_n, "p1"_n }; + auto sb = A.produce_block(); + auto orig_producer = sb->producer; auto orig_version = A.control->active_producers().version; - while (A.head().timestamp().slot % prod_rep >= prod_rep - 2) - A.produce_block(); // make sure we are not on the last two blocks of a round + while (A.head().timestamp().slot % prod_rep >= prod_rep - 4) + A.produce_block(); // make sure the next block is not one of the last three blocks of a round + + const vector producers { "pa"_n, "pb"_n }; A.create_accounts(producers); A.tester::set_producers(producers); // push the action to update the producer schedule - A.produce_blocks(13); // produce 13 blocks which guarantees that the proposer policy is pending - - BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); - BOOST_REQUIRE(!!A.control->pending_producers()); - BOOST_REQUIRE_GT(A.control->pending_producers()->version, orig_version); - auto orig_lib_num = A.lib_number; + A.produce_block(); // produce a block that will include the policy change transaction + A.produce_blocks(12); // produce 12 blocks which guarantees that the proposer policy is pending // split network { A, B } and { C, D } // Regardless of how many blocks A produces, finality will not advance @@ -254,12 +253,22 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, const std::vector partition {2, 3}; set_partition(partition); - // produce `2 * prod_rep` more blocks. If finality was advancing, the new proposer policy would be active, - // but with a split network finality will stall, and the new proposer policy should *not* become active. + sb = A.produce_block(); // produce one more block for lib final advance (in-flight QC) + + BOOST_REQUIRE_EQUAL(sb->producer, orig_producer); + BOOST_REQUIRE_EQUAL(A.control->active_producers().version, orig_version); + BOOST_REQUIRE(!!A.control->pending_producers()); + BOOST_REQUIRE_GT(A.control->pending_producers()->version, orig_version); + auto orig_lib_num = A.lib_number; + + // produce `prod_rep` more blocks. Finality will not be advancing anymore, but still the new policy + // will become active because it was already pending. // ----------------------------------------------------------------------------------------------------- - A.produce_blocks(2 * prod_rep); - BOOST_REQUIRE_LE(A.lib_number, orig_lib_num + 1); - BOOST_REQUIRE_GT(A.control->active_producers().version, orig_version); + sb = A.produce_blocks(prod_rep); + BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); // check lib didn't advance + BOOST_REQUIRE_GT(A.control->active_producers().version, orig_version);// check producer schedule version is greater + BOOST_REQUIRE_NE(sb->producer, orig_producer); // and the last block was produced by a different producer + } FC_LOG_AND_RETHROW() From 1e7dd28476b5d6e1fcaa77869cc450b9601fea62 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 21 Aug 2024 15:56:08 -0400 Subject: [PATCH 6/8] Minor changes (add a check) --- unittests/savanna_proposer_policy_tests.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index 74bf2531d8..8fe0629456 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -31,6 +31,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change_first_block_delay_check, savanna_cluster:: sb = A.produce_block(); bool using_new_sched = std::ranges::find(producers, sb->producer) != producers.end(); BOOST_REQUIRE(using_new_sched); // verify that we have just switched to new schedule + BOOST_REQUIRE_NE(sb->producer, orig_producer); // and that the producer has changed auto end_slot = sb->timestamp.slot; BOOST_REQUIRE_EQUAL(end_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep @@ -70,6 +71,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster:: sb = A.produce_block(); bool using_new_sched = std::ranges::find(producers, sb->producer) != producers.end(); BOOST_REQUIRE(using_new_sched); // verify that we have just switched to new schedule + BOOST_REQUIRE_NE(sb->producer, orig_producer); // and that the producer has changed auto end_slot = sb->timestamp.slot; BOOST_REQUIRE_EQUAL(end_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep @@ -109,6 +111,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change_last_block_delay_check, savanna_cluster::c sb = A.produce_block(); bool using_new_sched = std::ranges::find(producers, sb->producer) != producers.end(); BOOST_REQUIRE(using_new_sched); // verify that we have just switched to new schedule + BOOST_REQUIRE_NE(sb->producer, orig_producer); // and that the producer has changed auto end_slot = sb->timestamp.slot; BOOST_REQUIRE_EQUAL(end_slot % prod_rep, 0); // validate that the policy change occurs on the first block of prod_rep From 95620bfc7576f17b95508694597f66b5af72a353 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 21 Aug 2024 18:16:07 -0400 Subject: [PATCH 7/8] Address some PR comments. --- unittests/savanna_proposer_policy_tests.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index 8fe0629456..eb1ef3b2c3 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -51,8 +51,8 @@ BOOST_FIXTURE_TEST_CASE(policy_change_first_block_delay_check, savanna_cluster:: // --------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; - uint32_t middle = prod_rep / 2; - assert(middle > 0); + const uint32_t middle = prod_rep / 2; + static_assert(middle > 0); while(A.head().timestamp().slot % prod_rep != middle - 1) A.produce_block(); @@ -91,8 +91,8 @@ BOOST_FIXTURE_TEST_CASE(policy_change_sixth_block_delay_check, savanna_cluster:: // --------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(policy_change_last_block_delay_check, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; - uint32_t last = prod_rep - 1; - assert(last > 0); + const uint32_t last = prod_rep - 1; + static_assert(last > 0); while(A.head().timestamp().slot % prod_rep != last - 1) A.produce_block(); @@ -234,6 +234,7 @@ BOOST_FIXTURE_TEST_CASE(no_proposer_policy_change_without_finality_2, savanna_cl // --------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; + static_assert(prod_rep >= 4); auto sb = A.produce_block(); auto orig_producer = sb->producer; @@ -247,7 +248,7 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, A.create_accounts(producers); A.tester::set_producers(producers); // push the action to update the producer schedule A.produce_block(); // produce a block that will include the policy change transaction - A.produce_blocks(12); // produce 12 blocks which guarantees that the proposer policy is pending + A.produce_blocks(prod_rep); // produce `prod_rep` blocks which guarantees that the proposer policy is pending // split network { A, B } and { C, D } // Regardless of how many blocks A produces, finality will not advance @@ -266,7 +267,9 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, // produce `prod_rep` more blocks. Finality will not be advancing anymore, but still the new policy // will become active because it was already pending. - // ----------------------------------------------------------------------------------------------------- + // Indeed, the new policy would eventually become active as long as it was simply *proposed* prior to the + // last final block when finality stalled, but this is not verified in this test. + // ------------------------------------------------------------------------------------------------------ sb = A.produce_blocks(prod_rep); BOOST_REQUIRE_EQUAL(A.lib_number, orig_lib_num); // check lib didn't advance BOOST_REQUIRE_GT(A.control->active_producers().version, orig_version);// check producer schedule version is greater From e1c7d29f818a075989f0e7afec94a8e6df19e851 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 21 Aug 2024 20:15:10 -0400 Subject: [PATCH 8/8] Address PR comment by adjusting while condition so that the comment is accurate. --- unittests/savanna_proposer_policy_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/savanna_proposer_policy_tests.cpp b/unittests/savanna_proposer_policy_tests.cpp index eb1ef3b2c3..c2b99a56dc 100644 --- a/unittests/savanna_proposer_policy_tests.cpp +++ b/unittests/savanna_proposer_policy_tests.cpp @@ -240,7 +240,7 @@ BOOST_FIXTURE_TEST_CASE(pending_proposer_policy_becomes_active_without_finality, auto orig_producer = sb->producer; auto orig_version = A.control->active_producers().version; - while (A.head().timestamp().slot % prod_rep >= prod_rep - 4) + while ((A.head().timestamp().slot + 1) % prod_rep >= prod_rep - 3) A.produce_block(); // make sure the next block is not one of the last three blocks of a round const vector producers { "pa"_n, "pb"_n };