Fixing dynamic re-work and trend from elections (#1968)

* Add to/from work multiplier utility methods

* Use new method in RPC work_validate

* Update block in work watcher to avoid recomputing work every work watcher iteration

* Interim commit (not compiling)

* Fix adjusted difficulty, all arithmetic must be done in 'multiplier space'

* Move multiplier utility methods to lib/numbers under nano::difficulty namespace

* Check before division by zero

* Fix rework related tests

* Add difficulty manipulation methods core tests

* Simplify RPC work_validate test, difficulty manipulation methods are now tested elsewhere

* Add multiplier to RPC active_difficulty response

* Use difficulty methods in RPC active_difficulty

* Only ASSERT_DEATH if asserts enabled

* Format

* Fix adjusted difficulty (currently hanging in tests)

* Fix two tests

* Attempt debugging

* Fix type in test

* from_multiplier calculation is more clear

* Use a low value work for test_genesis_data

* Cleanup

* The median should be used, not the average. Average is biased towards the extreme one-off difficulties

* Only consider roots with ongoing elections

nano/core_test/CMakeLists.txt

@@ -4,6 +4,7 @@ add_executable (core_test
 	block.cpp
 	block_store.cpp
 	conflicts.cpp
+	difficulty.cpp
 	entry.cpp
 	gap_cache.cpp
 	interface.cpp

nano/core_test/difficulty.cpp

@@ -0,0 +1,35 @@
+#include <gtest/gtest.h>
+
+#include <nano/lib/numbers.hpp>
+
+TEST (difficulty, multipliers)
+{
+	{
+		uint64_t base = 0xff00000000000000;
+		uint64_t difficulty = 0xfff27e7a57c285cd;
+		double expected_multiplier = 18.95461493377003;
+
+		ASSERT_NEAR (expected_multiplier, nano::difficulty::to_multiplier (difficulty, base), 1e-10);
+		ASSERT_EQ (difficulty, nano::difficulty::from_multiplier (expected_multiplier, base));
+	}
+
+	{
+		uint64_t base = 0xffffffc000000000;
+		uint64_t difficulty = 0xfffffe0000000000;
+		double expected_multiplier = 0.125;
+
+		auto multiplier = nano::difficulty::to_multiplier (difficulty, base);
+		ASSERT_NEAR (expected_multiplier, nano::difficulty::to_multiplier (difficulty, base), 1e-10);
+		ASSERT_EQ (difficulty, nano::difficulty::from_multiplier (expected_multiplier, base));
+	}
+
+	{
+		uint64_t base = 0xffffffc000000000;
+		uint64_t difficulty_nil = 0;
+		double multiplier_nil = 0.;
+#ifndef NDEBUG
+		ASSERT_DEATH_IF_SUPPORTED (nano::difficulty::to_multiplier (difficulty_nil, base), "");
+		ASSERT_DEATH_IF_SUPPORTED (nano::difficulty::from_multiplier (multiplier_nil, base), "");
+#endif
+	}
+}

nano/core_test/node.cpp

@@ -2574,6 +2574,7 @@ TEST (active_difficulty, recalculate_work)
 	node1.work_generate_blocking (*send1);
 	uint64_t difficulty1;
 	nano::work_validate (*send1, &difficulty1);
+	auto multiplier1 = nano::difficulty::to_multiplier (difficulty1, node1.network_params.network.publish_threshold);
 	// Process as local block
 	node1.process_active (send1);
 	system.deadline_set (2s);
@@ -2581,13 +2582,13 @@ TEST (active_difficulty, recalculate_work)
 	{
 		ASSERT_NO_ERROR (system.poll ());
 	}
-	auto sum (std::accumulate (node1.active.difficulty_cb.begin (), node1.active.difficulty_cb.end (), nano::uint128_t (0)));
+	auto sum (std::accumulate (node1.active.multipliers_cb.begin (), node1.active.multipliers_cb.end (), double(0)));
-	ASSERT_EQ (node1.active.active_difficulty (), static_cast<uint64_t> (sum / node1.active.difficulty_cb.size ()));
+	ASSERT_EQ (node1.active.active_difficulty (), nano::difficulty::from_multiplier (sum / node1.active.multipliers_cb.size (), node1.network_params.network.publish_threshold));
 	std::unique_lock<std::mutex> lock (node1.active.mutex);
 	// Fake history records to force work recalculation
-	for (auto i (0); i < node1.active.difficulty_cb.size (); i++)
+	for (auto i (0); i < node1.active.multipliers_cb.size (); i++)
 	{
-		node1.active.difficulty_cb.push_back (difficulty1 + 10000);
+		node1.active.multipliers_cb.push_back (multiplier1 * (1 + i / 100.));
 	}
 	node1.work_generate_blocking (*send1);
 	uint64_t difficulty2;
@@ -2595,8 +2596,8 @@ TEST (active_difficulty, recalculate_work)
 	node1.process_active (send1);
 	node1.active.update_active_difficulty (lock);
 	lock.unlock ();
-	sum = std::accumulate (node1.active.difficulty_cb.begin (), node1.active.difficulty_cb.end (), nano::uint128_t (0));
+	sum = std::accumulate (node1.active.multipliers_cb.begin (), node1.active.multipliers_cb.end (), double(0));
-	ASSERT_EQ (node1.active.active_difficulty (), static_cast<uint64_t> (sum / node1.active.difficulty_cb.size ()));
+	ASSERT_EQ (node1.active.active_difficulty (), nano::difficulty::from_multiplier (sum / node1.active.multipliers_cb.size (), node1.network_params.network.publish_threshold));
 }
 
 namespace

nano/core_test/wallet.cpp

@@ -1051,16 +1051,17 @@ TEST (wallet, update_work_action)
 	auto const block (wallet.send_action (nano::test_genesis_key.pub, key.pub, nano::genesis_amount));
 	uint64_t difficulty1 (0);
 	nano::work_validate (*block, &difficulty1);
+	auto multiplier1 = nano::difficulty::to_multiplier (difficulty1, node.network_params.network.publish_threshold);
 	system.deadline_set (10s);
 	auto updated (false);
 	uint64_t updated_difficulty;
 	while (!updated)
 	{
 		std::unique_lock<std::mutex> lock (node.active.mutex);
-		//fill difficulty_cb and update active difficulty;
+		//fill multipliers_cb and update active difficulty;
-		for (auto i (0); i < node.active.difficulty_cb.size (); i++)
+		for (auto i (0); i < node.active.multipliers_cb.size (); i++)
 		{
-			node.active.difficulty_cb.push_back (difficulty1 + 10000);
+			node.active.multipliers_cb.push_back (multiplier1 * (1 + i / 100.));
 		}
 		node.active.update_active_difficulty (lock);
 		auto const existing (node.active.roots.find (block->qualified_root ()));

nano/lib/numbers.cpp

@@ -773,3 +773,15 @@ std::string nano::uint128_union::to_string_dec () const
 	encode_dec (result);
 	return result;
 }
+
+uint64_t nano::difficulty::from_multiplier (double const multiplier_a, uint64_t const base_difficulty_a)
+{
+	assert (multiplier_a > 0.);
+	return (-static_cast<uint64_t> ((-base_difficulty_a) / multiplier_a));
+}
+
+double nano::difficulty::to_multiplier (uint64_t const difficulty_a, uint64_t const base_difficulty_a)
+{
+	assert (difficulty_a > 0);
+	return static_cast<double> (-base_difficulty_a) / (-difficulty_a);
+}

nano/lib/numbers.hpp

@@ -126,6 +126,12 @@ bool validate_message (nano::public_key const &, nano::uint256_union const &, na
 bool validate_message_batch (const unsigned char **, size_t *, const unsigned char **, const unsigned char **, size_t, int *);
 void deterministic_key (nano::uint256_union const &, uint32_t, nano::uint256_union &);
 nano::public_key pub_key (nano::private_key const &);
+
+namespace difficulty
+{
+	uint64_t from_multiplier (double const, uint64_t const);
+	double to_multiplier (uint64_t const, uint64_t const);
+}
 }
 
 namespace std

nano/node/active_transactions.cpp

@@ -9,7 +9,7 @@ using namespace std::chrono;
 
 nano::active_transactions::active_transactions (nano::node & node_a, bool delay_frontier_confirmation_height_updating) :
 node (node_a),
-difficulty_cb (20, node.network_params.network.publish_threshold),
+multipliers_cb (20, 1.),
 trended_active_difficulty (node.network_params.network.publish_threshold),
 next_frontier_check (steady_clock::now () + (delay_frontier_confirmation_height_updating ? 60s : 0s)),
 thread ([this]() {
@@ -447,7 +447,7 @@ void nano::active_transactions::adjust_difficulty (nano::block_hash const & hash
 	remaining_blocks.emplace_back (hash_a, 0);
 	std::unordered_set<nano::block_hash> processed_blocks;
 	std::vector<std::pair<nano::qualified_root, int64_t>> elections_list;
-	uint128_t sum (0);
+	double sum (0.);
 	while (!remaining_blocks.empty ())
 	{
 		auto const & item (remaining_blocks.front ());
@@ -482,61 +482,61 @@ void nano::active_transactions::adjust_difficulty (nano::block_hash const & hash
 				auto existing_root (roots.find (root));
 				if (existing_root != roots.end ())
 				{
-					sum += existing_root->difficulty;
+					sum += nano::difficulty::to_multiplier (existing_root->difficulty, node.network_params.network.publish_threshold);
 					elections_list.emplace_back (root, level);
 				}
 			}
 		}
 		remaining_blocks.pop_front ();
 	}
-	if (elections_list.size () > 1)
+	if (!elections_list.empty ())
 	{
-		uint64_t average (static_cast<uint64_t> (sum / elections_list.size ()));
+		double multiplier = sum / elections_list.size ();
-		// Potential overflow check
+		uint64_t average = nano::difficulty::from_multiplier (multiplier, node.network_params.network.publish_threshold);
-		uint64_t divider (1);
+		auto highest_level = elections_list.back ().second;
-		if (elections_list.size () > 1000000 && (average - node.network_params.network.publish_threshold) > elections_list.size ())
+		uint64_t divider = 1;
+		// Possible overflow check, will not occur for negative levels
+		if ((multiplier + highest_level) > 10000000000)
 		{
-			divider = ((average - node.network_params.network.publish_threshold) / elections_list.size ()) + 1;
+			divider = ((multiplier + highest_level) / 10000000000) + 1;
 		}
+
 		// Set adjusted difficulty
 		for (auto & item : elections_list)
 		{
 			auto existing_root (roots.find (item.first));
-			uint64_t difficulty_a (average + (item.second / divider));
+			uint64_t difficulty_a = average + item.second / divider;
 			roots.modify (existing_root, [difficulty_a](nano::conflict_info & info_a) {
 				info_a.adjusted_difficulty = difficulty_a;
 			});
 		}
 	}
-	// Set adjusted difficulty equals to difficulty
-	else if (elections_list.size () == 1)
-	{
-		auto existing_root (roots.find (elections_list.begin ()->first));
-		if (existing_root->difficulty != existing_root->adjusted_difficulty)
-		{
-			roots.modify (existing_root, [](nano::conflict_info & info_a) {
-				info_a.adjusted_difficulty = info_a.difficulty;
-			});
-		}
-	}
 }
 
 void nano::active_transactions::update_active_difficulty (std::unique_lock<std::mutex> & lock_a)
 {
 	assert (lock_a.mutex () == &mutex && lock_a.owns_lock ());
-	uint64_t difficulty (node.network_params.network.publish_threshold);
+	double multiplier (1.);
 	if (!roots.empty ())
 	{
-		uint128_t min = roots.get<1> ().begin ()->adjusted_difficulty;
+		std::vector<uint64_t> active_root_difficulties;
-		assert (min >= node.network_params.network.publish_threshold);
+		active_root_difficulties.reserve (roots.size ());
-		uint128_t max = (--roots.get<1> ().end ())->adjusted_difficulty;
+		for (auto & root : roots)
-		assert (max >= node.network_params.network.publish_threshold);
+		{
-		difficulty = static_cast<uint64_t> ((min + max) / 2);
+			if (!root.election->confirmed && !root.election->stopped)
+			{
+				active_root_difficulties.push_back (root.adjusted_difficulty);
+			}
+		}
+		if (!active_root_difficulties.empty ())
+		{
+			multiplier = nano::difficulty::to_multiplier (active_root_difficulties[active_root_difficulties.size () / 2], node.network_params.network.publish_threshold);
+		}
 	}
-	assert (difficulty >= node.network_params.network.publish_threshold);
+	assert (multiplier >= 1);
-	difficulty_cb.push_front (difficulty);
+	multipliers_cb.push_front (multiplier);
-	auto sum (std::accumulate (node.active.difficulty_cb.begin (), node.active.difficulty_cb.end (), uint128_t (0)));
+	auto sum (std::accumulate (multipliers_cb.begin (), multipliers_cb.end (), double(0)));
-	difficulty = static_cast<uint64_t> (sum / difficulty_cb.size ());
+	auto difficulty = nano::difficulty::from_multiplier (sum / multipliers_cb.size (), node.network_params.network.publish_threshold);
 	assert (difficulty >= node.network_params.network.publish_threshold);
 	trended_active_difficulty = difficulty;
 }

nano/node/active_transactions.hpp

@@ -95,7 +95,7 @@ public:
 	static unsigned constexpr announcement_long = 20;
 	static size_t constexpr election_history_size = 2048;
 	static size_t constexpr max_broadcast_queue = 1000;
-	boost::circular_buffer<uint64_t> difficulty_cb;
+	boost::circular_buffer<double> multipliers_cb;
 	uint64_t trended_active_difficulty;
 
 private:

nano/node/json_handler.cpp

@@ -836,7 +836,10 @@ void nano::json_handler::accounts_pending ()
 void nano::json_handler::active_difficulty ()
 {
 	response_l.put ("difficulty_threshold", nano::to_string_hex (node.network_params.network.publish_threshold));
-	response_l.put ("difficulty_active", nano::to_string_hex (node.active.active_difficulty ()));
+	auto difficulty_active = node.active.active_difficulty ();
+	response_l.put ("difficulty_active", nano::to_string_hex (difficulty_active));
+	float multiplier = nano::difficulty::to_multiplier (difficulty_active, node.network_params.network.publish_threshold);
+	response_l.put ("multiplier", std::to_string (multiplier));
 	response_errors ();
 }
 
@@ -4446,7 +4449,7 @@ void nano::json_handler::work_validate ()
 		bool valid (!invalid && result_difficulty >= difficulty);
 		response_l.put ("valid", valid ? "1" : "0");
 		response_l.put ("value", nano::to_string_hex (result_difficulty));
-		float multiplier = static_cast<float> (-node.network_params.network.publish_threshold) / (-result_difficulty);
+		float multiplier = nano::difficulty::to_multiplier (result_difficulty, node.network_params.network.publish_threshold);
 		response_l.put ("multiplier", std::to_string (multiplier));
 	}
 	response_errors ();

nano/node/wallet.cpp

@@ -1472,6 +1472,9 @@ void nano::work_watcher::run ()
 					}
 					node.network.flood_block (block);
 					node.active.update_difficulty (*block.get ());
