Skip to content

Commit

Permalink
Investigate and fix some of the disabled unit tests (#3679)
Browse files Browse the repository at this point in the history 
Fixed unit tests:

- `active_transactions.confirm_active`
- `election.quorum_minimum_update_weight_before_quorum_checks`
- `active_transactions.inactive_votes_cache_fork`
- `confirmation_height.election_winner_details_clearing`
- `node.vote_by_hash_republish`
  • Loading branch information
@theohax
theohax committed Feb 2, 2022
1 parent 1214b2c commit c9112b5
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 257 deletions.
150 changes: 78 additions & 72 deletions nano/core_test/active_transactions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,62 +11,73 @@ using namespace std::chrono_literals;

namespace nano
{
// Test disabled because it's failing intermittently.
// PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3512
// Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3522
TEST (active_transactions, DISABLED_confirm_active)
// Tests that an election can be confirmed as the result of a confirmation request
//
// Set-up:
// - node1 with:
// - enabled frontiers_confirmation (default) -> allows it to confirm blocks and subsequently generates votes
// - node2 with:
// - disabled rep crawler -> this inhibits node2 from learning that node1 is a rep
//
// Steps:
// - create a block (send1), process it locally (without creating an election for it) on node1
// - process send1 (as incoming from network -- process_active) on node2
// - expect that election has been started for send1 on node2, but no confirmation_requests are sent for it
// - stick genesis key into node1, then add node1 as a rep to node2's probable reps list
// - expect at least one confirmation_request for the election (having been sent to node1 -- which is a rep now)
// - expect a (non-final) vote to come back
// - expected confirmation_request count has increased -- two round trips for the election to get confirmed
// - expect election is confirmed

TEST (active_transactions, confirm_election_by_request)
{
nano::system system;
nano::node_flags node_flags;
node_flags.disable_request_loop = true;
auto & node1 = *system.add_node (node_flags);
auto send = nano::send_block_builder ()
.previous (nano::dev::genesis->hash ())
.destination (nano::public_key ())
.balance (nano::dev::constants.genesis_amount - 100)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build_shared ();
ASSERT_EQ (nano::process_result::progress, node1.process (*send).code);
nano::node_config node_config2 (nano::get_available_port (), system.logging);
node_config2.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
nano::node_flags node_flags2;
// The rep crawler would otherwise request confirmations in order to find representatives
nano::system system{};
auto & node1 = *system.add_node ();

nano::node_flags node_flags2{};
node_flags2.disable_rep_crawler = true;
auto & node2 = *system.add_node (node_config2, node_flags2);
system.deadline_set (5s);
// Let node2 know about the block
while (node2.active.empty ())
{
node1.network.flood_block (send, nano::buffer_drop_policy::no_limiter_drop);
ASSERT_NO_ERROR (system.poll ());
}
// Save election to check request count afterwards
auto election = node2.active.election (send->qualified_root ());
ASSERT_NE (nullptr, election);
auto & node2 = *system.add_node (node_flags2);

auto send1 = nano::state_block_builder{}.make_block ().account (nano::dev::genesis_key.pub).representative (nano::dev::genesis_key.pub).previous (nano::dev::genesis->hash ()).link (nano::public_key ()).balance (nano::dev::constants.genesis_amount - 100).sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub).work (*system.work.generate (nano::dev::genesis->hash ())).build_shared ();

// Process send1 locally on node1
ASSERT_EQ (nano::process_result::progress, node1.process (*send1).code);

// Start an election for send1 on node2
node2.process_active (send1);
std::shared_ptr<nano::election> election{};
ASSERT_TIMELY (5s, (election = node2.active.election (send1->qualified_root ())) != nullptr);

// Expect that node2 has nobody to send a confirmation_request to (no reps)
ASSERT_EQ (0, election->confirmation_request_count);

// Add key to node1
system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
// Add representative to disabled rep crawler
auto peers (node2.network.random_set (1));

// Get random peer list (of size 1) from node2 -- so basically just node1
auto const peers = node2.network.random_set (1);
ASSERT_FALSE (peers.empty ());

// Add representative (node1) to disabled rep crawler of node2
{
nano::lock_guard<nano::mutex> guard (node2.rep_crawler.probable_reps_mutex);
node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, *peers.begin ());
node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, *peers.cbegin ());
}
ASSERT_TIMELY (5s, election->votes ().size () != 1); // Votes were inserted (except for the null account)
auto confirm_req_count (election->confirmation_request_count.load ());
// At least one confirmation request
ASSERT_GT (confirm_req_count, 0u);
ASSERT_FALSE (election->confirmed ()); // Cannot be confirmed without final vote
node1.confirmation_height_processor.add (send); // Confirm block for node1 for final vote generation
ASSERT_TIMELY (5s, node1.ledger.block_confirmed (node1.store.tx_begin_read (), send->hash ()));
node1.history.erase (send->root ()); // Have to erase existing non-final vote for final vote generation (at runtime it should be replaced with automatically generated final vote from election)
// Waiting for final confirmation
ASSERT_TIMELY (10s, node2.ledger.cache.cemented_count == 2 && node2.active.empty ());
// At least one more confirmation request
ASSERT_GT (election->confirmation_request_count, confirm_req_count);
// Blocks were cleared (except for the null account)
ASSERT_EQ (1, election->blocks ().size ());

