Vote spacing (#2763)

* 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.
Co-authored-by: Russel <russel@nano.org>

nano/core_test/voting.cpp

@@ -119,3 +119,117 @@ TEST (vote_generator, session)
 	thread.join ();
 	ASSERT_TIMELY (2s, 1 == node->stats.count (nano::stat::type::vote, nano::stat::detail::vote_indeterminate));
 }
+
+TEST (vote_spacing, basic)
+{
+	nano::vote_spacing spacing{ std::chrono::milliseconds{ 100 } };
+	nano::root root1{ 1 };
+	nano::root root2{ 2 };
+	nano::block_hash hash3{ 3 };
+	nano::block_hash hash4{ 4 };
+	nano::block_hash hash5{ 5 };
+	ASSERT_EQ (0, spacing.size ());
+	ASSERT_TRUE (spacing.votable (root1, hash3));
+	spacing.flag (root1, hash3);
+	ASSERT_EQ (1, spacing.size ());
+	ASSERT_TRUE (spacing.votable (root1, hash3));
+	ASSERT_FALSE (spacing.votable (root1, hash4));
+	spacing.flag (root2, hash5);
+	ASSERT_EQ (2, spacing.size ());
+}
+
+TEST (vote_spacing, prune)
+{
+	auto length = std::chrono::milliseconds{ 100 };
+	nano::vote_spacing spacing{ length };
+	nano::root root1{ 1 };
+	nano::root root2{ 2 };
+	nano::block_hash hash3{ 3 };
+	nano::block_hash hash4{ 4 };
+	spacing.flag (root1, hash3);
+	ASSERT_EQ (1, spacing.size ());
+	std::this_thread::sleep_for (length);
+	spacing.flag (root2, hash4);
+	ASSERT_EQ (1, spacing.size ());
+}
+
+TEST (vote_spacing, vote_generator)
+{
+	nano::node_config config;
+	config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
+	nano::system system;
+	auto & node = *system.add_node (config);
+	auto & wallet = *system.wallet (0);
+	wallet.insert_adhoc (nano::dev_genesis_key.prv);
+	nano::state_block_builder builder;
+	auto send1 = builder.make_block ()
+	             .account (nano::dev_genesis_key.pub)
+	             .previous (nano::genesis_hash)
+	             .representative (nano::dev_genesis_key.pub)
+	             .balance (nano::genesis_amount - nano::Gxrb_ratio)
+	             .link (nano::dev_genesis_key.pub)
+	             .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+	             .work (*system.work.generate (nano::genesis_hash))
+	             .build_shared ();
+	auto send2 = builder.make_block ()
+	             .account (nano::dev_genesis_key.pub)
+	             .previous (nano::genesis_hash)
+	             .representative (nano::dev_genesis_key.pub)
+	             .balance (nano::genesis_amount - nano::Gxrb_ratio - 1)
+	             .link (nano::dev_genesis_key.pub)
+	             .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+	             .work (*system.work.generate (nano::genesis_hash))
+	             .build_shared ();
+	ASSERT_EQ (nano::process_result::progress, node.ledger.process (node.store.tx_begin_write (), *send1).code);
+	ASSERT_EQ (0, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
+	node.active.generator.add (nano::genesis_hash, send1->hash ());
+	ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts) == 1);
+	ASSERT_FALSE (node.ledger.rollback (node.store.tx_begin_write (), send1->hash ()));
+	ASSERT_EQ (nano::process_result::progress, node.ledger.process (node.store.tx_begin_write (), *send2).code);
+	node.active.generator.add (nano::genesis_hash, send2->hash ());
+	ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing) == 1);
+	ASSERT_EQ (1, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
+	std::this_thread::sleep_for (config.network_params.voting.delay);
+	node.active.generator.add (nano::genesis_hash, send2->hash ());
+	ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts) == 2);
+}
+
+TEST (vote_spacing, rapid)
+{
+	nano::node_config config;
+	config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
+	nano::system system;
+	auto & node = *system.add_node (config);
+	auto & wallet = *system.wallet (0);
+	wallet.insert_adhoc (nano::dev_genesis_key.prv);
+	nano::state_block_builder builder;
+	auto send1 = builder.make_block ()
+				 .account (nano::dev_genesis_key.pub)
+				 .previous (nano::genesis_hash)
+				 .representative (nano::dev_genesis_key.pub)
+				 .balance (nano::genesis_amount - nano::Gxrb_ratio)
+				 .link (nano::dev_genesis_key.pub)
+				 .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+				 .work (*system.work.generate (nano::genesis_hash))
+				 .build_shared ();
+	auto send2 = builder.make_block ()
+				 .account (nano::dev_genesis_key.pub)
+				 .previous (nano::genesis_hash)
+				 .representative (nano::dev_genesis_key.pub)
+				 .balance (nano::genesis_amount - nano::Gxrb_ratio - 1)
+				 .link (nano::dev_genesis_key.pub)
+				 .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+				 .work (*system.work.generate (nano::genesis_hash))
+				 .build_shared ();
+	ASSERT_EQ (nano::process_result::progress, node.ledger.process (node.store.tx_begin_write (), *send1).code);
+	node.active.generator.add (nano::genesis_hash, send1->hash ());
+	ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts) == 1);
+	ASSERT_FALSE (node.ledger.rollback (node.store.tx_begin_write (), send1->hash ()));
+	ASSERT_EQ (nano::process_result::progress, node.ledger.process (node.store.tx_begin_write (), *send2).code);
+	node.active.generator.add (nano::genesis_hash, send2->hash ());
+	ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing) == 1);
+	ASSERT_TIMELY (3s, 1 == node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts));
+	std::this_thread::sleep_for (config.network_params.voting.delay);
+	node.active.generator.add (nano::genesis_hash, send2->hash ());
+	ASSERT_TIMELY (3s, node.stats.count (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts) == 2);
+}