+					lock.lock ();
+					i.second = block;
+					lock.unlock ();
 				}
 				lock.lock ();
 			}

nano/rpc_test/rpc.cpp

@@ -3216,6 +3216,8 @@ TEST (rpc, work_validate)
 		uint64_t value;
 		ASSERT_FALSE (nano::from_string_hex (value_text, value));
 		ASSERT_GE (value, params.network.publish_threshold);
+		double multiplier (response.json.get<double> ("multiplier"));
+		ASSERT_NEAR (multiplier, nano::difficulty::to_multiplier (value, params.network.publish_threshold), 1e-6);
 	}
 	uint64_t work2 (0);
 	request.put ("work", nano::to_string_hex (work2));
@@ -3233,6 +3235,8 @@ TEST (rpc, work_validate)
 		uint64_t value;
 		ASSERT_FALSE (nano::from_string_hex (value_text, value));
 		ASSERT_GE (params.network.publish_threshold, value);
+		double multiplier (response.json.get<double> ("multiplier"));
+		ASSERT_NEAR (multiplier, nano::difficulty::to_multiplier (value, params.network.publish_threshold), 1e-6);
 	}
 	uint64_t result_difficulty;
 	ASSERT_FALSE (nano::work_validate (hash, work1, &result_difficulty));
@@ -3277,28 +3281,6 @@ TEST (rpc, work_validate)
 		bool validate (response.json.get<bool> ("valid"));
 		ASSERT_TRUE (validate);
 	}
