Question: Finalizing a fully signed PSBT

andreasgriffin commented 6 months ago

I have the following fully signed (regtest) PSBT

cHNidP8BAIkBAAAAATuuOwH+YN3lM9CHZuaxhXU+P/xWQQUpwldxTxng2/NWAAAAAAD9////AhAnAAAAAAAAIgAgbnxIFWJ84RPQEHQJIBWYVALEGgr6e99xVLT2DDykpha+kQ0AAAAAACIAIH+2seEetNM9J6mtfXwz2EwP7E1gqjpvr0HHI97D3b5IcwAAAAABAP2HAQEAAAAAAQHSc/5077HT+IqRaNwhhb9WuzlFYINsZk1BxhahFNsqlQAAAAAA/f///wKYuQ0AAAAAACIAIKteOph2G5lDpTD98oWJkrif3i6FX/eHTr2kmU4KN1w1oIYBAAAAAAAiACBYU+aHAWVhSe4DMfwzQhq9NzO6smI694/A7MoURBK4nAQARzBEAiBFFZVQQjC5SlDRCAuC5AkoQgMXyrG54gp71Ro2W6g0fgIgTbg94g7liL0T7DwEeWqOiJfurgpuTv1Q+7bAzFlV/yQBRzBEAiB76jOyWL28VWQzn32ITyy4JlRYAASEaPB9C7mANDLtzAIgCjyov+Y9xRQicB2+v0iDA09RcC7hQHzLxXA9klITMXkBaVIhApBlhYUDvuGXybpbsvXzcXHMb+NikjYe3kqp8xvXMoeJIQPPm4n6VeT9fEoPYLoiy9a3O0mxnSA3wNRunj9xLxmoXSED6TWmEfTbB6zewl0TlxSPr3xmEqifQu5Ou9xoOocqvQlTrnMAAAABASuYuQ0AAAAAACIAIKteOph2G5lDpTD98oWJkrif3i6FX/eHTr2kmU4KN1w1IgICov3eEPPwwA+7S2NpB1FCBk3wZDu+EXBOrtIJi8IgI1lHMEQCIFuO9cKoLEM1v3juKeV+D9yotGzHONOlHdmXaA4qsa7PAiBNN4i+JleuHBXl3NFV8rQIgwCmTJkx4yykF5qnkvtJvAEiAgNcn6+q0vKTzCKLEvlMWVwbS/PuzlVVbsGfMcoQt6OjUkcwRAIgdEIjn8cnj7/WHz3nR9xpMRzcfTCQqzzCYBKhwYKYlo0CICdppsT+bQm1ktrRAJCpc8enfGcjcaS92mMbRDvCOo5qAQEFaVIhAqL93hDz8MAPu0tjaQdRQgZN8GQ7vhFwTq7SCYvCICNZIQMUwP2b3HF9IhzCZzdJeJyJNXq/XTr3cxJzv0tDXlIFkSEDXJ+vqtLyk8wiixL5TFlcG0vz7s5VVW7BnzHKELejo1JTriIGAqL93hDz8MAPu0tjaQdRQgZN8GQ7vhFwTq7SCYvCICNZHCWX5CkwAACAAQAAgAAAAIACAACAAQAAAAMAAAAiBgMUwP2b3HF9IhzCZzdJeJyJNXq/XTr3cxJzv0tDXlIFkRwm6/kqMAAAgAEAAIAAAACAAgAAgAEAAAADAAAAIgYDXJ+vqtLyk8wiixL5TFlcG0vz7s5VVW7BnzHKELejo1Ic9OSVdDAAAIABAACAAAAAgAIAAIABAAAAAwAAAAABAWlSIQLISLAcXFc1NEPLF6RMYI6oMkX9QY+zC5br755VTKeAPSEDHSnh2CYwJ5xK2W0KK3A+ddEd6ju4zLbrS6dad7iGRJkhA3eBBjx8GKzvNoyDo89SlQGapVMV4cvzxm2uSYZggi1YU64iAgLISLAcXFc1NEPLF6RMYI6oMkX9QY+zC5br755VTKeAPRz05JV0MAAAgAEAAIAAAACAAgAAgAAAAAAHAAAAIgIDHSnh2CYwJ5xK2W0KK3A+ddEd6ju4zLbrS6dad7iGRJkcJZfkKTAAAIABAACAAAAAgAIAAIAAAAAABwAAACICA3eBBjx8GKzvNoyDo89SlQGapVMV4cvzxm2uSYZggi1YHCbr+SowAACAAQAAgAAAAIACAACAAAAAAAcAAAAAAQFpUiECpfNLzexJfZ5G++SEVaypCms5w4g/8ExmROHke623xxEhAzc7rxaDI9x7Lvd7U3faPs9e34DNsrDE/V2mRVYn0WLvIQNkHWK1/HMUBxz9Cbs7elmC528iAQWIW104ND9B6ZI1+lOuIgICpfNLzexJfZ5G++SEVaypCms5w4g/8ExmROHke623xxEcJuv5KjAAAIABAACAAAAAgAIAAIABAAAABAAAACICAzc7rxaDI9x7Lvd7U3faPs9e34DNsrDE/V2mRVYn0WLvHPTklXQwAACAAQAAgAAAAIACAACAAQAAAAQAAAAiAgNkHWK1/HMUBxz9Cbs7elmC528iAQWIW104ND9B6ZI1+hwll+QpMAAAgAEAAIAAAACAAgAAgAEAAAAEAAAAAA==

