From 39d3b538e6a2af8db85077e958970cdcd0ee7f7d Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 31 Oct 2024 10:56:16 -0700 Subject: [PATCH 1/4] Rename merkle branch to path --- src/consensus/merkle.cpp | 29 +++++++++++++++-------------- src/consensus/merkle.h | 2 +- src/node/interfaces.cpp | 2 +- src/test/merkle_tests.cpp | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index dc32f0ab80..4b321d6c88 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -84,8 +84,9 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) } /* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ -static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector* pbranch) { - if (pbranch) pbranch->clear(); +static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t leaf_pos, std::vector* path) +{ + if (path) path->clear(); if (leaves.size() == 0) { if (pmutated) *pmutated = false; if (proot) *proot = uint256(); @@ -105,18 +106,18 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot // First process all leaves into 'inner' values. while (count < leaves.size()) { uint256 h = leaves[count]; - bool matchh = count == branchpos; + bool matchh = count == leaf_pos; count++; int level; // For each of the lower bits in count that are 0, do 1 step. Each // corresponds to an inner value that existed before processing the // current leaf, and each needs a hash to combine it. for (level = 0; !(count & ((uint32_t{1}) << level)); level++) { - if (pbranch) { + if (path) { if (matchh) { - pbranch->push_back(inner[level]); + path->push_back(inner[level]); } else if (matchlevel == level) { - pbranch->push_back(h); + path->push_back(h); matchh = true; } } @@ -144,8 +145,8 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot // If we reach this point, h is an inner value that is not the top. // We combine it with itself (Bitcoin's special rule for odd levels in // the tree) to produce a higher level one. - if (pbranch && matchh) { - pbranch->push_back(h); + if (path && matchh) { + path->push_back(h); } h = Hash(h, h); // Increment count to the value it would have if two entries at this @@ -154,11 +155,11 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot level++; // And propagate the result upwards accordingly. while (!(count & ((uint32_t{1}) << level))) { - if (pbranch) { + if (path) { if (matchh) { - pbranch->push_back(inner[level]); + path->push_back(inner[level]); } else if (matchlevel == level) { - pbranch->push_back(h); + path->push_back(h); matchh = true; } } @@ -171,18 +172,18 @@ static void MerkleComputation(const std::vector& leaves, uint256* proot if (proot) *proot = h; } -static std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position) { +static std::vector ComputeMerklePath(const std::vector& leaves, uint32_t position) { std::vector ret; MerkleComputation(leaves, nullptr, nullptr, position, &ret); return ret; } -std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) +std::vector TransactionMerklePath(const CBlock& block, uint32_t position) { std::vector leaves; leaves.resize(block.vtx.size()); for (size_t s = 0; s < block.vtx.size(); s++) { leaves[s] = block.vtx[s]->GetHash(); } - return ComputeMerkleBranch(leaves, position); + return ComputeMerklePath(leaves, position); } diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h index 363f68039c..ac622602e7 100644 --- a/src/consensus/merkle.h +++ b/src/consensus/merkle.h @@ -32,6 +32,6 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr); * * @return merkle path ordered from the deepest */ -std::vector BlockMerkleBranch(const CBlock& block, uint32_t position = 0); +std::vector TransactionMerklePath(const CBlock& block, uint32_t position = 0); #endif // BITCOIN_CONSENSUS_MERKLE_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index e4ae9400e3..6c06620ed2 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -913,7 +913,7 @@ public: std::vector getCoinbaseMerklePath() override { - return BlockMerkleBranch(m_block_template->block); + return TransactionMerklePath(m_block_template->block); } bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) override diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 2b1cf8595d..f8cb0094fc 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(merkle_test) if (ntx > 16) { mtx = m_rng.randrange(ntx); } - std::vector newBranch = BlockMerkleBranch(block, mtx); + std::vector newBranch = TransactionMerklePath(block, mtx); std::vector oldBranch = BlockGetMerkleBranch(block, merkleTree, mtx); BOOST_CHECK(oldBranch == newBranch); BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx]->GetHash(), newBranch, mtx) == oldRoot); From 2e81791d907288c174aa05dc1b3816e6d988127c Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 31 Oct 2024 11:00:12 -0700 Subject: [PATCH 2/4] Drop TransactionMerklePath default position arg --- src/consensus/merkle.h | 4 ++-- src/node/interfaces.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h index ac622602e7..c722cbe446 100644 --- a/src/consensus/merkle.h +++ b/src/consensus/merkle.h @@ -28,10 +28,10 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr); * Compute merkle path to the specified transaction * * @param[in] block the block - * @param[in] position transaction for which to calculate the merkle path, defaults to coinbase + * @param[in] position transaction for which to calculate the merkle path (0 is the coinbase) * * @return merkle path ordered from the deepest */ -std::vector TransactionMerklePath(const CBlock& block, uint32_t position = 0); +std::vector TransactionMerklePath(const CBlock& block, uint32_t position); #endif // BITCOIN_CONSENSUS_MERKLE_H diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 6c06620ed2..f2518298c0 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -913,7 +913,7 @@ public: std::vector getCoinbaseMerklePath() override { - return TransactionMerklePath(m_block_template->block); + return TransactionMerklePath(m_block_template->block, 0); } bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) override From 4d572882463b20818fcfbd0a2f6fa6c0168e4e4a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 31 Oct 2024 11:05:05 -0700 Subject: [PATCH 3/4] refactor: use CTransactionRef in submitSolution --- src/interfaces/mining.h | 2 +- src/node/interfaces.cpp | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 6f23bf3bb5..05d1ee7e7b 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -55,7 +55,7 @@ public: * * @returns if the block was processed, independent of block validity */ - virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) = 0; + virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) = 0; }; //! Interface giving clients (RPC, Stratum v2 Template Provider in the future) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index f2518298c0..26d2fcb1c6 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -916,16 +916,14 @@ public: return TransactionMerklePath(m_block_template->block, 0); } - bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) override + bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) override { CBlock block{m_block_template->block}; - auto cb = MakeTransactionRef(std::move(coinbase)); - if (block.vtx.size() == 0) { - block.vtx.push_back(cb); + block.vtx.push_back(coinbase); } else { - block.vtx[0] = cb; + block.vtx[0] = coinbase; } block.nVersion = version; From f86678156a3d3ce6488b383a2d6d91d28fcd2b73 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 17 Dec 2024 10:11:54 +0700 Subject: [PATCH 4/4] Check leaves size maximum in MerkleComputation Belt and suspenders for future code changes. Currently this function is only called from TransactionMerklePath() which sets leaves to the block transactions, so the Assume always holds. --- src/consensus/merkle.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index 4b321d6c88..7dd24e1868 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -4,6 +4,7 @@ #include #include +#include /* WARNING! If you're reading this because you're learning about crypto and/or designing a new system that will use merkle trees, keep in mind @@ -87,6 +88,7 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated) static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t leaf_pos, std::vector* path) { if (path) path->clear(); + Assume(leaves.size() <= UINT32_MAX); if (leaves.size() == 0) { if (pmutated) *pmutated = false; if (proot) *proot = uint256();