Wallet representative counts consistency (#2438)

* Wallet representative counts consistency

* Make counts_mutex private

* Format

* Wrap in a struct rather than using std::pair

nano/core_test/node.cpp

@@ -1751,7 +1751,7 @@ TEST (node, rep_self_vote)
 	}
 	system.wallet (0)->insert_adhoc (rep_big.prv);
 	system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
-	ASSERT_EQ (system.wallet (0)->wallets.reps_count, 2);
+	ASSERT_EQ (system.wallet (0)->wallets.rep_counts ().voting, 2);
 	auto block0 (std::make_shared<nano::send_block> (node0->latest (nano::test_genesis_key.pub), rep_big.pub, nano::uint128_t ("0x60000000000000000000000000000000"), nano::test_genesis_key.prv, nano::test_genesis_key.pub, 0));
 	node0->work_generate_blocking (*block0);
 	ASSERT_EQ (nano::process_result::progress, node0->process (*block0).code);

nano/node/active_transactions.cpp

@@ -35,8 +35,9 @@ nano::active_transactions::~active_transactions ()
 void nano::active_transactions::search_frontiers (nano::transaction const & transaction_a)
 {
 	// Limit maximum count of elections to start
-	bool representative (node.config.enable_voting && node.wallets.reps_count > 0);
-	bool half_princpal_representative (representative && node.wallets.half_principal_reps_count > 0);
+	auto rep_counts (node.wallets.rep_counts ());
+	bool representative (node.config.enable_voting && rep_counts.voting > 0);
+	bool half_princpal_representative (representative && rep_counts.half_principal > 0);
 	/* Check less frequently for regular nodes in auto mode */
 	bool agressive_mode (half_princpal_representative || node.config.frontiers_confirmation == nano::frontiers_confirmation_mode::always);
 	auto request_interval (std::chrono::milliseconds (node.network_params.network.request_interval_ms));

nano/node/blockprocessor.cpp

@@ -379,7 +379,7 @@ void nano::block_processor::process_live (nano::block_hash const & hash_a, std::
 	}
 	// Announce block contents to the network
 	node.network.flood_block (block_a, false);
-	if (node.config.enable_voting)
+	if (node.config.enable_voting && node.wallets.rep_counts ().voting > 0)
 	{
 		// Announce our weighted vote to the network
 		generator.add (hash_a);

nano/node/network.cpp

@@ -142,7 +142,7 @@ template <typename T>
 bool confirm_block (nano::transaction const & transaction_a, nano::node & node_a, T & list_a, std::shared_ptr<nano::block> block_a, bool also_publish)
 {
 	bool result (false);
-	if (node_a.config.enable_voting)
+	if (node_a.config.enable_voting && node_a.wallets.rep_counts ().voting > 0)
 	{
 		auto hash (block_a->hash ());
 		// Search in cache
@@ -458,7 +458,7 @@ public:
 		}
 		node.stats.inc (nano::stat::type::message, nano::stat::detail::confirm_req, nano::stat::dir::in);
 		// Don't load nodes with disabled voting
-		if (node.config.enable_voting && node.wallets.reps_count)
+		if (node.config.enable_voting && node.wallets.rep_counts ().voting > 0)
 		{
 			if (message_a.block != nullptr)
 			{

nano/node/node.cpp

@@ -1060,7 +1060,7 @@ void nano::node::block_confirm (std::shared_ptr<nano::block> block_a)
 	active.start (block_a, false);
 	network.broadcast_confirm_req (block_a);
 	// Calculate votes for local representatives
-	if (config.enable_voting && active.active (*block_a))
+	if (config.enable_voting && wallets.rep_counts ().voting > 0 && active.active (*block_a))
 	{
 		block_processor.generator.add (block_a->hash ());
 	}

nano/node/wallet.cpp

@@ -1833,17 +1833,28 @@ void nano::wallets::clear_send_ids (nano::transaction const & transaction_a)
 	assert (status == 0);
 }
 
-bool nano::wallets::check_rep (nano::account const & account_a, nano::uint128_t const & half_principal_weight_a)
+nano::wallet_representative_counts nano::wallets::rep_counts ()
+{
+	nano::lock_guard<std::mutex> counts_guard (counts_mutex);
+	return counts;
+}
+
+bool nano::wallets::check_rep (nano::account const & account_a, nano::uint128_t const & half_principal_weight_a, const bool acquire_lock_a)
 {
 	bool result (false);
 	auto weight (node.ledger.weight (account_a));
 	if (weight >= node.config.vote_minimum.number ())
 	{
+		nano::unique_lock<std::mutex> lock;
+		if (acquire_lock_a)
+		{
+			lock = nano::unique_lock<std::mutex> (counts_mutex);
+		}
 		result = true;
-		++reps_count;
+		++counts.voting;
 		if (weight >= half_principal_weight_a)
 		{
-			++half_principal_reps_count;
+			++counts.half_principal;
 		}
 	}
 	return result;
@@ -1851,9 +1862,9 @@ bool nano::wallets::check_rep (nano::account const & account_a, nano::uint128_t
 
 void nano::wallets::compute_reps ()
 {
-	nano::lock_guard<std::mutex> lock (mutex);
-	reps_count = 0;
-	half_principal_reps_count = 0;
+	nano::lock_guard<std::mutex> guard (mutex);
+	nano::lock_guard<std::mutex> counts_guard (counts_mutex);
+	counts = { 0, 0 };
 	auto half_principal_weight (node.minimum_principal_weight () / 2);
 	auto transaction (tx_begin_read ());
 	for (auto i (items.begin ()), n (items.end ()); i != n; ++i)
@@ -1863,12 +1874,12 @@ void nano::wallets::compute_reps ()
 		for (auto ii (wallet.store.begin (transaction)), nn (wallet.store.end ()); ii != nn; ++ii)
 		{
 			auto account (ii->first);
-			if (check_rep (account, half_principal_weight))
+			if (check_rep (account, half_principal_weight, false))
 			{
 				representatives_l.insert (account);
 			}
 		}
-		nano::lock_guard<std::mutex> representatives_lock (wallet.representatives_mutex);
+		nano::lock_guard<std::mutex> representatives_guard (wallet.representatives_mutex);
 		wallet.representatives.swap (representatives_l);
 	}
 }

nano/node/wallet.hpp

@@ -179,6 +179,14 @@ public:
 	std::unordered_map<nano::qualified_root, std::shared_ptr<nano::state_block>> watched;
 	std::atomic<bool> stopped;
 };
+
+class wallet_representative_counts
+{
+public:
+	uint64_t voting{ 0 }; // Representatives with at least the configured minimum voting weight
+	uint64_t half_principal{ 0 }; // Representatives with at least 50% of principal representative requirements
+};
+
 /**
  * The wallets set is all the wallets a node controls.
  * A node may contain multiple wallets independently encrypted and operated.
@@ -200,7 +208,8 @@ public:
 	bool exists (nano::transaction const &, nano::public_key const &);
 	void stop ();
 	void clear_send_ids (nano::transaction const &);
-	bool check_rep (nano::account const &, nano::uint128_t const &);
+	nano::wallet_representative_counts rep_counts ();
+	bool check_rep (nano::account const &, nano::uint128_t const &, const bool = true);
 	void compute_reps ();
 	void ongoing_compute_reps ();
 	void split_if_needed (nano::transaction &, nano::block_store &);
@@ -222,14 +231,15 @@ public:
 	std::thread thread;
 	static nano::uint128_t const generate_priority;
 	static nano::uint128_t const high_priority;
-	std::atomic<uint64_t> reps_count{ 0 };
-	std::atomic<uint64_t> half_principal_reps_count{ 0 }; // Representatives with at least 50% of principal representative requirements
-
 	/** Start read-write transaction */
 	nano::write_transaction tx_begin_write ();
 
 	/** Start read-only transaction */
 	nano::read_transaction tx_begin_read ();
+
+private:
+	std::mutex counts_mutex;
+	nano::wallet_representative_counts counts;
 };
 
 std::unique_ptr<seq_con_info_component> collect_seq_con_info (wallets & wallets, const std::string & name);