diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index b2d1e338a1..2e6b5e3f5a 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -769,6 +769,8 @@ TEST (node, fork_multi_flip) auto election = nano::test::start_election (system, node2, send2->hash ()); ASSERT_NE (nullptr, election); + ASSERT_TIMELY (5s, election->contains (send1->hash ())); + nano::test::confirm (node1.ledger, send1); ASSERT_TIMELY (5s, node2.block_or_pruned_exists (send1->hash ())); ASSERT_TRUE (nano::test::block_or_pruned_none_exists (node2, { send2, send3 })); auto winner = *election->tally ().begin (); @@ -781,17 +783,15 @@ TEST (node, fork_multi_flip) TEST (node, fork_bootstrap_flip) { nano::test::system system; - nano::test::system system0; - nano::test::system system1; nano::node_config config0{ system.get_available_port () }; config0.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled; nano::node_flags node_flags; node_flags.disable_bootstrap_bulk_push_client = true; node_flags.disable_lazy_bootstrap = true; - auto & node1 = *system0.add_node (config0, node_flags); + auto & node1 = *system.add_node (config0, node_flags); + system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); nano::node_config config1 (system.get_available_port ()); - auto & node2 = *system1.add_node (config1, node_flags); - system0.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); + auto & node2 = *system.make_disconnected_node (config1, node_flags); nano::block_hash latest = node1.latest (nano::dev::genesis_key.pub); nano::keypair key1; nano::send_block_builder builder; @@ -800,7 +800,7 @@ TEST (node, fork_bootstrap_flip) .destination (key1.pub) .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*system0.work.generate (latest)) + .work (*system.work.generate (latest)) .build (); nano::keypair key2; auto send2 = builder.make_block () @@ -808,22 +808,19 @@ TEST (node, fork_bootstrap_flip) .destination (key2.pub) .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (*system0.work.generate (latest)) + .work (*system.work.generate (latest)) .build (); // Insert but don't rebroadcast, simulating settled blocks ASSERT_EQ (nano::block_status::progress, node1.ledger.process (node1.ledger.tx_begin_write (), send1)); ASSERT_EQ (nano::block_status::progress, node2.ledger.process (node2.ledger.tx_begin_write (), send2)); - ASSERT_TRUE (node2.ledger.any.block_exists (node2.ledger.tx_begin_read (), send2->hash ())); - node2.bootstrap_initiator.bootstrap (node1.network.endpoint ()); // Additionally add new peer to confirm & replace bootstrap block - auto again (true); - system0.deadline_set (50s); - system1.deadline_set (50s); - while (again) - { - ASSERT_NO_ERROR (system0.poll ()); - ASSERT_NO_ERROR (system1.poll ()); - again = !node2.ledger.any.block_exists (node2.ledger.tx_begin_read (), send1->hash ()); - } + nano::test::confirm (node1.ledger, send1); + ASSERT_TIMELY (1s, node1.ledger.any.block_exists (node1.ledger.tx_begin_read (), send1->hash ())); + ASSERT_TIMELY (1s, node2.ledger.any.block_exists (node2.ledger.tx_begin_read (), send2->hash ())); + + // Additionally add new peer to confirm & replace bootstrap block + node2.network.merge_peer (node1.network.endpoint ()); + + ASSERT_TIMELY (10s, node2.ledger.any.block_exists (node2.ledger.tx_begin_read (), send1->hash ())); } TEST (node, fork_open) @@ -1820,7 +1817,8 @@ TEST (node, confirm_quorum) ASSERT_EQ (0, node1.balance (nano::dev::genesis_key.pub)); } -TEST (node, local_votes_cache) +// TODO: Local vote cache is no longer used when generating votes +TEST (node, DISABLED_local_votes_cache) { nano::test::system system; nano::node_config node_config (system.get_available_port ()); @@ -1903,6 +1901,7 @@ TEST (node, local_votes_cache) // Test disabled because it's failing intermittently. // PR in which it got disabled: https://github.com/nanocurrency/nano-node/pull/3532 // Issue for investigating it: https://github.com/nanocurrency/nano-node/issues/3481 +// TODO: Local vote cache is no longer used when generating votes TEST (node, DISABLED_local_votes_cache_batch) { nano::test::system system; @@ -1976,7 +1975,8 @@ TEST (node, DISABLED_local_votes_cache_batch) * There is a cache for locally generated votes. This test checks that the node * properly caches and uses those votes when replying to confirm_req requests. */ -TEST (node, local_votes_cache_generate_new_vote) +// TODO: Local vote cache is no longer used when generating votes +TEST (node, DISABLED_local_votes_cache_generate_new_vote) { nano::test::system system; nano::node_config node_config (system.get_available_port ()); @@ -2022,7 +2022,8 @@ TEST (node, local_votes_cache_generate_new_vote) ASSERT_TIMELY_EQ (3s, 3, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); } -TEST (node, local_votes_cache_fork) +// TODO: Local vote cache is no longer used when generating votes +TEST (node, DISABLED_local_votes_cache_fork) { nano::test::system system; nano::node_flags node_flags; @@ -2959,13 +2960,10 @@ TEST (node, rollback_vote_self) // Insert genesis key in the wallet system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); - // Even without the rollback being finished, the aggregator must reply with a vote for the new winner, not the old one - ASSERT_TRUE (node.history.votes (send2->root (), send2->hash ()).empty ()); - ASSERT_TRUE (node.history.votes (fork->root (), fork->hash ()).empty ()); + // Without the rollback being finished, the aggregator should not reply with any vote auto channel = std::make_shared (node); node.aggregator.request ({ { send2->hash (), send2->root () } }, channel); - ASSERT_TIMELY (5s, !node.history.votes (fork->root (), fork->hash ()).empty ()); - ASSERT_TRUE (node.history.votes (send2->root (), send2->hash ()).empty ()); + ASSERT_ALWAYS_EQ (1s, node.stats.count (nano::stat::type::request_aggregator_replies), 0); // Going out of the scope allows the rollback to complete } diff --git a/nano/core_test/request_aggregator.cpp b/nano/core_test/request_aggregator.cpp index a6ec1c3ee6..7d7148404f 100644 --- a/nano/core_test/request_aggregator.cpp +++ b/nano/core_test/request_aggregator.cpp @@ -34,27 +34,34 @@ TEST (request_aggregator, one) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*node.work_generate_blocking (nano::dev::genesis->hash ())) .build (); - std::vector> request; - request.emplace_back (send1->hash (), send1->root ()); + + std::vector> request{ { send1->hash (), send1->root () } }; + auto client = std::make_shared (node); std::shared_ptr dummy_channel = std::make_shared (node, client); + + // Not yet in the ledger node.aggregator.request (request, dummy_channel); ASSERT_TIMELY (3s, node.aggregator.empty ()); - // Not yet in the ledger ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); + + // Process and confirm ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send1)); - node.aggregator.request (request, dummy_channel); + nano::test::confirm (node.ledger, send1); + // In the ledger but no vote generated yet - ASSERT_TIMELY (3s, 0 < node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_TIMELY (3s, node.aggregator.empty ()); node.aggregator.request (request, dummy_channel); + ASSERT_TIMELY (3s, node.aggregator.empty ()); + ASSERT_TIMELY (3s, 0 < node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); + // Already cached + // TODO: This is outdated, aggregator should not be using cache + node.aggregator.request (request, dummy_channel); ASSERT_TIMELY (3s, node.aggregator.empty ()); ASSERT_TIMELY_EQ (3s, 3, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_accepted)); ASSERT_TIMELY_EQ (3s, 0, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_dropped)); ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); - ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_votes)); + ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); ASSERT_TIMELY_EQ (3s, 0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); } @@ -78,8 +85,7 @@ TEST (request_aggregator, one_update) .work (*node.work_generate_blocking (nano::dev::genesis->hash ())) .build (); ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send1)); - node.confirming_set.add (send1->hash ()); - ASSERT_TIMELY (5s, node.ledger.confirmed.block_exists_or_pruned (node.ledger.tx_begin_read (), send1->hash ())); + nano::test::confirm (node.ledger, send1); auto send2 = nano::state_block_builder () .account (nano::dev::genesis_key.pub) .previous (send1->hash ()) @@ -90,6 +96,7 @@ TEST (request_aggregator, one_update) .work (*node.work_generate_blocking (send1->hash ())) .build (); ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send2)); + nano::test::confirm (node.ledger, send2); auto receive1 = nano::state_block_builder () .account (key1.pub) .previous (0) @@ -100,6 +107,7 @@ TEST (request_aggregator, one_update) .work (*node.work_generate_blocking (key1.pub)) .build (); ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), receive1)); + nano::test::confirm (node.ledger, receive1); auto dummy_channel = nano::test::fake_channel (node); @@ -143,8 +151,7 @@ TEST (request_aggregator, two) .work (*node.work_generate_blocking (nano::dev::genesis->hash ())) .build (); ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send1)); - node.confirming_set.add (send1->hash ()); - ASSERT_TIMELY (5s, node.ledger.confirmed.block_exists_or_pruned (node.ledger.tx_begin_read (), send1->hash ())); + nano::test::confirm (node.ledger, send1); auto send2 = builder.make_block () .account (nano::dev::genesis_key.pub) .previous (send1->hash ()) @@ -164,7 +171,10 @@ TEST (request_aggregator, two) .work (*node.work_generate_blocking (key1.pub)) .build (); ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), send2)); + nano::test::confirm (node.ledger, send2); ASSERT_EQ (nano::block_status::progress, node.ledger.process (node.ledger.tx_begin_write (), receive1)); + nano::test::confirm (node.ledger, receive1); + std::vector> request; request.emplace_back (send2->hash (), send2->root ()); request.emplace_back (receive1->hash (), receive1->root ()); @@ -181,10 +191,8 @@ TEST (request_aggregator, two) ASSERT_EQ (2, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_accepted)); ASSERT_EQ (0, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_dropped)); ASSERT_TIMELY_EQ (3s, 0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); - ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_hashes)); - ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_hashes)); - ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_votes)); + ASSERT_TIMELY_EQ (3s, 4, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_hashes)); + ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); ASSERT_TIMELY_EQ (3s, 0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); // Make sure the cached vote is for both hashes @@ -217,13 +225,15 @@ TEST (request_aggregator, two_endpoints) .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) .work (*node1.work_generate_blocking (nano::dev::genesis->hash ())) .build (); - std::vector> request; - request.emplace_back (send1->hash (), send1->root ()); ASSERT_EQ (nano::block_status::progress, node1.ledger.process (node1.ledger.tx_begin_write (), send1)); + nano::test::confirm (node1.ledger, send1); + auto dummy_channel1 = std::make_shared (node1, node1); auto dummy_channel2 = std::make_shared (node2, node2); ASSERT_NE (nano::transport::map_endpoint_to_v6 (dummy_channel1->get_endpoint ()), nano::transport::map_endpoint_to_v6 (dummy_channel2->get_endpoint ())); + std::vector> request{ { send1->hash (), send1->root () } }; + // For the first request, aggregator should generate a new vote node1.aggregator.request (request, dummy_channel1); ASSERT_TIMELY (5s, node1.aggregator.empty ()); @@ -234,11 +244,10 @@ TEST (request_aggregator, two_endpoints) ASSERT_TIMELY_EQ (5s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_hashes)); ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_TIMELY_EQ (3s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_hashes)); - ASSERT_TIMELY_EQ (3s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_votes)); ASSERT_TIMELY_EQ (3s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); // For the second request, aggregator should use the cache + // TODO: This is outdated, aggregator should not be using cache node1.aggregator.request (request, dummy_channel1); ASSERT_TIMELY (5s, node1.aggregator.empty ()); @@ -246,10 +255,8 @@ TEST (request_aggregator, two_endpoints) ASSERT_EQ (0, node1.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_dropped)); ASSERT_TIMELY_EQ (5s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); - ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_hashes)); - ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_TIMELY_EQ (3s, 1, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_hashes)); - ASSERT_TIMELY_EQ (3s, 1, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_votes)); + ASSERT_TIMELY_EQ (5s, 2, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_hashes)); + ASSERT_TIMELY_EQ (5s, 2, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); ASSERT_TIMELY_EQ (3s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); } @@ -365,8 +372,6 @@ TEST (request_aggregator, DISABLED_unique) ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); } -namespace nano -{ TEST (request_aggregator, cannot_vote) { nano::test::system system; @@ -400,44 +405,41 @@ TEST (request_aggregator, cannot_vote) request.emplace_back (send2->hash (), send2->root ()); // Incorrect hash, correct root request.emplace_back (1, send2->root ()); + auto client = std::make_shared (node); std::shared_ptr dummy_channel = std::make_shared (node, client); node.aggregator.request (request, dummy_channel); ASSERT_TIMELY (3s, node.aggregator.empty ()); ASSERT_EQ (1, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_accepted)); ASSERT_EQ (0, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_dropped)); - ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); + ASSERT_TIMELY_EQ (3s, 2, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_non_final)); ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_votes)); ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); ASSERT_EQ (0, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); // With an ongoing election node.start_election (send2); + ASSERT_TIMELY (5s, node.active.election (send2->qualified_root ())); + node.aggregator.request (request, dummy_channel); ASSERT_TIMELY (3s, node.aggregator.empty ()); ASSERT_EQ (2, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_accepted)); ASSERT_EQ (0, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_dropped)); - ASSERT_TIMELY_EQ (3s, 4, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); + ASSERT_TIMELY_EQ (3s, 4, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_non_final)); ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); - ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cached_votes)); ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); ASSERT_EQ (0, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); - // Confirm send1 - node.start_election (send1); - std::shared_ptr election; - ASSERT_TIMELY (5s, election = node.active.election (send1->qualified_root ())); - election->force_confirm (); - ASSERT_TIMELY (3s, node.ledger.dependents_confirmed (node.ledger.tx_begin_read (), *send2)); + // Confirm send1 and send2 + nano::test::confirm (node.ledger, { send1, send2 }); + node.aggregator.request (request, dummy_channel); ASSERT_TIMELY (3s, node.aggregator.empty ()); ASSERT_EQ (3, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_accepted)); ASSERT_EQ (0, node.stats.count (nano::stat::type::aggregator, nano::stat::detail::aggregator_dropped)); - ASSERT_EQ (4, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); + ASSERT_EQ (4, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_non_final)); ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_hashes)); ASSERT_TIMELY_EQ (3s, 1, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); ASSERT_EQ (0, node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_unknown)); ASSERT_TIMELY (3s, 1 <= node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out)); } -} diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 6caefff4e2..081e4779b2 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -45,6 +45,8 @@ enum class type aggregator, requests, request_aggregator, + request_aggregator_vote, + request_aggregator_replies, filter, telemetry, vote_generator, @@ -332,10 +334,14 @@ enum class detail requests_generated_votes, requests_cannot_vote, requests_unknown, + requests_non_final, + requests_final, // request_aggregator request_hashes, overfill_hashes, + normal_vote, + final_vote, // duplicate duplicate_publish_message, diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 3f7d7baeec..908d18a210 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -542,6 +542,7 @@ bool nano::active_elections::publish (std::shared_ptr const & block node.vote_cache_processor.trigger (block_a->hash ()); node.stats.inc (nano::stat::type::active, nano::stat::detail::election_block_conflict); + node.logger.debug (nano::log::type::active_elections, "Block was added to an existing election: {}", block_a->hash ().to_string ()); } } return result; diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 8ac2fc5628..d0e7631e24 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -756,6 +756,12 @@ nano::election_state nano::election::state () const return state_m; } +bool nano::election::contains (nano::block_hash const & hash) const +{ + nano::lock_guard guard{ mutex }; + return last_blocks.contains (hash); +} + // TODO: Remove the need for .to_string () calls void nano::election::operator() (nano::object_stream & obs) const { diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 6111191e2b..b65d2f1c0f 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -138,6 +138,10 @@ class election final : public std::enable_shared_from_this nano::election_behavior behavior () const; nano::election_state state () const; + std::unordered_map votes () const; + std::unordered_map> blocks () const; + bool contains (nano::block_hash const &) const; + private: nano::tally_t tally_impl () const; bool confirmed_locked () const; @@ -188,8 +192,6 @@ class election final : public std::enable_shared_from_this public: // Only used in tests void force_confirm (); - std::unordered_map votes () const; - std::unordered_map> blocks () const; friend class confirmation_solicitor_different_hash_Test; friend class confirmation_solicitor_bypass_max_requests_cap_Test; diff --git a/nano/node/request_aggregator.cpp b/nano/node/request_aggregator.cpp index 3eb366c90f..e846950bc8 100644 --- a/nano/node/request_aggregator.cpp +++ b/nano/node/request_aggregator.cpp @@ -175,12 +175,16 @@ void nano::request_aggregator::process (nano::secure::transaction const & transa if (!remaining.remaining_normal.empty ()) { + stats.inc (nano::stat::type::request_aggregator_replies, nano::stat::detail::normal_vote); + // Generate votes for the remaining hashes auto const generated = generator.generate (remaining.remaining_normal, channel); stats.add (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote, stat::dir::in, remaining.remaining_normal.size () - generated); } if (!remaining.remaining_final.empty ()) { + stats.inc (nano::stat::type::request_aggregator_replies, nano::stat::detail::final_vote); + // Generate final votes for the remaining hashes auto const generated = final_generator.generate (remaining.remaining_final, channel); stats.add (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote, stat::dir::in, remaining.remaining_final.size () - generated); @@ -208,65 +212,52 @@ auto nano::request_aggregator::aggregate (nano::secure::transaction const & tran { std::vector> to_generate; std::vector> to_generate_final; - std::vector> cached_votes; - std::unordered_set cached_hashes; for (auto const & [hash, root] : requests_a) { - // 0. Hashes already sent - if (cached_hashes.count (hash) > 0) - { - continue; - } + bool generate_final_vote (false); + std::shared_ptr block; - // 1. Votes in cache - auto find_votes (local_votes.votes (root, hash)); - if (!find_votes.empty ()) + // 2. Final votes + auto final_vote_hashes (ledger.store.final_vote.get (transaction, root)); + if (!final_vote_hashes.empty ()) { - for (auto & found_vote : find_votes) + generate_final_vote = true; + block = ledger.any.block_get (transaction, final_vote_hashes[0]); + // Allow same root vote + if (block != nullptr && final_vote_hashes.size () > 1) { - cached_votes.push_back (found_vote); - for (auto & found_hash : found_vote->hashes) - { - cached_hashes.insert (found_hash); - } + // WTF? This shouldn't be done like this + to_generate_final.push_back (block); + block = ledger.any.block_get (transaction, final_vote_hashes[1]); + debug_assert (final_vote_hashes.size () == 2); } } - else - { - bool generate_vote (true); - bool generate_final_vote (false); - std::shared_ptr block; - // 2. Final votes - auto final_vote_hashes (ledger.store.final_vote.get (transaction, root)); - if (!final_vote_hashes.empty ()) + // 4. Ledger by hash + if (block == nullptr) + { + block = ledger.any.block_get (transaction, hash); + // Confirmation status. Generate final votes for confirmed + if (block != nullptr) { - generate_final_vote = true; - block = ledger.any.block_get (transaction, final_vote_hashes[0]); - // Allow same root vote - if (block != nullptr && final_vote_hashes.size () > 1) - { - to_generate_final.push_back (block); - block = ledger.any.block_get (transaction, final_vote_hashes[1]); - debug_assert (final_vote_hashes.size () == 2); - } + nano::confirmation_height_info confirmation_height_info; + ledger.store.confirmation_height.get (transaction, block->account (), confirmation_height_info); + generate_final_vote = (confirmation_height_info.height >= block->sideband ().height); } + } - // 3. Election winner by hash - if (block == nullptr) + // 5. Ledger by root + if (block == nullptr && !root.is_zero ()) + { + // Search for block root + auto successor = ledger.any.block_successor (transaction, root.as_block_hash ()); + if (successor) { - auto election = vote_router.election (hash); - if (election != nullptr) - { - block = election->winner (); - } - } + auto successor_block = ledger.any.block_get (transaction, successor.value ()); + release_assert (successor_block != nullptr); + block = std::move (successor_block); - // 4. Ledger by hash - if (block == nullptr) - { - block = ledger.any.block_get (transaction, hash); - // Confirmation status. Generate final votes for confirmed + // Confirmation status. Generate final votes for confirmed successor if (block != nullptr) { nano::confirmation_height_info confirmation_height_info; @@ -274,81 +265,33 @@ auto nano::request_aggregator::aggregate (nano::secure::transaction const & tran generate_final_vote = (confirmation_height_info.height >= block->sideband ().height); } } + } - // 5. Ledger by root - if (block == nullptr && !root.is_zero ()) - { - // Search for block root - auto successor = ledger.any.block_successor (transaction, root.as_block_hash ()); - if (successor) - { - auto successor_block = ledger.any.block_get (transaction, successor.value ()); - debug_assert (successor_block != nullptr); - block = std::move (successor_block); - // 5. Votes in cache for successor - auto find_successor_votes (local_votes.votes (root, successor.value ())); - if (!find_successor_votes.empty ()) - { - cached_votes.insert (cached_votes.end (), find_successor_votes.begin (), find_successor_votes.end ()); - generate_vote = false; - } - // Confirmation status. Generate final votes for confirmed successor - if (block != nullptr && generate_vote) - { - nano::confirmation_height_info confirmation_height_info; - ledger.store.confirmation_height.get (transaction, block->account (), confirmation_height_info); - generate_final_vote = (confirmation_height_info.height >= block->sideband ().height); - } - } - } - - if (block) + if (block) + { + if (generate_final_vote) { - // Generate new vote - if (generate_vote) - { - if (generate_final_vote) - { - to_generate_final.push_back (block); - } - else - { - to_generate.push_back (block); - } - } - - // Let the node know about the alternative block - if (block->hash () != hash) - { - nano::publish publish (network_constants, block); - channel_a->send (publish); - } + to_generate_final.push_back (block); + stats.inc (nano::stat::type::requests, nano::stat::detail::requests_final); } else { - stats.inc (nano::stat::type::requests, nano::stat::detail::requests_unknown, stat::dir::in); + stats.inc (nano::stat::type::requests, nano::stat::detail::requests_non_final); } } + else + { + stats.inc (nano::stat::type::requests, nano::stat::detail::requests_unknown); + } } - // Unique votes - std::sort (cached_votes.begin (), cached_votes.end ()); - cached_votes.erase (std::unique (cached_votes.begin (), cached_votes.end ()), cached_votes.end ()); - for (auto const & vote : cached_votes) - { - reply_action (vote, channel_a); - } - - stats.add (nano::stat::type::requests, nano::stat::detail::requests_cached_hashes, stat::dir::in, cached_hashes.size ()); - stats.add (nano::stat::type::requests, nano::stat::detail::requests_cached_votes, stat::dir::in, cached_votes.size ()); - return { .remaining_normal = to_generate, .remaining_final = to_generate_final }; } -std::unique_ptr nano::request_aggregator::collect_container_info (std::string const & name) +std::unique_ptr nano::request_aggregator::collect_container_info (std::string const & name) const { nano::lock_guard guard{ mutex }; diff --git a/nano/node/request_aggregator.hpp b/nano/node/request_aggregator.hpp index eb3d7fd934..d0a72074af 100644 --- a/nano/node/request_aggregator.hpp +++ b/nano/node/request_aggregator.hpp @@ -61,7 +61,7 @@ class request_aggregator final std::size_t size () const; bool empty () const; - std::unique_ptr collect_container_info (std::string const &); + std::unique_ptr collect_container_info (std::string const & name) const; private: void run (); diff --git a/nano/node/vote_processor.cpp b/nano/node/vote_processor.cpp index d2d22f45b4..7b480e07bf 100644 --- a/nano/node/vote_processor.cpp +++ b/nano/node/vote_processor.cpp @@ -195,6 +195,7 @@ nano::vote_code nano::vote_processor::vote_blocking (std::shared_ptr logger.trace (nano::log::type::vote_processor, nano::log::detail::vote_processed, nano::log::arg{ "vote", vote }, + nano::log::arg{ "vote_source", source }, nano::log::arg{ "result", result }); return result;