Skip to content

Commit

Permalink
Introduce nano::test::start_election method (#4100)
Browse files Browse the repository at this point in the history
* Introduce the method systen::start_election()

In many tests, we currently use the method node::block_confirm(), which
uses block processor flush and is therefore broken. It is also terribly
named.

I introduce the method start_election with the aim of replacing
node::block_confirm in unit tests.

I converted a few unit tests to use this new function as a way to test the functionality.
The rest will be done progressively.
  • Loading branch information
dsiganos committed Feb 8, 2023
1 parent 1fcbe63 commit 352f66a
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 52 deletions.
12 changes: 4 additions & 8 deletions nano/core_test/active_transactions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,7 @@ TEST (active_transactions, dropped_cleanup)
ASSERT_FALSE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));
ASSERT_TRUE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));

node.block_confirm (nano::dev::genesis);
ASSERT_TIMELY (5s, node.active.election (nano::dev::genesis->qualified_root ()));
auto election = node.active.election (nano::dev::genesis->qualified_root ());
auto election = nano::test::start_election (system, node, nano::dev::genesis);
ASSERT_NE (nullptr, election);

// Not yet removed
Expand All @@ -654,9 +652,8 @@ TEST (active_transactions, dropped_cleanup)

// Repeat test for a confirmed election
ASSERT_TRUE (node.network.publish_filter.apply (block_bytes.data (), block_bytes.size ()));
node.block_confirm (nano::dev::genesis);
ASSERT_TIMELY (5s, node.active.election (nano::dev::genesis->qualified_root ()));
election = node.active.election (nano::dev::genesis->qualified_root ());

election = nano::test::start_election (system, node, nano::dev::genesis);
ASSERT_NE (nullptr, election);
election->force_confirm ();
ASSERT_TRUE (election->confirmed ());
Expand Down Expand Up @@ -1222,8 +1219,7 @@ TEST (active_transactions, activate_inactive)
ASSERT_EQ (nano::process_result::progress, node.process (*send2).code);
ASSERT_EQ (nano::process_result::progress, node.process (*open).code);

node.block_confirm (send2);
auto election = node.active.election (send2->qualified_root ());
auto election = nano::test::start_election (system, node, send2);
ASSERT_NE (nullptr, election);
election->force_confirm ();

Expand Down
40 changes: 15 additions & 25 deletions nano/core_test/confirmation_height.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,7 @@ TEST (confirmation_height, multiple_accounts)
.work (*system.work.generate (open3->hash ()))
.build_shared ();
node->process_active (receive3);
node->block_processor.flush ();
node->block_confirm (receive3);
auto election = node->active.election (receive3->qualified_root ());
auto election = nano::test::start_election (system, *node, receive3);
ASSERT_NE (nullptr, election);
election->force_confirm ();

Expand Down Expand Up @@ -535,7 +533,7 @@ TEST (confirmation_height, gap_live)
}

// Vote and confirm all existing blocks
node->block_confirm (send1);
nano::test::start_election (system, *node, send1);
ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 3);

// Now complete the chain where the block comes in on the live network
Expand Down Expand Up @@ -690,9 +688,7 @@ TEST (confirmation_height, send_receive_between_2_accounts)
add_callback_stats (*node);

node->process_active (receive4);
node->block_processor.flush ();
node->block_confirm (receive4);
auto election = node->active.election (receive4->qualified_root ());
auto election = nano::test::start_election (system, *node, receive4);
ASSERT_NE (nullptr, election);
election->force_confirm ();

Expand Down Expand Up @@ -809,12 +805,11 @@ TEST (confirmation_height, send_receive_self)

add_callback_stats (*node);

node->block_confirm (receive3);
auto election = node->active.election (receive3->qualified_root ());
auto election = nano::test::start_election (system, *node, receive3);
ASSERT_NE (nullptr, election);
election->force_confirm ();

ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 6);
ASSERT_TIMELY (5s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 6);

auto transaction (node->store.tx_begin_read ());
ASSERT_TRUE (node->ledger.block_confirmed (transaction, receive3->hash ()));
Expand Down Expand Up @@ -1055,12 +1050,11 @@ TEST (confirmation_height, all_block_types)
}

add_callback_stats (*node);
node->block_confirm (state_send2);
auto election = node->active.election (state_send2->qualified_root ());
auto election = nano::test::start_election (system, *node, state_send2);
ASSERT_NE (nullptr, election);
election->force_confirm ();

ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 15);
ASSERT_TIMELY (5s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 15);

