From f9ed20c78ef77e9df90e6097ae5169a1362c0f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sun, 23 Feb 2025 21:18:33 +0100 Subject: [PATCH] Rework online reps tracking callback --- nano/core_test/election.cpp | 3 --- nano/node/active_elections.cpp | 10 +++++---- nano/node/election.cpp | 18 +++++++++------ nano/node/election.hpp | 4 +++- nano/node/node.cpp | 40 ++++++++++++++++++---------------- 5 files changed, 41 insertions(+), 34 deletions(-) diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index d329852211..8cabb88cd3 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -198,8 +198,6 @@ TEST (election, quorum_minimum_confirm_fail) ASSERT_FALSE (election->confirmed ()); } -namespace nano -{ // FIXME: this test fails on rare occasions. It needs a review. TEST (election, quorum_minimum_update_weight_before_quorum_checks) { @@ -267,7 +265,6 @@ TEST (election, quorum_minimum_update_weight_before_quorum_checks) ASSERT_TIMELY (5s, election->confirmed ()); ASSERT_NE (nullptr, node1.block (send1->hash ())); } -} TEST (election, continuous_voting) { diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index bf099c4f86..1d4810a54b 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -406,12 +406,14 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr< if (!recently_confirmed.exists (root)) { result.inserted = true; - auto observe_rep_cb = [&node = node] (auto const & rep_a) { - // TODO: Is this neccessary? Move this outside of the election class - // Representative is defined as online if replying to live votes or rep_crawler queries + + // Passing this callback into the election is important + // We need to observe and update the online voting weight *before* election quorum is checked + auto observe_rep_callback = [&node = node] (auto const & rep_a) { node.online_reps.observe (rep_a); }; - result.election = nano::make_shared (node, block_a, nullptr, observe_rep_cb, election_behavior_a); + result.election = nano::make_shared (node, block_a, nullptr, observe_rep_callback, election_behavior_a); + roots.get ().emplace (entry{ root, result.election, std::move (erased_callback_a) }); node.vote_router.connect (hash, result.election); diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 3ee56c1f0b..fc0025a8d6 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -23,9 +23,9 @@ std::chrono::milliseconds nano::election::base_latency () const * election */ -nano::election::election (nano::node & node_a, std::shared_ptr const & block_a, std::function const &)> const & confirmation_action_a, std::function const & live_vote_action_a, nano::election_behavior election_behavior_a) : +nano::election::election (nano::node & node_a, std::shared_ptr const & block_a, std::function const &)> const & confirmation_action_a, std::function const & vote_action_a, nano::election_behavior election_behavior_a) : confirmation_action (confirmation_action_a), - live_vote_action (live_vote_action_a), + vote_action (vote_action_a), node (node_a), behavior_m (election_behavior_a), status (block_a), @@ -502,7 +502,8 @@ std::shared_ptr nano::election::find (nano::block_hash const & hash nano::vote_code nano::election::vote (nano::account const & rep, uint64_t timestamp_a, nano::block_hash const & block_hash_a, nano::vote_source vote_source_a) { - auto weight = node.ledger.weight (rep); + auto const weight = node.ledger.weight (rep); + if (!node.network_params.network.is_dev_network () && weight <= node.minimum_principal_weight ()) { return vote_code::indeterminate; @@ -538,11 +539,8 @@ nano::vote_code nano::election::vote (nano::account const & rep, uint64_t timest } } + // Update voter list entry last_votes[rep] = { std::chrono::steady_clock::now (), timestamp_a, block_hash_a }; - if (vote_source_a != vote_source::cache) - { - live_vote_action (rep); - } node.stats.inc (nano::stat::type::election, nano::stat::detail::vote); node.stats.inc (nano::stat::type::election_vote, to_stat_detail (vote_source_a)); @@ -565,6 +563,12 @@ nano::vote_code nano::election::vote (nano::account const & rep, uint64_t timest weight, to_string (vote_source_a)); + // This must execute before calculating the vote tally to ensure accurate online weight and quorum numbers are used + if (vote_action) + { + vote_action (rep); + } + if (!confirmed_locked ()) { confirm_if_quorum (lock); diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 78e6c5d8d9..ac82afd403 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -64,8 +64,10 @@ class election final : public std::enable_shared_from_this private: // Minimum time between broadcasts of the current winner of an election, as a backup to requesting confirmations std::chrono::milliseconds base_latency () const; + + // Callbacks std::function const &)> confirmation_action; - std::function live_vote_action; + std::function vote_action; private: // State management static unsigned constexpr passive_duration_factor = 5; diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 5b64f1da1f..25cdff33bf 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -252,6 +252,27 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy } }); + // Representative is defined as online if replying to live votes or rep crawler queries + observers.vote.add ([this] (std::shared_ptr vote, std::shared_ptr const & channel, nano::vote_source source, nano::vote_code code) { + release_assert (vote != nullptr); + release_assert (channel != nullptr); + debug_assert (code != nano::vote_code::invalid); + + // Track rep weight voting on live elections + bool should_observe = (code != nano::vote_code::indeterminate); + + // Ignore republished votes when rep crawling + if (source == nano::vote_source::live) + { + should_observe |= rep_crawler.process (vote, channel); + } + + if (should_observe) + { + online_reps.observe (vote->account); + } + }); + if (!init_error ()) { wallets.observer = [this] (bool active) { @@ -265,25 +286,6 @@ nano::node::node (std::shared_ptr io_ctx_a, std::filesy network.send_keepalive_self (channel); }); - observers.vote.add ([this] (std::shared_ptr vote, std::shared_ptr const & channel, nano::vote_source source, nano::vote_code code) { - debug_assert (vote != nullptr); - debug_assert (code != nano::vote_code::invalid); - if (channel == nullptr) - { - return; // Channel expired when waiting for vote to be processed - } - // Ignore republished votes - if (source == nano::vote_source::live) - { - bool active_in_rep_crawler = rep_crawler.process (vote, channel); - if (active_in_rep_crawler) - { - // Representative is defined as online if replying to live votes or rep_crawler queries - online_reps.observe (vote->account); - } - } - }); - // Cancelling local work generation observers.work_cancel.add ([this] (nano::root const & root_a) { this->work.cancel (root_a);