Skip to content

fix: preserve explicit zero tapleaf versions - #2355

Open
Harshil-1603 wants to merge 2 commits into
bitcoinjs:masterfrom
Harshil-1603:fix/tapleaf-zero-version-2354
Open

Harshil-1603 wants to merge 2 commits into
bitcoinjs:masterfrom
Harshil-1603:fix/tapleaf-zero-version-2354

Conversation

@Harshil-1603

Copy link
Copy Markdown

tapleafHash used a truthiness fallback, so an explicit leaf version 0x00 was hashed as 0xc0. Use nullish coalescing to preserve the caller's version and default only when it is absent.

Add independently calculated BIP341 hash vectors for OP_TRUE at versions 0x00, 0xc0, and 0xc2, covering both direct hashing and toHashTree, plus the omitted-version default. Include the regenerated CommonJS and ESM files.

Fixes #2354.

Validation:

  • The version-zero regression failed on the original code and passes with the fix.
  • npm test: 2,664 tests passed; build, formatting, lint, and coverage thresholds passed.
  • npm run lint:tests and npm run gitdiff:ci passed.
  • Regtest integration was not run locally; it requires the project's Bitcoin Core test service.

@fametrano fametrano left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK b70cda9, with one more line: tapTreeToList in bip371.ts still has tree.version || LEAF_VERSION_TAPSCRIPT, so tapTreeToList({ output, version: 0 }) returns leaf version 192. Changing it to ?? makes it consistent with this fix. On the committed esm build, tapleafHash for versions 0x00, 0xc0 and 0xc2 matches an independent BIP341 tagged-hash computation, and the tapTreeToList call above still gives 192.

@fametrano

Copy link
Copy Markdown

I reviewed this on b70cda9, but its "Run Tests" run has been waiting for a workflow approval, so there is no CI result. Could a maintainer approve it?

Preserve explicit leaf versions when converting Taproot trees to BIP371 lists. Cover zero, tapscript, custom versions, and mixed default/zero leaves. Regenerate CommonJS and ESM builds.

Assisted-by: OpenAI Codex (GPT-6)

@fametrano fametrano left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reACK 458d2c6. The new commit changes only the tapTreeToList line I flagged, from || to ??, and adds tests; npm test passes, and the two new zero-version tests fail on b70cda9's code.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tapleafHash hashes leaf version 0x00 as 0xc0 (|| instead of ??)

3 participants