bdk-cli icon indicating copy to clipboard operation
bdk-cli copied to clipboard

Broadcast should tell user to finalize PSBTs

Open icota opened this issue 2 years ago • 11 comments

When trying to broadcast a PSBT such as

cHNidP8BAHEBAAAAAQU2MK4mbnsx/zjbmKwHGUAMVT1zXRFTPBArkMACRZjxAQAAAAD9////AhAnAAAAAAAAFgAU/52lZ+YvMOqGVPodX71HvvjjvhP7FQEAAAAAABYAFKqBwKFeT43famR0JJGaAhoMZKrXAAAAAAABAN4BAAAAAAEBArVvNyXe4TvWObt2vPpIv4IJPeFG1hRK8RtTgzVx3MEAAAAAAP3///8CECcAAAAAAAAWABT/naVn5i8w6oZU+h1fvUe++OO+E+ZAAQAAAAAAFgAUvv5IaH57cUVjoZ35bLxinO1BwkwCRzBEAiABTG7xfKRyZF2ezFCByBLSMGTA2FXYpMN881YXQZB/TgIgbU/OqbuWbe4KhWnjKeglVbnkko70H6glFe/zoo069fUBIQIeb6icGFSB9miQOCV9IMVmJdbEkXG8Hi86LaEyJRa5swAAAAABAR/mQAEAAAAAABYAFL7+SGh+e3FFY6Gd+Wy8YpztQcJMIgICZNLhaTjDm6k30vM/U2+d1TKQ02pk3MFxixFp4EQN1G5IMEUCIQCYRpRlO8YiUXZHLRYmS7fxhgCCDtYRVJ7Tb1rVOKqsHAIgcAeg6Dh8l2N9cixRUyCKwe0jWC9Xc7lNMrxJnDnlmN8BAQMEAQAAACIGAmTS4Wk4w5upN9LzP1NvndUykNNqZNzBcYsRaeBEDdRuGCrNFF1UAACAAQAAgAAAAIABAAAAAQAAAAAAIgIDRcB4bLvY45WvIPqto3nRP4nAQm1FHeIBWcqo2UiIHYoYKs0UXVQAAIABAACAAAAAgAEAAAACAAAAAA==

the transaction gets (rightfully) rejected by the network

[2022-05-26T13:05:29Z ERROR bdk_cli] Electrum(Protocol(String("sendrawtransaction RPC error: {\"code\":-26,\"message\":\"non-mandatory-script-verify-flag (Witness program hash mismatch)\"}")))

due to not being finalized.

To avoid user confusion I think the user should be either instructed to finalize the PSBT or have this be done automatically (which would be in line with what other wallets are doing).

icota avatar Jun 02 '22 14:06 icota

Good idea, rather than complicating the broadcast command with the extra options finalize has I'd prefer to give a nicer error if someone tries to broadcast a non-finalized psbt.

bdk-cli wallet help finalize_psbt
bdk-cli-wallet-finalize_psbt 0.4.0
Finalizes a PSBT

USAGE:
    bdk-cli wallet finalize_psbt [OPTIONS] --psbt <BASE64_PSBT>

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
        --psbt <BASE64_PSBT>              Sets the PSBT to finalize
        --assume_height <HEIGHT>          Assume the blockchain has reached a specific height
        --trust_witness_utxo <WITNESS>    Whether the signer should trust the witness_utxo, if the non_witness_utxo hasn’t been provided

notmandatory avatar Jun 02 '22 15:06 notmandatory

@notmandatory TBH before I opened this issue I didn't even realise there is a finalize_psbt (with the options) in the CLI. In that case it's obviously better to improve the error - I've changed the title to reflect that.

icota avatar Jun 03 '22 10:06 icota

Do the changes in https://github.com/bitcoindevkit/bdk/pull/621 solve your issue if we bring that into bdk-cli?

notmandatory avatar Jul 06 '22 23:07 notmandatory

I don't see how? My issue came about from trying to broadcast an unfinalized transaction and getting confused by the error message.

Are you saying changes in https://github.com/bitcoindevkit/bdk/pull/621 would make it less likely for the user to forget to finalize?

icota avatar Jul 07 '22 07:07 icota

Yes my thinking is for the sign command to always finalize (if possible), and then add a --no-finalize for those who don't want to finalize for some reason. The result of sign already lets the user know if the PSBT is in fact finalized. The problem with doing an automatic finalize on broadcast (and then throwing an error if that fails) is the user would have to repeat the signing options they may have already used when signing the PSBT.

notmandatory avatar Jul 11 '22 21:07 notmandatory

My flow was from outside (hardware device) coming in (to bdk-cli).

But also I agree with what you say @notmandatory, in all cases broadcast shouldn't finalize. That's why I changed the title to merely suggest to the user to finalize.

non-mandatory-script-verify-flag (Witness program hash mismatch)

was way too cryptic for me.

Would be great to say something like "WARNING: trying to broadcast a non-finalized transaction" beforehand.

icota avatar Jul 12 '22 06:07 icota

When using the command broadcast --psbt I get the following error, I don't know how to solve it, any idea. [2022-07-20T15:04:04Z ERROR bdk_cli] Electrum(Protocol(String("sendrawtransaction RPC error: {"code":-26,"message":"non-mandatory-script-verify-flag (Witness program hash mismatch)"}")))

w2k31984 avatar Jul 20 '22 15:07 w2k31984

@w2k31984 It seems you might of forgot to sign the psbt. You need to sign the PSBT using the sign --psbt $UNSIGNED_PSBT command.

Creating a transaction using bdk-cli looks something like this...

  • First you create a transactioncreate_tx --to addressgoeshere:amountSats
  • Second you sign the transaction sign --psbt $UNSIGNED_PSBT
  • Third and final step is to brodcast the transaction using brodcast --psbt $SIGNED
  • A full preview of the command --> bdk-cli wallet -w mywalletName --descriptor "wpkh($XPRV_GoesHere/84'/0'/0'/0/*)" broadcast --psbt $SIGNED_PSBT .

One final note: The instance when I got the error message seen below was when I did not sign the PSBT

  • ERROR bdk_cli] Electrum(Protocol(String("sendrawtransaction RPC error: {\"code\":-26,\"message\":\"non-mandatory-script-verify-flag (Witness program hash mismatch)\"}")))

When using the command broadcast --psbt I get the following error, I don't know how to solve it, any idea. [2022-07-20T15:04:04Z ERROR bdk_cli] Electrum(Protocol(String("sendrawtransaction RPC error: {"code":-26,"message":"non-mandatory-script-verify-flag (Witness program hash mismatch)"}")))

waterst0ne avatar Jul 22 '22 18:07 waterst0ne

Hmm.. This seems like a possible improvement.. The "Witness program hash moissmatch" is not really enlightening.. Its should say something more friendly than that..

Putting this down in my todos..

rajarshimaitra avatar Jul 28 '22 12:07 rajarshimaitra

This one is up for grab btw.. If we don't get anyone on this in few weeks I will take it up..

rajarshimaitra avatar Sep 22 '22 05:09 rajarshimaitra

@notmandatory I am adding back the good first issue for this one in case someone wanna try this on. This doesn't seem like a pressing need but surely a nice thing to have..

rajarshimaitra avatar Sep 22 '22 06:09 rajarshimaitra