Skip to content

Commit

Permalink
Merge pull request #4755 from pwojcikdev/remove-election-winner-details
Browse files Browse the repository at this point in the history
Remove `election_winner_details` container
  • Loading branch information
pwojcikdev authored Oct 18, 2024
2 parents 55e9065 + 6883c4d commit 5c31172
Show file tree
Hide file tree
Showing 20 changed files with 227 additions and 254 deletions.
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

0 comments on commit 5c31172

Please sign in to comment.