Difficulty updates for elections with multiple blocks (#2710)

* Difficulty updates for elections with multiple blocks

`active_transactions::publish` now performs a difficulty update for a new block, ensuring elections with forks have equivalent priority accross the network as long as they see the same blocks. `election->publish` is no longer called from `update_difficulty(_impl)` as it has no effect.

Difficulty normalization (#2691) made this more difficult as forks don't have a loaded sideband and we wish to avoid extra disk reads. https://github.com/clemahieu suggested storing the previous block's epoch and balance in order to infer we can infer block details, implemented in this PR.

In elections for live blocks, the only case where it is not possible to correctly infer the threshold is during an epoch upgrade, for blocks performing an upgrade (epoch will mismatch from root, and the block itself might be an epoch block). This only affects prioritization and the window is short. In the future, this can be disambiguated by including some flags in the block itself.

New tests added ensuring correct difficulty updates for old blocks and forks.

* Check block previous zero first to avoid tx_begin_read (Serg review)

* Assign previous_balance for all blocks (Serg review)

* Use const ref (Wes review)

nano/core_test/active_transactions.cpp

@@ -768,6 +768,10 @@ TEST (active_transactions, insertion_prioritization)
 
 	// Sort by difficulty, descending
 	std::vector<std::shared_ptr<nano::block>> blocks{ send1, send2, send3, send4, send5, send6, send7 };
+	for (auto const & block : blocks)
+	{
+		ASSERT_EQ (nano::process_result::progress, node.process (*block).code);
+	}
 	std::sort (blocks.begin (), blocks.end (), [](auto const & blockl, auto const & blockr) { return blockl->difficulty () > blockr->difficulty (); });
 
 	auto update_active_multiplier = [&node] {
@@ -904,3 +908,94 @@ TEST (active_transactions, vote_generator_session)
 	}
 }
 }
