Skip to content

Commit a95a8ba

Browse files
committed
Merge bitcoin/bitcoin#31197: refactor: mining interface 30955 followups
f866781 Check leaves size maximum in MerkleComputation (Sjors Provoost) 4d57288 refactor: use CTransactionRef in submitSolution (Sjors Provoost) 2e81791 Drop TransactionMerklePath default position arg (Sjors Provoost) 39d3b53 Rename merkle branch to path (Sjors Provoost) Pull request description: This PR implements the refactors suggested in bitcoin/bitcoin#30955 (review). ACKs for top commit: tdb3: code review re-ACK f866781 itornaza: re ACK f866781 ryanofsky: Code review ACK f866781 only changes since last review are a whitespace change and adding an Assume statement to check for size_t -> uint32_t overflows Tree-SHA512: 661b5d5d0e24b2269bf33ab1484e37c36e67b32a7796d77ca3b1856d3043378b081ad43c32a8638b46fa8c0de51c823fd9747dd9fc81f958f20d327bf330a47c
2 parents cd3d9fa + f866781 commit a95a8ba

File tree

5 files changed

+25
-24
lines changed

5 files changed

+25
-24
lines changed

src/consensus/merkle.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <consensus/merkle.h>
66
#include <hash.h>
7+
#include <util/check.h>
78

89
/* WARNING! If you're reading this because you're learning about crypto
910
and/or designing a new system that will use merkle trees, keep in mind
@@ -84,8 +85,10 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated)
8485
}
8586

8687
/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */
87-
static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector<uint256>* pbranch) {
88-
if (pbranch) pbranch->clear();
88+
static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot, bool* pmutated, uint32_t leaf_pos, std::vector<uint256>* path)
89+
{
90+
if (path) path->clear();
91+
Assume(leaves.size() <= UINT32_MAX);
8992
if (leaves.size() == 0) {
9093
if (pmutated) *pmutated = false;
9194
if (proot) *proot = uint256();
@@ -105,18 +108,18 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot
105108
// First process all leaves into 'inner' values.
106109
while (count < leaves.size()) {
107110
uint256 h = leaves[count];
108-
bool matchh = count == branchpos;
111+
bool matchh = count == leaf_pos;
109112
count++;
110113
int level;
111114
// For each of the lower bits in count that are 0, do 1 step. Each
112115
// corresponds to an inner value that existed before processing the
113116
// current leaf, and each needs a hash to combine it.
114117
for (level = 0; !(count & ((uint32_t{1}) << level)); level++) {
115-
if (pbranch) {
118+
if (path) {
116119
if (matchh) {
117-
pbranch->push_back(inner[level]);
120+
path->push_back(inner[level]);
118121
} else if (matchlevel == level) {
119-
pbranch->push_back(h);
122+
path->push_back(h);
120123
matchh = true;
121124
}
122125
}
@@ -144,8 +147,8 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot
144147
// If we reach this point, h is an inner value that is not the top.
145148
// We combine it with itself (Bitcoin's special rule for odd levels in
146149
// the tree) to produce a higher level one.
147-
if (pbranch && matchh) {
148-
pbranch->push_back(h);
150+
if (path && matchh) {
151+
path->push_back(h);
149152
}
150153
h = Hash(h, h);
151154
// Increment count to the value it would have if two entries at this
@@ -154,11 +157,11 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot
154157
level++;
155158
// And propagate the result upwards accordingly.
156159
while (!(count & ((uint32_t{1}) << level))) {
157-
if (pbranch) {
160+
if (path) {
158161
if (matchh) {
159-
pbranch->push_back(inner[level]);
162+
path->push_back(inner[level]);
160163
} else if (matchlevel == level) {
161-
pbranch->push_back(h);
164+
path->push_back(h);
162165
matchh = true;
163166
}
164167
}
@@ -171,18 +174,18 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot
171174
if (proot) *proot = h;
172175
}
173176

174-
static std::vector<uint256> ComputeMerkleBranch(const std::vector<uint256>& leaves, uint32_t position) {
177+
static std::vector<uint256> ComputeMerklePath(const std::vector<uint256>& leaves, uint32_t position) {
175178
std::vector<uint256> ret;
176179
MerkleComputation(leaves, nullptr, nullptr, position, &ret);
177180
return ret;
178181
}
179182

180-
std::vector<uint256> BlockMerkleBranch(const CBlock& block, uint32_t position)
183+
std::vector<uint256> TransactionMerklePath(const CBlock& block, uint32_t position)
181184
{
182185
std::vector<uint256> leaves;
183186
leaves.resize(block.vtx.size());
184187
for (size_t s = 0; s < block.vtx.size(); s++) {
185188
leaves[s] = block.vtx[s]->GetHash();
186189
}
187-
return ComputeMerkleBranch(leaves, position);
190+
return ComputeMerklePath(leaves, position);
188191
}

src/consensus/merkle.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ uint256 BlockWitnessMerkleRoot(const CBlock& block, bool* mutated = nullptr);
2828
* Compute merkle path to the specified transaction
2929
*
3030
* @param[in] block the block
31-
* @param[in] position transaction for which to calculate the merkle path, defaults to coinbase
31+
* @param[in] position transaction for which to calculate the merkle path (0 is the coinbase)
3232
*
3333
* @return merkle path ordered from the deepest
3434
*/
35-
std::vector<uint256> BlockMerkleBranch(const CBlock& block, uint32_t position = 0);
35+
std::vector<uint256> TransactionMerklePath(const CBlock& block, uint32_t position);
3636

3737
#endif // BITCOIN_CONSENSUS_MERKLE_H

src/interfaces/mining.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ class BlockTemplate
5555
*
5656
* @returns if the block was processed, independent of block validity
5757
*/
58-
virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) = 0;
58+
virtual bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) = 0;
5959
};
6060

6161
//! Interface giving clients (RPC, Stratum v2 Template Provider in the future)

src/node/interfaces.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -913,19 +913,17 @@ class BlockTemplateImpl : public BlockTemplate
913913

914914
std::vector<uint256> getCoinbaseMerklePath() override
915915
{
916-
return BlockMerkleBranch(m_block_template->block);
916+
return TransactionMerklePath(m_block_template->block, 0);
917917
}
918918

919-
bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CMutableTransaction coinbase) override
919+
bool submitSolution(uint32_t version, uint32_t timestamp, uint32_t nonce, CTransactionRef coinbase) override
920920
{
921921
CBlock block{m_block_template->block};
922922

923-
auto cb = MakeTransactionRef(std::move(coinbase));
924-
925923
if (block.vtx.size() == 0) {
926-
block.vtx.push_back(cb);
924+
block.vtx.push_back(coinbase);
927925
} else {
928-
block.vtx[0] = cb;
926+
block.vtx[0] = coinbase;
929927
}
930928

931929
block.nVersion = version;

src/test/merkle_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(merkle_test)
135135
if (ntx > 16) {
136136
mtx = m_rng.randrange(ntx);
137137
}
138-
std::vector<uint256> newBranch = BlockMerkleBranch(block, mtx);
138+
std::vector<uint256> newBranch = TransactionMerklePath(block, mtx);
139139
std::vector<uint256> oldBranch = BlockGetMerkleBranch(block, merkleTree, mtx);
140140
BOOST_CHECK(oldBranch == newBranch);
141141
BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx]->GetHash(), newBranch, mtx) == oldRoot);

0 commit comments

Comments
 (0)