and create a dummy_wallet (unrelated to the PSBT inputs): dummy_wallet.sign(psbt, None) returns False

It fails to finalize the transaction. Comparison: Sparrow can finalize the PSBT to a fully signed tx.

Am I doing something wrong, or why does bdk need the source wallet to finalize the PSBT?

thunderbiscuit commented 6 months ago

Moving this to the Rust bdk repo for better visibility.

notmandatory commented 6 months ago

You said you are using a dummy descriptor unrelated to the given inputs of the PSBT, but your PSBT is missing a "final script sig" or "final script witness" (decode with https://bip174.org/). In the BDK code if both of these are missing for an input it tries to create them from your descriptor, but since you aren't using a matching descriptor this is likely why the finalize fails. See the finalize logic check here: https://github.com/bitcoindevkit/bdk/blob/2a8c8c2bb6041a800c596c90139af9b086179e90/src/wallet/mod.rs#L1229

notmandatory commented 6 months ago

How are you finalizing the PSBT with Sparrow? did you use a wallet that has a valid descriptor for the PSBT?

andreasgriffin commented 6 months ago

Sparrow can finalize it without any wallet. And the bitcointx library can too:

from bitcointx.core.psbt import (
    PartiallySignedTransaction as TXPartiallySignedTransaction,
)

psbt_data = "cHNidP8BAIkBAAAAATuuOwH+YN3lM9CHZuaxhXU+P/xWQQUpwldxTxng2/NWAAAAAAD9////AhAnAAAAAAAAIgAgbnxIFWJ84RPQEHQJIBWYVALEGgr6e99xVLT2DDykpha+kQ0AAAAAACIAIH+2seEetNM9J6mtfXwz2EwP7E1gqjpvr0HHI97D3b5IcwAAAAABAP2HAQEAAAAAAQHSc/5077HT+IqRaNwhhb9WuzlFYINsZk1BxhahFNsqlQAAAAAA/f///wKYuQ0AAAAAACIAIKteOph2G5lDpTD98oWJkrif3i6FX/eHTr2kmU4KN1w1oIYBAAAAAAAiACBYU+aHAWVhSe4DMfwzQhq9NzO6smI694/A7MoURBK4nAQARzBEAiBFFZVQQjC5SlDRCAuC5AkoQgMXyrG54gp71Ro2W6g0fgIgTbg94g7liL0T7DwEeWqOiJfurgpuTv1Q+7bAzFlV/yQBRzBEAiB76jOyWL28VWQzn32ITyy4JlRYAASEaPB9C7mANDLtzAIgCjyov+Y9xRQicB2+v0iDA09RcC7hQHzLxXA9klITMXkBaVIhApBlhYUDvuGXybpbsvXzcXHMb+NikjYe3kqp8xvXMoeJIQPPm4n6VeT9fEoPYLoiy9a3O0mxnSA3wNRunj9xLxmoXSED6TWmEfTbB6zewl0TlxSPr3xmEqifQu5Ou9xoOocqvQlTrnMAAAABASuYuQ0AAAAAACIAIKteOph2G5lDpTD98oWJkrif3i6FX/eHTr2kmU4KN1w1IgICov3eEPPwwA+7S2NpB1FCBk3wZDu+EXBOrtIJi8IgI1lHMEQCIFuO9cKoLEM1v3juKeV+D9yotGzHONOlHdmXaA4qsa7PAiBNN4i+JleuHBXl3NFV8rQIgwCmTJkx4yykF5qnkvtJvAEiAgNcn6+q0vKTzCKLEvlMWVwbS/PuzlVVbsGfMcoQt6OjUkcwRAIgdEIjn8cnj7/WHz3nR9xpMRzcfTCQqzzCYBKhwYKYlo0CICdppsT+bQm1ktrRAJCpc8enfGcjcaS92mMbRDvCOo5qAQEFaVIhAqL93hDz8MAPu0tjaQdRQgZN8GQ7vhFwTq7SCYvCICNZIQMUwP2b3HF9IhzCZzdJeJyJNXq/XTr3cxJzv0tDXlIFkSEDXJ+vqtLyk8wiixL5TFlcG0vz7s5VVW7BnzHKELejo1JTriIGAqL93hDz8MAPu0tjaQdRQgZN8GQ7vhFwTq7SCYvCICNZHCWX5CkwAACAAQAAgAAAAIACAACAAQAAAAMAAAAiBgMUwP2b3HF9IhzCZzdJeJyJNXq/XTr3cxJzv0tDXlIFkRwm6/kqMAAAgAEAAIAAAACAAgAAgAEAAAADAAAAIgYDXJ+vqtLyk8wiixL5TFlcG0vz7s5VVW7BnzHKELejo1Ic9OSVdDAAAIABAACAAAAAgAIAAIABAAAAAwAAAAABAWlSIQLISLAcXFc1NEPLF6RMYI6oMkX9QY+zC5br755VTKeAPSEDHSnh2CYwJ5xK2W0KK3A+ddEd6ju4zLbrS6dad7iGRJkhA3eBBjx8GKzvNoyDo89SlQGapVMV4cvzxm2uSYZggi1YU64iAgLISLAcXFc1NEPLF6RMYI6oMkX9QY+zC5br755VTKeAPRz05JV0MAAAgAEAAIAAAACAAgAAgAAAAAAHAAAAIgIDHSnh2CYwJ5xK2W0KK3A+ddEd6ju4zLbrS6dad7iGRJkcJZfkKTAAAIABAACAAAAAgAIAAIAAAAAABwAAACICA3eBBjx8GKzvNoyDo89SlQGapVMV4cvzxm2uSYZggi1YHCbr+SowAACAAQAAgAAAAIACAACAAAAAAAcAAAAAAQFpUiECpfNLzexJfZ5G++SEVaypCms5w4g/8ExmROHke623xxEhAzc7rxaDI9x7Lvd7U3faPs9e34DNsrDE/V2mRVYn0WLvIQNkHWK1/HMUBxz9Cbs7elmC528iAQWIW104ND9B6ZI1+lOuIgICpfNLzexJfZ5G++SEVaypCms5w4g/8ExmROHke623xxEcJuv5KjAAAIABAACAAAAAgAIAAIABAAAABAAAACICAzc7rxaDI9x7Lvd7U3faPs9e34DNsrDE/V2mRVYn0WLvHPTklXQwAACAAQAAgAAAAIACAACAAQAAAAQAAAAiAgNkHWK1/HMUBxz9Cbs7elmC528iAQWIW104ND9B6ZI1+hwll+QpMAAAgAEAAIAAAACAAgAAgAEAAAAEAAAAAA=="

# Decode the PSBT from base64 to a PSBT object.
psbt = TXPartiallySignedTransaction.from_base64(psbt_data)
print(f"is_final={psbt.is_final()}")
psbt.extract_transaction()
print(f"is_final={psbt.is_final()}")
print(psbt.extract_transaction().serialize().hex())

gives

is_final=False
is_final=True
010000000001013bae3b01fe60dde533d08766e6b185753e3ffc56410529c257714f19e0dbf3560000000000fdffffff0210270000000000002200206e7c4815627ce113d01074092015985402c41a0afa7bdf7154b4f60c3ca4a616be910d00000000002200207fb6b1e11eb4d33d27a9ad7d7c33d84c0fec4d60aa3a6faf41c723dec3ddbe48040047304402205b8ef5c2a82c4335bf78ee29e57e0fdca8b46cc738d3a51dd997680e2ab1aecf02204d3788be2657ae1c15e5dcd155f2b4088300a64c9931e32ca4179aa792fb49bc0147304402207442239fc7278fbfd61f3de747dc69311cdc7d3090ab3cc26012a1c18298968d02202769a6c4fe6d09b592dad10090a973c7a77c672371a4bdda631b443bc23a8e6a0169522102a2fdde10f3f0c00fbb4b6369075142064df0643bbe11704eaed2098bc2202359210314c0fd9bdc717d221cc2673749789c89357abf5d3af7731273bf4b435e52059121035c9fafaad2f293cc228b12f94c595c1b4bf3eece55556ec19f31ca10b7a3a35253ae73000000

also bitcoincore finalizepsbt can finalize it (again without any wallet):

{
  "hex": "010000000001013bae3b01fe60dde533d08766e6b185753e3ffc56410529c257714f19e0dbf3560000000000fdffffff0210270000000000002200206e7c4815627ce113d01074092015985402c41a0afa7bdf7154b4f60c3ca4a616be910d00000000002200207fb6b1e11eb4d33d27a9ad7d7c33d84c0fec4d60aa3a6faf41c723dec3ddbe48040047304402205b8ef5c2a82c4335bf78ee29e57e0fdca8b46cc738d3a51dd997680e2ab1aecf02204d3788be2657ae1c15e5dcd155f2b4088300a64c9931e32ca4179aa792fb49bc0147304402207442239fc7278fbfd61f3de747dc69311cdc7d3090ab3cc26012a1c18298968d02202769a6c4fe6d09b592dad10090a973c7a77c672371a4bdda631b443bc23a8e6a0169522102a2fdde10f3f0c00fbb4b6369075142064df0643bbe11704eaed2098bc2202359210314c0fd9bdc717d221cc2673749789c89357abf5d3af7731273bf4b435e52059121035c9fafaad2f293cc228b12f94c595c1b4bf3eece55556ec19f31ca10b7a3a35253ae73000000",
  "complete": true
}

My impression is this type of finalization step, doesn't really fit into bdk.wallet, but into bdk.PartiallySignedTransaction similar to bitcointx.

notmandatory commented 6 months ago

Can you give me that bitcointx finalized version (or the sparrow one) in base64 encoding so I can plug it on the bip174.org page? I want to see what they do with the final script fields.

andreasgriffin commented 6 months ago

sure, here is the finalized one:

cHNidP8BAIkBAAAAATuuOwH+YN3lM9CHZuaxhXU+P/xWQQUpwldxTxng2/NWAAAAAAD9////AhAnAAAAAAAAIgAgbnxIFWJ84RPQEHQJIBWYVALEGgr6e99xVLT2DDykpha+kQ0AAAAAACIAIH+2seEetNM9J6mtfXwz2EwP7E1gqjpvr0HHI97D3b5IcwAAAAABAP2HAQEAAAAAAQHSc/5077HT+IqRaNwhhb9WuzlFYINsZk1BxhahFNsqlQAAAAAA/f///wKYuQ0AAAAAACIAIKteOph2G5lDpTD98oWJkrif3i6FX/eHTr2kmU4KN1w1oIYBAAAAAAAiACBYU+aHAWVhSe4DMfwzQhq9NzO6smI694/A7MoURBK4nAQARzBEAiBFFZVQQjC5SlDRCAuC5AkoQgMXyrG54gp71Ro2W6g0fgIgTbg94g7liL0T7DwEeWqOiJfurgpuTv1Q+7bAzFlV/yQBRzBEAiB76jOyWL28VWQzn32ITyy4JlRYAASEaPB9C7mANDLtzAIgCjyov+Y9xRQicB2+v0iDA09RcC7hQHzLxXA9klITMXkBaVIhApBlhYUDvuGXybpbsvXzcXHMb+NikjYe3kqp8xvXMoeJIQPPm4n6VeT9fEoPYLoiy9a3O0mxnSA3wNRunj9xLxmoXSED6TWmEfTbB6zewl0TlxSPr3xmEqifQu5Ou9xoOocqvQlTrnMAAAABASuYuQ0AAAAAACIAIKteOph2G5lDpTD98oWJkrif3i6FX/eHTr2kmU4KN1w1AQj8BABHMEQCIFuO9cKoLEM1v3juKeV+D9yotGzHONOlHdmXaA4qsa7PAiBNN4i+JleuHBXl3NFV8rQIgwCmTJkx4yykF5qnkvtJvAFHMEQCIHRCI5/HJ4+/1h8950fcaTEc3H0wkKs8wmASocGCmJaNAiAnaabE/m0JtZLa0QCQqXPHp3xnI3GkvdpjG0Q7wjqOagFpUiECov3eEPPwwA+7S2NpB1FCBk3wZDu+EXBOrtIJi8IgI1khAxTA/ZvccX0iHMJnN0l4nIk1er9dOvdzEnO/S0NeUgWRIQNcn6+q0vKTzCKLEvlMWVwbS/PuzlVVbsGfMcoQt6OjUlOuAAEBaVIhAshIsBxcVzU0Q8sXpExgjqgyRf1Bj7MLluvvnlVMp4A9IQMdKeHYJjAnnErZbQorcD510R3qO7jMtutLp1p3uIZEmSEDd4EGPHwYrO82jIOjz1KVAZqlUxXhy/PGba5JhmCCLVhTriICAshIsBxcVzU0Q8sXpExgjqgyRf1Bj7MLluvvnlVMp4A9HPTklXQwAACAAQAAgAAAAIACAACAAAAAAAcAAAAiAgMdKeHYJjAnnErZbQorcD510R3qO7jMtutLp1p3uIZEmRwll+QpMAAAgAEAAIAAAACAAgAAgAAAAAAHAAAAIgIDd4EGPHwYrO82jIOjz1KVAZqlUxXhy/PGba5JhmCCLVgcJuv5KjAAAIABAACAAAAAgAIAAIAAAAAABwAAAAABAWlSIQKl80vN7El9nkb75IRVrKkKaznDiD/wTGZE4eR7rbfHESEDNzuvFoMj3Hsu93tTd9o+z17fgM2ysMT9XaZFVifRYu8hA2QdYrX8cxQHHP0Juzt6WYLnbyIBBYhbXTg0P0HpkjX6U64iAgKl80vN7El9nkb75IRVrKkKaznDiD/wTGZE4eR7rbfHERwm6/kqMAAAgAEAAIAAAACAAgAAgAEAAAAEAAAAIgIDNzuvFoMj3Hsu93tTd9o+z17fgM2ysMT9XaZFVifRYu8c9OSVdDAAAIABAACAAAAAgAIAAIABAAAABAAAACICA2QdYrX8cxQHHP0Juzt6WYLnbyIBBYhbXTg0P0HpkjX6HCWX5CkwAACAAQAAgAAAAIACAACAAQAAAAQAAAAA

notmandatory commented 6 months ago

It looks like in BDK we are doing more than bip174 specifies for finalizing. We assume any un-finalized PSBT inputs are from our wallet. For those inputs we use rust-miniscript's Descriptor::satisfy() to re-create the final script sig or script witness (see: https://docs.rs/miniscript/11.0.0/miniscript/descriptor/enum.Descriptor.html#method.satisfy).

What's your use case for using a BDK based wallet to finalize PSBT inputs un-associated with it's wallet descriptor? One possibility is we add a SignOption flag to say "try to finalize without the descriptor", but I want to make sure it's worth the work to add it.

andreasgriffin commented 6 months ago

I encountered this problem, when creating the gif of psbt sharing between wallets. With some effort I could recreate the exact way this happened
In principle it is not clear why the user should keep the wallet open, when he is signing a transaction with a hardware signer.
To solve this issue I had to bring in bitcointx https://github.com/andreasgriffin/bitcoin-usb/blob/a68600f89d028166403ee99351ed421409526bb2/bitcoin_usb/psbt_finalizer.py#L13 to ensure I can finalize a psbt. (I would rather of course stick with 1 library than needing multiple)
Please consider extending bdk.PSBT, and not SignOption. Finalizing a psbt should not need any wallet instance. The user doesn't need to have an open software wallet when signing with a hardware signer and then expects the software still to finalize it.

ValuedMammal commented 5 months ago

BDK can possibly expose a method, probably from rust-miniscript, that just handles the finalizer role for a signed psbt. That said, I would expect a method called sign to fail if the descriptor is not available.

notmandatory commented 5 months ago

Good point @ValuedMammal. We already export all of the rust-miniscript crate for rust users. One of the rust-miniscript finalize* functions look like what's required. See: https://docs.rs/miniscript/latest/miniscript/psbt/trait.PsbtExt.html#tymethod.finalize

But @andreasgriffin is using the python bindings which only exports a limited set of bdk wallet apis.

@thunderbiscuit I think we should move this one back to the bdk-ffi project and then we can figure out exactly which rust-miniscript PSBT functions we need to expose, ok?

thunderbiscuit commented 5 months ago

Sounds good.

ValuedMammal commented 5 months ago

I do think our signing flow could be smarter - it would be worth keeping an issue open to think about.

bitcoindevkit / bdk-ffi

Question: Finalizing a fully signed PSBT #469