+
+TEST (active_transactions, election_difficulty_update_old)
+{
+	nano::system system;
+	nano::node_flags node_flags;
+	node_flags.disable_request_loop = true;
+	auto & node = *system.add_node (node_flags);
+	nano::genesis genesis;
+	nano::keypair key;
+	auto send1 (std::make_shared<nano::state_block> (nano::test_genesis_key.pub, genesis.hash (), nano::test_genesis_key.pub, nano::genesis_amount - 10 * nano::xrb_ratio, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ())));
+	auto send1_copy (std::make_shared<nano::state_block> (*send1));
+	node.process_active (send1);
+	node.block_processor.flush ();
+	auto root (send1->qualified_root ());
+	ASSERT_EQ (1, node.active.size ());
+	auto multiplier = node.active.roots.begin ()->multiplier;
+	{
+		nano::lock_guard<std::mutex> guard (node.active.mutex);
+		ASSERT_EQ (node.active.normalized_multiplier (*send1), multiplier);
+	}
+	// Should not update with a lower difficulty
+	send1_copy->block_work_set (0);
+	ASSERT_EQ (nano::process_result::old, node.process (*send1_copy).code);
+	ASSERT_FALSE (send1_copy->has_sideband ());
+	node.process_active (send1);
+	node.block_processor.flush ();
+	ASSERT_EQ (1, node.active.size ());
+	ASSERT_EQ (node.active.roots.begin ()->multiplier, multiplier);
+	// Update work, even without a sideband it should find the block in the election and update the election multiplier
+	ASSERT_TRUE (node.work_generate_blocking (*send1_copy, send1->difficulty () + 1).is_initialized ());
+	node.process_active (send1_copy);
+	node.block_processor.flush ();
+	ASSERT_EQ (1, node.active.size ());
+	ASSERT_GT (node.active.roots.begin ()->multiplier, multiplier);
+}
+
+TEST (active_transactions, election_difficulty_update_fork)
+{
+	nano::system system;
+	nano::node_flags node_flags;
+	node_flags.disable_request_loop = true;
+	auto & node = *system.add_node (node_flags);
+
+	ASSERT_NE (nullptr, system.upgrade_genesis_epoch (node, nano::epoch::epoch_1));
+	auto epoch2 = system.upgrade_genesis_epoch (node, nano::epoch::epoch_2);
+	ASSERT_NE (nullptr, epoch2);
+	nano::keypair key;
+	auto send1 (std::make_shared<nano::state_block> (nano::test_genesis_key.pub, epoch2->hash (), nano::test_genesis_key.pub, nano::genesis_amount - nano::Gxrb_ratio, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (epoch2->hash ())));
+	auto open1 (std::make_shared<nano::state_block> (key.pub, 0, key.pub, nano::Gxrb_ratio, send1->hash (), key.prv, key.pub, *system.work.generate (key.pub)));
+	auto send2 (std::make_shared<nano::state_block> (nano::test_genesis_key.pub, send1->hash (), nano::test_genesis_key.pub, nano::genesis_amount - 2 * nano::Gxrb_ratio, key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (send1->hash ())));
+	ASSERT_EQ (nano::process_result::progress, node.process (*send1).code);
+	ASSERT_EQ (nano::process_result::progress, node.process (*open1).code);
+	ASSERT_EQ (nano::process_result::progress, node.process (*send2).code);
+
+	// Verify an election with multiple blocks is correctly updated on arrival of another block
+	// Each subsequent block has difficulty at least higher than the previous one
+	auto fork_change (std::make_shared<nano::state_block> (key.pub, open1->hash (), nano::test_genesis_key.pub, nano::Gxrb_ratio, 0, key.prv, key.pub, *system.work.generate (open1->hash ())));
+	auto fork_send (std::make_shared<nano::state_block> (key.pub, open1->hash (), key.pub, 0, key.pub, key.prv, key.pub, *system.work.generate (open1->hash (), fork_change->difficulty ())));
+	auto fork_receive (std::make_shared<nano::state_block> (key.pub, open1->hash (), key.pub, 2 * nano::Gxrb_ratio, send2->hash (), key.prv, key.pub, *system.work.generate (open1->hash (), fork_send->difficulty ())));
+	ASSERT_GT (fork_send->difficulty (), fork_change->difficulty ());
+	ASSERT_GT (fork_receive->difficulty (), fork_send->difficulty ());
+
+	node.process_active (fork_change);
+	node.block_processor.flush ();
+	ASSERT_EQ (1, node.active.size ());
+	auto multiplier_change = node.active.roots.begin ()->multiplier;
+	node.process_active (fork_send);
+	node.block_processor.flush ();
+	ASSERT_EQ (1, node.active.size ());
+	auto multiplier_send = node.active.roots.begin ()->multiplier;
+	node.process_active (fork_receive);
+	node.block_processor.flush ();
+	ASSERT_EQ (1, node.active.size ());
+	auto multiplier_receive = node.active.roots.begin ()->multiplier;
+
+	ASSERT_GT (multiplier_send, multiplier_change);
+	ASSERT_GT (multiplier_receive, multiplier_send);
+
+	EXPECT_FALSE (fork_receive->has_sideband ());
+	auto threshold = nano::work_threshold (fork_receive->work_version (), nano::block_details (nano::epoch::epoch_2, false, true, false));
+	auto denormalized = nano::denormalized_multiplier (multiplier_receive, threshold);
+	ASSERT_NEAR (nano::difficulty::to_multiplier (fork_receive->difficulty (), threshold), denormalized, 1e-10);
+
+	// Ensure a fork with updated difficulty will also update the election difficulty
+	fork_receive->block_work_set (*system.work.generate (fork_receive->root (), fork_receive->difficulty () + 1));
+	node.process_active (fork_receive);
+	node.block_processor.flush ();
+	ASSERT_EQ (1, node.active.size ());
+	auto multiplier_receive_updated = node.active.roots.begin ()->multiplier;
+	ASSERT_GT (multiplier_receive_updated, multiplier_receive);
+}

nano/core_test/conflicts.cpp

@@ -39,6 +39,7 @@ TEST (conflicts, add_existing)
 	node1.active.insert (send1);
 	nano::keypair key2;
 	auto send2 (std::make_shared<nano::send_block> (genesis.hash (), key2.pub, 0, nano::test_genesis_key.prv, nano::test_genesis_key.pub, 0));
+	send2->sideband_set ({});
 	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));

nano/core_test/ledger.cpp

@@ -928,32 +928,33 @@ TEST (votes, add_old_different_account)
 	node1.work_generate_blocking (*send1);
 	auto send2 (std::make_shared<nano::send_block> (send1->hash (), key1.pub, 0, nano::test_genesis_key.prv, nano::test_genesis_key.pub, 0));
 	node1.work_generate_blocking (*send2);
