From f448e29279da7348bdf26952f3d7f3714b37f603 Mon Sep 17 00:00:00 2001 From: Wesley Shillingford Date: Thu, 7 Jan 2021 16:50:46 +0000 Subject: [PATCH] Update online weight before checking quorum (#3048) * Update online weight before checking quorum * Make test compatible with quorum hardcoded changes * Fix test network quorum overflow Co-authored-by: Sergey Kroshnin --- nano/core_test/active_transactions.cpp | 38 +++++----- nano/core_test/confirmation_solicitor.cpp | 10 +-- nano/core_test/conflicts.cpp | 2 +- nano/core_test/election.cpp | 92 ++++++++++++++++++++++- nano/core_test/ledger.cpp | 14 ++-- nano/core_test/node.cpp | 2 +- nano/core_test/vote_processor.cpp | 6 +- nano/node/active_transactions.cpp | 14 +++- nano/node/active_transactions.hpp | 2 +- nano/node/election.cpp | 25 +++--- nano/node/election.hpp | 5 +- nano/node/node.cpp | 10 +-- nano/node/node.hpp | 4 +- nano/node/online_reps.cpp | 6 +- nano/node/online_reps.hpp | 1 + nano/node/repcrawler.cpp | 2 +- nano/node/repcrawler.hpp | 4 +- nano/node/vote_processor.cpp | 6 +- nano/node/vote_processor.hpp | 4 +- 19 files changed, 171 insertions(+), 76 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index 5ed593a9..5a973da5 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -613,19 +613,19 @@ TEST (active_transactions, vote_replays) ASSERT_EQ (2, node.active.size ()); // 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 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, send1)); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1, true)); ASSERT_EQ (2, node.active.size ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1, true)); // Wait until the election is removed, at which point the vote is still a replay since it's been recently confirmed ASSERT_TIMELY (3s, 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, true)); // Open new account auto vote_open1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, open1)); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1, true)); ASSERT_EQ (1, node.active.size ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1, true)); ASSERT_TIMELY (3s, 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, true)); ASSERT_EQ (nano::Gxrb_ratio, node.ledger.weight (key.pub)); auto send2 = builder.make_block () @@ -643,27 +643,27 @@ TEST (active_transactions, vote_replays) ASSERT_EQ (1, node.active.size ()); auto vote1_send2 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, send2)); auto vote2_send2 (std::make_shared (key.pub, key.prv, 0, send2)); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2_send2)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2_send2, true)); ASSERT_EQ (1, node.active.size ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2, true)); ASSERT_EQ (1, node.active.size ()); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2, true)); ASSERT_EQ (1, node.active.size ()); - ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2, true)); ASSERT_TIMELY (3s, node.active.empty ()); ASSERT_EQ (0, node.active.size ()); - 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, true)); + ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2, true)); // 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, true)); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote_open1, true)); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote1_send2, true)); + ASSERT_EQ (nano::vote_code::indeterminate, node.active.vote (vote2_send2, true)); } } @@ -1498,7 +1498,7 @@ TEST (active_transactions, conflicting_block_vote_existing_election) ASSERT_EQ (1, node.active.size ()); // Vote for conflicting block, but the block does not yet exist in the ledger - node.active.vote (vote_fork); + node.active.vote (vote_fork, true); // Block now gets processed ASSERT_EQ (nano::process_result::fork, node.process_local (fork).code); @@ -1735,9 +1735,9 @@ TEST (active_transactions, pessimistic_elections) // Make dummy election with winner. { nano::election election1 ( - node, send, [](auto const & block) {}, false, nano::election_behavior::normal); + node, send, [](auto const &) {}, [](auto const &, bool) {}, false, nano::election_behavior::normal); nano::election election2 ( - node, open, [](auto const & block) {}, false, nano::election_behavior::normal); + node, open, [](auto const &) {}, [](auto const &, bool) {}, false, nano::election_behavior::normal); node.active.add_expired_optimistic_election (election1); node.active.add_expired_optimistic_election (election2); } diff --git a/nano/core_test/confirmation_solicitor.cpp b/nano/core_test/confirmation_solicitor.cpp index 0dd1d286..dbcf3290 100644 --- a/nano/core_test/confirmation_solicitor.cpp +++ b/nano/core_test/confirmation_solicitor.cpp @@ -34,12 +34,12 @@ TEST (confirmation_solicitor, batches) nano::lock_guard guard (node2.active.mutex); for (size_t i (0); i < nano::network::confirm_req_hashes_max; ++i) { - auto election (std::make_shared (node2, send, nullptr, false, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, false, nano::election_behavior::normal)); ASSERT_FALSE (solicitor.add (*election)); } ASSERT_EQ (1, solicitor.max_confirm_req_batches); // Reached the maximum amount of requests for the channel - auto election (std::make_shared (node2, send, nullptr, false, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, false, nano::election_behavior::normal)); ASSERT_TRUE (solicitor.add (*election)); // Broadcasting should be immediate ASSERT_EQ (0, node2.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::out)); @@ -75,7 +75,7 @@ TEST (confirmation_solicitor, different_hash) ASSERT_TIMELY (3s, node2.network.size () == 1); auto send (std::make_shared (nano::genesis_hash, nano::keypair ().pub, nano::genesis_amount - 100, nano::dev_genesis_key.prv, nano::dev_genesis_key.pub, *system.work.generate (nano::genesis_hash))); send->sideband_set ({}); - auto election (std::make_shared (node2, send, nullptr, false, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, false, nano::election_behavior::normal)); // Add a vote for something else, not the winner election->last_votes[representative.account] = { std::chrono::steady_clock::now (), 1, 1 }; // Ensure the request and broadcast goes through @@ -111,7 +111,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap) ASSERT_TIMELY (3s, node2.network.size () == 1); auto send (std::make_shared (nano::genesis_hash, nano::keypair ().pub, nano::genesis_amount - 100, nano::dev_genesis_key.prv, nano::dev_genesis_key.pub, *system.work.generate (nano::genesis_hash))); send->sideband_set ({}); - auto election (std::make_shared (node2, send, nullptr, false, nano::election_behavior::normal)); + auto election (std::make_shared (node2, send, nullptr, nullptr, false, nano::election_behavior::normal)); // Add a vote for something else, not the winner for (auto const & rep : representatives) { @@ -125,7 +125,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap) ASSERT_EQ (max_representatives + 1, node2.stats.count (nano::stat::type::message, nano::stat::detail::confirm_req, nano::stat::dir::out)); solicitor.prepare (representatives); - auto election2 (std::make_shared (node2, send, nullptr, false, nano::election_behavior::normal)); + auto election2 (std::make_shared (node2, send, nullptr, nullptr, false, nano::election_behavior::normal)); ASSERT_FALSE (solicitor.add (*election2)); ASSERT_FALSE (solicitor.broadcast (*election2)); diff --git a/nano/core_test/conflicts.cpp b/nano/core_test/conflicts.cpp index a7912336..9c26fcfc 100644 --- a/nano/core_test/conflicts.cpp +++ b/nano/core_test/conflicts.cpp @@ -40,7 +40,7 @@ TEST (conflicts, add_existing) auto election1 = node1.active.insert (send2); ASSERT_EQ (1, node1.active.size ()); auto vote1 (std::make_shared (key2.pub, key2.prv, 0, send2)); - node1.active.vote (vote1); + node1.active.vote (vote1, true); ASSERT_NE (nullptr, election1.election); ASSERT_EQ (2, election1.election->votes ().size ()); auto votes (election1.election->votes ()); diff --git a/nano/core_test/election.cpp b/nano/core_test/election.cpp index f2af540d..a6793ce7 100644 --- a/nano/core_test/election.cpp +++ b/nano/core_test/election.cpp @@ -4,6 +4,8 @@ #include +using namespace std::chrono_literals; + TEST (election, construction) { nano::system system (1); @@ -52,7 +54,7 @@ TEST (election, quorum_minimum_flip_success) ASSERT_NE (nullptr, election.election); ASSERT_EQ (2, election.election->blocks ().size ()); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send2)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); node1.block_processor.flush (); ASSERT_NE (nullptr, node1.block (send2->hash ())); ASSERT_TRUE (election.election->confirmed ()); @@ -96,7 +98,7 @@ TEST (election, quorum_minimum_flip_fail) ASSERT_NE (nullptr, election.election); ASSERT_EQ (2, election.election->blocks ().size ()); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send2)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); node1.block_processor.flush (); ASSERT_NE (nullptr, node1.block (send1->hash ())); ASSERT_FALSE (election.election->confirmed ()); @@ -128,7 +130,7 @@ TEST (election, quorum_minimum_confirm_success) ASSERT_NE (nullptr, election.election); ASSERT_EQ (1, election.election->blocks ().size ()); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); node1.block_processor.flush (); ASSERT_NE (nullptr, node1.block (send1->hash ())); ASSERT_TRUE (election.election->confirmed ()); @@ -160,8 +162,90 @@ TEST (election, quorum_minimum_confirm_fail) ASSERT_NE (nullptr, election.election); ASSERT_EQ (1, election.election->blocks ().size ()); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); node1.block_processor.flush (); ASSERT_NE (nullptr, node1.block (send1->hash ())); ASSERT_FALSE (election.election->confirmed ()); } + +namespace nano +{ +TEST (election, quorum_minimum_update_weight_before_quorum_checks) +{ + nano::system system; + nano::node_config node_config (nano::get_available_port (), system.logging); + node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled; + auto & node1 = *system.add_node (node_config); + system.wallet (0)->insert_adhoc (nano::dev_genesis_key.prv); + auto amount = ((nano::uint256_t (node_config.online_weight_minimum.number ()) * nano::online_reps::online_weight_quorum) / 100).convert_to () - 1; + nano::keypair key1; + nano::block_builder builder; + auto send1 = builder.state () + .account (nano::dev_genesis_key.pub) + .previous (nano::genesis_hash) + .representative (nano::dev_genesis_key.pub) + .balance (amount) + .link (key1.pub) + .work (0) + .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub) + .build_shared (); + node1.work_generate_blocking (*send1); + auto open1 = builder.state () + .account (key1.pub) + .previous (0) + .representative (key1.pub) + .balance (nano::genesis_amount - amount) + .link (send1->hash ()) + .work (0) + .sign (key1.prv, key1.pub) + .build_shared (); + nano::keypair key2; + auto send2 = builder.state () + .account (key1.pub) + .previous (open1->hash ()) + .representative (key1.pub) + .balance (3) + .link (key2.pub) + .work (0) + .sign (key1.prv, key1.pub) + .build_shared (); + node1.work_generate_blocking (*open1); + node1.work_generate_blocking (*send2); + node1.process_active (send1); + node1.block_processor.flush (); + node1.process (*open1); + node1.process (*send2); + node1.block_processor.flush (); + ASSERT_EQ (node1.ledger.cache.block_count, 4); + + node_config.peering_port = nano::get_available_port (); + auto & node2 = *system.add_node (node_config); + node2.process (*send1); + node2.process (*open1); + node2.process (*send2); + system.wallet (1)->insert_adhoc (key1.prv); + node2.block_processor.flush (); + ASSERT_EQ (node2.ledger.cache.block_count, 4); + + auto election{ node1.active.insert (send1) }; + ASSERT_FALSE (election.inserted); + ASSERT_NE (nullptr, election.election); + ASSERT_EQ (1, election.election->blocks ().size ()); + auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), send1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); + auto vote2 (std::make_shared (key1.pub, key1.prv, nano::milliseconds_since_epoch (), send1)); + auto channel = node1.network.find_channel (node2.network.endpoint ()); + ASSERT_NE (channel, nullptr); + ASSERT_TIMELY (10s, !node1.rep_crawler.response (channel, vote2)); + ASSERT_FALSE (election.election->confirmed ()); + { + nano::lock_guard guard (node1.online_reps.mutex); + // 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, true)); + node1.block_processor.flush (); + ASSERT_NE (nullptr, node1.block (send1->hash ())); + ASSERT_TRUE (election.election->confirmed ()); +} +} diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 483a7b02..71a66525 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -790,9 +790,9 @@ TEST (votes, add_one) auto election1 = node1.active.insert (send1); ASSERT_EQ (1, election1.election->votes ().size ()); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); auto vote2 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 2, send1)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2, true)); ASSERT_EQ (2, election1.election->votes ().size ()); auto votes1 (election1.election->votes ()); auto existing1 (votes1.find (nano::dev_genesis_key.pub)); @@ -818,9 +818,9 @@ TEST (votes, add_two) nano::keypair key2; auto send2 (std::make_shared (genesis.hash (), key2.pub, 0, nano::dev_genesis_key.prv, nano::dev_genesis_key.pub, 0)); auto vote2 (std::make_shared (key2.pub, key2.prv, 1, send2)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2, true)); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); ASSERT_EQ (3, election1.election->votes ().size ()); auto votes1 (election1.election->votes ()); ASSERT_NE (votes1.end (), votes1.find (nano::dev_genesis_key.pub)); @@ -855,7 +855,7 @@ TEST (votes, add_existing) ASSERT_EQ (nano::process_result::progress, node1.ledger.process (node1.store.tx_begin_write (), *send1).code); auto election1 = node1.active.insert (send1); auto vote1 (std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1)); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true)); // Block is already processed from vote ASSERT_TRUE (node1.active.publish (send1)); ASSERT_EQ (1, election1.election->last_votes[nano::dev_genesis_key.pub].timestamp); @@ -875,14 +875,14 @@ TEST (votes, add_existing) nano::unique_lock lock (election1.election->mutex); election1.election->last_votes[nano::dev_genesis_key.pub].time = std::chrono::steady_clock::now () - std::chrono::seconds (20); lock.unlock (); - ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2, true)); ASSERT_FALSE (node1.active.publish (send2)); ASSERT_EQ (2, election1.election->last_votes[nano::dev_genesis_key.pub].timestamp); // Also resend the old vote, and see if we respect the timestamp lock.lock (); election1.election->last_votes[nano::dev_genesis_key.pub].time = std::chrono::steady_clock::now () - std::chrono::seconds (20); lock.unlock (); - ASSERT_EQ (nano::vote_code::replay, node1.active.vote (vote1)); + ASSERT_EQ (nano::vote_code::replay, node1.active.vote (vote1, true)); ASSERT_EQ (2, election1.election->votes ()[nano::dev_genesis_key.pub].timestamp); auto votes (election1.election->votes ()); ASSERT_EQ (2, votes.size ()); diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index dd41c021..025f7703 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -3946,7 +3946,7 @@ TEST (node, rollback_vote_self) { ASSERT_EQ (1, election->votes ().size ()); // Vote with key to switch the winner - election->vote (key.pub, 0, fork->hash ()); + election->vote (key.pub, 0, fork->hash (), true); ASSERT_EQ (2, election->votes ().size ()); // The winner changed ASSERT_EQ (election->winner (), fork); diff --git a/nano/core_test/vote_processor.cpp b/nano/core_test/vote_processor.cpp index 8fe35965..52a915ec 100644 --- a/nano/core_test/vote_processor.cpp +++ b/nano/core_test/vote_processor.cpp @@ -209,7 +209,7 @@ TEST (vote_processor, no_broadcast_local) ASSERT_FALSE (node.wallets.reps ().have_half_rep ()); // Process a vote auto vote = std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector{ send->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote, true)); // Make sure the vote was processed auto election (node.active.election (send->qualified_root ())); ASSERT_NE (nullptr, election); @@ -242,7 +242,7 @@ TEST (vote_processor, no_broadcast_local) node.block_confirm (send2); // Process a vote auto vote2 = std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector{ send2->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2, true)); // Make sure the vote was processed auto election2 (node.active.election (send2->qualified_root ())); ASSERT_NE (nullptr, election2); @@ -276,7 +276,7 @@ TEST (vote_processor, no_broadcast_local) ASSERT_TRUE (node.wallets.reps ().have_half_rep ()); // Process a vote auto vote3 = std::make_shared (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector{ open->hash () }); - ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote3)); + ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote3, true)); // Make sure the vote was processed auto election3 (node.active.election (open->qualified_root ())); ASSERT_NE (nullptr, election3); diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 713e1ed0..c3d56390 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -825,7 +825,15 @@ nano::election_insertion_result nano::active_transactions::insert_impl (nano::un } double multiplier (normalized_multiplier (*block_a)); bool prioritized = roots.size () < prioritized_cutoff || multiplier > last_prioritized_multiplier.value_or (0); - result.election = nano::make_shared (node, block_a, confirmation_action_a, prioritized, election_behavior_a); + result.election = nano::make_shared ( + node, block_a, confirmation_action_a, [& node = node](auto const & rep_a, bool rep_is_active_a) { + if (rep_is_active_a) + { + // Representative is defined as online if replying to live votes or rep_crawler queries + node.online_reps.observe (rep_a); + } + }, + prioritized, election_behavior_a); roots.get ().emplace (nano::active_transactions::conflict_info{ root, multiplier, result.election, epoch, previous_balance }); blocks.emplace (hash, result.election); auto const cache = find_inactive_votes_cache_impl (hash); @@ -861,7 +869,7 @@ 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 vote_a) +nano::vote_code nano::active_transactions::vote (std::shared_ptr vote_a, bool active_in_rep_crawler_a) { nano::vote_code result{ nano::vote_code::indeterminate }; // If all hashes were recently confirmed then it is a replay @@ -915,7 +923,7 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr vot bool processed (false); for (auto const & [election, block_hash] : process) { - auto const result_l = election->vote (vote_a->account, vote_a->timestamp, block_hash); + auto const result_l = election->vote (vote_a->account, vote_a->timestamp, block_hash, active_in_rep_crawler_a); processed = processed || result_l.processed; replay = replay || result_l.replay; } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 1474ebc7..e2858f99 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -178,7 +178,7 @@ public: nano::election_insertion_result insert (std::shared_ptr const &, boost::optional const & = boost::none, nano::election_behavior = nano::election_behavior::normal, std::function)> const & = nullptr); // clang-format on // Distinguishes replay votes, cannot be determined if the block is not in any election - nano::vote_code vote (std::shared_ptr); + nano::vote_code vote (std::shared_ptr, bool); // Is the root of this block in the roots container bool active (nano::block const &); bool active (nano::qualified_root const &); diff --git a/nano/node/election.cpp b/nano/node/election.cpp index cb716c16..1d55bcef 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -18,8 +18,9 @@ nano::election_vote_result::election_vote_result (bool replay_a, bool processed_ processed = processed_a; } -nano::election::election (nano::node & node_a, std::shared_ptr block_a, std::function)> const & confirmation_action_a, bool prioritized_a, nano::election_behavior election_behavior_a) : +nano::election::election (nano::node & node_a, std::shared_ptr block_a, std::function)> const & confirmation_action_a, std::function const & pre_confirm_action_a, bool prioritized_a, nano::election_behavior election_behavior_a) : confirmation_action (confirmation_action_a), +pre_confirm_action (pre_confirm_action_a), prioritized_m (prioritized_a), behavior (election_behavior_a), node (node_a), @@ -235,7 +236,8 @@ bool nano::election::have_quorum (nano::tally_t const & tally_a) const ++i; auto second (i != tally_a.end () ? i->first : 0); auto delta_l (node.online_reps.delta ()); - bool result{ tally_a.begin ()->first >= (second + delta_l) }; + release_assert (tally_a.begin ()->first >= second); + bool result{ (tally_a.begin ()->first - second) >= delta_l }; return result; } @@ -254,12 +256,12 @@ nano::tally_t nano::election::tally_impl () const } last_tally = block_weights; nano::tally_t result; - for (auto item : block_weights) + for (auto const & [hash, amount] : block_weights) { - auto block (last_blocks.find (item.first)); + auto block (last_blocks.find (hash)); if (block != last_blocks.end ()) { - result.emplace (item.second, block->second); + result.emplace (amount, block->second); } } return result; @@ -326,9 +328,8 @@ std::shared_ptr nano::election::find (nano::block_hash const & hash return result; } -nano::election_vote_result nano::election::vote (nano::account const & rep, uint64_t timestamp_a, nano::block_hash const & block_hash) +nano::election_vote_result nano::election::vote (nano::account const & rep, uint64_t timestamp_a, nano::block_hash const & block_hash_a, bool rep_is_active_a) { - // see republish_vote documentation for an explanation of these rules auto replay (false); auto online_stake (node.online_reps.trended ()); auto weight (node.ledger.weight (rep)); @@ -359,12 +360,9 @@ nano::election_vote_result nano::election::vote (nano::account const & rep, uint else { auto last_vote_l (last_vote_it->second); - if (last_vote_l.timestamp < timestamp_a || (last_vote_l.timestamp == timestamp_a && last_vote_l.hash < block_hash)) + if (last_vote_l.timestamp < timestamp_a || (last_vote_l.timestamp == timestamp_a && last_vote_l.hash < block_hash_a)) { - if (last_vote_l.time <= std::chrono::steady_clock::now () - std::chrono::seconds (cooldown)) - { - should_process = true; - } + should_process = last_vote_l.time <= std::chrono::steady_clock::now () - std::chrono::seconds (cooldown); } else { @@ -374,7 +372,8 @@ nano::election_vote_result nano::election::vote (nano::account const & rep, uint if (should_process) { node.stats.inc (nano::stat::type::election, nano::stat::detail::vote_new); - last_votes[rep] = { std::chrono::steady_clock::now (), timestamp_a, block_hash }; + last_votes[rep] = { std::chrono::steady_clock::now (), timestamp_a, block_hash_a }; + pre_confirm_action (rep, rep_is_active_a); if (!confirmed ()) { confirm_if_quorum (lock); diff --git a/nano/node/election.hpp b/nano/node/election.hpp index db938c61..6bc0f316 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -54,6 +54,7 @@ class election final : public std::enable_shared_from_this // Minimum time between broadcasts of the current winner of an election, as a backup to requesting confirmations std::chrono::milliseconds base_latency () const; std::function)> confirmation_action; + std::function pre_confirm_action; private: // State management enum class state_t @@ -102,9 +103,9 @@ public: // Status nano::election_status status; public: // Interface - election (nano::node &, std::shared_ptr, std::function)> const &, bool, nano::election_behavior); + election (nano::node &, std::shared_ptr, std::function)> const &, std::function const &, bool, nano::election_behavior); std::shared_ptr find (nano::block_hash const &) const; - nano::election_vote_result vote (nano::account const &, uint64_t, nano::block_hash const &); + nano::election_vote_result vote (nano::account const &, uint64_t, nano::block_hash const &, bool); bool publish (std::shared_ptr const & block_a); size_t insert_inactive_votes_cache (nano::inactive_cache_information const &); // Confirm this block if quorum is met diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 6b0c9f94..f6a22b5a 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -108,8 +108,8 @@ bootstrap_initiator (*this), bootstrap (config.peering_port, *this), application_path (application_path_a), port_mapping (*this), -vote_processor (checker, active, observers, stats, config, flags, logger, online_reps, ledger, network_params), rep_crawler (*this), +vote_processor (checker, active, observers, stats, config, flags, logger, online_reps, rep_crawler, ledger, network_params), warmed_up (0), block_processor (*this, write_database_queue), // clang-format off @@ -299,17 +299,15 @@ node_seq (seq) }); observers.vote.add ([this](std::shared_ptr vote_a, std::shared_ptr channel_a, nano::vote_code code_a) { debug_assert (code_a != nano::vote_code::invalid); - if (code_a != nano::vote_code::replay) + // The vote_code::vote is handled inside the election + if (code_a == nano::vote_code::indeterminate) { auto active_in_rep_crawler (!this->rep_crawler.response (channel_a, vote_a)); - if (active_in_rep_crawler || code_a == nano::vote_code::vote) + if (active_in_rep_crawler) { // Representative is defined as online if replying to live votes or rep_crawler queries this->online_reps.observe (vote_a->account); } - } - if (code_a == nano::vote_code::indeterminate) - { this->gap_cache.vote (vote_a); } }); diff --git a/nano/node/node.hpp b/nano/node/node.hpp index c2635d6c..275d4657 100644 --- a/nano/node/node.hpp +++ b/nano/node/node.hpp @@ -184,13 +184,13 @@ public: boost::filesystem::path application_path; nano::node_observers observers; nano::port_mapping port_mapping; - nano::vote_processor vote_processor; + nano::online_reps online_reps; nano::rep_crawler rep_crawler; + nano::vote_processor vote_processor; unsigned warmed_up; nano::block_processor block_processor; std::thread block_processor_thread; nano::block_arrival block_arrival; - nano::online_reps online_reps; nano::local_vote_history history; nano::keypair node_id; nano::block_uniquer block_uniquer; diff --git a/nano/node/online_reps.cpp b/nano/node/online_reps.cpp index 0108d2d4..e901d89a 100644 --- a/nano/node/online_reps.cpp +++ b/nano/node/online_reps.cpp @@ -96,9 +96,9 @@ nano::uint128_t nano::online_reps::online () const nano::uint128_t nano::online_reps::delta () const { nano::lock_guard lock (mutex); - auto weight = std::max ({ online_m, trended_m, config.online_weight_minimum.number () }); - auto result ((weight / 100) * online_weight_quorum); - return result; + // Using a larger container to ensure maximum precision + auto weight = static_cast (std::max ({ online_m, trended_m, config.online_weight_minimum.number () })); + return ((weight * online_weight_quorum) / 100).convert_to (); } std::vector nano::online_reps::list () diff --git a/nano/node/online_reps.hpp b/nano/node/online_reps.hpp index aa28119e..c9c30aab 100644 --- a/nano/node/online_reps.hpp +++ b/nano/node/online_reps.hpp @@ -68,6 +68,7 @@ private: nano::uint128_t online_m; nano::uint128_t minimum; + friend class election_quorum_minimum_update_weight_before_quorum_checks_Test; friend std::unique_ptr collect_container_info (online_reps & online_reps, const std::string & name); }; diff --git a/nano/node/repcrawler.cpp b/nano/node/repcrawler.cpp index 7e762e6b..16cb078d 100644 --- a/nano/node/repcrawler.cpp +++ b/nano/node/repcrawler.cpp @@ -179,7 +179,7 @@ bool nano::rep_crawler::is_pr (nano::transport::channel const & channel_a) const return result; } -bool nano::rep_crawler::response (std::shared_ptr & channel_a, std::shared_ptr & vote_a) +bool nano::rep_crawler::response (std::shared_ptr const & channel_a, std::shared_ptr const & vote_a) { bool error = true; nano::lock_guard lock (active_mutex); diff --git a/nano/node/repcrawler.hpp b/nano/node/repcrawler.hpp index 7d8a2974..e14bfbab 100644 --- a/nano/node/repcrawler.hpp +++ b/nano/node/repcrawler.hpp @@ -93,11 +93,11 @@ public: bool is_pr (nano::transport::channel const &) const; /** - * Called when a non-replay vote on a block previously sent by query() is received. This indiciates + * Called when a non-replay vote on a block previously sent by query() is received. This indicates * with high probability that the endpoint is a representative node. * @return false if the vote corresponded to any active hash. */ - bool response (std::shared_ptr &, std::shared_ptr &); + bool response (std::shared_ptr const &, std::shared_ptr const &); /** Get total available weight from representatives */ nano::uint128_t total_weight () const; diff --git a/nano/node/vote_processor.cpp b/nano/node/vote_processor.cpp index 8e7dec94..ed86c05d 100644 --- a/nano/node/vote_processor.cpp +++ b/nano/node/vote_processor.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -14,7 +15,7 @@ #include -nano::vote_processor::vote_processor (nano::signature_checker & checker_a, nano::active_transactions & active_a, nano::node_observers & observers_a, nano::stat & stats_a, nano::node_config & config_a, nano::node_flags & flags_a, nano::logger_mt & logger_a, nano::online_reps & online_reps_a, nano::ledger & ledger_a, nano::network_params & network_params_a) : +nano::vote_processor::vote_processor (nano::signature_checker & checker_a, nano::active_transactions & active_a, nano::node_observers & observers_a, nano::stat & stats_a, nano::node_config & config_a, nano::node_flags & flags_a, nano::logger_mt & logger_a, nano::online_reps & online_reps_a, nano::rep_crawler & rep_crawler_a, nano::ledger & ledger_a, nano::network_params & network_params_a) : checker (checker_a), active (active_a), observers (observers_a), @@ -22,6 +23,7 @@ stats (stats_a), config (config_a), logger (logger_a), online_reps (online_reps_a), +rep_crawler (rep_crawler_a), ledger (ledger_a), network_params (network_params_a), max_votes (flags_a.vote_processor_capacity), @@ -170,7 +172,7 @@ nano::vote_code nano::vote_processor::vote_blocking (std::shared_ptr auto result (nano::vote_code::invalid); if (validated || !vote_a->validate ()) { - result = active.vote (vote_a); + result = active.vote (vote_a, !rep_crawler.response (channel_a, vote_a)); observers.vote.notify (vote_a, channel_a, result); } std::string status; diff --git a/nano/node/vote_processor.hpp b/nano/node/vote_processor.hpp index e0df2681..d9707123 100644 --- a/nano/node/vote_processor.hpp +++ b/nano/node/vote_processor.hpp @@ -20,6 +20,7 @@ class stats; class node_config; class logger_mt; class online_reps; +class rep_crawler; class ledger; class network_params; class node_flags; @@ -33,7 +34,7 @@ namespace transport class vote_processor final { public: - explicit vote_processor (nano::signature_checker & checker_a, nano::active_transactions & active_a, nano::node_observers & observers_a, nano::stat & stats_a, nano::node_config & config_a, nano::node_flags & flags_a, nano::logger_mt & logger_a, nano::online_reps & online_reps_a, nano::ledger & ledger_a, nano::network_params & network_params_a); + explicit vote_processor (nano::signature_checker & checker_a, nano::active_transactions & active_a, nano::node_observers & observers_a, nano::stat & stats_a, nano::node_config & config_a, nano::node_flags & flags_a, nano::logger_mt & logger_a, nano::online_reps & online_reps_a, nano::rep_crawler & rep_crawler_a, nano::ledger & ledger_a, nano::network_params & network_params_a); /** Returns false if the vote was processed */ bool vote (std::shared_ptr, std::shared_ptr); /** Note: node.active.mutex lock is required */ @@ -58,6 +59,7 @@ private: nano::node_config & config; nano::logger_mt & logger; nano::online_reps & online_reps; + nano::rep_crawler & rep_crawler; nano::ledger & ledger; nano::network_params & network_params; size_t max_votes;