auto transaction (node->store.tx_begin_read ());
ASSERT_TRUE (node->ledger.block_confirmed (transaction, state_send2->hash ()));
Expand Down Expand Up @@ -1552,8 +1546,7 @@ TEST (confirmation_height, callback_confirmed_history)
add_callback_stats (*node);

node->process_active (send1);
node->block_processor.flush ();
node->block_confirm (send1);
ASSERT_NE (nano::test::start_election (system, *node, send1), nullptr);
{
node->process_active (send);
node->block_processor.flush ();
Expand Down Expand Up @@ -1654,14 +1647,13 @@ TEST (confirmation_height, dependent_election)
add_callback_stats (*node);

// This election should be confirmed as active_conf_height
node->block_confirm (send1);
ASSERT_TRUE (nano::test::start_election (system, *node, send1));
// Start an election and confirm it
node->block_confirm (send2);
auto election = node->active.election (send2->qualified_root ());
auto election = nano::test::start_election (system, *node, send2);
ASSERT_NE (nullptr, election);
election->force_confirm ();

ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 3);
ASSERT_TIMELY (5s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 3);

ASSERT_EQ (1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out));
ASSERT_EQ (1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_conf_height, nano::stat::dir::out));
Expand Down Expand Up @@ -1811,11 +1803,10 @@ TEST (confirmation_height, cemented_gap_below_receive)
nano::mutex mutex;
add_callback_stats (*node, &observer_order, &mutex);

node->block_confirm (open1);
auto election = node->active.election (open1->qualified_root ());
auto election = nano::test::start_election (system, *node, open1);
ASSERT_NE (nullptr, election);
election->force_confirm ();
ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 10);
ASSERT_TIMELY (5s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 10);

auto transaction = node->store.tx_begin_read ();
ASSERT_TRUE (node->ledger.block_confirmed (transaction, open1->hash ()));
Expand Down Expand Up @@ -1977,11 +1968,10 @@ TEST (confirmation_height, cemented_gap_below_no_cache)

add_callback_stats (*node);

node->block_confirm (open1);
auto election = node->active.election (open1->qualified_root ());
auto election = nano::test::start_election (system, *node, open1);
ASSERT_NE (nullptr, election);
election->force_confirm ();
ASSERT_TIMELY (10s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 6);
ASSERT_TIMELY (5s, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out) == 6);

auto transaction = node->store.tx_begin_read ();
ASSERT_TRUE (node->ledger.block_confirmed (transaction, open1->hash ()));
Expand Down
7 changes: 3 additions & 4 deletions nano/core_test/gap_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,17 @@ TEST (gap_cache, gap_bootstrap)
ASSERT_EQ (nano::dev::constants.genesis_amount - 100, node1.balance (nano::dev::genesis->account ()));
ASSERT_EQ (nano::dev::constants.genesis_amount, node2.balance (nano::dev::genesis->account ()));
// Confirm send block, allowing voting on the upcoming block
node1.block_confirm (send);
auto election = node1.active.election (send->qualified_root ());
auto election = nano::test::start_election (system, node1, send);
ASSERT_NE (nullptr, election);
election->force_confirm ();
ASSERT_TIMELY (2s, node1.block_confirmed (send->hash ()));
ASSERT_TIMELY (5s, node1.block_confirmed (send->hash ()));
node1.active.erase (*send);
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
auto latest_block (system.wallet (0)->send_action (nano::dev::genesis_key.pub, key.pub, 100));
ASSERT_NE (nullptr, latest_block);
ASSERT_EQ (nano::dev::constants.genesis_amount - 200, node1.balance (nano::dev::genesis->account ()));
ASSERT_EQ (nano::dev::constants.genesis_amount, node2.balance (nano::dev::genesis->account ()));
ASSERT_TIMELY (10s, node2.balance (nano::dev::genesis->account ()) == nano::dev::constants.genesis_amount - 200);
ASSERT_TIMELY (5s, node2.balance (nano::dev::genesis->account ()) == nano::dev::constants.genesis_amount - 200);
}

TEST (gap_cache, two_dependencies)
Expand Down
12 changes: 5 additions & 7 deletions nano/core_test/vote_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,14 @@ TEST (vote_processor, invalid_signature)
vote_invalid->signature.bytes[0] ^= 1;
auto channel = std::make_shared<nano::transport::inproc::channel> (node, node);

