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

FinalizePsbt fails if input utxo data is absent but fetches that utxo data anyway #7266

Open
DanGould opened this issue Dec 20, 2022 · 1 comment
Labels
feature request Requests for new features psbt

Comments

@DanGould
Copy link

FinalizePsbt fails if input utxo data is absent but fetches that utxo data anyway. Because the psbt's unsigned_tx input has a valid outpoint, lnd could fetch the missing utxo data. This behavior differs from bitcoin core, which retrieves the utxo data if it is missing.

See that FinalizePsbt does in fact already call `FetchInputInfo(&txIn.PreviousOutPoint). I think the first utxo check should be removed for convenience and to match the behavior of bitcoind. I have also suggested the spec which removes this utxo data (BIP 78) and led me to find this problem changes to be more compliant with BIP 174. BIP 174 says one should keep utxo data around.

I think the existing LND behavior is in fact BIP 174 compliant, but could be more convenient and match bitcoind while preserving compliance.

// We can only sign if we have UTXO information available. We
// can just continue here as a later step will fail with a more
// precise error message.
if in.WitnessUtxo == nil && in.NonWitnessUtxo == nil {
continue
}
// Skip this input if it's got final witness data attached.
if len(in.FinalScriptWitness) > 0 {
continue
}
// We can only sign this input if it's ours, so we try to map it
// to a coin we own. If we can't, then we'll continue as it
// isn't our input.
utxo, err := r.FetchInputInfo(&txIn.PreviousOutPoint)
if err != nil {
continue
}
fullTx := utxo.PrevTx


Proposed BIP 78 change: bitcoin/bips#1396
Proposed PayJoin library change: Kixunil/payjoin#39

@nickfarrow

@ellemouton ellemouton added feature request Requests for new features psbt labels Jan 3, 2023
@guggero
Copy link
Collaborator

guggero commented Jan 3, 2023

I think this check was just added to make sure the FinalizePsbt call isn't called with an incomplete PSBT.
I see why it looks weird that we require the UTXO information to be present just to fetch it anyway.

But making sure you have the UTXO information for each input (not just the one you want to sign) is especially important once P2TR inputs are present. Because you can only sign a P2TR input if you have the UTXO information of all of the inputs, otherwise the sighash will be incorrect.

So I'm not sure if removing the UTXO check here is a good idea, even if it would fix your immediate problem...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new features psbt
Projects
None yet
Development

No branches or pull requests

3 participants