btcsuite / btcd

An alternative full node bitcoin implementation written in Go (golang)
https://github.com/btcsuite/btcd/blob/master/README.md
ISC License
6.29k stars 2.38k forks source link

txscript.ComputePkScript incorrectly assumes P2WSH #1767

Open chappjc opened 3 years ago

chappjc commented 3 years ago

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

https://github.com/btcsuite/btcd/blob/31791ba4dc6ef913b1e8eb7bfb6746b1a118e405/txscript/pkscript.go#L253-L262

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 commented 3 years ago

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 commented 3 years ago

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