Fix online reps live votes update (#3084)

* Fix online reps live votes update Representative is defined as online if: - replying to live votes (elections) or - replying to rep_crawler queries * Remove bool, and assume callback is always for live votes Co-authored-by Wesley Shillingford <[email protected]> Co-authored-by: Wesley Shillingford <[email protected]>
nanocurrency · Jan 28, 2021 · 6f4ca51 · 6f4ca51
1 parent bec9e21
commit 6f4ca51
Show file tree

Hide file tree

Showing 12 changed files with 104 additions and 54 deletions.
diff --git 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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, send1));
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_send1));
 	ASSERT_EQ (2, node.active.size ());
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1));
 	// 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, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_send1));
 	// Open new account
 	auto vote_open1 (std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, open1));
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote_open1));
 	ASSERT_EQ (1, node.active.size ());
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1));
 	ASSERT_TIMELY (3s, node.active.empty ());
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote_open1));
 	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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 0, send2));
 	auto vote2_send2 (std::make_shared<nano::vote> (key.pub, key.prv, 0, send2));
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2_send2, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2_send2));
 	ASSERT_EQ (1, node.active.size ());
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2));
 	ASSERT_EQ (1, node.active.size ());
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote1_send2));
 	ASSERT_EQ (1, node.active.size ());
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2));
 	ASSERT_TIMELY (3s, node.active.empty ());
 	ASSERT_EQ (0, node.active.size ());
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2, true));
-	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2, true));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote1_send2));
+	ASSERT_EQ (nano::vote_code::replay, node.active.vote (vote2_send2));
 
 	// Removing blocks as recently confirmed makes every vote indeterminate
 	{
 		nano::lock_guard<std::mutex> guard (node.active.mutex);
 		node.active.recently_confirmed.clear ();
 	}
-	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));
+	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));
 }
 }
 
@@ -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, true);
+	node.active.vote (vote_fork);
 
 	// 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 &) {}, [](auto const &, bool) {}, false, nano::election_behavior::normal);
+		node, send, [](auto const &) {}, [](auto const &) {}, false, nano::election_behavior::normal);
 		nano::election election2 (
-		node, open, [](auto const &) {}, [](auto const &, bool) {}, false, nano::election_behavior::normal);
+		node, open, [](auto const &) {}, [](auto const &) {}, 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/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<nano::vote> (key2.pub, key2.prv, 0, send2));
-	node1.active.vote (vote1, true);
+	node1.active.vote (vote1);
 	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
@@ -54,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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send2));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	node1.block_processor.flush ();
 	ASSERT_NE (nullptr, node1.block (send2->hash ()));
 	ASSERT_TRUE (election.election->confirmed ());
@@ -98,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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send2));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	node1.block_processor.flush ();
 	ASSERT_NE (nullptr, node1.block (send1->hash ()));
 	ASSERT_FALSE (election.election->confirmed ());
@@ -130,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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	node1.block_processor.flush ();
 	ASSERT_NE (nullptr, node1.block (send1->hash ()));
 	ASSERT_TRUE (election.election->confirmed ());
@@ -162,7 +162,7 @@ TEST (election, quorum_minimum_confirm_fail)
 	ASSERT_NE (nullptr, election.election);
 	ASSERT_EQ (1, election.election->blocks ().size ());
 	auto vote1 (std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	node1.block_processor.flush ();
 	ASSERT_NE (nullptr, node1.block (send1->hash ()));
 	ASSERT_FALSE (election.election->confirmed ());
@@ -232,7 +232,7 @@ TEST (election, quorum_minimum_update_weight_before_quorum_checks)
 	ASSERT_NE (nullptr, election.election);
 	ASSERT_EQ (1, election.election->blocks ().size ());
 	auto vote1 (std::make_shared<nano::vote> (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));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	auto vote2 (std::make_shared<nano::vote> (key1.pub, key1.prv, nano::milliseconds_since_epoch (), send1));
 	auto channel = node1.network.find_channel (node2.network.endpoint ());
 	ASSERT_NE (channel, nullptr);
@@ -243,7 +243,7 @@ TEST (election, quorum_minimum_update_weight_before_quorum_checks)
 		// 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));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2));
 	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
@@ -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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	auto vote2 (std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 2, send1));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2));
 	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<nano::send_block> (genesis.hash (), key2.pub, 0, nano::dev_genesis_key.prv, nano::dev_genesis_key.pub, 0));
 	auto vote2 (std::make_shared<nano::vote> (key2.pub, key2.prv, 1, send2));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2));
 	auto vote1 (std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, 1, send1));
-	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote1));
 	// 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<std::mutex> 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, true));
+	ASSERT_EQ (nano::vote_code::vote, node1.active.vote (vote2));
 	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, true));
+	ASSERT_EQ (nano::vote_code::replay, node1.active.vote (vote1));
 	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