node.block_confirm (nano::dev::genesis);
auto election = node.active.election (nano::dev::genesis->qualified_root ());
ASSERT_TRUE (election);
auto election = nano::test::start_election (system, node, nano::dev::genesis);
ASSERT_NE (election, nullptr);
ASSERT_EQ (1, election->votes ().size ());

node.vote_processor.vote (vote_invalid, channel);
node.vote_processor.flush ();
ASSERT_TIMELY (3s, 1 == election->votes ().size ());
ASSERT_TIMELY (5s, 1 == election->votes ().size ());
node.vote_processor.vote (vote, channel);
node.vote_processor.flush ();
ASSERT_TIMELY (3s, 2 == election->votes ().size ());
ASSERT_TIMELY (5s, 2 == election->votes ().size ());
}

TEST (vote_processor, no_capacity)
Expand Down
12 changes: 4 additions & 8 deletions nano/rpc_test/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5874,9 +5874,9 @@ TEST (rpc, confirmation_height_currently_processing)
{
// Write guard prevents the confirmation height processor writing the blocks, so that we can inspect contents during the response
auto write_guard = node->write_database_queue.wait (nano::writer::testing);
node->block_confirm (frontier);
nano::test::start_election (system, *node, frontier);

ASSERT_TIMELY (10s, node->confirmation_height_processor.current () == frontier->hash ());
ASSERT_TIMELY (5s, node->confirmation_height_processor.current () == frontier->hash ());

// Make the request
{
Expand Down Expand Up @@ -6516,14 +6516,10 @@ TEST (rpc, block_confirmed)
.work (*system.work.generate (latest))
.build_shared ();
node->process_active (send);
node->block_processor.flush ();
node->block_confirm (send);
auto election = node->active.election (send->qualified_root ());
ASSERT_NE (nullptr, election);
election->force_confirm ();
ASSERT_TIMELY (5s, nano::test::confirm (*node, { send }));

// Wait until the confirmation height has been set
ASSERT_TIMELY (10s, node->ledger.block_confirmed (node->store.tx_begin_read (), send->hash ()) && !node->confirmation_height_processor.is_processing_block (send->hash ()));
ASSERT_TIMELY (5s, node->ledger.block_confirmed (node->store.tx_begin_read (), send->hash ()) && !node->confirmation_height_processor.is_processing_block (send->hash ()));

// Requesting confirmation for this should now succeed
request.put ("hash", send->hash ().to_string ());
Expand Down
30 changes: 30 additions & 0 deletions nano/test_common/testutil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,34 @@ std::shared_ptr<nano::transport::channel> nano::test::fake_channel (nano::node &
channel->set_node_id (node_id);
}
return channel;
}

std::shared_ptr<nano::election> nano::test::start_election (nano::test::system & system_a, nano::node & node_a, const std::shared_ptr<nano::block> & block_a)
{
system_a.deadline_set (5s);

// wait until and ensure that the block is in the ledger
while (!node_a.block (block_a->hash ()))
{
if (system_a.poll ())
{
return nullptr;
}
}

node_a.scheduler.manual (block_a);

// wait for the election to appear
std::shared_ptr<nano::election> election = node_a.active.election (block_a->qualified_root ());
while (!election)
{
if (system_a.poll ())
{
return nullptr;
}
election = node_a.active.election (block_a->qualified_root ());
}

election->transition_active ();
return election;
}
7 changes: 7 additions & 0 deletions nano/test_common/testutil.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ class telemetry_data;
class network_params;
class vote;
class block;
class election;

extern nano::uint128_t const & genesis_amount;

Expand Down Expand Up @@ -406,5 +407,11 @@ namespace test
* Creates a new fake channel associated with `node`
*/
std::shared_ptr<nano::transport::channel> fake_channel (nano::node & node, nano::account node_id = { 0 });
/*
* Start an election on system system_a, node node_a and block block_a by adding the block to the manual election scheduler queue.
* It waits up to 5 seconds for the election to start and calls the system poll function while waiting.
* Returns nullptr if the election did not start within the timeframe.
*/
std::shared_ptr<nano::election> start_election (nano::test::system & system_a, nano::node & node_a, const std::shared_ptr<nano::block> & block_a);
}
}

0 comments on commit 352f66a

Please sign in to comment.