mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-21 14:34:49 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#25132: consensus: Add BIP-341 specified constraints in ComputeTaprootMerkleRoot

Browse source 
bd7c5e2f0a Add BIP-341 specified constraints to `ComputeTaprootMerkleRoot` (David Bakin)

Pull request description:

  [**N.B.:** This PR **_does not change the consensus_**.  It only adds `assert` statements according to the current consensus in consensus-sensitive code (`interpreter.cpp`). So that's why the bot added the "consensus" tag and I prefixed the PR title with "consensus".]

  BIP 341 specifies [constraints on the size of the control block _c_ used to compute the taproot merkle root](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules).

  > The last stack element is called the control block _c_, and must have length _33 + 32m_, for a value of _m_ that is an integer between 0 and 128, inclusive. Fail if it does not have such a length.

  The actual merkle root is computed in `ComputeTaprootMerkleRoot` ([interpreter.cpp@1833](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1833)) - this code does _not_ check these constraints.

  All the callers do check the constraints before calling `ComputeTaprootMerkleRoot`.  But in the future there may be more callers, and these checks may be inadvertently omitted at those future calls.  Also, code at/near the current call sites may also change and skip these checks.  Therefore _this PR adds those checks as `asserts` directly in `ComputeTaprootMerkleRoot`_ to help prevent that error.

  No unit tests provided: they'd have to be death tests as these are `assert` statements which raise `SIGABRT` and kill the program.  Boost Test has a way to implement death tests (see the in-progress draft PR #25097 at [this code (you may have to click to expand the diff)](https://github.com/bitcoin/bitcoin/pull/25097/files#diff-21483d0e032747850208f21325b29cde89e9c1f55f83a7a166a388cc5c27115aR1089) and could be added here if desired by reviewers.

  Current callers of `ComputeTaprootMerkleRoot`:
  - `InferTaprootTree` ([standard.cpp@1552](https://github.com/bitcoin/bitcoin/blob/master/src/script/standard.cpp#L546))
  - `VerifyTaprootCommittment` ([interpreter.cpp@1859](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1859)) does a partial check, but it is called from `VerifyWitnessProgram` ([interpreter.cpp@1922](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1918)) where a full check is done

ACKs for top commit:
  sipa:
    ACK bd7c5e2f0a
  theStack:
    ACK bd7c5e2f0a

Tree-SHA512: cec5aebfdf9fc9d13011abdf8427c934edf1df1ffc32b3e7cc5580f12809f24cf83e64ab0c843fabfce3591873bfcfa248277b30820fd786684319a196e4e550
This commit is contained in:
laanwj 2022-05-26 14:14:03 +02:00
commit cacbdbaa95
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
1 changed files with 4 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/script/interpreter.cpp
View file

@ -1832,6 +1832,10 @@ uint256 ComputeTapleafHash(uint8_t leaf_version, const CScript& script)
uint256 ComputeTaprootMerkleRoot(Span<const unsigned char> control, const uint256& tapleaf_hash)
{
assert(control.size() >= TAPROOT_CONTROL_BASE_SIZE);
assert(control.size() <= TAPROOT_CONTROL_MAX_SIZE);
assert((control.size() - TAPROOT_CONTROL_BASE_SIZE) % TAPROOT_CONTROL_NODE_SIZE == 0);
const int path_len = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE;
uint256 k = tapleaf_hash;
for (int i = 0; i < path_len; ++i) {