nano/lib/stats.cpp

@@ -760,6 +760,9 @@ std::string nano::stat::detail_to_string (uint32_t key)
 		case nano::stat::detail::generator_replies_discarded:
 			res = "generator_replies_discarded";
 			break;
+		case nano::stat::detail::generator_spacing:
+			res = "generator_spacing";
+			break;
 	}
 	return res;
 }

nano/lib/stats.hpp

@@ -342,7 +342,8 @@ public:
 		// vote generator
 		generator_broadcasts,
 		generator_replies,
-		generator_replies_discarded
+		generator_replies_discarded,
+		generator_spacing
 	};
 
 	/** Direction of the stat. If the direction is irrelevant, use in */

nano/node/voting.cpp

@@ -11,6 +11,44 @@
 
 #include <chrono>
 
+void nano::vote_spacing::trim ()
+{
+	recent.get<tag_time> ().erase (recent.get<tag_time> ().begin (), recent.get<tag_time> ().upper_bound (std::chrono::steady_clock::now () - delay));
+}
+
+bool nano::vote_spacing::votable (nano::root const & root_a, nano::block_hash const & hash_a) const
+{
+	bool result = true;
+	for (auto range = recent.get<tag_root> ().equal_range (root_a); result && range.first != range.second; ++range.first)
+	{
+		auto & item = *range.first;
+		result = hash_a == item.hash || item.time < std::chrono::steady_clock::now () - delay;
+	}
+	return result;
+}
+
+void nano::vote_spacing::flag (nano::root const & root_a, nano::block_hash const & hash_a)
+{
+	trim ();
+	auto now = std::chrono::steady_clock::now ();
+	auto existing = recent.get<tag_root> ().find (root_a);
+	if (existing != recent.end ())
+	{
+		recent.get<tag_root> ().modify (existing, [now] (entry & entry) {
+			entry.time = now;
+		});
+	}
+	else
+	{
+		recent.insert ({ root_a, now, hash_a });
+	}
+}
+
+size_t nano::vote_spacing::size () const
+{
+	return recent.size ();
+}
+
 bool nano::local_vote_history::consistency_check (nano::root const & root_a) const
 {
 	auto & history_by_root (history.get<tag_root> ());
@@ -117,6 +155,7 @@ ledger (ledger_a),
 wallets (wallets_a),
 vote_processor (vote_processor_a),
 history (history_a),
+spacing{ config_a.network_params.voting.delay },
 network (network_a),
 stats (stats_a),
 thread ([this]() { run (); })
@@ -218,18 +257,27 @@ void nano::vote_generator::broadcast (nano::unique_lock<std::mutex> & lock_a)
 		}
 		if (cached_votes.empty () && std::find (roots.begin (), roots.end (), root) == roots.end ())
 		{
-			roots.push_back (root);
+			if (spacing.votable (root, hash))
-			hashes.push_back (hash);
+			{
+				roots.push_back (root);
+				hashes.push_back (hash);
+			}
+			else
+			{
+				stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing);
+			}
 		}
 		candidates.pop_front ();
 	}
 	if (!hashes.empty ())
 	{
 		lock_a.unlock ();
-		vote (hashes, roots, [this](auto const & vote_a) { this->broadcast_action (vote_a); });
+		vote (hashes, roots, [this](auto const & vote_a) {
+			this->broadcast_action (vote_a);
+			this->stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts);
+		});
 		lock_a.lock ();
 	}
