From 9af7abf867f0bc993c19512793fdcc6ad4ad7fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Wed, 20 Mar 2024 16:19:20 +0100 Subject: [PATCH] Return map of results from `active_transactions::vote (...)` --- nano/core_test/active_transactions.cpp | 32 ++++++++++---------- nano/core_test/election.cpp | 12 ++++---- nano/core_test/ledger.cpp | 10 +++---- nano/core_test/vote_processor.cpp | 6 ++-- nano/lib/stats_enums.hpp | 1 + nano/node/active_transactions.cpp | 41 +++++++++++++------------- nano/node/active_transactions.hpp | 2 +- nano/node/vote_processor.cpp | 20 +++++++++++-- 8 files changed, 71 insertions(+), 53 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index e40e817015..653fcb1136 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -582,19 +582,19 @@ TEST (active_transactions, vote_replays) // First vote is not a replay and confirms the election, second vote should be a replay since the election has confirmed but not yet removed auto vote_send1 = nano::test::make_final_vote (nano::dev::genesis_key, { send1 }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1)); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1).at (send1->hash ())); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1).at (send1->hash ())); // Wait until the election is removed, at which point the vote is still a replay since it's been recently confirmed ASSERT_TIMELY_EQ (5s, node.active.size (), 1); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1).at (send1->hash ())); // Open new account auto vote_open1 = nano::test::make_final_vote (nano::dev::genesis_key, { open1 }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1)); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1).at (open1->hash ())); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1).at (open1->hash ())); ASSERT_TIMELY (5s, node.active.empty ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1).at (open1->hash ())); ASSERT_EQ (nano::Gxrb_ratio, node.ledger.weight (key.pub)); // send 1 raw to key to key @@ -615,27 +615,27 @@ TEST (active_transactions, vote_replays) // vote2_send2 is a non final vote with little weight, vote1_send2 is the vote that confirms the election auto vote1_send2 = nano::test::make_final_vote (nano::dev::genesis_key, { send2 }); auto vote2_send2 = nano::test::make_vote (key, { send2 }, 0, 0); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2_send2)); // this vote cannot confirm the election + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2_send2).at (send2->hash ())); // this vote cannot confirm the election ASSERT_EQ (1, node.active.size ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2)); // this vote cannot confirm the election + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2).at (send2->hash ())); // this vote cannot confirm the election ASSERT_EQ (1, node.active.size ()); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2)); // this vote confirms the election + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2).at (send2->hash ())); // this vote confirms the election // this should still return replay, either because the election is still in the AEC or because it is recently confirmed - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2).at (send2->hash ())); ASSERT_TIMELY (5s, node.active.empty ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2)); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2).at (send2->hash ())); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2).at (send2->hash ())); // Removing blocks as recently confirmed makes every vote indeterminate { nano::lock_guard guard (node.active.mutex); node.active.recently_confirmed.clear (); } - ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_send1)); - ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_open1)); - ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote1_send2)); - ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote2_send2)); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_send1).at (send1->hash ())); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_open1).at (open1->hash ())); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote1_send2).at (send2->hash ())); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote2_send2).at (send2->hash ())); } } diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index 766b41c069..a56dfe04c2 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -71,7 +71,7 @@ TEST (election, quorum_minimum_flip_success) ASSERT_TIMELY_EQ (5s, election->blocks ().size (), 2); auto vote = nano::test::make_final_vote (nano::dev::genesis_key, { send2->hash () }); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote).at (send2->hash ())); ASSERT_TIMELY (5s, election->confirmed ()); auto const winner = election->winner (); @@ -120,7 +120,7 @@ TEST (election, quorum_minimum_flip_fail) // genesis generates a final vote for send2 but it should not be enough to reach quorum due to the online_weight_minimum being so high auto vote = nano::test::make_final_vote (nano::dev::genesis_key, { send2->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote).at (send2->hash ())); // give the election some time before asserting it is not confirmed so that in case // it would be wrongfully confirmed, have that immediately fail instead of race @@ -156,7 +156,7 @@ TEST (election, quorum_minimum_confirm_success) ASSERT_NE (nullptr, election); ASSERT_EQ (1, election->blocks ().size ()); auto vote = nano::test::make_final_vote (nano::dev::genesis_key, { send1->hash () }); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote).at (send1->hash ())); ASSERT_NE (nullptr, node1.block (send1->hash ())); ASSERT_TIMELY (5s, election->confirmed ()); } @@ -187,7 +187,7 @@ TEST (election, quorum_minimum_confirm_fail) ASSERT_EQ (1, election->blocks ().size ()); auto vote = nano::test::make_final_vote (nano::dev::genesis_key, { send1->hash () }); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote).at (send1->hash ())); // give the election a chance to confirm WAIT (1s); @@ -250,7 +250,7 @@ TEST (election, quorum_minimum_update_weight_before_quorum_checks) ASSERT_EQ (1, election->blocks ().size ()); auto vote1 = nano::test::make_final_vote (nano::dev::genesis_key, { send1->hash () }); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1).at (send1->hash ())); auto channel = node1.network.find_node_id (node2.get_node_id ()); ASSERT_NE (channel, nullptr); @@ -264,7 +264,7 @@ TEST (election, quorum_minimum_update_weight_before_quorum_checks) // Modify online_m for online_reps to more than is available, this checks that voting below updates it to current online reps. node1.online_reps.online_m = node_config.online_weight_minimum.number () + 20; } - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2).at (send1->hash ())); ASSERT_TIMELY (5s, election->confirmed ()); ASSERT_NE (nullptr, node1.block (send1->hash ())); } diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 773057e894..12567e02be 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -931,9 +931,9 @@ TEST (votes, add_one) auto election1 = node1.active.election (send1->qualified_root ()); ASSERT_EQ (1, election1->votes ().size ()); auto vote1 = nano::test::make_vote (nano::dev::genesis_key, { send1 }, nano::vote::timestamp_min * 1, 0); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1).at (send1->hash ())); auto vote2 = nano::test::make_vote (nano::dev::genesis_key, { send1 }, nano::vote::timestamp_min * 2, 0); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2).at (send1->hash ())); ASSERT_EQ (2, election1->votes ().size ()); auto votes1 (election1->votes ()); auto existing1 (votes1.find (nano::dev::genesis_key.pub)); @@ -972,7 +972,7 @@ TEST (votes, add_existing) ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ())); auto election1 = node1.active.election (send1->qualified_root ()); auto vote1 = nano::test::make_vote (nano::dev::genesis_key, { send1 }, nano::vote::timestamp_min * 1, 0); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1).at (send1->hash ())); // Block is already processed from vote ASSERT_TRUE (node1.active.publish (send1)); ASSERT_EQ (nano::vote::timestamp_min * 1, election1->last_votes[nano::dev::genesis_key.pub].timestamp); @@ -994,13 +994,13 @@ TEST (votes, add_existing) auto vote_info1 = election1->get_last_vote (nano::dev::genesis_key.pub); vote_info1.time = std::chrono::steady_clock::now () - std::chrono::seconds (20); election1->set_last_vote (nano::dev::genesis_key.pub, vote_info1); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2).at (send2->hash ())); ASSERT_EQ (nano::vote::timestamp_min * 2, election1->last_votes[nano::dev::genesis_key.pub].timestamp); // Also resend the old vote, and see if we respect the timestamp auto vote_info2 = election1->get_last_vote (nano::dev::genesis_key.pub); vote_info2.time = std::chrono::steady_clock::now () - std::chrono::seconds (20); election1->set_last_vote (nano::dev::genesis_key.pub, vote_info2); - ASSERT_EQ (nano::vote_code::replay, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::replay, node1.active.vote (vote1).at (send1->hash ())); ASSERT_EQ (nano::vote::timestamp_min * 2, election1->votes ()[nano::dev::genesis_key.pub].timestamp); auto votes (election1->votes ()); ASSERT_EQ (2, votes.size ()); diff --git a/nano/core_test/vote_processor.cpp b/nano/core_test/vote_processor.cpp index f755eb8ac0..e0b0d56390 100644 --- a/nano/core_test/vote_processor.cpp +++ b/nano/core_test/vote_processor.cpp @@ -206,7 +206,7 @@ TEST (vote_processor, no_broadcast_local) ASSERT_FALSE (node.wallets.reps ().have_half_rep ()); // Genesis balance remaining after `send' is less than the half_rep threshold // Process a vote with a key that is in the local wallet. auto vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::milliseconds_since_epoch (), nano::vote::duration_max, std::vector{ send->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote).at (send->hash ())); // Make sure the vote was processed. auto election (node.active.election (send->qualified_root ())); ASSERT_NE (nullptr, election); @@ -254,7 +254,7 @@ TEST (vote_processor, local_broadcast_without_a_representative) node.start_election (send); // Process a vote without a representative auto vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::milliseconds_since_epoch (), nano::vote::duration_max, std::vector{ send->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote).at (send->hash ())); // Make sure the vote was processed. std::shared_ptr election; ASSERT_TIMELY (5s, election = node.active.election (send->qualified_root ())); @@ -307,7 +307,7 @@ TEST (vote_processor, no_broadcast_local_with_a_principal_representative) ASSERT_TRUE (node.wallets.reps ().have_half_rep ()); // Genesis balance after `send' is over both half_rep and PR threshold. // Process a vote with a key that is in the local wallet. auto vote = std::make_shared (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::milliseconds_since_epoch (), nano::vote::duration_max, std::vector{ send->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote).at (send->hash ())); // Make sure the vote was processed. auto election (node.active.election (send->qualified_root ())); ASSERT_NE (nullptr, election); diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index b1baaa9631..64d5f8ba21 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -173,6 +173,7 @@ enum class detail : uint8_t vote_indeterminate, vote_invalid, vote_overflow, + vote_ignored, // election specific vote_new, diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index b90c5510ae..7dc65b0f95 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -438,31 +438,31 @@ nano::election_insertion_result nano::active_transactions::insert (std::shared_p } // Validate a vote and apply it to the current election if one exists -nano::vote_code nano::active_transactions::vote (std::shared_ptr const & vote_a) +std::unordered_map nano::active_transactions::vote (std::shared_ptr const & vote) { - nano::vote_code result{ nano::vote_code::indeterminate }; - // If all hashes were recently confirmed then it is a replay - unsigned recently_confirmed_counter (0); + std::unordered_map results; std::vector, nano::block_hash>> process; std::vector inactive; // Hashes that should be added to inactive vote cache { nano::unique_lock lock{ mutex }; - for (auto const & hash : vote_a->hashes) + for (auto const & hash : vote->hashes) { - auto existing (blocks.find (hash)); - if (existing != blocks.end ()) + debug_assert (results.find (hash) == results.end ()); + + if (auto existing = blocks.find (hash); existing != blocks.end ()) { process.emplace_back (existing->second, hash); } else if (!recently_confirmed.exists (hash)) { inactive.emplace_back (hash); + results[hash] = nano::vote_code::indeterminate; } else { - ++recently_confirmed_counter; + results[hash] = nano::vote_code::replay; } } } @@ -470,37 +470,38 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr con // Process inactive votes outside of the critical section for (auto & hash : inactive) { - add_vote_cache (hash, vote_a); + add_vote_cache (hash, vote); } if (!process.empty ()) { - bool replay = false; bool processed = false; for (auto const & [election, block_hash] : process) { - auto const vote_result = election->vote (vote_a->account, vote_a->timestamp (), block_hash); + auto const vote_result = election->vote (vote->account, vote->timestamp (), block_hash); + results[block_hash] = vote_result; + processed |= (vote_result == nano::vote_code::vote); - replay |= (vote_result == nano::vote_code::replay); } // Republish vote if it is new and the node does not host a principal representative (or close to) if (processed) { auto const reps (node.wallets.reps ()); - if (!reps.have_half_rep () && !reps.exists (vote_a->account)) + if (!reps.have_half_rep () && !reps.exists (vote->account)) { - node.network.flood_vote (vote_a, 0.5f); + node.network.flood_vote (vote, 0.5f); } } - result = replay ? nano::vote_code::replay : nano::vote_code::vote; - } - else if (recently_confirmed_counter == vote_a->hashes.size ()) - { - result = nano::vote_code::replay; } - return result; + + // All hashes should have their result set + debug_assert (std::all_of (vote->hashes.begin (), vote->hashes.end (), [&results] (auto const & hash) { + return results.find (hash) != results.end (); + })); + + return results; } bool nano::active_transactions::active (nano::qualified_root const & root_a) const diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 7323bf51dc..1c7b493d02 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -142,7 +142,7 @@ class active_transactions final */ nano::election_insertion_result insert (std::shared_ptr const &, nano::election_behavior = nano::election_behavior::normal); // Distinguishes replay votes, cannot be determined if the block is not in any election - nano::vote_code vote (std::shared_ptr const &); + std::unordered_map vote (std::shared_ptr const &); // Is the root of this block in the roots container bool active (nano::block const &) const; bool active (nano::qualified_root const &) const; diff --git a/nano/node/vote_processor.cpp b/nano/node/vote_processor.cpp index 337b0bddfb..080f64914e 100644 --- a/nano/node/vote_processor.cpp +++ b/nano/node/vote_processor.cpp @@ -163,12 +163,24 @@ void nano::vote_processor::verify_votes (decltype (votes) const & votes_a) nano::vote_code nano::vote_processor::vote_blocking (std::shared_ptr const & vote_a, std::shared_ptr const & channel_a, bool validated) { - auto result (nano::vote_code::invalid); + auto result = nano::vote_code::invalid; if (validated || !vote_a->validate ()) { - result = active.vote (vote_a); + auto vote_results = active.vote (vote_a); + + // Aggregate results for individual hashes + bool replay = false; + bool processed = false; + for (auto const & [hash, hash_result] : vote_results) + { + replay |= (hash_result == nano::vote_code::replay); + processed |= (hash_result == nano::vote_code::vote); + } + result = replay ? nano::vote_code::replay : (processed ? nano::vote_code::vote : nano::vote_code::indeterminate); + observers.vote.notify (vote_a, channel_a, result); } + std::string status; switch (result) { @@ -188,6 +200,10 @@ nano::vote_code nano::vote_processor::vote_blocking (std::shared_ptr status = "Indeterminate"; stats.inc (nano::stat::type::vote, nano::stat::detail::vote_indeterminate); break; + case nano::vote_code::ignored: + status = "Ignored"; + stats.inc (nano::stat::type::vote, nano::stat::detail::vote_ignored); + break; } logger.trace (nano::log::type::vote_processor, nano::log::detail::vote_processed,