Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove election_winner_details container #4755

Merged
20 changes: 6 additions & 14 deletions nano/core_test/active_elections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1224,7 +1224,7 @@ TEST (active_elections, activate_inactive)
ASSERT_NE (nullptr, election);
election->force_confirm ();

ASSERT_TIMELY (5s, !node.confirming_set.exists (send2->hash ()));
ASSERT_TIMELY (5s, !node.confirming_set.contains (send2->hash ()));
ASSERT_TIMELY (5s, node.block_confirmed (send2->hash ()));
ASSERT_TIMELY (5s, node.block_confirmed (send->hash ()));

Expand Down Expand Up @@ -1398,6 +1398,8 @@ TEST (active_elections, bound_election_winners)
nano::node_config config = system.default_config ();
// Set election winner limit to a low value
config.active_elections.max_election_winners = 5;
// Large batch size would complicate this testcase
config.confirming_set.batch_size = 1;
auto & node = *system.add_node (config);

// Start elections for a couple of blocks, number of elections is larger than the election winner set limit
Expand All @@ -1411,22 +1413,12 @@ TEST (active_elections, bound_election_winners)
auto guard = node.ledger.tx_begin_write (nano::store::writer::testing);

// Ensure that when the number of election winners reaches the limit, AEC vacancy reflects that
// Confirming more elections should make the vacancy negative
ASSERT_TRUE (node.active.vacancy (nano::election_behavior::priority) > 0);

int index = 0;
for (; index < config.active_elections.max_election_winners; ++index)
{
auto election = node.vote_router.election (blocks[index]->hash ());
ASSERT_TRUE (election);
election->force_confirm ();
}

ASSERT_TIMELY_EQ (5s, node.active.vacancy (nano::election_behavior::priority), 0);

// Confirming more elections should make the vacancy negative
for (; index < blocks.size (); ++index)
for (auto const & block : blocks)
{
auto election = node.vote_router.election (blocks[index]->hash ());
auto election = node.vote_router.election (block->hash ());
ASSERT_TRUE (election);
election->force_confirm ();
}
Expand Down
42 changes: 7 additions & 35 deletions nano/core_test/confirming_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,24 @@ TEST (confirming_set, construction)
{
auto ctx = nano::test::ledger_empty ();
nano::confirming_set_config config{};
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats () };
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats (), ctx.logger () };
}

TEST (confirming_set, add_exists)
{
auto ctx = nano::test::ledger_send_receive ();
nano::confirming_set_config config{};
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats () };
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats (), ctx.logger () };
auto send = ctx.blocks ()[0];
confirming_set.add (send->hash ());
ASSERT_TRUE (confirming_set.exists (send->hash ()));
ASSERT_TRUE (confirming_set.contains (send->hash ()));
}

TEST (confirming_set, process_one)
{
auto ctx = nano::test::ledger_send_receive ();
nano::confirming_set_config config{};
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats () };
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats (), ctx.logger () };
std::atomic<int> count = 0;
std::mutex mutex;
std::condition_variable condition;
Expand All @@ -54,7 +54,7 @@ TEST (confirming_set, process_multiple)
{
auto ctx = nano::test::ledger_send_receive ();
nano::confirming_set_config config{};
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats () };
nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats (), ctx.logger () };
std::atomic<int> count = 0;
std::mutex mutex;
std::condition_variable condition;
Expand Down Expand Up @@ -111,7 +111,6 @@ TEST (confirmation_callback, observer_callbacks)

ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
ASSERT_EQ (3, node->ledger.cemented_count ());
ASSERT_EQ (0, node->active.election_winner_details_size ());
}

// The callback and confirmation history should only be updated after confirmation height is set (and not just after voting)
Expand Down Expand Up @@ -171,8 +170,7 @@ TEST (confirmation_callback, confirmed_history)

ASSERT_TIMELY (10s, !node->store.write_queue.contains (nano::store::writer::confirmation_height));

auto transaction = node->ledger.tx_begin_read ();
ASSERT_TRUE (node->ledger.confirmed.block_exists (transaction, send->hash ()));
ASSERT_TIMELY (5s, node->ledger.confirmed.block_exists (node->ledger.tx_begin_read (), send->hash ()));

ASSERT_TIMELY_EQ (10s, node->active.size (), 0);
ASSERT_TIMELY_EQ (10s, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out), 1);
Expand All @@ -186,7 +184,6 @@ TEST (confirmation_callback, confirmed_history)
ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
ASSERT_TIMELY_EQ (5s, 2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
ASSERT_EQ (3, node->ledger.cemented_count ());
ASSERT_EQ (0, node->active.election_winner_details_size ());
}

TEST (confirmation_callback, dependent_election)
Expand Down Expand Up @@ -242,35 +239,10 @@ TEST (confirmation_callback, dependent_election)
// Wait for blocks to be confirmed in ledger, callbacks will happen after
ASSERT_TIMELY_EQ (5s, 3, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
// Once the item added to the confirming set no longer exists, callbacks have completed
ASSERT_TIMELY (5s, !node->confirming_set.exists (send2->hash ()));
ASSERT_TIMELY (5s, !node->confirming_set.contains (send2->hash ()));

ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out));
ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_conf_height, nano::stat::dir::out));
ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
ASSERT_EQ (4, node->ledger.cemented_count ());

ASSERT_EQ (0, node->active.election_winner_details_size ());
}

