From af4ef0550c17aa7547a43de8d4ac3418a8d003f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Fri, 18 Oct 2024 18:19:58 +0200 Subject: [PATCH] Remove `node::process_confirmed ()` --- nano/core_test/election_scheduler.cpp | 4 +-- nano/core_test/node.cpp | 12 ++++----- nano/node/election.cpp | 8 +++--- nano/node/node.cpp | 38 ++++----------------------- nano/node/node.hpp | 11 ++------ 5 files changed, 18 insertions(+), 55 deletions(-) diff --git a/nano/core_test/election_scheduler.cpp b/nano/core_test/election_scheduler.cpp index 14f840d4fa..422df99719 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 (send->hash ()); + node.confirming_set.add (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 (receive->hash ()); + node.confirming_set.add (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 13fe91ae8a..a69d4fed39 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 (send1->hash ()); + node1.confirming_set.add (send1->hash ()); ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ())); - node1.process_confirmed (send2->hash ()); + node1.confirming_set.add (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 (send1->hash ()); + node1.confirming_set.add (send1->hash ()); ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ())); - node1.process_confirmed (send2->hash ()); + node1.confirming_set.add (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 (send1->hash ()); + node1.confirming_set.add (send1->hash ()); ASSERT_TIMELY (5s, node1.block_confirmed (send1->hash ())); - node1.process_confirmed (send2->hash ()); + node1.confirming_set.add (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 93b9e8f7dc..a8a392dbe5 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -60,13 +60,11 @@ void nano::election::confirm_once (nano::unique_lock & lock) nano::log::arg{ "qualified_root", qualified_root }, nano::log::arg{ "status", current_status_locked () }); - lock.unlock (); + node.confirming_set.add (status_l.winner->hash (), shared_from_this ()); - 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); + lock.unlock (); + node.election_workers.push_task ([status_l, confirmation_action_l = confirmation_action] () { if (confirmation_action_l) { confirmation_action_l (status_l.winner); diff --git a/nano/node/node.cpp b/nano/node/node.cpp index c57f9c0a97..af3cadfc39 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -185,6 +185,11 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy } }); + // Do some cleanup due to this block never being processed by confirmation height processor + confirming_set.cementing_failed.add ([this] (auto const & hash) { + active.recently_confirmed.erase (hash); + }); + if (!init_error ()) { // Notify election schedulers when AEC frees election slot @@ -1151,39 +1156,6 @@ void nano::node::ongoing_online_weight_calculation () ongoing_online_weight_calculation_queue (); } -// 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); - - // 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 }); - - confirming_set.add (block->hash (), election); - } - else if (iteration < max_iterations) - { - 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.recently_confirmed.erase (hash); - } -} - std::shared_ptr nano::node::shared () { return shared_from_this (); diff --git a/nano/node/node.hpp b/nano/node/node.hpp index 49ca17d65e..a1a4538ae8 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -95,7 +95,6 @@ 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::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 &); @@ -235,20 +234,14 @@ class node final : public std::enable_shared_from_this std::atomic stopped{ false }; static double constexpr price_max = 16.0; static double constexpr free_cutoff = 1024.0; - // For tests only + +public: // For tests only unsigned node_seq; - // For tests only std::optional work_generate_blocking (nano::block &); - // For tests only std::optional work_generate_blocking (nano::root const &, uint64_t); - // For tests only std::optional work_generate_blocking (nano::root const &); public: // Testing convenience functions - /** - Creates a new write transaction and inserts `block' and returns result - Transaction is comitted before function return - */ [[nodiscard]] nano::block_status process (std::shared_ptr block); [[nodiscard]] nano::block_status process (secure::write_transaction const &, std::shared_ptr block); nano::block_hash latest (nano::account const &);