From d391b18e1097348e2503c2ce56fc060f7a30ee4b Mon Sep 17 00:00:00 2001 From: clemahieu Date: Wed, 16 Dec 2020 19:33:46 +0000 Subject: [PATCH] Moving voting constants in to their own class (#3071) * Currently there is no rate limiter on vote generation for a particular root and this can lead to increased, unnecessary vote traffic. This patch adds a delay between constructing non-cached votes. * Removing check on hash since we won't produce a new vote if there's an existing one. * Moving delay in to voting_constants and referencing voting_constants from local_vote_history instead of copying values on construction. * Erasing duplicate roots. * Adding test for correct broadcast deduplication. * Retaining historical blocks until election completion. history::votes filters by root+hash so it won't pick up votes for hashes it's not looking for. * Moving vote spacing to its own data structure. * Testing vote_spacing class and adding trimming. * Revert "Retaining historical blocks until election completion. history::votes filters by root+hash so it won't pick up votes for hashes it's not looking for." This reverts commit 9890305eaeb745b4c86bac36b8ad19d55975c61d. Co-authored-by: Russel --- nano/core_test/voting.cpp | 3 ++- nano/node/node.cpp | 1 + nano/node/voting.cpp | 7 ++++--- nano/node/voting.hpp | 6 +++++- nano/secure/common.cpp | 5 +++-- nano/secure/common.hpp | 3 ++- 6 files changed, 17 insertions(+), 8 deletions(-) diff --git a/nano/core_test/voting.cpp b/nano/core_test/voting.cpp index b7a9f0dc..f0fc36ed 100644 --- a/nano/core_test/voting.cpp +++ b/nano/core_test/voting.cpp @@ -11,7 +11,8 @@ namespace nano { TEST (local_vote_history, basic) { - nano::local_vote_history history; + nano::network_params params; + nano::local_vote_history history{ params.voting }; ASSERT_FALSE (history.exists (1)); ASSERT_FALSE (history.exists (2)); ASSERT_TRUE (history.votes (1).empty ()); diff --git a/nano/node/node.cpp b/nano/node/node.cpp index da8a47e0..7d036fcb 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -119,6 +119,7 @@ block_processor_thread ([this]() { }), // clang-format on online_reps (ledger, config), +history{ config.network_params.voting }, vote_uniquer (block_uniquer), confirmation_height_processor (ledger, write_database_queue, config.conf_height_processor_batch_min_time, config.logging, logger, node_initialized_latch, flags.confirmation_height_processor_mode), active (*this, confirmation_height_processor), diff --git a/nano/node/voting.cpp b/nano/node/voting.cpp index f0393068..f8b0d868 100644 --- a/nano/node/voting.cpp +++ b/nano/node/voting.cpp @@ -87,9 +87,9 @@ bool nano::local_vote_history::exists (nano::root const & root_a) const void nano::local_vote_history::clean () { - debug_assert (max_size > 0); + debug_assert (constants.max_cache > 0); auto & history_by_sequence (history.get ()); - while (history_by_sequence.size () > max_size) + while (history_by_sequence.size () > constants.max_cache) { history_by_sequence.erase (history_by_sequence.begin ()); } @@ -247,7 +247,8 @@ void nano::vote_generator::reply (nano::unique_lock & lock_a, reques roots.reserve (nano::network::confirm_ack_hashes_max); for (; i != n && hashes.size () < nano::network::confirm_ack_hashes_max; ++i) { - auto cached_votes = history.votes (i->first, i->second); + auto const & [root, hash] = *i; + auto cached_votes = history.votes (root, hash); for (auto const & cached_vote : cached_votes) { if (cached_sent.insert (cached_vote).second) diff --git a/nano/node/voting.hpp b/nano/node/voting.hpp index 03ee6ae5..09502e17 100644 --- a/nano/node/voting.hpp +++ b/nano/node/voting.hpp @@ -48,6 +48,10 @@ class local_vote_history final }; public: + local_vote_history (nano::voting_constants const & constants) : + constants{ constants } + { + } void add (nano::root const & root_a, nano::block_hash const & hash_a, std::shared_ptr const & vote_a); void erase (nano::root const & root_a); @@ -65,7 +69,7 @@ private: history; // clang-format on - size_t const max_size{ nano::network_params{}.voting.max_cache }; + nano::voting_constants const & constants; void clean (); std::vector> votes (nano::root const & root_a) const; // Only used in Debug diff --git a/nano/secure/common.cpp b/nano/secure/common.cpp index 630511a5..06d6078e 100644 --- a/nano/secure/common.cpp +++ b/nano/secure/common.cpp @@ -155,9 +155,10 @@ nano::node_constants::node_constants (nano::network_constants & network_constant weight_period = 5 * 60; // 5 minutes } -nano::voting_constants::voting_constants (nano::network_constants & network_constants) +nano::voting_constants::voting_constants (nano::network_constants & network_constants) : +max_cache{ network_constants.is_dev_network () ? 256U : 128U * 1024 }, +delay{ network_constants.is_dev_network () ? 1 : 15 } { - max_cache = network_constants.is_dev_network () ? 256 : 128 * 1024; } nano::portmapping_constants::portmapping_constants (nano::network_constants & network_constants) diff --git a/nano/secure/common.hpp b/nano/secure/common.hpp index a0515b2f..9696ca32 100644 --- a/nano/secure/common.hpp +++ b/nano/secure/common.hpp @@ -428,7 +428,8 @@ class voting_constants { public: voting_constants (nano::network_constants & network_constants); - size_t max_cache; + size_t const max_cache; + std::chrono::seconds const delay; }; /** Port-mapping related constants whose value depends on the active network */