From 48c98a2aa76540640b2a0d84d885f22e87333305 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 16:33:25 +0100 Subject: [PATCH 1/7] Fix active_elections.activate_account_chain This test is supposed to be testing the node's internal election activation logic but this is being circumvented by activating elections within the test itself. --- nano/core_test/active_elections.cpp | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index 9818755d07..179ce2fbec 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -1092,6 +1092,7 @@ TEST (active_elections, conflicting_block_vote_existing_election) ASSERT_TIMELY (3s, election->confirmed ()); } +// This tests the node's internal block activation logic TEST (active_elections, activate_account_chain) { nano::test::system system; @@ -1153,32 +1154,24 @@ TEST (active_elections, activate_account_chain) ASSERT_EQ (nano::block_status::progress, node.process (open)); ASSERT_EQ (nano::block_status::progress, node.process (receive)); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_TIMELY (5s, node.active.election (send->qualified_root ())); - auto election1 = node.active.election (send->qualified_root ()); + auto election1 = nano::test::start_election (system, node, send->hash ()); ASSERT_EQ (1, node.active.size ()); ASSERT_EQ (1, election1->blocks ().count (send->hash ())); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - auto election2 = node.active.election (send->qualified_root ()); - ASSERT_EQ (election2, election1); - election1->force_confirm (); + election1->force_confirm (); // Force confirm to trigger successor activation ASSERT_TIMELY (3s, node.block_confirmed (send->hash ())); // On cementing, the next election is started ASSERT_TIMELY (3s, node.active.active (send2->qualified_root ())); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); auto election3 = node.active.election (send2->qualified_root ()); ASSERT_NE (nullptr, election3); ASSERT_EQ (1, election3->blocks ().count (send2->hash ())); - election3->force_confirm (); + election3->force_confirm (); // Force confirm to trigger successor and destination activation ASSERT_TIMELY (3s, node.block_confirmed (send2->hash ())); // On cementing, the next election is started - ASSERT_TIMELY (3s, node.active.active (open->qualified_root ())); - ASSERT_TIMELY (3s, node.active.active (send3->qualified_root ())); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); + ASSERT_TIMELY (3s, node.active.active (open->qualified_root ())); // Destination account activated + ASSERT_TIMELY (3s, node.active.active (send3->qualified_root ())); // Block successor activated auto election4 = node.active.election (send3->qualified_root ()); ASSERT_NE (nullptr, election4); ASSERT_EQ (1, election4->blocks ().count (send3->hash ())); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), key.pub); auto election5 = node.active.election (open->qualified_root ()); ASSERT_NE (nullptr, election5); ASSERT_EQ (1, election5->blocks ().count (open->hash ())); @@ -1186,10 +1179,10 @@ TEST (active_elections, activate_account_chain) ASSERT_TIMELY (3s, node.block_confirmed (open->hash ())); // Until send3 is also confirmed, the receive block should not activate std::this_thread::sleep_for (200ms); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), key.pub); + ASSERT_FALSE (node.active.active (receive->qualified_root ())); election4->force_confirm (); ASSERT_TIMELY (3s, node.block_confirmed (send3->hash ())); - ASSERT_TIMELY (3s, node.active.active (receive->qualified_root ())); + ASSERT_TIMELY (3s, node.active.active (receive->qualified_root ())); // Destination account activated } TEST (active_elections, activate_inactive) From 06143a6c8828c088745c7aea427bad01ab2e05ae Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 16:58:20 +0100 Subject: [PATCH 2/7] Fix receivable_processor tests that weren't testing anything --- nano/core_test/network.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index d84ac7da05..cfd7cc4297 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -364,18 +365,21 @@ TEST (receivable_processor, confirm_insufficient_pos) .send () .previous (nano::dev::genesis->hash ()) .destination (0) - .balance (0) + .balance (nano::dev::constants.genesis_amount - 1) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (0) .build (); node1.work_generate_blocking (*block1); ASSERT_EQ (nano::block_status::progress, node1.process (block1)); - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); + auto election = nano::test::start_election (system, node1, block1->hash ()); nano::keypair key1; - auto vote = nano::test::make_vote (key1, { block1 }, 0, 0); + auto vote = nano::test::make_final_vote (key1, { block1 }); nano::confirm_ack con1{ nano::dev::network_params.network, vote }; auto channel1 = std::make_shared (node1, node1); + ASSERT_EQ (1, election->votes ().size ()); node1.network.inbound (con1, channel1); + ASSERT_TIMELY_EQ (5s, 2, election->votes ().size ()) + ASSERT_FALSE (election->confirmed ()); } TEST (receivable_processor, confirm_sufficient_pos) @@ -387,17 +391,20 @@ TEST (receivable_processor, confirm_sufficient_pos) .send () .previous (nano::dev::genesis->hash ()) .destination (0) - .balance (0) + .balance (nano::dev::constants.genesis_amount - 1) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (0) .build (); node1.work_generate_blocking (*block1); ASSERT_EQ (nano::block_status::progress, node1.process (block1)); - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - auto vote = nano::test::make_vote (nano::dev::genesis_key, { block1 }, 0, 0); + auto election = nano::test::start_election (system, node1, block1->hash ()); + auto vote = nano::test::make_final_vote (nano::dev::genesis_key, { block1 }); nano::confirm_ack con1{ nano::dev::network_params.network, vote }; auto channel1 = std::make_shared (node1, node1); + ASSERT_EQ (1, election->votes ().size ()); node1.network.inbound (con1, channel1); + ASSERT_TIMELY_EQ (5s, 2, election->votes ().size ()) + ASSERT_TRUE (election->confirmed ()); } TEST (receivable_processor, send_with_receive) From d9bdec707f515a70b6cef5014ecf57b2de86dfa9 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 17:05:01 +0100 Subject: [PATCH 3/7] Use nano::test::start_election to manually start elections instead of directly calling scheduler. --- nano/core_test/active_elections.cpp | 8 ++------ nano/core_test/conflicts.cpp | 6 ++---- nano/core_test/election.cpp | 4 +--- nano/core_test/ledger.cpp | 8 ++------ nano/core_test/node.cpp | 4 +--- 5 files changed, 8 insertions(+), 22 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index 179ce2fbec..d58a91e968 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -425,9 +425,7 @@ TEST (inactive_votes_cache, multiple_votes) ASSERT_TIMELY_EQ (5s, node.vote_cache.find (send1->hash ()).size (), 2); ASSERT_EQ (1, node.vote_cache.size ()); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - std::shared_ptr election; - ASSERT_TIMELY (5s, election = node.active.election (send1->qualified_root ())); + auto election = nano::test::start_election (system, node, send1->hash ()); ASSERT_TIMELY_EQ (5s, 3, election->votes ().size ()); // 2 votes and 1 default not_an_acount ASSERT_EQ (2, node.stats.count (nano::stat::type::election, nano::stat::detail::vote_cached)); } @@ -1320,13 +1318,11 @@ TEST (active_elections, vacancy) ASSERT_EQ (nano::block_status::progress, node.process (send)); ASSERT_EQ (1, node.active.vacancy (nano::election_behavior::priority)); ASSERT_EQ (0, node.active.size ()); - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); + auto election1 = nano::test::start_election (system, node, send->hash ()); ASSERT_TIMELY (1s, updated); updated = false; ASSERT_EQ (0, node.active.vacancy (nano::election_behavior::priority)); ASSERT_EQ (1, node.active.size ()); - auto election1 = node.active.election (send->qualified_root ()); - ASSERT_NE (nullptr, election1); election1->force_confirm (); ASSERT_TIMELY (1s, updated); ASSERT_EQ (1, node.active.vacancy (nano::election_behavior::priority)); diff --git a/nano/core_test/conflicts.cpp b/nano/core_test/conflicts.cpp index 8a4ca3a481..43c1938158 100644 --- a/nano/core_test/conflicts.cpp +++ b/nano/core_test/conflicts.cpp @@ -31,9 +31,7 @@ TEST (conflicts, start_stop) node1.work_generate_blocking (*send1); ASSERT_EQ (nano::block_status::progress, node1.process (send1)); ASSERT_EQ (0, node1.active.size ()); - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ())); - auto election1 = node1.active.election (send1->qualified_root ()); + auto election1 = nano::test::start_election (system, node1, send1->hash ()); ASSERT_EQ (1, node1.active.size ()); ASSERT_NE (nullptr, election1); ASSERT_EQ (1, election1->votes ().size ()); @@ -64,7 +62,7 @@ TEST (conflicts, add_existing) ASSERT_TIMELY (5s, node1.block (send1->hash ())); // instruct the election scheduler to trigger an election for send1 - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); + nano::test::start_election (system, node1, send1->hash ()); // wait for election to be started before processing send2 ASSERT_TIMELY (5s, node1.active.active (*send1)); diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index 82bcb7f131..0ad811ab4f 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -152,9 +152,7 @@ TEST (election, quorum_minimum_confirm_success) .build (); node1.work_generate_blocking (*send1); node1.process_active (send1); - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ())); - auto election = node1.active.election (send1->qualified_root ()); + auto election = nano::test::start_election (system, node1, send1->hash ()); ASSERT_NE (nullptr, election); ASSERT_EQ (1, election->blocks ().size ()); auto vote = nano::test::make_final_vote (nano::dev::genesis_key, { send1->hash () }); diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index d850343849..a258d0c201 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -964,9 +964,7 @@ TEST (votes, check_signature) auto transaction = node1.ledger.tx_begin_write (); ASSERT_EQ (nano::block_status::progress, node1.ledger.process (transaction, send1)); } - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ())); - auto election1 = node1.active.election (send1->qualified_root ()); + auto election1 = nano::test::start_election (system, node1, send1->hash ()); ASSERT_EQ (1, election1->votes ().size ()); auto vote1 = nano::test::make_vote (nano::dev::genesis_key, { send1 }, nano::vote::timestamp_min * 1, 0); vote1->signature.bytes[0] ^= 1; @@ -1035,9 +1033,7 @@ TEST (votes, add_existing) .build (); node1.work_generate_blocking (*send1); ASSERT_EQ (nano::block_status::progress, node1.ledger.process (node1.ledger.tx_begin_write (), send1)); - node1.scheduler.priority.activate (node1.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ())); - auto election1 = node1.active.election (send1->qualified_root ()); + auto election1 = nano::test::start_election (system, node1, send1->hash ()); auto vote1 = nano::test::make_vote (nano::dev::genesis_key, { send1 }, nano::vote::timestamp_min * 1, 0); ASSERT_EQ (nano::vote_code::vote, node1.vote_router.vote (vote1).at (send1->hash ())); // Block is already processed from vote diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index 5ccf56c234..9a8111454b 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -1363,9 +1363,7 @@ TEST (node, rep_self_vote) ASSERT_EQ (nano::block_status::progress, node0->process (block0)); auto & active = node0->active; auto & scheduler = node0->scheduler; - scheduler.priority.activate (node0->ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_TIMELY (5s, active.election (block0->qualified_root ())); - auto election1 = active.election (block0->qualified_root ()); + auto election1 = nano::test::start_election (system, *node0, block0->hash ()); ASSERT_NE (nullptr, election1); // Wait until representatives are activated & make vote ASSERT_TIMELY_EQ (1s, election1->votes ().size (), 3); From def7f0b7489f5d302da94cee9c4634bf948ffc79 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 17:48:45 +0100 Subject: [PATCH 4/7] Remove manual election activation and confirmation height checking from rpc.account_representative_set_epoch_2_insufficient_work Comment about slow confirmation seems out of date as there is no performance difference simply checking confirmed status. --- nano/rpc_test/rpc.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 3093ca9ab6..90edadafef 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -2477,14 +2477,10 @@ TEST (rpc, account_representative_set_epoch_2_insufficient_work) system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv, false); // Upgrade the genesis account to epoch 2 + std::shared_ptr head; ASSERT_NE (nullptr, system.upgrade_genesis_epoch (*node, nano::epoch::epoch_1)); - ASSERT_NE (nullptr, system.upgrade_genesis_epoch (*node, nano::epoch::epoch_2)); - - // speed up the cementing process, otherwise the node waits for frontiers confirmation to notice the unconfirmed epoch blocks, which takes time - node->scheduler.priority.activate (node->ledger.tx_begin_read (), nano::dev::genesis_key.pub); - - // wait for the epoch blocks to be cemented - ASSERT_TIMELY_EQ (5s, node->ledger.confirmed.account_height (node->ledger.tx_begin_read (), nano::dev::genesis_key.pub), 3); + ASSERT_NE (nullptr, (head = system.upgrade_genesis_epoch (*node, nano::epoch::epoch_2))); + ASSERT_TIMELY (5s, node->block_confirmed (head->hash ())); auto target_difficulty = nano::dev::network_params.work.threshold (nano::work_version::work_1, nano::block_details (nano::epoch::epoch_2, false, false, false)); ASSERT_LT (node->network_params.work.entry, target_difficulty); From 6cbb97fac4cf60906ddf15badb9e503b798df23c Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 17:56:04 +0100 Subject: [PATCH 5/7] Replace manual testing loop with ASSERT_TIMELY. --- nano/core_test/active_elections.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index d58a91e968..dc4d419f00 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -1006,12 +1006,7 @@ TEST (active_elections, confirmation_consistency) { auto block (system.wallet (0)->send_action (nano::dev::genesis_key.pub, nano::public_key (), node.config.receive_minimum.number ())); ASSERT_NE (nullptr, block); - system.deadline_set (5s); - while (!node.ledger.confirmed.block_exists_or_pruned (node.ledger.tx_begin_read (), block->hash ())) - { - node.scheduler.priority.activate (node.ledger.tx_begin_read (), nano::dev::genesis_key.pub); - ASSERT_NO_ERROR (system.poll (5ms)); - } + ASSERT_TIMELY (5s, node.block_confirmed (block->hash ())); ASSERT_NO_ERROR (system.poll_until_true (1s, [&node, &block, i] { nano::lock_guard guard (node.active.mutex); EXPECT_EQ (i + 1, node.active.recently_confirmed.size ()); From 43938618e721fc3ab4015e62c1217074a2d6d9df Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 18:44:55 +0100 Subject: [PATCH 6/7] Rewrite inactive_vote_cache.existing_vote to directly process blocks and start elections. --- nano/core_test/active_elections.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index dc4d419f00..a1331add8d 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -346,11 +346,9 @@ TEST (inactive_votes_cache, existing_vote) .sign (key.prv, key.pub) .work (*system.work.generate (key.pub)) .build (); - node.process_active (send); - node.block_processor.add (open); - ASSERT_TIMELY_EQ (5s, node.active.size (), 1); - auto election (node.active.election (send->qualified_root ())); - ASSERT_NE (nullptr, election); + ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send)); + ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), open)); + auto election = nano::test::start_election (system, node, send->hash ()); ASSERT_GT (node.weight (key.pub), node.minimum_principal_weight ()); // Insert vote auto vote1 = nano::test::make_vote (key, { send }, nano::vote::timestamp_min * 1, 0); From 6d3c3f52cf64755bdb03406eb28d1f96d4c9af79 Mon Sep 17 00:00:00 2001 From: Colin LeMahieu Date: Mon, 13 May 2024 19:05:33 +0100 Subject: [PATCH 7/7] Rewrite confirmation_callback.confirrmed_history to directly start elections rather than using node::process_active. --- nano/core_test/confirming_set.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/nano/core_test/confirming_set.cpp b/nano/core_test/confirming_set.cpp index 0a699182d1..7b0df16c0c 100644 --- a/nano/core_test/confirming_set.cpp +++ b/nano/core_test/confirming_set.cpp @@ -133,10 +133,7 @@ TEST (confirmation_callback, confirmed_history) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*system.work.generate (latest)) .build (); - { - auto transaction = node->ledger.tx_begin_write (); - ASSERT_EQ (nano::block_status::progress, node->ledger.process (transaction, send)); - } + ASSERT_EQ (nano::block_status::progress, node->ledger.process (node->ledger.tx_begin_write (), send)); auto send1 = builder .send () @@ -146,8 +143,8 @@ TEST (confirmation_callback, confirmed_history) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*system.work.generate (send->hash ())) .build (); + ASSERT_EQ (nano::block_status::progress, node->ledger.process (node->ledger.tx_begin_write (), send1)); - node->process_active (send1); std::shared_ptr election; ASSERT_TIMELY (5s, election = nano::test::start_election (system, *node, send1->hash ())); {