-	stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_broadcasts);
 }
 
 void nano::vote_generator::reply (nano::unique_lock<std::mutex> & lock_a, request_t && request_a)
@@ -260,8 +308,15 @@ void nano::vote_generator::reply (nano::unique_lock<std::mutex> & lock_a, reques
 			}
 			if (cached_votes.empty () && std::find (roots.begin (), roots.end (), root) == roots.end ())
 			{
-				roots.push_back (i->first);
+				if (spacing.votable (root, hash))
-				hashes.push_back (i->second);
+				{
+					roots.push_back (root);
+					hashes.push_back (hash);
+				}
+				else
+				{
+					stats.inc (nano::stat::type::vote_generator, nano::stat::detail::generator_spacing);
+				}
 			}
 		}
 		if (!hashes.empty ())
@@ -289,6 +344,7 @@ void nano::vote_generator::vote (std::vector<nano::block_hash> const & hashes_a,
 		for (size_t i (0), n (hashes_a.size ()); i != n; ++i)
 		{
 			history.add (roots_a[i], hashes_a[i], vote_l);
+			spacing.flag (roots_a[i], hashes_a[i]);
 		}
 		action_a (vote_l);
 	}

nano/node/voting.hpp

@@ -7,6 +7,7 @@
 #include <nano/secure/common.hpp>
 
 #include <boost/multi_index/hashed_index.hpp>
+#include <boost/multi_index/ordered_index.hpp>
 #include <boost/multi_index/member.hpp>
 #include <boost/multi_index/sequenced_index.hpp>
 #include <boost/multi_index_container.hpp>
@@ -31,6 +32,34 @@ namespace transport
 	class channel;
 }
 
+class vote_spacing final
+{
+	class entry
+	{
+	public:
+		nano::root root;
+		std::chrono::steady_clock::time_point time;
+		nano::block_hash hash;
+	};
+	
+	boost::multi_index_container<entry,
+	mi::indexed_by<
+		mi::hashed_non_unique<mi::tag<class tag_root>,
+			mi::member<entry, nano::root, &entry::root>>,
+		mi::ordered_non_unique<mi::tag<class tag_time>,
+			mi::member<entry, std::chrono::steady_clock::time_point, &entry::time>>
+	>>
+	recent;
+	std::chrono::milliseconds const delay;
+	void trim ();
+public:
+	vote_spacing (std::chrono::milliseconds const & delay) :
+	delay{ delay } {}
+	bool votable (nano::root const & root_a, nano::block_hash const & hash_a) const;
+	void flag (nano::root const & root_a, nano::block_hash const & hash_a);
+	size_t size () const;
+};
+
 class local_vote_history final
 {
 	class local_vote final
@@ -77,7 +106,6 @@ private:
 	mutable std::mutex mutex;
 
 	friend std::unique_ptr<container_info_component> collect_container_info (local_vote_history & history, const std::string & name);
-
 	friend class local_vote_history_basic_Test;
 };
 
@@ -110,6 +138,7 @@ private:
 	nano::wallets & wallets;
 	nano::vote_processor & vote_processor;
 	nano::local_vote_history & history;
+	nano::vote_spacing spacing;
 	nano::network & network;
 	nano::stat & stats;
 	mutable std::mutex mutex;