Removing tracking weight from rep_crawler class. (#4187)
This information is tracked accurately by nano::ledger and keeping the containers in sync is error prone.
This commit is contained in:
		
					parent
					
						
							
								7e6a3107ff
							
						
					
				
			
			
				commit
				
					
						e8ddd831db
					
				
			
		
					 6 changed files with 30 additions and 64 deletions
				
			
		|  | @ -79,7 +79,7 @@ TEST (active_transactions, confirm_election_by_request) | |||
| 	// Add representative (node1) to disabled rep crawler of node2
 | ||||
| 	{ | ||||
| 		nano::lock_guard<nano::mutex> guard (node2.rep_crawler.probable_reps_mutex); | ||||
| 		node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, *peers.cbegin ()); | ||||
| 		node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, *peers.cbegin ()); | ||||
| 	} | ||||
| 
 | ||||
| 	// Expect a vote to come back
 | ||||
|  | @ -117,7 +117,7 @@ TEST (active_transactions, confirm_frontier) | |||
| 	ASSERT_FALSE (peers.empty ()); | ||||
| 	{ | ||||
| 		nano::lock_guard<nano::mutex> guard (node2.rep_crawler.probable_reps_mutex); | ||||
| 		node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, *peers.begin ()); | ||||
| 		node2.rep_crawler.probable_reps.emplace (nano::dev::genesis_key.pub, *peers.begin ()); | ||||
| 	} | ||||
| 
 | ||||
| 	nano::state_block_builder builder; | ||||
|  |  | |||
|  | @ -20,7 +20,7 @@ TEST (confirmation_solicitor, batches) | |||
| 	auto & node2 = *system.add_node (node_flags); | ||||
| 	auto channel1 = nano::test::establish_tcp (system, node2, node1.network.endpoint ()); | ||||
| 	// Solicitor will only solicit from this representative
 | ||||
| 	nano::representative representative (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, channel1); | ||||
| 	nano::representative representative (nano::dev::genesis_key.pub, channel1); | ||||
| 	std::vector<nano::representative> representatives{ representative }; | ||||
| 	nano::confirmation_solicitor solicitor (node2.network, node2.config); | ||||
| 	solicitor.prepare (representatives); | ||||
|  | @ -70,7 +70,7 @@ TEST (confirmation_solicitor, different_hash) | |||
| 	auto & node2 = *system.add_node (node_flags); | ||||
| 	auto channel1 = nano::test::establish_tcp (system, node2, node1.network.endpoint ()); | ||||
| 	// Solicitor will only solicit from this representative
 | ||||
