Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

txscript.ComputePkScript incorrectly assumes P2WSH #1767

Open
chappjc opened this issue Oct 29, 2021 · 2 comments
Open

txscript.ComputePkScript incorrectly assumes P2WSH #1767

chappjc opened this issue Oct 29, 2021 · 2 comments

Comments

@chappjc
Copy link
Contributor

chappjc commented Oct 29, 2021

When using txscript.ComputePkScript with any witness data that is not a P2WPKH, it is assumed to be P2WSH:

btcd/txscript/pkscript.go

Lines 253 to 262 in 31791ba

// For any other witnesses, we'll assume it's a P2WSH witness.
default:
scriptHash := sha256.Sum256(lastWitnessItem)
script, err := payToWitnessScriptHashScript(scriptHash[:])
if err != nil {
return pkScript, err
}
pkScript.class = WitnessV0ScriptHashTy
copy(pkScript.script[:], script)

However, this will return an incorrect pkScript for anything but P2WSH witness data, such as when the prevout is a witness_v1_taproot type. For example: https://play.golang.org/p/cRsmrGFMzVT

computeWitnessPkScript should return txscript.ErrUnsupportedScriptType for any wire.TxWitness that cannot be reliably identified by its witness version and number of items on the witness stack.

This is important to avoid incorrect pkScript computation in neutrino's block-filter validation: https://github.com/lightninglabs/neutrino/blob/51686db787e01a3709b1583af3e20e916811d585/verification.go#L107

@chappjc
Copy link
Contributor Author

chappjc commented Oct 30, 2021

The more I think about this, the more I think inferring the PkScript from witness data is not possible. The witness script version is part of the pkScript itself, and even if v0 were the only version, a p2wsh could be misidentified as a p2wpkh if it has a script of length 33 following another witness stack item.
neutrino/VerifyBasicBlockFilter I think should not attempt to check that the filter includes any pk scripts of the inputs being spent.

@guggero
Copy link
Collaborator

guggero commented Nov 1, 2021

Good find. I'll take a look at this since this indeed seems to break the Neutrino filter validation.

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

No branches or pull requests

2 participants