// At least one confirmation request sent to the freshly inserted rep (node1)
std::size_t confirm_req_count{};
ASSERT_TIMELY (5s, (confirm_req_count = election->confirmation_request_count) > 0);

// Expect a (non-final) vote come back
ASSERT_TIMELY (5s, election->votes ().size () > 1);

// There need to be 2 round trips in order for the election to get confirmed
ASSERT_TIMELY (5s, election->confirmation_request_count > confirm_req_count);

// Expect election was confirmed
ASSERT_TIMELY (5s, election->confirmed ());
ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ()));
ASSERT_TIMELY (5s, node2.block_confirmed (send1->hash ()));
}
}

Expand Down Expand Up @@ -231,48 +242,43 @@ TEST (active_transactions, inactive_votes_cache_non_final)
ASSERT_EQ (nano::dev::constants.genesis_amount - 100, election->tally ().begin ()->first);
}

// Test disabled because it's failing intermittently.
// PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3604
// Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3605
TEST (active_transactions, DISABLED_inactive_votes_cache_fork)
TEST (active_transactions, inactive_votes_cache_fork)
{
nano::system system (1);
nano::system system{ 1 };
auto & node = *system.nodes[0];
nano::block_hash latest (node.latest (nano::dev::genesis_key.pub));
nano::keypair key;
nano::send_block_builder builder;

auto const latest = node.latest (nano::dev::genesis_key.pub);
nano::keypair key{};

nano::send_block_builder builder{};
auto send1 = builder.make_block ()
.previous (latest)
.destination (key.pub)
.balance (nano::dev::constants.genesis_amount - 100)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (latest))
.build_shared ();

auto send2 = builder.make_block ()
.previous (latest)
.destination (key.pub)
.balance (nano::dev::constants.genesis_amount - 200)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (latest))
.build_shared ();
auto vote (std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector<nano::block_hash> (1, send1->hash ())));

auto const vote = std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_max, nano::vote::duration_max, std::vector<nano::block_hash> (1, send1->hash ()));
node.vote_processor.vote (vote, std::make_shared<nano::transport::channel_loopback> (node));
auto channel1 (node.network.udp_channels.create (node.network.endpoint ()));
ASSERT_TIMELY (5s, node.active.inactive_votes_cache_size () == 1);
node.network.inbound (nano::publish{ nano::dev::network_params.network, send2 }, channel1);
node.block_processor.flush ();
ASSERT_NE (nullptr, node.block (send2->hash ()));
node.scheduler.flush (); // Start election, otherwise conflicting block won't be inserted into election
node.network.inbound (nano::publish{ nano::dev::network_params.network, send1 }, channel1);
node.block_processor.flush ();
bool confirmed (false);
system.deadline_set (5s);
while (!confirmed)
{
auto transaction (node.store.tx_begin_read ());
confirmed = node.block (send1->hash ()) != nullptr && node.ledger.block_confirmed (transaction, send1->hash ()) && node.active.empty ();
ASSERT_NO_ERROR (system.poll ());
}

node.process_active (send2);

std::shared_ptr<nano::election> election{};
ASSERT_TIMELY (5s, (election = node.active.election (send1->qualified_root ())) != nullptr);

node.process_active (send1);
ASSERT_TIMELY (5s, election->blocks ().size () == 2);
ASSERT_TIMELY (5s, node.block_confirmed (send1->hash ()));
ASSERT_EQ (1, node.stats.count (nano::stat::type::election, nano::stat::detail::vote_cached));
}

Expand Down
104 changes: 49 additions & 55 deletions nano/core_test/confirmation_height.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1265,72 +1265,66 @@ TEST (confirmation_height, cemented_gap_below_no_cache)
test_mode (nano::confirmation_height_mode::unbounded);
}

