diff --git a/nano/core_test/active_elections.cpp b/nano/core_test/active_elections.cpp index 6c13d4a02c..9bd1d6c204 100644 --- a/nano/core_test/active_elections.cpp +++ b/nano/core_test/active_elections.cpp @@ -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 ())); @@ -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 @@ -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 (); } diff --git a/nano/core_test/confirming_set.cpp b/nano/core_test/confirming_set.cpp index 7d25017874..596f65444c 100644 --- a/nano/core_test/confirming_set.cpp +++ b/nano/core_test/confirming_set.cpp @@ -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 count = 0; std::mutex mutex; std::condition_variable condition; @@ -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 count = 0; std::mutex mutex; std::condition_variable condition; @@ -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) @@ -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); @@ -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) @@ -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 ()); } diff --git a/nano/core_test/election_scheduler.cpp b/nano/core_test/election_scheduler.cpp index 56c1a5c7b8..14f840d4fa 100644 --- a/nano/core_test/election_scheduler.cpp +++ b/nano/core_test/election_scheduler.cpp @@ -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) @@ -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 })); diff --git a/nano/core_test/ledger_confirm.cpp b/nano/core_test/ledger_confirm.cpp index 1301c9ff95..f9bfeff998 100644 --- a/nano/core_test/ledger_confirm.cpp +++ b/nano/core_test/ledger_confirm.cpp @@ -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; diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index 86cb7d2581..13fe91ae8a 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -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 @@ -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 @@ -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 diff --git a/nano/lib/logging_enums.hpp b/nano/lib/logging_enums.hpp index 922c62baa1..85136b98b7 100644 --- a/nano/lib/logging_enums.hpp +++ b/nano/lib/logging_enums.hpp @@ -82,6 +82,7 @@ enum class type message_processor, local_block_broadcaster, monitor, + confirming_set, // bootstrap bulk_pull_client, diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index ee9d8799dc..1e3889d180 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -93,6 +93,7 @@ enum class type message_processor, message_processor_overfill, message_processor_type, + process_confirmed, _last // Must be the last enum }; @@ -134,6 +135,8 @@ enum class detail cemented, cooldown, empty, + done, + retry, // processing queue queue, @@ -249,6 +252,8 @@ enum class detail generate_vote_final, broadcast_block_initial, broadcast_block_repeat, + confirm_once, + confirm_once_failed, // election types manual, @@ -407,6 +412,7 @@ enum class detail // active_elections started, stopped, + confirm_dependent, // unchecked put, @@ -499,6 +505,7 @@ enum class detail already_cemented, cementing, cemented_hash, + cementing_failed, // election_state passive, diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 134c649dfa..0cc4953f0e 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -25,36 +25,24 @@ nano::active_elections::active_elections (nano::node & node_a, nano::confirming_ confirming_set{ confirming_set_a }, block_processor{ block_processor_a }, recently_confirmed{ config.confirmation_cache }, - recently_cemented{ config.confirmation_history_size }, - election_time_to_live{ node_a.network_params.network.is_dev_network () ? 0s : 2s } + recently_cemented{ config.confirmation_history_size } { count_by_behavior.fill (0); // Zero initialize array - confirming_set.batch_cemented.add ([this] (nano::confirming_set::cemented_notification const & notification) { + confirming_set.batch_cemented.add ([this] (auto const & cemented) { + auto transaction = node.ledger.tx_begin_read (); + for (auto const & [block, confirmation_root, source_election] : cemented) { - auto transaction = node.ledger.tx_begin_read (); - for (auto const & [block, confirmation_root] : notification.cemented) - { - transaction.refresh_if_needed (); - - block_cemented_callback (transaction, block, confirmation_root); - } - } - for (auto const & hash : notification.already_cemented) - { - block_already_cemented_callback (hash); + transaction.refresh_if_needed (); + block_cemented (transaction, block, confirmation_root, source_election); } }); // Notify elections about alternative (forked) blocks block_processor.block_processed.add ([this] (auto const & result, auto const & context) { - switch (result) + if (result == nano::block_status::fork) { - case nano::block_status::fork: - publish (context.block); - break; - default: - break; + publish (context.block); } }); } @@ -91,28 +79,31 @@ void nano::active_elections::stop () clear (); } -void nano::active_elections::block_cemented_callback (nano::secure::transaction const & transaction, std::shared_ptr const & block, nano::block_hash const & confirmation_root) +void nano::active_elections::block_cemented (nano::secure::transaction const & transaction, std::shared_ptr const & block, nano::block_hash const & confirmation_root, std::shared_ptr const & source_election) { debug_assert (node.block_confirmed (block->hash ())); - if (auto election_l = election (block->qualified_root ())) + // Dependent elections are implicitly confirmed when their block is cemented + auto dependend_election = election (block->qualified_root ()); + if (dependend_election) { - election_l->try_confirm (block->hash ()); + node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::confirm_dependent); + dependend_election->try_confirm (block->hash ()); } - auto election = remove_election_winner_details (block->hash ()); + nano::election_status status; std::vector votes; status.winner = block; - if (election) - { - status = election->get_status (); - votes = election->votes_with_weight (); - } - if (block->hash () == confirmation_root) + + // Check if the currently cemented block was part of an election that triggered the confirmation + if (source_election && source_election->qualified_root == block->qualified_root ()) { + status = source_election->get_status (); + debug_assert (status.winner->hash () == block->hash ()); + votes = source_election->votes_with_weight (); status.type = nano::election_status_type::active_confirmed_quorum; } - else if (election) + else if (dependend_election) { status.type = nano::election_status_type::active_confirmation_height; } @@ -120,12 +111,16 @@ void nano::active_elections::block_cemented_callback (nano::secure::transaction { status.type = nano::election_status_type::inactive_confirmation_height; } + recently_cemented.put (status); node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::cemented); node.stats.inc (nano::stat::type::active_elections_cemented, to_stat_detail (status.type)); - node.logger.trace (nano::log::type::active_elections, nano::log::detail::active_cemented, nano::log::arg{ "election", election }); + node.logger.trace (nano::log::type::active_elections, nano::log::detail::active_cemented, + nano::log::arg{ "block", block }, + nano::log::arg{ "confirmation_root", confirmation_root }, + nano::log::arg{ "source_election", source_election }); notify_observers (transaction, status, votes); @@ -185,39 +180,6 @@ void nano::active_elections::activate_successors (nano::secure::transaction cons } } -void nano::active_elections::add_election_winner_details (nano::block_hash const & hash_a, std::shared_ptr const & election_a) -{ - nano::lock_guard guard{ election_winner_details_mutex }; - election_winner_details.emplace (hash_a, election_a); -} - -std::shared_ptr nano::active_elections::remove_election_winner_details (nano::block_hash const & hash_a) -{ - std::shared_ptr result; - { - nano::lock_guard guard{ election_winner_details_mutex }; - auto existing = election_winner_details.find (hash_a); - if (existing != election_winner_details.end ()) - { - result = existing->second; - election_winner_details.erase (existing); - } - } - - vacancy_update (); - - return result; -} - -void nano::active_elections::block_already_cemented_callback (nano::block_hash const & hash_a) -{ - // Depending on timing there is a situation where the election_winner_details is not reset. - // This can happen when a block wins an election, and the block is confirmed + observer - // called before the block hash gets added to election_winner_details. If the block is confirmed - // callbacks have already been done, so we can safely just remove it. - remove_election_winner_details (hash_a); -} - int64_t nano::active_elections::limit (nano::election_behavior behavior) const { switch (behavior) @@ -265,7 +227,7 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const }; auto election_winners_vacancy = [this] () -> int64_t { - return static_cast (config.max_election_winners) - static_cast (election_winner_details_size ()); + return static_cast (config.max_election_winners) - static_cast (confirming_set.size ()); }; return std::min (election_vacancy (behavior), election_winners_vacancy ()); @@ -572,12 +534,6 @@ bool nano::active_elections::publish (std::shared_ptr const & block return result; } -std::size_t nano::active_elections::election_winner_details_size () const -{ - nano::lock_guard guard{ election_winner_details_mutex }; - return election_winner_details.size (); -} - void nano::active_elections::clear () { // TODO: Call erased_callback for each election @@ -595,7 +551,6 @@ nano::container_info nano::active_elections::container_info () const nano::container_info info; info.put ("roots", roots.size ()); - info.put ("election_winner_details", election_winner_details_size ()); info.put ("normal", static_cast (count_by_behavior[nano::election_behavior::priority])); info.put ("hinted", static_cast (count_by_behavior[nano::election_behavior::hinted])); info.put ("optimistic", static_cast (count_by_behavior[nano::election_behavior::optimistic])); diff --git a/nano/node/active_elections.hpp b/nano/node/active_elections.hpp index 222d5d278d..1a76cf0448 100644 --- a/nano/node/active_elections.hpp +++ b/nano/node/active_elections.hpp @@ -123,10 +123,6 @@ class active_elections final int64_t vacancy (nano::election_behavior behavior) const; std::function vacancy_update{ [] () {} }; - std::size_t election_winner_details_size () const; - void add_election_winner_details (nano::block_hash const &, std::shared_ptr const &); - std::shared_ptr remove_election_winner_details (nano::block_hash const &); - nano::container_info container_info () const; private: @@ -139,8 +135,7 @@ class active_elections final std::vector> list_active_impl (std::size_t) const; void activate_successors (nano::secure::transaction const &, std::shared_ptr const & block); void notify_observers (nano::secure::transaction const &, nano::election_status const & status, std::vector const & votes) const; - void block_cemented_callback (nano::secure::transaction const &, std::shared_ptr const & block, nano::block_hash const & confirmation_root); - void block_already_cemented_callback (nano::block_hash const & hash); + void block_cemented (nano::secure::transaction const &, std::shared_ptr const & block, nano::block_hash const & confirmation_root, std::shared_ptr const & source_election); private: // Dependencies active_elections_config const & config; @@ -157,12 +152,6 @@ class active_elections final mutable nano::mutex mutex{ mutex_identifier (mutexes::active) }; private: - mutable nano::mutex election_winner_details_mutex{ mutex_identifier (mutexes::election_winner_details) }; - std::unordered_map> election_winner_details; - - // Maximum time an election can be kept active if it is extending the container - std::chrono::seconds const election_time_to_live; - /** Keeps track of number of elections by election behavior (normal, hinted, optimistic) */ nano::enum_array count_by_behavior{}; diff --git a/nano/node/confirming_set.cpp b/nano/node/confirming_set.cpp index c499e35057..4d8d145084 100644 --- a/nano/node/confirming_set.cpp +++ b/nano/node/confirming_set.cpp @@ -1,20 +1,23 @@ +#include #include #include #include +#include #include #include #include -nano::confirming_set::confirming_set (confirming_set_config const & config_a, nano::ledger & ledger_a, nano::stats & stats_a) : +nano::confirming_set::confirming_set (confirming_set_config const & config_a, nano::ledger & ledger_a, nano::stats & stats_a, nano::logger & logger_a) : config{ config_a }, ledger{ ledger_a }, stats{ stats_a }, + logger{ logger_a }, notification_workers{ 1, nano::thread_role::name::confirmation_height_notifications } { - batch_cemented.add ([this] (auto const & notification) { - for (auto const & [block, confirmation_root] : notification.cemented) + batch_cemented.add ([this] (auto const & cemented) { + for (auto const & context : cemented) { - cemented_observers.notify (block); + cemented_observers.notify (context.block); } }); } @@ -24,12 +27,12 @@ nano::confirming_set::~confirming_set () debug_assert (!thread.joinable ()); } -void nano::confirming_set::add (nano::block_hash const & hash) +void nano::confirming_set::add (nano::block_hash const & hash, std::shared_ptr const & election) { bool added = false; { std::lock_guard lock{ mutex }; - auto [it, inserted] = set.insert (hash); + auto [it, inserted] = set.push_back ({ hash, election }); added = inserted; } if (added) @@ -47,6 +50,11 @@ void nano::confirming_set::start () { debug_assert (!thread.joinable ()); + if (!config.enable) + { + return; + } + thread = std::thread{ [this] () { nano::thread_role::set (nano::thread_role::name::confirmation_height); run (); @@ -67,10 +75,10 @@ void nano::confirming_set::stop () notification_workers.stop (); } -bool nano::confirming_set::exists (nano::block_hash const & hash) const +bool nano::confirming_set::contains (nano::block_hash const & hash) const { std::lock_guard lock{ mutex }; - return set.count (hash) != 0; + return set.get ().contains (hash) || current.contains (hash); } std::size_t nano::confirming_set::size () const @@ -99,17 +107,16 @@ void nano::confirming_set::run () } } -std::deque nano::confirming_set::next_batch (size_t max_count) +auto nano::confirming_set::next_batch (size_t max_count) -> std::deque { debug_assert (!mutex.try_lock ()); debug_assert (!set.empty ()); - std::deque results; + std::deque results; while (!set.empty () && results.size () < max_count) { - auto it = set.begin (); - results.push_back (*it); - set.erase (it); + results.push_back (set.front ()); + set.pop_front (); } return results; } @@ -120,20 +127,27 @@ void nano::confirming_set::run_batch (std::unique_lock & lock) debug_assert (!mutex.try_lock ()); debug_assert (!set.empty ()); - std::deque cemented; + std::deque cemented; std::deque already; - auto batch = next_batch (256); + auto batch = next_batch (config.batch_size); + + // Keep track of the blocks we're currently cementing, so that the .contains (...) check is accurate + debug_assert (current.empty ()); + for (auto const & [hash, election] : batch) + { + current.insert (hash); + } lock.unlock (); - auto notify = [this, &cemented, &already] () { - cemented_notification notification{}; - notification.cemented.swap (cemented); - notification.already_cemented.swap (already); + auto notify = [this, &cemented] () { + std::deque batch; + batch.swap (cemented); std::unique_lock lock{ mutex }; + // It's possible that ledger cementing happens faster than the notifications can be processed by other components, cooldown here while (notification_workers.num_queued_tasks () >= config.max_queued_notifications) { stats.inc (nano::stat::type::confirming_set, nano::stat::detail::cooldown); @@ -144,9 +158,9 @@ void nano::confirming_set::run_batch (std::unique_lock & lock) } } - notification_workers.push_task ([this, notification = std::move (notification)] () { + notification_workers.push_task ([this, batch = std::move (batch)] () { stats.inc (nano::stat::type::confirming_set, nano::stat::detail::notify); - batch_cemented.notify (notification); + batch_cemented.notify (batch); }); }; @@ -163,8 +177,10 @@ void nano::confirming_set::run_batch (std::unique_lock & lock) { auto transaction = ledger.tx_begin_write (nano::store::writer::confirmation_height); - for (auto const & hash : batch) + for (auto const & [hash, election] : batch) { + size_t cemented_count = 0; + bool success = false; do { transaction.refresh_if_needed (); @@ -180,6 +196,13 @@ void nano::confirming_set::run_batch (std::unique_lock & lock) stats.inc (nano::stat::type::confirming_set, nano::stat::detail::cementing); + // The block might be rolled back before it's fully cemented + if (!ledger.any.block_exists (transaction, hash)) + { + stats.inc (nano::stat::type::confirming_set, nano::stat::detail::missing_block); + break; + } + auto added = ledger.confirm (transaction, hash, config.max_blocks); if (!added.empty ()) { @@ -187,8 +210,9 @@ void nano::confirming_set::run_batch (std::unique_lock & lock) stats.add (nano::stat::type::confirming_set, nano::stat::detail::cemented, added.size ()); for (auto & block : added) { - cemented.emplace_back (block, hash); + cemented.push_back ({ block, hash, election }); } + cemented_count += added.size (); } else { @@ -196,16 +220,31 @@ void nano::confirming_set::run_batch (std::unique_lock & lock) already.push_back (hash); debug_assert (ledger.confirmed.block_exists (transaction, hash)); } - } while (!ledger.confirmed.block_exists (transaction, hash)); - stats.inc (nano::stat::type::confirming_set, nano::stat::detail::cemented_hash); + success = ledger.confirmed.block_exists (transaction, hash); + } while (!success); + + if (success) + { + stats.inc (nano::stat::type::confirming_set, nano::stat::detail::cemented_hash); + logger.debug (nano::log::type::confirming_set, "Cemented block: {} (total cemented: {})", hash.to_string (), cemented_count); + } + else + { + stats.inc (nano::stat::type::confirming_set, nano::stat::detail::cementing_failed); + logger.debug (nano::log::type::confirming_set, "Failed to cement block: {}", hash.to_string ()); + } } } notify (); - release_assert (cemented.empty ()); - release_assert (already.empty ()); + + already_cemented.notify (already); + + lock.lock (); + current.clear (); + lock.unlock (); } nano::container_info nano::confirming_set::container_info () const diff --git a/nano/node/confirming_set.hpp b/nano/node/confirming_set.hpp index e5ef4857f1..99569ce1e6 100644 --- a/nano/node/confirming_set.hpp +++ b/nano/node/confirming_set.hpp @@ -4,6 +4,13 @@ #include #include #include +#include + +#include +#include +#include +#include +#include #include #include @@ -11,6 +18,8 @@ #include #include +namespace mi = boost::multi_index; + namespace nano { class confirming_set_config final @@ -19,6 +28,9 @@ class confirming_set_config final // TODO: Serialization & deserialization public: + bool enable{ true }; + size_t batch_size{ 256 }; + /** Maximum number of dependent blocks to be stored in memory during processing */ size_t max_blocks{ 128 * 1024 }; size_t max_queued_notifications{ 8 }; @@ -33,45 +45,65 @@ class confirming_set final friend class confirmation_height_pruned_source_Test; public: - confirming_set (confirming_set_config const &, nano::ledger &, nano::stats &); + confirming_set (confirming_set_config const &, nano::ledger &, nano::stats &, nano::logger &); ~confirming_set (); void start (); void stop (); // Adds a block to the set of blocks to be confirmed - void add (nano::block_hash const & hash); + void add (nano::block_hash const & hash, std::shared_ptr const & election = nullptr); // Added blocks will remain in this set until after ledger has them marked as confirmed. - bool exists (nano::block_hash const & hash) const; + bool contains (nano::block_hash const & hash) const; std::size_t size () const; nano::container_info container_info () const; public: // Events - // Observers will be called once ledger has blocks marked as confirmed - using cemented_t = std::pair, nano::block_hash>; // - - struct cemented_notification + struct context { - std::deque cemented; - std::deque already_cemented; + std::shared_ptr block; + nano::block_hash confirmation_root; + std::shared_ptr election; }; - nano::observer_set batch_cemented; + nano::observer_set const &> batch_cemented; + nano::observer_set const &> already_cemented; + nano::observer_set> cemented_observers; private: // Dependencies confirming_set_config const & config; nano::ledger & ledger; nano::stats & stats; + nano::logger & logger; private: + struct entry + { + nano::block_hash hash; + std::shared_ptr election; + }; + void run (); void run_batch (std::unique_lock &); - std::deque next_batch (size_t max_count); + std::deque next_batch (size_t max_count); private: - std::unordered_set set; + // clang-format off + class tag_hash {}; + class tag_sequenced {}; + + using ordered_entries = boost::multi_index_container>, + mi::hashed_unique, + mi::member> + >>; + // clang-format on + + ordered_entries set; + std::unordered_set current; nano::thread_pool notification_workers; diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 98fa9ba491..93b9e8f7dc 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -35,18 +35,16 @@ nano::election::election (nano::node & node_a, std::shared_ptr cons last_blocks.emplace (block_a->hash (), block_a); } -void nano::election::confirm_once (nano::unique_lock & lock_a) +void nano::election::confirm_once (nano::unique_lock & lock) { - debug_assert (lock_a.owns_lock ()); + debug_assert (lock.owns_lock ()); + debug_assert (!mutex.try_lock ()); - // This must be kept above the setting of election state, as dependent confirmed elections require up to date changes to election_winner_details - nano::unique_lock election_winners_lk{ node.active.election_winner_details_mutex }; - auto just_confirmed = state_m != nano::election_state::confirmed; + bool just_confirmed = state_m != nano::election_state::confirmed; state_m = nano::election_state::confirmed; - if (just_confirmed && (node.active.election_winner_details.count (status.winner->hash ()) == 0)) + + if (just_confirmed) { - node.active.election_winner_details.emplace (status.winner->hash (), shared_from_this ()); - election_winners_lk.unlock (); status.election_end = std::chrono::duration_cast (std::chrono::system_clock::now ().time_since_epoch ()); status.election_duration = std::chrono::duration_cast (std::chrono::steady_clock::now () - election_start); status.confirmation_request_count = confirmation_request_count; @@ -56,15 +54,18 @@ void nano::election::confirm_once (nano::unique_lock & lock_a) node.active.recently_confirmed.put (qualified_root, status_l.winner->hash ()); + node.stats.inc (nano::stat::type::election, nano::stat::detail::confirm_once); node.logger.trace (nano::log::type::election, nano::log::detail::election_confirmed, nano::log::arg{ "id", id }, nano::log::arg{ "qualified_root", qualified_root }, nano::log::arg{ "status", current_status_locked () }); - lock_a.unlock (); + lock.unlock (); - node.election_workers.push_task ([node_l = node.shared (), status_l, confirmation_action_l = confirmation_action] () { - node_l->process_confirmed (status_l); + node.election_workers.push_task ([this_l = shared_from_this (), status_l, confirmation_action_l = confirmation_action] () { + // This is necessary if the winner of the election is one of the forks. + // In that case the winning block is not yet in the ledger and cementing needs to wait for rollbacks to complete. + this_l->node.process_confirmed (status_l.winner->hash (), this_l); if (confirmation_action_l) { @@ -74,7 +75,8 @@ void nano::election::confirm_once (nano::unique_lock & lock_a) } else { - lock_a.unlock (); + node.stats.inc (nano::stat::type::election, nano::stat::detail::confirm_once_failed); + lock.unlock (); } } @@ -414,6 +416,7 @@ void nano::election::confirm_if_quorum (nano::unique_lock & lock_a) if (final_weight >= node.online_reps.delta ()) { confirm_once (lock_a); + debug_assert (!lock_a.owns_lock ()); } } } @@ -427,6 +430,7 @@ void nano::election::try_confirm (nano::block_hash const & hash) if (!confirmed_locked ()) { confirm_once (election_lock); + debug_assert (!election_lock.owns_lock ()); } } } diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 198bdeac13..8b75b9d0f7 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -1187,7 +1187,7 @@ void nano::json_handler::block_confirm () if (!node.ledger.confirmed.block_exists_or_pruned (transaction, hash)) { // Start new confirmation for unconfirmed (or not being confirmed) block - if (!node.confirming_set.exists (hash)) + if (!node.confirming_set.contains (hash)) { node.start_election (std::move (block_l)); } diff --git a/nano/node/node.cpp b/nano/node/node.cpp index ceb034f361..3cb0e03534 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -115,7 +115,7 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy port_mapping_impl{ std::make_unique (*this) }, port_mapping{ *port_mapping_impl }, block_processor (*this), - confirming_set_impl{ std::make_unique (config.confirming_set, ledger, stats) }, + confirming_set_impl{ std::make_unique (config.confirming_set, ledger, stats, logger) }, confirming_set{ *confirming_set_impl }, active_impl{ std::make_unique (*this, confirming_set, block_processor) }, active{ *active_impl }, @@ -1114,14 +1114,14 @@ void nano::node::start_election (std::shared_ptr const & block) scheduler.manual.push (block); } -bool nano::node::block_confirmed (nano::block_hash const & hash_a) +bool nano::node::block_confirmed (nano::block_hash const & hash) { - return ledger.confirmed.block_exists_or_pruned (ledger.tx_begin_read (), hash_a); + return ledger.confirmed.block_exists_or_pruned (ledger.tx_begin_read (), hash); } -bool nano::node::block_confirmed_or_being_confirmed (nano::secure::transaction const & transaction, nano::block_hash const & hash_a) +bool nano::node::block_confirmed_or_being_confirmed (nano::secure::transaction const & transaction, nano::block_hash const & hash) { - return confirming_set.exists (hash_a) || ledger.confirmed.block_exists_or_pruned (transaction, hash_a); + return confirming_set.contains (hash) || ledger.confirmed.block_exists_or_pruned (transaction, hash); } bool nano::node::block_confirmed_or_being_confirmed (nano::block_hash const & hash_a) @@ -1151,31 +1151,36 @@ void nano::node::ongoing_online_weight_calculation () ongoing_online_weight_calculation_queue (); } -void nano::node::process_confirmed (nano::election_status const & status_a, uint64_t iteration_a) +// TODO: Replace this with a queue of some sort. Blocks submitted here could be in a limbo for a while: neither part of an active election nor cemented +void nano::node::process_confirmed (nano::block_hash hash, std::shared_ptr election, uint64_t iteration) { - auto hash (status_a.winner->hash ()); - decltype (iteration_a) const num_iters = (config.block_processor_batch_max_time / network_params.node.process_confirmed_interval) * 4; - if (auto block_l = ledger.any.block_get (ledger.tx_begin_read (), hash)) + stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::initiate); + + // Limit the maximum number of iterations to avoid getting stuck + uint64_t const max_iterations = (config.block_processor_batch_max_time / network_params.node.process_confirmed_interval) * 4; + + if (auto block = ledger.any.block_get (ledger.tx_begin_read (), hash)) { - logger.trace (nano::log::type::node, nano::log::detail::process_confirmed, nano::log::arg{ "block", block_l }); + stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::done); + logger.trace (nano::log::type::node, nano::log::detail::process_confirmed, nano::log::arg{ "block", block }); - confirming_set.add (block_l->hash ()); + confirming_set.add (block->hash (), election); } - else if (iteration_a < num_iters) + else if (iteration < max_iterations) { - iteration_a++; - std::weak_ptr node_w (shared ()); - election_workers.add_timed_task (std::chrono::steady_clock::now () + network_params.node.process_confirmed_interval, [node_w, status_a, iteration_a] () { - if (auto node_l = node_w.lock ()) - { - node_l->process_confirmed (status_a, iteration_a); - } + stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::retry); + + // Try again later + election_workers.add_timed_task (std::chrono::steady_clock::now () + network_params.node.process_confirmed_interval, [this, hash, election, iteration] () { + process_confirmed (hash, election, iteration + 1); }); } else { + stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::timeout); + // Do some cleanup due to this block never being processed by confirmation height processor - active.remove_election_winner_details (hash); + active.recently_confirmed.erase (hash); } } diff --git a/nano/node/node.hpp b/nano/node/node.hpp index af5f0e41d0..49ca17d65e 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -95,7 +95,7 @@ class node final : public std::enable_shared_from_this void keepalive (std::string const &, uint16_t); int store_version (); void inbound (nano::message const &, std::shared_ptr const &); - void process_confirmed (nano::election_status const &, uint64_t = 0); + void process_confirmed (nano::block_hash, std::shared_ptr = nullptr, uint64_t iteration = 0); void process_active (std::shared_ptr const &); std::optional process_local (std::shared_ptr const &); void process_local_async (std::shared_ptr const &); diff --git a/nano/node/wallet.cpp b/nano/node/wallet.cpp index 837a2ca417..716f78ee2f 100644 --- a/nano/node/wallet.cpp +++ b/nano/node/wallet.cpp @@ -1201,7 +1201,7 @@ bool nano::wallet::search_receivable (store::transaction const & wallet_transact // Receive confirmed block receive_async (hash, representative, amount, account, [] (std::shared_ptr const &) {}); } - else if (!wallets.node.confirming_set.exists (hash)) + else if (!wallets.node.confirming_set.contains (hash)) { auto block = wallets.node.ledger.any.block_get (block_transaction, hash); if (block) diff --git a/nano/secure/ledger.cpp b/nano/secure/ledger.cpp index efe7c5ad86..cba173a6c8 100644 --- a/nano/secure/ledger.cpp +++ b/nano/secure/ledger.cpp @@ -865,7 +865,10 @@ std::deque> nano::ledger::confirm (secure::write_tr bool refreshed = transaction.refresh_if_needed (); if (refreshed) { - release_assert (any.block_exists (transaction, target_hash), "block was rolled back during cementing"); + if (!any.block_exists (transaction, target_hash)) + { + break; // Block was rolled back during cementing + } } // Early return might leave parts of the dependency tree unconfirmed diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index 4e4781d412..b9d4e873c7 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -717,7 +717,6 @@ TEST (confirmation_height, many_accounts_single_confirmation) ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), 0); ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); - ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } TEST (confirmation_height, many_accounts_many_confirmations) @@ -792,8 +791,6 @@ TEST (confirmation_height, many_accounts_many_confirmations) ASSERT_EQ (cemented_count, node->ledger.cemented_count ()); ASSERT_TIMELY_EQ (20s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); - - ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } TEST (confirmation_height, long_chains) @@ -939,7 +936,6 @@ TEST (confirmation_height, long_chains) ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), 0); ASSERT_TIMELY_EQ (40s, (node->ledger.cemented_count () - 1), node->stats.count (nano::stat::type::confirmation_observer, nano::stat::dir::out)); - ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } TEST (confirmation_height, dynamic_algorithm) @@ -987,7 +983,6 @@ TEST (confirmation_height, dynamic_algorithm) ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in), num_blocks); ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_bounded, nano::stat::dir::in), 1); ASSERT_EQ (node->ledger.stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed_unbounded, nano::stat::dir::in), num_blocks - 1); - ASSERT_TIMELY_EQ (10s, node->active.election_winner_details_size (), 0); } TEST (confirmation_height, many_accounts_send_receive_self) @@ -1118,10 +1113,6 @@ TEST (confirmation_height, many_accounts_send_receive_self) } system.deadline_set (60s); - while (node->active.election_winner_details_size () > 0) - { - ASSERT_NO_ERROR (system.poll ()); - } } // Same as the many_accounts_send_receive_self test, except works on the confirmation height processor directly @@ -1147,7 +1138,7 @@ TEST (confirmation_height, many_accounts_send_receive_self_no_elections) nano::block_hash block_hash_being_processed{ 0 }; nano::store::write_queue write_queue; nano::confirming_set_config confirming_set_config{}; - nano::confirming_set confirming_set{ confirming_set_config, ledger, stats }; + nano::confirming_set confirming_set{ confirming_set_config, ledger, stats, logger }; auto const num_accounts = 100000; diff --git a/nano/test_common/ledger_context.cpp b/nano/test_common/ledger_context.cpp index 578aaa6342..e9a6b33c49 100644 --- a/nano/test_common/ledger_context.cpp +++ b/nano/test_common/ledger_context.cpp @@ -4,8 +4,8 @@ #include nano::test::ledger_context::ledger_context (std::deque> && blocks) : - store_m{ nano::make_store (logger, nano::unique_path (), nano::dev::constants) }, - stats_m{ logger }, + store_m{ nano::make_store (logger_m, nano::unique_path (), nano::dev::constants) }, + stats_m{ logger_m }, ledger_m{ *store_m, stats_m, nano::dev::constants }, blocks_m{ blocks }, pool_m{ nano::dev::network_params.network, 1 } @@ -35,6 +35,11 @@ nano::stats & nano::test::ledger_context::stats () return stats_m; } +nano::logger & nano::test::ledger_context::logger () +{ + return logger_m; +} + std::deque> const & nano::test::ledger_context::blocks () const { return blocks_m; diff --git a/nano/test_common/ledger_context.hpp b/nano/test_common/ledger_context.hpp index 8edca08508..62b376a4fd 100644 --- a/nano/test_common/ledger_context.hpp +++ b/nano/test_common/ledger_context.hpp @@ -16,12 +16,13 @@ class ledger_context ledger_context (std::deque> && blocks = std::deque>{}); nano::ledger & ledger (); nano::store::component & store (); - nano::stats & stats (); std::deque> const & blocks () const; nano::work_pool & pool (); + nano::stats & stats (); + nano::logger & logger (); private: - nano::logger logger; + nano::logger logger_m; std::unique_ptr store_m; nano::stats stats_m; nano::ledger ledger_m;