diff --git a/nano/core_test/confirming_set.cpp b/nano/core_test/confirming_set.cpp index ee7302eb..596f6544 100644 --- a/nano/core_test/confirming_set.cpp +++ b/nano/core_test/confirming_set.cpp @@ -170,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); diff --git a/nano/core_test/election_scheduler.cpp b/nano/core_test/election_scheduler.cpp index 56c1a5c7..14f840d4 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/node.cpp b/nano/core_test/node.cpp index 54d1ef29..218ceb23 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -3564,9 +3564,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 @@ -3615,9 +3615,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 @@ -3676,9 +3676,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/node/election.cpp b/nano/node/election.cpp index a8a392db..93b9e8f7 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -60,11 +60,13 @@ void nano::election::confirm_once (nano::unique_lock & lock) nano::log::arg{ "qualified_root", qualified_root }, nano::log::arg{ "status", current_status_locked () }); - node.confirming_set.add (status_l.winner->hash (), shared_from_this ()); - lock.unlock (); - node.election_workers.push_task ([status_l, confirmation_action_l = confirmation_action] () { + 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) { confirmation_action_l (status_l.winner); diff --git a/nano/node/node.cpp b/nano/node/node.cpp index da9b9c5d..c6f550c9 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -1141,30 +1141,28 @@ 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) { stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::initiate); - 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)) + // 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)) { 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_l }); + 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) { stats.inc (nano::stat::type::process_confirmed, nano::stat::detail::retry); - 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); - } + // 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 @@ -1172,6 +1170,7 @@ void nano::node::process_confirmed (nano::election_status const & status_a, uint 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.recently_confirmed.erase (hash); } } diff --git a/nano/node/node.hpp b/nano/node/node.hpp index c411b6d9..9a79d5bd 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -94,7 +94,7 @@ public: bool copy_with_compaction (std::filesystem::path const &); void keepalive (std::string const &, uint16_t); int store_version (); - 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 &);