// Test disabled because it's failing intermittently.
// PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3607
// Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3608
TEST (confirmation_height, DISABLED_election_winner_details_clearing)
TEST (confirmation_height, election_winner_details_clearing)
{
auto test_mode = [] (nano::confirmation_height_mode mode_a) {
nano::system system;
nano::node_flags node_flags;
node_flags.confirmation_height_processor_mode = mode_a;
nano::node_config node_config (nano::get_available_port (), system.logging);
node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
auto node = system.add_node (node_config, node_flags);

nano::block_hash latest (node->latest (nano::dev::genesis_key.pub));

nano::keypair key1;
auto send = std::make_shared<nano::send_block> (latest, key1.pub, nano::dev::constants.genesis_amount - nano::Gxrb_ratio, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *system.work.generate (latest));
auto send1 = std::make_shared<nano::send_block> (send->hash (), key1.pub, nano::dev::constants.genesis_amount - nano::Gxrb_ratio * 2, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *system.work.generate (send->hash ()));
auto send2 = std::make_shared<nano::send_block> (send1->hash (), key1.pub, nano::dev::constants.genesis_amount - nano::Gxrb_ratio * 3, nano::dev::genesis_key.prv, nano::dev::genesis_key.pub, *system.work.generate (send1->hash ()));

{
auto transaction = node->store.tx_begin_write ();
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, *send).code);
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, *send1).code);
ASSERT_EQ (nano::process_result::progress, node->ledger.process (transaction, *send2).code);
}

add_callback_stats (*node);
nano::system system{};

node->block_confirm (send1);
auto election = node->active.election (send1->qualified_root ());
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) == 2);

ASSERT_EQ (0, node->active.election_winner_details_size ());
node->block_confirm (send);
election = node->active.election (send->qualified_root ());
ASSERT_NE (nullptr, election);
election->force_confirm ();
nano::node_flags node_flags{};
node_flags.confirmation_height_processor_mode = mode_a;

// Wait until this block is confirmed
ASSERT_TIMELY (10s, node->active.election_winner_details_size () == 1 || node->confirmation_height_processor.current ().is_zero ());
nano::node_config node_config{ nano::get_available_port (), system.logging };
node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;

auto node = system.add_node (node_config, node_flags);
auto const latest = node->latest (nano::dev::genesis_key.pub);

nano::keypair key1{};
nano::send_block_builder builder{};

auto const send1 = builder.make_block ()
.previous (latest)
.destination (key1.pub)
.balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (latest))
.build_shared ();
ASSERT_EQ (nano::process_result::progress, node->process (*send1).code);

auto const send2 = builder.make_block ()
.previous (send1->hash ())
.destination (key1.pub)
.balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio * 2)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (send1->hash ()))
.build_shared ();
ASSERT_EQ (nano::process_result::progress, node->process (*send2).code);

auto const send3 = builder.make_block ()
.previous (send2->hash ())
.destination (key1.pub)
.balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio * 3)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (send2->hash ()))
.build_shared ();
ASSERT_EQ (nano::process_result::progress, node->process (*send3).code);

node->process_confirmed (nano::election_status{ send2 });
ASSERT_TIMELY (5s, node->block_confirmed (send2->hash ()));
ASSERT_EQ (1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));

node->block_confirm (send2);
election = node->active.election (send2->qualified_root ());
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);
node->process_confirmed (nano::election_status{ send3 });
ASSERT_TIMELY (5s, node->block_confirmed (send3->hash ()));

// Add an already cemented block with fake election details. It should get removed
node->active.add_election_winner_details (send2->hash (), nullptr);
node->confirmation_height_processor.add (send2);

node->active.add_election_winner_details (send3->hash (), nullptr);
node->confirmation_height_processor.add (send3);
ASSERT_TIMELY (10s, node->active.election_winner_details_size () == 0);

ASSERT_EQ (1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
ASSERT_EQ (3, node->stats.count (nano::stat::type::http_callback, nano::stat::detail::http_callback, nano::stat::dir::out));
ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out));
ASSERT_EQ (3, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
ASSERT_EQ (3, node->stats.count (nano::stat::type::confirmation_height, get_stats_detail (mode_a), nano::stat::dir::in));
ASSERT_EQ (4, node->ledger.cache.cemented_count);

EXPECT_EQ (1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
EXPECT_EQ (3, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
EXPECT_EQ (3, node->stats.count (nano::stat::type::confirmation_height, get_stats_detail (mode_a), nano::stat::dir::in));
};

test_mode (nano::confirmation_height_mode::bounded);
Expand Down
Loading

0 comments on commit c9112b5

Please sign in to comment.