diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index 3bad3c28b0..b730b489a4 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) @@ -3050,13 +3047,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 7fce48d08e..9ca767cb3a 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, 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 ()); @@ -176,6 +186,7 @@ TEST (request_aggregator, two) ASSERT_TIMELY (3s, 0 < node.stats.count (nano::stat::type::requests, nano::stat::detail::requests_generated_votes)); ASSERT_TRUE (node.aggregator.empty ()); // The same request should now send the cached vote + // TODO: This is outdated, aggregator should not be using cache 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)); @@ -183,8 +194,6 @@ TEST (request_aggregator, two) 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, 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 +226,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 +245,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 ()); @@ -248,8 +258,6 @@ 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, 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 (3s, 0, node1.stats.count (nano::stat::type::requests, nano::stat::detail::requests_cannot_vote)); } @@ -365,8 +373,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 +406,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 0b39c9f6ad..6d70b144b2 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -45,6 +45,7 @@ enum class type requests, request_aggregator, request_aggregator_vote, + request_aggregator_replies, filter, telemetry, vote_generator, @@ -329,6 +330,8 @@ enum class detail // 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 780b22d39c..fe1817b042 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -526,6 +526,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 8e4854db2e..868b8a9ff8 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 fc651558dc..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);