-	auto transaction (node1.store.tx_begin_write ());
-	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send1).code);
-	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send2).code);
-	auto election1 = node1.active.insert (send1);
-	auto election2 = node1.active.insert (send2);
-	ASSERT_EQ (1, election1.election->last_votes_size ());
-	ASSERT_EQ (1, election2.election->last_votes_size ());
+	ASSERT_EQ (nano::process_result::progress, node1.process_local (send1).code);
+	ASSERT_EQ (nano::process_result::progress, node1.process_local (send2).code);
+	auto election1 = node1.active.election (send1->qualified_root ());
+	ASSERT_NE (nullptr, election1);
+	auto election2 = node1.active.election (send2->qualified_root ());
+	ASSERT_NE (nullptr, election2);
+	ASSERT_EQ (1, election1->last_votes_size ());
+	ASSERT_EQ (1, election2->last_votes_size ());
 	auto vote1 (std::make_shared<nano::vote> (nano::test_genesis_key.pub, nano::test_genesis_key.prv, 2, send1));
 	auto channel (std::make_shared<nano::transport::channel_udp> (node1.network.udp_channels, node1.network.endpoint (), node1.network_params.protocol.protocol_version));
 	auto vote_result1 (node1.vote_processor.vote_blocking (vote1, channel));
 	ASSERT_EQ (nano::vote_code::vote, vote_result1);
-	ASSERT_EQ (2, election1.election->last_votes_size ());
-	ASSERT_EQ (1, election2.election->last_votes_size ());
+	ASSERT_EQ (2, election1->last_votes_size ());
+	ASSERT_EQ (1, election2->last_votes_size ());
 	auto vote2 (std::make_shared<nano::vote> (nano::test_genesis_key.pub, nano::test_genesis_key.prv, 1, send2));
 	auto vote_result2 (node1.vote_processor.vote_blocking (vote2, channel));
 	ASSERT_EQ (nano::vote_code::vote, vote_result2);
-	ASSERT_EQ (2, election1.election->last_votes_size ());
-	ASSERT_EQ (2, election2.election->last_votes_size ());
+	ASSERT_EQ (2, election1->last_votes_size ());
+	ASSERT_EQ (2, election2->last_votes_size ());
 	nano::unique_lock<std::mutex> lock (node1.active.mutex);
-	ASSERT_NE (election1.election->last_votes.end (), election1.election->last_votes.find (nano::test_genesis_key.pub));
-	ASSERT_NE (election2.election->last_votes.end (), election2.election->last_votes.find (nano::test_genesis_key.pub));
-	ASSERT_EQ (send1->hash (), election1.election->last_votes[nano::test_genesis_key.pub].hash);
-	ASSERT_EQ (send2->hash (), election2.election->last_votes[nano::test_genesis_key.pub].hash);
-	auto winner1 (*election1.election->tally ().begin ());
+	ASSERT_NE (election1->last_votes.end (), election1->last_votes.find (nano::test_genesis_key.pub));
+	ASSERT_NE (election2->last_votes.end (), election2->last_votes.find (nano::test_genesis_key.pub));
+	ASSERT_EQ (send1->hash (), election1->last_votes[nano::test_genesis_key.pub].hash);
+	ASSERT_EQ (send2->hash (), election2->last_votes[nano::test_genesis_key.pub].hash);
+	auto winner1 (*election1->tally ().begin ());
 	ASSERT_EQ (*send1, *winner1.second);
-	auto winner2 (*election2.election->tally ().begin ());
+	auto winner2 (*election2->tally ().begin ());
 	ASSERT_EQ (*send2, *winner2.second);
 }
 
@@ -2659,20 +2660,23 @@ TEST (ledger, block_hash_account_conflict)
 	node1.work_generate_blocking (*receive1);
 	node1.work_generate_blocking (*send2);
 	node1.work_generate_blocking (*open_epoch1);
-	auto transaction (node1.store.tx_begin_write ());
-	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send1).code);
-	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *receive1).code);
-	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send2).code);
-	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *open_epoch1).code);
-	auto election1 = node1.active.insert (send1);
-	auto election2 = node1.active.insert (receive1);
-	auto election3 = node1.active.insert (send2);
-	auto election4 = node1.active.insert (open_epoch1);
+	ASSERT_EQ (nano::process_result::progress, node1.process_local (send1).code);
+	ASSERT_EQ (nano::process_result::progress, node1.process_local (receive1).code);
+	ASSERT_EQ (nano::process_result::progress, node1.process_local (send2).code);
+	ASSERT_EQ (nano::process_result::progress, node1.process_local (open_epoch1).code);
+	auto election1 = node1.active.election (send1->qualified_root ());
+	ASSERT_NE (nullptr, election1);
+	auto election2 = node1.active.election (receive1->qualified_root ());
+	ASSERT_NE (nullptr, election2);
+	auto election3 = node1.active.election (send2->qualified_root ());
+	ASSERT_NE (nullptr, election3);
+	auto election4 = node1.active.election (open_epoch1->qualified_root ());
+	ASSERT_NE (nullptr, election4);
 	nano::lock_guard<std::mutex> lock (node1.active.mutex);
