Add partial support for PSBT Taproot Fields (BIP371) #128
Conversation
sstone
force-pushed
the
snapshot/psbt-v2
branch
from
a2e1477 to
66b9c4f
Compare
src/commonMain/kotlin/fr/acinq/bitcoin/LexicographicalOrdering.kt
Outdated
Show resolved
Hide resolved
|
It seems to me that PSBTv2 is something completely different from adding taproot or musig fields to PSBT. Can you clarify exactly what is covered by this PR? As far as I understand, Bitcoin Core hasn't merged support for PSBTv2 yet (which gets rid of the global tx field), so we should stick to PSBTv0 and just add support for BIPs 371 and 373? |
sstone
changed the title
Here we only support taproot fields that would allow us to sign BIP86 transactions.
sstone
force-pushed
the
snapshot/psbt-v2
branch
from
66b9c4f to
a9cc7c7
Compare
Yes I've updated the PR description and commit message, here I'm just adding what we need to support p2tr wallets in eclair. |
| Script.parse(input.txOut.publicKeyScript) | ||
| }.getOrElse { | ||
| return Either.Left(UpdateFailure.InvalidWitnessUtxo("failed to parse redeem script")) | ||
| val spentOutputs = this.inputs.mapNotNull { it.witnessUtxo ?: it.nonWitnessUtxo?.txOut?.get(this.global.tx.txIn[inputIndex].outPoint.index.toInt()) } |
There was a problem hiding this comment.
I don't think this is correct: you are trying to get the list of TxOut that are spent by this transaction, right? But the issue is that you're always using inputIndex instead of using each input's index? We must use a mapIndexed here I believe, and since we need all the txOut, we should fail if one of them is null?
Otherwise we will fail to sign transaction that also include non-witness inputs, which is something that could be controlled by a malicious counterparty providing the PSBT.
This is fixed in #134
There was a problem hiding this comment.
Yes that was wrong, thanks for the fix!
| createOutput(redeemScript, witnessScript, derivationPaths, unknown) | ||
| val taprootInternalKey = known.find { it.key[0] == 0x05.toByte() }?.let { | ||
| when { | ||
| it.key.size() != 1 -> return Either.Left(ParseFailure.InvalidTxInput("taproot internal key entry must have an empty key")) |
There was a problem hiding this comment.
The error should be InvalidTxOutput here and below (fixed in #134).
- fix a bug in the index used for previous inputs - use `InvalidTxOutput` for invalid taproot output fields - add test vectors from BIP 371 - reformat to match the rest of the file Note that I updated the "build and sign a BIP86 psbt" test to include a non-segwit input, this way it fails without the fix to the input index.
We only add what is necessary to create, update and sign taproot (BIP86) outputs.