-	// Test the multiplier field in the response
-	// It's a multiplier from the base network threshold, so we make sure the test network threshold has not been changed
-	// The work and its value/multiplier were calculated beforehand
-	ASSERT_EQ (params.network.publish_threshold, 0xff00000000000000);
-	request.put ("work", nano::to_string_hex (0x4b52c90f538bbb60));
-	{
-		test_response response (request, rpc.config.port, system.io_ctx);
-		system.deadline_set (5s);
-		while (response.status == 0)
-		{
-			ASSERT_NO_ERROR (system.poll ());
-		}
-		ASSERT_EQ (200, response.status);
-		bool validate (response.json.get<bool> ("valid"));
-		ASSERT_TRUE (validate);
-		std::string value_text (response.json.get<std::string> ("value"));
-		uint64_t value;
-		ASSERT_FALSE (nano::from_string_hex (value_text, value));
-		ASSERT_EQ (value, 0xfff27e7a57c285cd);
-		double multiplier (response.json.get<double> ("multiplier"));
-		ASSERT_NEAR (multiplier, 18.9546, 1e-4);
-	}
 }
 
 TEST (rpc, successors)

nano/secure/common.cpp

@@ -38,7 +38,7 @@ char const * test_genesis_data = R"%%%({
 	"source": "B0311EA55708D6A53C75CDBF88300259C6D018522FE3D4D0A242E431F9E8B6D0",
 	"representative": "xrb_3e3j5tkog48pnny9dmfzj1r16pg8t1e76dz5tmac6iq689wyjfpiij4txtdo",
 	"account": "xrb_3e3j5tkog48pnny9dmfzj1r16pg8t1e76dz5tmac6iq689wyjfpiij4txtdo",
-	"work": "9680625b39d3363d",
+	"work": "7b42a00ee91d5810",
 	"signature": "ECDA914373A2F0CA1296475BAEE40500A7F0A7AD72A5A80C81D7FAB7F6C802B2CC7DB50F5DD0FB25B2EF11761FA7344A158DD5A700B21BD47DE5BD0F63153A02"
 	})%%%";