Skip to content

Conversation

@summraznboi
Copy link
Contributor

getTaprootHashesForSig is an overly reused function for both signing as well as signature validation.
To better make each path clearer, we created two wrappers on top to clearly state its intent, getTaprootHashesForSigValidation and getTaprootHashesForSigning. Note that getAllTaprootHashesForSig is just renamed to getAllTaprootHashesForSigValidation, since there isn't a corresponding usage for signing.

This separation allows for us to clearly see that for sig validation, we only need to see the existence of tapKeySig to know that this is a key spend path. This is especially important for nonstandard key spend path patterns such as MuSig2 or other offchain multisig signing protocols.

@summraznboi summraznboi force-pushed the tap-internal-key-unneeded-for-sig-validation branch from 1a42dfd to 2d8d57a Compare October 13, 2025 20:32
@junderw junderw requested a review from Copilot October 14, 2025 00:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the taproot signature hash generation logic to separate signing and validation concerns. Previously, a single function getTaprootHashesForSig was used for both purposes, making the code path unclear.

  • Split getTaprootHashesForSig into two wrapper functions: getTaprootHashesForSigning and getTaprootHashesForSigValidation
  • Updated key spend detection logic to use tapKeySig for validation and tapInternalKey for signing
  • Renamed getAllTaprootHashesForSig to getAllTaprootHashesForSigValidation for clarity

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
ts_src/psbt.ts Main TypeScript implementation with function splitting and key spend logic updates
src/esm/psbt.js ES module version with identical refactoring changes
src/cjs/psbt.cjs CommonJS version with identical refactoring changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@junderw junderw merged commit 13aea8c into bitcoinjs:master Oct 14, 2025
11 checks passed
@summraznboi
Copy link
Contributor Author

Thanks for merging this! It would be great to have a bug fix release soon so we can make use of it :)

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.

2 participants