From 89e3d8df92e53d34ee6f70cbc82337b351378f10 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Sat, 30 Mar 2024 15:41:21 -0400 Subject: [PATCH] Address PR comments, templatize `calculate_merkle`. --- libraries/chain/block_state.cpp | 4 +- libraries/chain/controller.cpp | 16 +++---- .../chain/include/eosio/chain/merkle.hpp | 47 ++++++++++++++----- .../include/eosio/chain/merkle_legacy.hpp | 2 +- unittests/merkle_tree_tests.cpp | 26 +++++----- 5 files changed, 59 insertions(+), 36 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 1528284875..bae2ea5d43 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -88,8 +88,8 @@ block_state_ptr block_state::create_if_genesis_block(const block_state_legacy& b result.valid_qc = {}; // best qc received from the network inside block extension, empty until first savanna proper IF block // Calculate Merkle tree root in Savanna way so that it is stored in Leaf Node when building block_state. - auto digests = *bsp.action_receipt_digests_savanna; - auto action_mroot_svnn = calculate_merkle(std::move(digests)); + const auto& digests = *bsp.action_receipt_digests_savanna; + auto action_mroot_svnn = calculate_merkle(digests); // build leaf_node and validation_tree valid_t::finality_leaf_node_t leaf_node { diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 9d481ca7e8..9d51f738e0 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -708,13 +708,13 @@ struct building_block { auto [transaction_mroot, action_mroot] = std::visit( overloaded{[&](digests_t& trx_receipts) { // calculate the two merkle roots in separate threads auto trx_merkle_fut = - post_async_task(ioc, [&]() { return calculate_merkle(std::move(trx_receipts)); }); + post_async_task(ioc, [&]() { return calculate_merkle(trx_receipts); }); auto action_merkle_fut = - post_async_task(ioc, [&]() { return calculate_merkle(std::move(*action_receipts.digests_s)); }); + post_async_task(ioc, [&]() { return calculate_merkle(*action_receipts.digests_s); }); return std::make_pair(trx_merkle_fut.get(), action_merkle_fut.get()); }, [&](const checksum256_type& trx_checksum) { - return std::make_pair(trx_checksum, calculate_merkle(std::move(*action_receipts.digests_s))); + return std::make_pair(trx_checksum, calculate_merkle(*action_receipts.digests_s)); }}, trx_mroot_or_receipt_digests()); @@ -1308,8 +1308,8 @@ struct controller_impl { // IRREVERSIBLE applies (validates) blocks when irreversible, new_valid will be done after apply in log_irreversible assert(read_mode == db_read_mode::IRREVERSIBLE || legacy->action_receipt_digests_savanna); if (legacy->action_receipt_digests_savanna) { - auto digests = *legacy->action_receipt_digests_savanna; - auto action_mroot = calculate_merkle(std::move(digests)); + const auto& digests = *legacy->action_receipt_digests_savanna; + auto action_mroot = calculate_merkle(digests); // Create the valid structure for producing new_bsp->valid = prev->new_valid(*new_bsp, action_mroot, new_bsp->strong_digest); } @@ -1522,8 +1522,8 @@ struct controller_impl { validator_t{}, skip_validate_signee); // legacy_branch is from head, all should be validated assert(bspl->action_receipt_digests_savanna); - auto digests = *bspl->action_receipt_digests_savanna; - auto action_mroot = calculate_merkle(std::move(digests)); + const auto& digests = *bspl->action_receipt_digests_savanna; + auto action_mroot = calculate_merkle(digests); // Create the valid structure for producing new_bsp->valid = prev->new_valid(*new_bsp, action_mroot, new_bsp->strong_digest); prev = new_bsp; @@ -4066,7 +4066,7 @@ struct controller_impl { // @param if_active true if instant finality is active static checksum256_type calc_merkle( deque&& digests, bool if_active ) { if (if_active) { - return calculate_merkle( std::move(digests) ); + return calculate_merkle( digests ); } else { return calculate_merkle_legacy( std::move(digests) ); } diff --git a/libraries/chain/include/eosio/chain/merkle.hpp b/libraries/chain/include/eosio/chain/merkle.hpp index b6e78c73f6..91d26d5e21 100644 --- a/libraries/chain/include/eosio/chain/merkle.hpp +++ b/libraries/chain/include/eosio/chain/merkle.hpp @@ -21,11 +21,6 @@ inline digest_type hash_combine(const digest_type& a, const digest_type& b) { return digest_type::hash(std::make_pair(std::cref(a), std::cref(b))); } -// does not overwrite passed sequence -// -// log2 recursion OK, uses less than 5KB stack space for 4 billion digests -// appended (or 0.25% of default 2MB thread stack size on Ubuntu). -// ----------------------------------------------------------------------- template requires std::is_same_v::value_type>, digest_type> inline digest_type calculate_merkle_pow2(const It& start, const It& end) { @@ -56,8 +51,26 @@ inline digest_type calculate_merkle_pow2(const It& start, const It& end) { } } +} // namespace detail + +// ************* public interface starts here ************************************************ + +// ------------------------------------------------------------------------ +// calculate_merkle: +// ----------------- +// takes two random access iterators delimiting a sequence of `digest_type`, +// returns the root digest for the provided sequence. +// +// does not overwrite passed sequence +// +// log2 recursion OK, uses less than 5KB stack space for 4 billion digests +// appended (or 0.25% of default 2MB thread stack size on Ubuntu). +// ------------------------------------------------------------------------ template -requires std::is_same_v::value_type>, digest_type> +#if __cplusplus >= 202002L +requires std::random_access_iterator && + std::is_same_v::value_type>, digest_type> +#endif inline digest_type calculate_merkle(const It& start, const It& end) { assert(end >= start); auto size = static_cast(end - start); @@ -66,16 +79,26 @@ inline digest_type calculate_merkle(const It& start, const It& end) { auto midpoint = detail::bit_floor(size); if (size == midpoint) - return calculate_merkle_pow2(start, end); + return detail::calculate_merkle_pow2(start, end); auto mid = start + midpoint; - return hash_combine(calculate_merkle_pow2(start, mid), calculate_merkle(mid, end)); + return detail::hash_combine(detail::calculate_merkle_pow2(start, mid), + calculate_merkle(mid, end)); } -} - -inline digest_type calculate_merkle(const deque& ids) { - return detail::calculate_merkle(ids.cbegin(), ids.cend()); +// -------------------------------------------------------------------------- +// calculate_merkle: +// ----------------- +// takes a container or `std::span` of `digest_type`, returns the root digest +// for the sequence of digests in the container. +// -------------------------------------------------------------------------- +template +#if __cplusplus >= 202002L +requires std::random_access_iterator && + std::is_same_v, digest_type> +#endif +inline digest_type calculate_merkle(const Cont& ids) { + return calculate_merkle(ids.begin(), ids.end()); // cbegin not supported for std::span until C++23. } diff --git a/libraries/chain/include/eosio/chain/merkle_legacy.hpp b/libraries/chain/include/eosio/chain/merkle_legacy.hpp index dc4a0ce2f8..5746f2e07f 100644 --- a/libraries/chain/include/eosio/chain/merkle_legacy.hpp +++ b/libraries/chain/include/eosio/chain/merkle_legacy.hpp @@ -30,7 +30,7 @@ namespace detail { return make_pair(make_legacy_left_digest(l), make_legacy_right_digest(r)); }; -} +} // namespace detail /** * Calculates the merkle root of a set of digests, if ids is odd it will duplicate the last id. diff --git a/unittests/merkle_tree_tests.cpp b/unittests/merkle_tree_tests.cpp index 52e7a74bba..066faafdcd 100644 --- a/unittests/merkle_tree_tests.cpp +++ b/unittests/merkle_tree_tests.cpp @@ -104,7 +104,7 @@ BOOST_AUTO_TEST_CASE(basic_append_and_root_check) { auto node1 = fc::sha256::hash("Node1"); tree.append(node1); BOOST_CHECK_EQUAL(tree.get_root(), node1); - BOOST_CHECK_EQUAL(calculate_merkle({node1}), node1); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(&node1, 1)), node1); } BOOST_AUTO_TEST_CASE(multiple_appends) { @@ -124,48 +124,48 @@ BOOST_AUTO_TEST_CASE(multiple_appends) { tree.append(node1); BOOST_CHECK_EQUAL(tree.get_root(), node1); - BOOST_CHECK_EQUAL(calculate_merkle({node1}), node1); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 1)), node1); tree.append(node2); BOOST_CHECK_EQUAL(tree.get_root(), hash(node1, node2)); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 2}), hash(node1, node2)); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 2)), hash(node1, node2)); tree.append(node3); auto calculated_root = hash(hash(node1, node2), node3); BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 3}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 3)), calculated_root); tree.append(node4); auto first_four_tree = hash(hash(node1, node2), hash(node3, node4)); calculated_root = first_four_tree; BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 4}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 4)), calculated_root); tree.append(node5); calculated_root = hash(first_four_tree, node5); BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 5}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 5)), calculated_root); tree.append(node6); calculated_root = hash(first_four_tree, hash(node5, node6)); BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 6}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 6)), calculated_root); tree.append(node7); calculated_root = hash(first_four_tree, hash(hash(node5, node6), node7)); BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 7}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 7)), calculated_root); tree.append(node8); auto next_four_tree = hash(hash(node5, node6), hash(node7, node8)); calculated_root = hash(first_four_tree, next_four_tree); BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 8}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 8)), calculated_root); tree.append(node9); calculated_root = hash(hash(first_four_tree, next_four_tree), node9); BOOST_CHECK_EQUAL(tree.get_root(), calculated_root); - BOOST_CHECK_EQUAL(calculate_merkle({first, first + 9}), calculated_root); + BOOST_CHECK_EQUAL(calculate_merkle(std::span(first, 9)), calculated_root); } BOOST_AUTO_TEST_CASE(consistency_over_large_range) { @@ -177,7 +177,7 @@ BOOST_AUTO_TEST_CASE(consistency_over_large_range) { for (size_t j=0; j>); BOOST_CHECK_EQUAL(incr_root, calc_root); } @@ -277,7 +277,7 @@ BOOST_AUTO_TEST_CASE(perf_test_many_small) { }; { - auto [incr_root, calc_root] = perf_test("savanna", incremental_merkle_tree(), calculate_merkle); + auto [incr_root, calc_root] = perf_test("savanna", incremental_merkle_tree(), calculate_merkle>); BOOST_CHECK_EQUAL(incr_root, calc_root); }