From 41506cb721a1af32d530cbaff2d3c7ce620f5c57 Mon Sep 17 00:00:00 2001 From: Guilherme Lawless Date: Mon, 13 May 2019 17:26:45 +0100 Subject: [PATCH] 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 | 1 + nano/core_test/difficulty.cpp | 35 +++++++++++++++++ nano/core_test/node.cpp | 13 ++++--- nano/core_test/wallet.cpp | 7 ++-- nano/lib/numbers.cpp | 12 ++++++ nano/lib/numbers.hpp | 6 +++ nano/node/active_transactions.cpp | 62 +++++++++++++++---------------- nano/node/active_transactions.hpp | 2 +- nano/node/json_handler.cpp | 7 +++- nano/node/wallet.cpp | 3 ++ nano/rpc_test/rpc.cpp | 26 ++----------- nano/secure/common.cpp | 2 +- 12 files changed, 110 insertions(+), 66 deletions(-) create mode 100644 nano/core_test/difficulty.cpp diff --git a/nano/core_test/CMakeLists.txt b/nano/core_test/CMakeLists.txt index fbb36b8f..ac7e4c6e 100644 --- a/nano/core_test/CMakeLists.txt +++ b/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 diff --git a/nano/core_test/difficulty.cpp b/nano/core_test/difficulty.cpp new file mode 100644 index 00000000..7b4228af --- /dev/null +++ b/nano/core_test/difficulty.cpp @@ -0,0 +1,35 @@ +#include + +#include + +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 + } +} diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index 05b09710..f51f283b 100644 --- a/nano/core_test/node.cpp +++ b/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))); - ASSERT_EQ (node1.active.active_difficulty (), static_cast (sum / node1.active.difficulty_cb.size ())); + auto sum (std::accumulate (node1.active.multipliers_cb.begin (), node1.active.multipliers_cb.end (), double(0))); + 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 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)); - ASSERT_EQ (node1.active.active_difficulty (), static_cast (sum / node1.active.difficulty_cb.size ())); + sum = std::accumulate (node1.active.multipliers_cb.begin (), node1.active.multipliers_cb.end (), double(0)); + ASSERT_EQ (node1.active.active_difficulty (), nano::difficulty::from_multiplier (sum / node1.active.multipliers_cb.size (), node1.network_params.network.publish_threshold)); } namespace diff --git a/nano/core_test/wallet.cpp b/nano/core_test/wallet.cpp index 3d3fd63b..32f769ff 100644 --- a/nano/core_test/wallet.cpp +++ b/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 lock (node.active.mutex); - //fill difficulty_cb and update active difficulty; - for (auto i (0); i < node.active.difficulty_cb.size (); i++) + //fill multipliers_cb and update active difficulty; + 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 ())); diff --git a/nano/lib/numbers.cpp b/nano/lib/numbers.cpp index 174a5572..b49b7f09 100644 --- a/nano/lib/numbers.cpp +++ b/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 ((-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 (-base_difficulty_a) / (-difficulty_a); +} \ No newline at end of file diff --git a/nano/lib/numbers.hpp b/nano/lib/numbers.hpp index 051a0083..1b583548 100644 --- a/nano/lib/numbers.hpp +++ b/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 diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 04d92539..f2c35142 100644 --- a/nano/node/active_transactions.cpp +++ b/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 processed_blocks; std::vector> 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 (sum / elections_list.size ())); - // Potential overflow check - uint64_t divider (1); - if (elections_list.size () > 1000000 && (average - node.network_params.network.publish_threshold) > elections_list.size ()) + double multiplier = sum / elections_list.size (); + uint64_t average = nano::difficulty::from_multiplier (multiplier, node.network_params.network.publish_threshold); + auto highest_level = elections_list.back ().second; + 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 & 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; - assert (min >= node.network_params.network.publish_threshold); - uint128_t max = (--roots.get<1> ().end ())->adjusted_difficulty; - assert (max >= node.network_params.network.publish_threshold); - difficulty = static_cast ((min + max) / 2); + std::vector active_root_difficulties; + active_root_difficulties.reserve (roots.size ()); + for (auto & root : roots) + { + 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); - difficulty_cb.push_front (difficulty); - auto sum (std::accumulate (node.active.difficulty_cb.begin (), node.active.difficulty_cb.end (), uint128_t (0))); - difficulty = static_cast (sum / difficulty_cb.size ()); + assert (multiplier >= 1); + multipliers_cb.push_front (multiplier); + auto sum (std::accumulate (multipliers_cb.begin (), multipliers_cb.end (), double(0))); + 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; } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index ded2a33f..61929a7b 100644 --- a/nano/node/active_transactions.hpp +++ b/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 difficulty_cb; + boost::circular_buffer multipliers_cb; uint64_t trended_active_difficulty; private: diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index f2a7a146..14d56b11 100644 --- a/nano/node/json_handler.cpp +++ b/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 (-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 (); diff --git a/nano/node/wallet.cpp b/nano/node/wallet.cpp index 07bbfa99..fff595f3 100644 --- a/nano/node/wallet.cpp +++ b/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 (); } diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 42aa3a25..eab8ab25 100644 --- a/nano/rpc_test/rpc.cpp +++ b/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 ("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 ("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 ("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 ("valid")); - ASSERT_TRUE (validate); - std::string value_text (response.json.get ("value")); - uint64_t value; - ASSERT_FALSE (nano::from_string_hex (value_text, value)); - ASSERT_EQ (value, 0xfff27e7a57c285cd); - double multiplier (response.json.get ("multiplier")); - ASSERT_NEAR (multiplier, 18.9546, 1e-4); - } } TEST (rpc, successors) diff --git a/nano/secure/common.cpp b/nano/secure/common.cpp index beb058a2..9a66cff6 100644 --- a/nano/secure/common.cpp +++ b/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" })%%%";