From 79a5d79e8867542a394f3c2b78bf9996509cc397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 1 Sep 2025 23:08:02 +0200 Subject: [PATCH] Activate highest gap optimistic elections first --- nano/core_test/optimistic_scheduler.cpp | 24 ++++-- nano/node/scheduler/optimistic.cpp | 98 +++++++++++++++---------- nano/node/scheduler/optimistic.hpp | 10 ++- 3 files changed, 82 insertions(+), 50 deletions(-) diff --git a/nano/core_test/optimistic_scheduler.cpp b/nano/core_test/optimistic_scheduler.cpp index 7466584b6..a61ac5783 100644 --- a/nano/core_test/optimistic_scheduler.cpp +++ b/nano/core_test/optimistic_scheduler.cpp @@ -17,8 +17,11 @@ using namespace std::chrono_literals; */ TEST (optimistic_scheduler, activate_one) { - nano::test::system system{}; - auto & node = *system.add_node (); + nano::test::system system; + + nano::node_config config; + config.priority_scheduler.enable = false; // Disable priority scheduler to avoid interference + auto & node = *system.add_node (config); // Needs to be greater than optimistic scheduler `gap_threshold` const int howmany_blocks = 64; @@ -41,8 +44,11 @@ TEST (optimistic_scheduler, activate_one) */ TEST (optimistic_scheduler, activate_one_zero_conf) { - nano::test::system system{}; - auto & node = *system.add_node (); + nano::test::system system; + + nano::node_config config; + config.priority_scheduler.enable = false; // Disable priority scheduler to avoid interference + auto & node = *system.add_node (config); // Can be smaller than optimistic scheduler `gap_threshold` // This is meant to activate short account chains (eg. binary tree spam leaf accounts) @@ -63,8 +69,11 @@ TEST (optimistic_scheduler, activate_one_zero_conf) */ TEST (optimistic_scheduler, activate_many) { - nano::test::system system{}; - auto & node = *system.add_node (); + nano::test::system system; + + nano::node_config config; + config.priority_scheduler.enable = false; // Disable priority scheduler to avoid interference + auto & node = *system.add_node (config); // Needs to be greater than optimistic scheduler `gap_threshold` const int howmany_blocks = 64; @@ -86,7 +95,8 @@ TEST (optimistic_scheduler, activate_many) */ TEST (optimistic_scheduler, under_gap_threshold) { - nano::test::system system{}; + nano::test::system system; + nano::node_config config = system.default_config (); config.backlog_scan.enable = false; auto & node = *system.add_node (config); diff --git a/nano/node/scheduler/optimistic.cpp b/nano/node/scheduler/optimistic.cpp index caab43c70..cd38ab9ae 100644 --- a/nano/node/scheduler/optimistic.cpp +++ b/nano/node/scheduler/optimistic.cpp @@ -72,34 +72,42 @@ bool nano::scheduler::optimistic::activate_predicate (const nano::account_info & bool nano::scheduler::optimistic::activate (const nano::account & account, const nano::account_info & account_info, const nano::confirmation_height_info & conf_info) { + debug_assert (account_info.block_count >= conf_info.height); + if (!config.enable) { return false; } - debug_assert (account_info.block_count >= conf_info.height); + bool activated = false; + if (activate_predicate (account_info, conf_info)) { + nano::lock_guard lock{ mutex }; + + // Assume gap_threshold for accounts with nothing confirmed + auto const unconfirmed_height = std::max (account_info.block_count - conf_info.height, config.gap_threshold); + + auto [it, inserted] = candidates.push_back ({ account, nano::clock::now (), unconfirmed_height }); + + stats.inc (nano::stat::type::optimistic_scheduler, inserted ? nano::stat::detail::activated : nano::stat::detail::duplicate); + + // Limit candidates container size + if (candidates.size () > config.max_size) { - nano::lock_guard lock{ mutex }; - - // Prevent duplicate candidate accounts - if (candidates.get ().contains (account)) - { - return false; // Not activated - } - // Limit candidates container size - if (candidates.size () >= config.max_size) - { - return false; // Not activated - } - - stats.inc (nano::stat::type::optimistic_scheduler, nano::stat::detail::activated); - candidates.push_back ({ account, nano::clock::now () }); + // Remove oldest candidate + candidates.pop_front (); } - return true; // Activated + + activated = inserted; } - return false; // Not activated + + if (activated) + { + condition.notify_all (); + } + + return activated; } bool nano::scheduler::optimistic::predicate () const @@ -115,9 +123,10 @@ bool nano::scheduler::optimistic::predicate () const return false; } - auto candidate = candidates.front (); - bool result = nano::elapsed (candidate.timestamp, network_constants.optimistic_activation_delay); - return result; + auto candidate = candidates.get ().begin (); + debug_assert (candidate != candidates.get ().end ()); + + return elapsed (candidate->timestamp, network_constants.optimistic_activation_delay); } void nano::scheduler::optimistic::run () @@ -127,27 +136,36 @@ void nano::scheduler::optimistic::run () { stats.inc (nano::stat::type::optimistic_scheduler, nano::stat::detail::loop); - if (predicate ()) - { - auto transaction = ledger.tx_begin_read (); - - while (predicate ()) - { - debug_assert (!candidates.empty ()); - auto candidate = candidates.front (); - candidates.pop_front (); - - lock.unlock (); - - run_one (transaction, candidate); - - lock.lock (); - } - } - condition.wait_for (lock, network_constants.optimistic_activation_delay / 2, [this] () { return stopped || predicate (); }); + + if (stopped) + { + return; + } + + lock.unlock (); + + // Acquire transaction outside of the lock + auto transaction = ledger.tx_begin_read (); + + lock.lock (); + + while (predicate () && !stopped) + { + debug_assert (!candidates.empty ()); + auto & height_index = candidates.get (); + auto candidate = *height_index.begin (); + height_index.erase (height_index.begin ()); + + lock.unlock (); + + transaction.refresh_if_needed (); + run_one (transaction, candidate); + + lock.lock (); + } } } @@ -160,7 +178,7 @@ void nano::scheduler::optimistic::run_one (secure::transaction const & transacti if (!node.block_confirmed_or_being_confirmed (transaction, block->hash ())) { // Try to insert it into AEC - // We check for AEC vacancy inside our predicate + // We check for AEC vacancy inside the predicate auto result = node.active.insert (block, nano::election_behavior::optimistic); stats.inc (nano::stat::type::optimistic_scheduler, result.inserted ? nano::stat::detail::insert : nano::stat::detail::insert_failed); diff --git a/nano/node/scheduler/optimistic.hpp b/nano/node/scheduler/optimistic.hpp index 24edd1e73..32557263d 100644 --- a/nano/node/scheduler/optimistic.hpp +++ b/nano/node/scheduler/optimistic.hpp @@ -33,10 +33,10 @@ public: bool enable{ true }; /** Minimum difference between confirmation frontier and account frontier to become a candidate for optimistic confirmation */ - std::size_t gap_threshold{ 32 }; + uint64_t gap_threshold{ 32 }; /** Maximum number of candidates stored in memory */ - std::size_t max_size{ 1024 * 64 }; + std::size_t max_size{ 1024 * 4 }; }; class optimistic final @@ -82,17 +82,21 @@ private: { nano::account account; nano::clock::time_point timestamp; + uint64_t unconfirmed_height; }; // clang-format off class tag_sequenced {}; class tag_account {}; + class tag_unconfirmed_height {}; using ordered_candidates = boost::multi_index_container>, mi::hashed_unique, - mi::member> + mi::member>, + mi::ordered_non_unique, + mi::member, std::greater> // Descending >>; // clang-format on