@@ -2432,6 +2432,58 @@ TEST (node, online_reps)
 	ASSERT_EQ (node1.config.online_weight_minimum, node1.online_reps.trended ());
 }
 
+namespace nano
+{
+TEST (node, online_reps_rep_crawler)
+{
+	nano::system system;
+	nano::node_flags flags;
+	flags.disable_rep_crawler = true;
+	auto & node1 = *system.add_node (flags);
+	auto vote = std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector<nano::block_hash>{ nano::genesis_hash });
+	ASSERT_EQ (0, node1.online_reps.online ());
+	// Without rep crawler
+	node1.vote_processor.vote_blocking (vote, std::make_shared<nano::transport::channel_loopback> (node1));
+	ASSERT_EQ (0, node1.online_reps.online ());
+	// After inserting to rep crawler
+	{
+		nano::lock_guard<std::mutex> guard (node1.rep_crawler.probable_reps_mutex);
+		node1.rep_crawler.active.insert (nano::genesis_hash);
+	}
+	node1.vote_processor.vote_blocking (vote, std::make_shared<nano::transport::channel_loopback> (node1));
+	ASSERT_EQ (nano::genesis_amount, node1.online_reps.online ());
+}
+}
+
+TEST (node, online_reps_election)
+{
+	nano::system system;
+	nano::node_flags flags;
+	flags.disable_rep_crawler = true;
+	auto & node1 = *system.add_node (flags);
+	// Start election
+	nano::genesis genesis;
+	nano::keypair key;
+	nano::state_block_builder builder;
+	auto send1 = builder.make_block ()
+	             .account (nano::dev_genesis_key.pub)
+	             .previous (genesis.hash ())
+	             .representative (nano::dev_genesis_key.pub)
+	             .balance (nano::genesis_amount - nano::Gxrb_ratio)
+	             .link (key.pub)
+	             .sign (nano::dev_genesis_key.prv, nano::dev_genesis_key.pub)
+	             .work (*node1.work_generate_blocking (genesis.hash ()))
+	             .build_shared ();
+	node1.process_active (send1);
+	node1.block_processor.flush ();
+	ASSERT_EQ (1, node1.active.size ());
+	// Process vote for ongoing election
+	auto vote = std::make_shared<nano::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector<nano::block_hash>{ send1->hash () });
+	ASSERT_EQ (0, node1.online_reps.online ());
+	node1.vote_processor.vote_blocking (vote, std::make_shared<nano::transport::channel_loopback> (node1));
+	ASSERT_EQ (nano::genesis_amount - nano::Gxrb_ratio, node1.online_reps.online ());
+}
+
 TEST (node, block_confirm)
 {
 	std::vector<nano::transport::transport_type> types{ nano::transport::transport_type::tcp, nano::transport::transport_type::udp };
@@ -3946,7 +3998,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 (), true);
+			election->vote (key.pub, 0, fork->hash ());
 			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
@@ -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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector<nano::block_hash>{ send->hash () });
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote));
 	// 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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector<nano::block_hash>{ send2->hash () });
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote2));
 	// 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::vote> (nano::dev_genesis_key.pub, nano::dev_genesis_key.prv, nano::milliseconds_since_epoch (), std::vector<nano::block_hash>{ open->hash () });
-	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote3, true));
+	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote3));
 	// 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
@@ -826,12 +826,9 @@ 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<nano::election> (
-				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);
-					}
+				node, block_a, confirmation_action_a, [& node = node](auto const & rep_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<tag_root> ().emplace (nano::active_transactions::conflict_info{ root, multiplier, result.election, epoch, previous_balance });
@@ -869,7 +866,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<nano::vote> const & vote_a, bool active_in_rep_crawler_a)
+nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> const & vote_a)
 {
 	nano::vote_code result{ nano::vote_code::indeterminate };
 	// If all hashes were recently confirmed then it is a replay
@@ -923,7 +920,7 @@ nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> con
 		bool processed (false);
 		for (auto const & [election, block_hash] : process)
 		{
-			auto const result_l = election->vote (vote_a->account, vote_a->timestamp, block_hash, active_in_rep_crawler_a);
+			auto const result_l = election->vote (vote_a->account, vote_a->timestamp, block_hash);
 			processed = processed || result_l.processed;
 			replay = replay || result_l.replay;
 		}

diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp
@@ -188,7 +188,7 @@ class active_transactions final
 	nano::election_insertion_result insert (std::shared_ptr<nano::block> const &, boost::optional<nano::uint128_t> const & = boost::none, nano::election_behavior = nano::election_behavior::normal, std::function<void(std::shared_ptr<nano::block> const&)> 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> const &, bool);
+	nano::vote_code vote (std::shared_ptr<nano::vote> const &);
 	// Is the root of this block in the roots container
 	bool active (nano::block const &);
 	bool active (nano::qualified_root const &);