-	auto winner1 (*election1.election->tally ().begin ());
-	auto winner2 (*election2.election->tally ().begin ());
-	auto winner3 (*election3.election->tally ().begin ());
-	auto winner4 (*election4.election->tally ().begin ());
+	auto winner1 (*election1->tally ().begin ());
+	auto winner2 (*election2->tally ().begin ());
+	auto winner3 (*election3->tally ().begin ());
+	auto winner4 (*election4->tally ().begin ());
 	ASSERT_EQ (*send1, *winner1.second);
 	ASSERT_EQ (*receive1, *winner2.second);
 	ASSERT_EQ (*send2, *winner3.second);

nano/node/active_transactions.cpp

@@ -493,8 +493,9 @@ void nano::active_transactions::stop ()
 	roots.clear ();
 }
 
-nano::election_insertion_result nano::active_transactions::insert_impl (std::shared_ptr<nano::block> block_a, std::function<void(std::shared_ptr<nano::block>)> const & confirmation_action_a)
+nano::election_insertion_result nano::active_transactions::insert_impl (std::shared_ptr<nano::block> const & block_a, boost::optional<nano::uint128_t> const & previous_balance_a, std::function<void(std::shared_ptr<nano::block>)> const & confirmation_action_a)
 {
+	debug_assert (block_a->has_sideband ());
 	nano::election_insertion_result result;
 	if (!stopped)
 	{
@@ -507,10 +508,14 @@ nano::election_insertion_result nano::active_transactions::insert_impl (std::sha
 				result.inserted = true;
 				auto hash (block_a->hash ());
 				auto difficulty (block_a->difficulty ());
+				auto epoch (block_a->sideband ().details.epoch);
+				auto previous_balance = block_a->previous ().is_zero () ? 0 : previous_balance_a.value_or_eval ([& node = node, &block_a] {
+					return node.ledger.balance (node.store.tx_begin_read (), block_a->previous ());
+				});
 				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, prioritized);
-				roots.get<tag_root> ().emplace (nano::conflict_info{ root, multiplier, multiplier, result.election });
+				roots.get<tag_root> ().emplace (nano::active_transactions::conflict_info{ root, multiplier, multiplier, result.election, epoch, previous_balance });
 				blocks.emplace (hash, result.election);
 				add_adjust_difficulty (hash);
 				result.election->insert_inactive_votes_cache (hash);
@@ -524,10 +529,10 @@ nano::election_insertion_result nano::active_transactions::insert_impl (std::sha
 	return result;
 }
 
-nano::election_insertion_result nano::active_transactions::insert (std::shared_ptr<nano::block> block_a, std::function<void(std::shared_ptr<nano::block>)> const & confirmation_action_a)
+nano::election_insertion_result nano::active_transactions::insert (std::shared_ptr<nano::block> const & block_a, boost::optional<nano::uint128_t> const & previous_balance_a, std::function<void(std::shared_ptr<nano::block>)> const & confirmation_action_a)
 {
 	nano::lock_guard<std::mutex> lock (mutex);
-	return insert_impl (block_a, confirmation_action_a);
+	return insert_impl (block_a, previous_balance_a, confirmation_action_a);
 }
 
 // Validate a vote and apply it to the current election if one exists
@@ -628,47 +633,59 @@ std::shared_ptr<nano::election> nano::active_transactions::election (nano::quali
 	return result;
 }
 
-void nano::active_transactions::update_difficulty (std::shared_ptr<nano::block> block_a)
+void nano::active_transactions::update_difficulty (nano::block const & block_a)
 {
 	nano::unique_lock<std::mutex> lock (mutex);
-	auto existing_election (roots.get<tag_root> ().find (block_a->qualified_root ()));
+	auto existing_election (roots.get<tag_root> ().find (block_a.qualified_root ()));
 	if (existing_election != roots.get<tag_root> ().end ())
 	{
-		double multiplier (normalized_multiplier (*block_a, existing_election->election->blocks));
-		if (multiplier > existing_election->multiplier)
+		update_difficulty_impl (existing_election, block_a);
+	}
+}
+
+void nano::active_transactions::update_difficulty_impl (nano::active_transactions::roots_iterator const & root_it_a, nano::block const & block_a)
+{
+	double multiplier (normalized_multiplier (block_a, root_it_a));
+	if (multiplier > root_it_a->multiplier)
+	{
+		if (node.config.logging.active_update_logging ())
 		{
-			if (node.config.logging.active_update_logging ())
-			{
-				node.logger.try_log (boost::str (boost::format ("Block %1% was updated from multiplier %2% to %3%") % block_a->hash ().to_string () % existing_election->multiplier % multiplier));
-			}
-			roots.get<tag_root> ().modify (existing_election, [multiplier](nano::conflict_info & info_a) {
-				info_a.multiplier = multiplier;
-			});
-			existing_election->election->publish (block_a);
-			add_adjust_difficulty (block_a->hash ());
+			node.logger.try_log (boost::str (boost::format ("Block %1% was updated from multiplier %2% to %3%") % block_a.hash ().to_string () % root_it_a->multiplier % multiplier));
 		}
+		roots.get<tag_root> ().modify (root_it_a, [multiplier](nano::active_transactions::conflict_info & info_a) {
+			info_a.multiplier = multiplier;
+		});
+		add_adjust_difficulty (block_a.hash ());
 	}
 }
 
-double nano::active_transactions::normalized_multiplier (nano::block const & block_a, std::unordered_map<nano::block_hash, std::shared_ptr<nano::block>> const & blocks_a)
+double nano::active_transactions::normalized_multiplier (nano::block const & block_a, boost::optional<nano::active_transactions::roots_iterator> const & root_it_a) const
 {
+	debug_assert (!mutex.try_lock ());
 	auto difficulty (block_a.difficulty ());
 	uint64_t threshold (0);
 	bool sideband_not_found (false);
 	if (block_a.has_sideband ())
 	{
 		threshold = nano::work_threshold (block_a.work_version (), block_a.sideband ().details);
 	}
-	else
+	else if (root_it_a.is_initialized ())
 	{
-		auto find_block (blocks_a.find (block_a.hash ()));
-		if (find_block != blocks_a.end () && find_block->second->has_sideband ())
+		auto election (*root_it_a);
+		debug_assert (election != roots.end ());
+		auto find_block (election->election->blocks.find (block_a.hash ()));
+		if (find_block != election->election->blocks.end () && find_block->second->has_sideband ())
 		{
 			threshold = nano::work_threshold (block_a.work_version (), find_block->second->sideband ().details);
 		}
 		else
 		{
-			threshold = nano::work_threshold_base (block_a.work_version ());
+			// This can have incorrect results during an epoch upgrade, but it only affects prioritization
+			bool is_send = election->previous_balance > block_a.balance ().number ();
+			bool is_receive = election->previous_balance < block_a.balance ().number ();
+			nano::block_details details (election->epoch, is_send, is_receive, false);
+
+			threshold = nano::work_threshold (block_a.work_version (), details);
 			sideband_not_found = true;
 		}
 	}
@@ -678,6 +695,11 @@ double nano::active_transactions::normalized_multiplier (nano::block const & blo
 	{
 		multiplier = nano::normalized_multiplier (multiplier, threshold);
 	}
+	else
+	{
+		// Inferred threshold was incorrect
+		multiplier = 1;
+	}
 	return multiplier;
 }
 
@@ -763,7 +785,7 @@ void nano::active_transactions::update_adjusted_multiplier ()
 				double multiplier_a = avg_multiplier + (double)item.second * min_unit;
 				if (existing_root->adjusted_multiplier != multiplier_a)
 				{
-					roots.get<tag_root> ().modify (existing_root, [multiplier_a](nano::conflict_info & info_a) {
+					roots.get<tag_root> ().modify (existing_root, [multiplier_a](nano::active_transactions::conflict_info & info_a) {
 						info_a.adjusted_multiplier = multiplier_a;
 					});
 				}
@@ -909,6 +931,7 @@ bool nano::active_transactions::publish (std::shared_ptr<nano::block> block_a)
 	auto result (true);
 	if (existing != roots.get<tag_root> ().end ())
 	{
+		update_difficulty_impl (existing, *block_a);
 		auto election (existing->election);
 		result = election->publish (block_a);
 		if (!result)