| 	nano::representative representative (nano::dev::genesis_key.pub, nano::dev::constants.genesis_amount, channel1); | ||||
| 	nano::representative representative (nano::dev::genesis_key.pub, channel1); | ||||
| 	std::vector<nano::representative> representatives{ representative }; | ||||
| 	nano::confirmation_solicitor solicitor (node2.network, node2.config); | ||||
| 	solicitor.prepare (representatives); | ||||
|  | @ -117,7 +117,7 @@ TEST (confirmation_solicitor, bypass_max_requests_cap) | |||
| 	{ | ||||
| 		// Make a temporary channel associated with node2
 | ||||
| 		auto channel = std::make_shared<nano::transport::inproc::channel> (node2, node2); | ||||
| 		nano::representative representative (nano::account (i), i, channel); | ||||
| 		nano::representative representative{ nano::account (i), channel }; | ||||
| 		representatives.push_back (representative); | ||||
| 	} | ||||
| 	ASSERT_EQ (max_representatives + 1, representatives.size ()); | ||||
|  |  | |||
|  | @ -1679,10 +1679,10 @@ TEST (node, rep_list) | |||
| 	auto done (false); | ||||
| 	while (!done) | ||||
| 	{ | ||||
| 		auto reps (node1.rep_crawler.representatives (1)); | ||||
| 		auto reps = node1.rep_crawler.representatives (1); | ||||
| 		if (!reps.empty ()) | ||||
| 		{ | ||||
| 			if (!reps[0].weight.is_zero ()) | ||||
| 			if (!node1.ledger.weight (reps[0].account).is_zero ()) | ||||
| 			{ | ||||
| 				done = true; | ||||
| 			} | ||||
|  | @ -1771,9 +1771,9 @@ TEST (node, rep_weight) | |||
| 	node.rep_crawler.response (channel3, vote2); | ||||
| 	ASSERT_TIMELY (5s, node.rep_crawler.representative_count () == 2); | ||||
| 	// Make sure we get the rep with the most weight first
 | ||||
| 	auto reps (node.rep_crawler.representatives (1)); | ||||
| 	auto reps = node.rep_crawler.representatives (1); | ||||
| 	ASSERT_EQ (1, reps.size ()); | ||||
| 	ASSERT_EQ (node.balance (nano::dev::genesis_key.pub), reps[0].weight.number ()); | ||||
| 	ASSERT_EQ (node.balance (nano::dev::genesis_key.pub), node.ledger.weight (reps[0].account)); | ||||
| 	ASSERT_EQ (nano::dev::genesis_key.pub, reps[0].account); | ||||
| 	ASSERT_EQ (*channel1, reps[0].channel_ref ()); | ||||
| 	ASSERT_TRUE (node.rep_crawler.is_pr (*channel1)); | ||||
|  | @ -1856,7 +1856,7 @@ TEST (node, rep_remove) | |||
| 	ASSERT_TIMELY (5s, searching_node.rep_crawler.representative_count () == 1); | ||||
| 	auto reps (searching_node.rep_crawler.representatives (1)); | ||||
| 	ASSERT_EQ (1, reps.size ()); | ||||
| 	ASSERT_EQ (searching_node.minimum_principal_weight () * 2, reps[0].weight.number ()); | ||||
| 	ASSERT_EQ (searching_node.minimum_principal_weight () * 2, searching_node.ledger.weight (reps[0].account)); | ||||
| 	ASSERT_EQ (keys_rep1.pub, reps[0].account); | ||||
| 	ASSERT_EQ (*channel_rep1, reps[0].channel_ref ()); | ||||
| 
 | ||||
|  |  | |||
|  | @ -2123,7 +2123,7 @@ void nano::json_handler::confirmation_quorum () | |||
| 			boost::property_tree::ptree peer_node; | ||||
| 			peer_node.put ("account", peer.account.to_account ()); | ||||
| 			peer_node.put ("ip", peer.channel->to_string ()); | ||||
| 			peer_node.put ("weight", peer.weight.to_string_dec ()); | ||||
| 			peer_node.put ("weight", nano::amount{ node.ledger.weight (peer.account) }.to_string_dec ()); | ||||
| 			peers.push_back (std::make_pair ("", peer_node)); | ||||
| 		} | ||||
| 		response_l.add_child ("peers", peers); | ||||
|  |  | |||
|  | @ -80,7 +80,6 @@ void nano::rep_crawler::validate () | |||
| 				{ | ||||
| 					debug_assert (info.account == vote->account); | ||||
| 					updated = true; | ||||
| 					info.weight = rep_weight; | ||||
| 					prev_channel = info.channel; | ||||
| 					info.channel = channel; | ||||
| 				} | ||||
|  | @ -88,7 +87,7 @@ void nano::rep_crawler::validate () | |||
| 		} | ||||
| 		else | ||||
| 		{ | ||||
| 			probable_reps.emplace (nano::representative (vote->account, rep_weight, channel)); | ||||
| 			probable_reps.emplace (nano::representative (vote->account, channel)); | ||||
| 			inserted = true; | ||||
| 		} | ||||
| 
 | ||||