TEST (confirmation_callback, election_winner_details_clearing_node_process_confirmed)
{
// Make sure election_winner_details is also cleared if the block never enters the confirmation height processor from node::process_confirmed
nano::test::system system (1);
auto node = system.nodes.front ();

nano::block_builder builder;
auto send = builder
.send ()
.previous (nano::dev::genesis->hash ())
.destination (nano::dev::genesis_key.pub)
.balance (nano::dev::constants.genesis_amount - nano::Knano_ratio)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
// Add to election_winner_details. Use an unrealistic iteration so that it should fall into the else case and do a cleanup
node->active.add_election_winner_details (send->hash (), nullptr);
nano::election_status election;
election.winner = send;
node->process_confirmed (election, 1000000);
ASSERT_EQ (0, node->active.election_winner_details_size ());
}
4 changes: 2 additions & 2 deletions nano/core_test/election_scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ TEST (election_scheduler, no_vacancy)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
ASSERT_EQ (nano::block_status::progress, node.process (send));
node.process_confirmed (nano::election_status{ send });
node.process_confirmed (send->hash ());

auto receive = builder.make_block ()
.account (key.pub)
Expand All @@ -207,7 +207,7 @@ TEST (election_scheduler, no_vacancy)
.work (*system.work.generate (key.pub))
.build ();
ASSERT_EQ (nano::block_status::progress, node.process (receive));
node.process_confirmed (nano::election_status{ receive });
node.process_confirmed (receive->hash ());

ASSERT_TIMELY (5s, nano::test::confirmed (node, { send, receive }));

Expand Down
23 changes: 0 additions & 23 deletions nano/core_test/ledger_confirm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -752,29 +752,6 @@ TEST (ledger_confirm, observers)
ASSERT_EQ (2, node1->ledger.cemented_count ());
}

TEST (ledger_confirm, election_winner_details_clearing_node_process_confirmed)
{
// Make sure election_winner_details is also cleared if the block never enters the confirmation height processor from node::process_confirmed
nano::test::system system (1);
auto node = system.nodes.front ();

nano::block_builder builder;
auto send = builder
.send ()
.previous (nano::dev::genesis->hash ())
.destination (nano::dev::genesis_key.pub)
.balance (nano::dev::constants.genesis_amount - nano::Knano_ratio)
.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
.work (*system.work.generate (nano::dev::genesis->hash ()))
.build ();
// Add to election_winner_details. Use an unrealistic iteration so that it should fall into the else case and do a cleanup
node->active.add_election_winner_details (send->hash (), nullptr);
nano::election_status election;
election.winner = send;
node->process_confirmed (election, 1000000);
ASSERT_EQ (0, node->active.election_winner_details_size ());
}

TEST (ledger_confirm, pruned_source)
{
nano::test::system system;
Expand Down
12 changes: 6 additions & 6 deletions nano/core_test/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3563,9 +3563,9 @@ TEST (node, pruning_automatic)
ASSERT_TIMELY (5s, node1.block (send2->hash ()) != nullptr);

// Force-confirm both blocks
node1.process_confirmed (nano::election_status{ send1 });
node1.process_confirmed (send1->hash ());
ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ()));
node1.process_confirmed (nano::election_status{ send2 });
node1.process_confirmed (send2->hash ());
ASSERT_TIMELY (5s, node1.block_confirmed (send2->hash ()));

// Check pruning result
Expand Down Expand Up @@ -3614,9 +3614,9 @@ TEST (node, DISABLED_pruning_age)
node1.process_active (send2);

// Force-confirm both blocks
node1.process_confirmed (nano::election_status{ send1 });
node1.process_confirmed (send1->hash ());
ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ()));
node1.process_confirmed (nano::election_status{ send2 });
node1.process_confirmed (send2->hash ());
ASSERT_TIMELY (5s, node1.block_confirmed (send2->hash ()));

// Three blocks in total, nothing pruned yet
Expand Down Expand Up @@ -3675,9 +3675,9 @@ TEST (node, DISABLED_pruning_depth)
node1.process_active (send2);

// Force-confirm both blocks
node1.process_confirmed (nano::election_status{ send1 });
node1.process_confirmed (send1->hash ());
ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ()));
node1.process_confirmed (nano::election_status{ send2 });
node1.process_confirmed (send2->hash ());
ASSERT_TIMELY (5s, node1.block_confirmed (send2->hash ()));

// Three blocks in total, nothing pruned yet
Expand Down
1 change: 1 addition & 0 deletions nano/lib/logging_enums.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ enum class type
message_processor,
local_block_broadcaster,
monitor,
confirming_set,

// bootstrap
bulk_pull_client,
Expand Down
7 changes: 7 additions & 0 deletions nano/lib/stats_enums.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ enum class type
message_processor,
message_processor_overfill,
message_processor_type,
process_confirmed,

_last // Must be the last enum
};
Expand Down Expand Up @@ -134,6 +135,8 @@ enum class detail
cemented,
cooldown,
empty,
done,
retry,

// processing queue
queue,
Expand Down Expand Up @@ -249,6 +252,8 @@ enum class detail
generate_vote_final,
broadcast_block_initial,
broadcast_block_repeat,
confirm_once,
confirm_once_failed,

// election types
manual,
Expand Down Expand Up @@ -407,6 +412,7 @@ enum class detail
// active_elections
started,
stopped,
confirm_dependent,

// unchecked
put,
Expand Down Expand Up @@ -499,6 +505,7 @@ enum class detail
already_cemented,
cementing,
cemented_hash,
cementing_failed,

// election_state
passive,
Expand Down
Loading
Loading