Fix taproot sig validation again. #1935
Conversation
|
Unit tests are broken... This code is used in signing too... |
ts_src/psbt.ts
Outdated
| @@ -1743,8 +1754,8 @@ function getTaprootHashesForSig( | |||
|
|
|||
| const hashes = []; | |||
| if (input.tapInternalKey && !tapLeafHashToSign) { | |||
| const outputKey = tweakInternalPubKey(inputIndex, input); | |||
| if (toXOnly(pubkey).equals(outputKey)) { | |||
| const outputKey = getPrevoutTaprootKey(inputIndex, input, cache); | |||
There was a problem hiding this comment.
This part is breaking one of the test vectors in the unit test for signing.
The test vector says Sign PSBT with 3 inputs [P2PKH, P2TR (key-path), P2WPKH] and at this point.
This is confusing...
ECPair keypair (pubkey) 4fef5c5163bea69a93e74a59672bbeb081837077cb94cfa6e481a5cf00d8ab18
tweak(tapInternalKey) 4fef5c5163bea69a93e74a59672bbeb081837077cb94cfa6e481a5cf00d8ab18
taproot prevOutput key 9421e734b0f9d2c467ea7dd197c61acb4467cdcbc9f4cb0c571f8b63a5c40cae
input.tapInternalKey 9421e734b0f9d2c467ea7dd197c61acb4467cdcbc9f4cb0c571f8b63a5c40cae
The tapInternalKey and the prevout script key are the same, and the WIF for the keypair is the self-tweaked version of tapInternalKey/prevOutputKey.
I think this test vector is incorrect...
There was a problem hiding this comment.
> p.data.inputs[1].witnessUtxo
{
script: <Buffer 51 20 94 21 e7 34 b0 f9 d2 c4 67 ea 7d d1 97 c6 1a cb 44 67 cd cb c9 f4 cb 0c 57 1f 8b 63 a5 c4 0c ae>,
value: 67000
}
> p.data.inputs[1].tapInternalKey
<Buffer 94 21 e7 34 b0 f9 d2 c4 67 ea 7d d1 97 c6 1a cb 44 67 cd cb c9 f4 cb 0c 57 1f 8b 63 a5 c4 0c ae>
junderw
force-pushed
the
fix/p2tr-sigs-pt2
branch
from
4a3a45e
to
299a5e0
Compare
Output keys are not guaranteed to be tweaked against their pubkey's hash. That is only a convention decided upon for NUMS properties.
junderw
force-pushed
the
fix/p2tr-sigs-pt2
branch
from
299a5e0
to
8d2d7db
Compare
|
In the end, a taproot signature is a signature with the key in the previous output, so when we call validateSignatures with a pubkey it should be only the prevout script pubkey we care about. Perhaps we should make another method to verify if a given pubkey will match the prevout script if it is tweaked...? Anywho... this is a big change so I've decided not to rush the publish. |
|
Re: test vector... how were these generated? btw |
|
@motorina0 Please post-publish review if possible. 6.1.2 is broken, so I will release this ASAP. |
Fixes #1934
The last PR made the "should be invalid" sig invalid because I was chopping off the wrong byte of the 65 byte signature.
After fixing it, I noticed we didn't actually fix the previous issue either.
This time we actually fixed the issue.
The problem was the assumption that the transaction is always using the "tweak the internal pubkey with a hash of itself" method.
Removing that assumption should fix the issue.
@motorina0 Please review the source and see my comments below first.