|  | @ -111,7 +110,6 @@ void nano::rep_crawler::ongoing_crawl () | |||
| 	auto now (std::chrono::steady_clock::now ()); | ||||
| 	auto total_weight_l (total_weight ()); | ||||
| 	cleanup_reps (); | ||||
| 	update_weights (); | ||||
| 	validate (); | ||||
| 	query (get_crawl_targets (total_weight_l)); | ||||
| 	auto sufficient_weight (total_weight_l > node.online_reps.delta ()); | ||||
|  | @ -225,7 +223,7 @@ bool nano::rep_crawler::is_pr (nano::transport::channel const & channel_a) const | |||
| 	bool result = false; | ||||
| 	if (existing != probable_reps.get<tag_channel_ref> ().end ()) | ||||
| 	{ | ||||
| 		result = existing->weight > node.minimum_principal_weight (); | ||||
| 		result = node.ledger.weight (existing->account) > node.minimum_principal_weight (); | ||||
| 	} | ||||
| 	return result; | ||||
| } | ||||
|  | @ -250,19 +248,11 @@ nano::uint128_t nano::rep_crawler::total_weight () const | |||
| { | ||||
| 	nano::lock_guard<nano::mutex> lock{ probable_reps_mutex }; | ||||
| 	nano::uint128_t result (0); | ||||
| 	for (auto i (probable_reps.get<tag_weight> ().begin ()), n (probable_reps.get<tag_weight> ().end ()); i != n; ++i) | ||||
| 	for (auto i (probable_reps.get<tag_account> ().begin ()), n (probable_reps.get<tag_account> ().end ()); i != n; ++i) | ||||
| 	{ | ||||
| 		if (i->channel->alive ()) | ||||
| 		{ | ||||
| 			auto weight (i->weight.number ()); | ||||
| 			if (weight > 0) | ||||
| 			{ | ||||
| 				result = result + weight; | ||||
| 			} | ||||
| 			else | ||||
| 			{ | ||||
| 				break; | ||||
| 			} | ||||
| 			result += node.ledger.weight (i->account); | ||||
| 		} | ||||
| 	} | ||||
| 	return result; | ||||
|  | @ -331,42 +321,24 @@ void nano::rep_crawler::cleanup_reps () | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| void nano::rep_crawler::update_weights () | ||||
| { | ||||
| 	nano::lock_guard<nano::mutex> lock{ probable_reps_mutex }; | ||||
| 	for (auto i (probable_reps.get<tag_last_request> ().begin ()), n (probable_reps.get<tag_last_request> ().end ()); i != n;) | ||||
| 	{ | ||||
| 		auto weight (node.ledger.weight (i->account)); | ||||
| 		if (weight > 0) | ||||
| 		{ | ||||
| 			if (i->weight.number () != weight) | ||||
| 			{ | ||||
| 				probable_reps.get<tag_last_request> ().modify (i, [weight] (nano::representative & info) { | ||||
| 					info.weight = weight; | ||||
| 				}); | ||||
| 			} | ||||
| 			++i; | ||||
| 		} | ||||
| 		else | ||||
| 		{ | ||||
| 			// Erase non representatives
 | ||||
| 			i = probable_reps.get<tag_last_request> ().erase (i); | ||||
| 		} | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| std::vector<nano::representative> nano::rep_crawler::representatives (std::size_t count_a, nano::uint128_t const weight_a, boost::optional<decltype (nano::network_constants::protocol_version)> const & opt_version_min_a) | ||||
| { | ||||
| 	auto version_min (opt_version_min_a.value_or (node.network_params.network.protocol_version_min)); | ||||
| 	std::vector<representative> result; | ||||
| 	std::multimap<nano::amount, representative, std::greater<nano::amount>> ordered; | ||||
| 	nano::lock_guard<nano::mutex> lock{ probable_reps_mutex }; | ||||
| 	for (auto i (probable_reps.get<tag_weight> ().begin ()), n (probable_reps.get<tag_weight> ().end ()); i != n && result.size () < count_a; ++i) | ||||
| 	for (auto i (probable_reps.get<tag_account> ().begin ()), n (probable_reps.get<tag_account> ().end ()); i != n; ++i) | ||||
| 	{ | ||||
| 		if (i->weight > weight_a && i->channel->get_network_version () >= version_min) | ||||
| 		auto weight = node.ledger.weight (i->account); | ||||
| 		if (weight > weight_a && i->channel->get_network_version () >= version_min) | ||||
| 		{ | ||||
| 			result.push_back (*i); | ||||
| 			ordered.insert ({ nano::amount{ weight }, *i }); | ||||
| 		} | ||||
| 	} | ||||
| 	std::vector<nano::representative> result; | ||||
| 	for (auto i = ordered.begin (), n = ordered.end (); i != n && result.size () < count_a; ++i) | ||||
| 	{ | ||||
| 		result.push_back (i->second); | ||||
| 	} | ||||
| 	return result; | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -27,8 +27,8 @@ class representative | |||
| { | ||||
| public: | ||||
| 	representative () = default; | ||||
| 	representative (nano::account account_a, nano::amount weight_a, std::shared_ptr<nano::transport::channel> const & channel_a) : | ||||
| 		account (account_a), weight (weight_a), channel (channel_a) | ||||
| 	representative (nano::account account_a, std::shared_ptr<nano::transport::channel> const & channel_a) : | ||||
| 		account (account_a), channel (channel_a) | ||||
| 	{ | ||||
| 		debug_assert (channel != nullptr); | ||||
| 	} | ||||
|  | @ -41,7 +41,6 @@ public: | |||
| 		return account == other_a.account; | ||||
| 	} | ||||
| 	nano::account account{}; | ||||
| 	nano::amount weight{ 0 }; | ||||
| 	std::shared_ptr<nano::transport::channel> channel; | ||||
| 	std::chrono::steady_clock::time_point last_request{ std::chrono::steady_clock::time_point () }; | ||||
| 	std::chrono::steady_clock::time_point last_response{ std::chrono::steady_clock::time_point () }; | ||||
|  | @ -59,16 +58,14 @@ class rep_crawler final | |||
| 	class tag_account {}; | ||||
| 	class tag_channel_ref {}; | ||||
| 	class tag_last_request {}; | ||||
| 	class tag_weight {}; | ||||
| 	class tag_sequenced {}; | ||||
| 
 | ||||
| 	using probably_rep_t = boost::multi_index_container<representative, | ||||
| 	mi::indexed_by< | ||||
| 		mi::hashed_unique<mi::member<representative, nano::account, &representative::account>>, | ||||
| 		mi::random_access<>, | ||||
| 		mi::hashed_unique<mi::tag<tag_account>, mi::member<representative, nano::account, &representative::account>>, | ||||
| 		mi::sequenced<mi::tag<tag_sequenced>>, | ||||
| 		mi::ordered_non_unique<mi::tag<tag_last_request>, | ||||
| 			mi::member<representative, std::chrono::steady_clock::time_point, &representative::last_request>>, | ||||
| 		mi::ordered_non_unique<mi::tag<tag_weight>, | ||||
| 			mi::member<representative, nano::amount, &representative::weight>, std::greater<nano::amount>>, | ||||
| 		mi::hashed_non_unique<mi::tag<tag_channel_ref>, | ||||
| 			mi::const_mem_fun<representative, std::reference_wrapper<nano::transport::channel const>, &representative::channel_ref>>>>; | ||||
| 	// clang-format on
 | ||||
|  | @ -141,9 +138,6 @@ private: | |||
| 	/** Clean representatives with inactive channels */ | ||||
| 	void cleanup_reps (); | ||||
| 
 | ||||
| 	/** Update representatives weights from ledger */ | ||||
| 	void update_weights (); | ||||
| 
 | ||||
| 	/** Protects the probable_reps container */ | ||||
| 	mutable nano::mutex probable_reps_mutex; | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 clemahieu
				clemahieu