From 39cea21ec51b9838669c38fefa14f25c36ae7096 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Fri, 28 Jun 2024 12:41:57 +0100 Subject: [PATCH 1/2] wallet: fix FillPSBT errantly showing as complete Fix cases of calls to `FillPSBT` returning `complete=true` when it's not the case. This can happen when some inputs have been signed but the transaction is subsequently modified, e.g. in the context of PayJoins. Also fixes a related bug where a finalized hex string is attempted to be added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort. Reported in #30077. --- src/wallet/wallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d569c64b433..fb4360cfd23 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2225,8 +2225,8 @@ std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bo // Complete if every input is now signed complete = true; - for (const auto& input : psbtx.inputs) { - complete &= PSBTInputSigned(input); + for (size_t i = 0; i < psbtx.inputs.size(); ++i) { + complete &= PSBTInputSignedAndVerified(psbtx, i, &txdata); } return {}; From 7e36dca657c66bc70b04d5b850e5a335aecfb902 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Fri, 28 Jun 2024 12:45:47 +0100 Subject: [PATCH 2/2] test: add test for modififed walletprocesspsbt calls This test checks that we can successfully process PSBTs and opt out of finalization. Previously trying to call `walletprocesspsbt` would attempt to auto-finalize (as a convenience), and would not permit opt-out of finalization, instead aborting via `CHECK_NONFATAL`. --- test/functional/rpc_psbt.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 6ee7e568868..7d4b6655a42 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -68,6 +68,28 @@ class PSBTTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + def test_psbt_incomplete_after_invalid_modification(self): + self.log.info("Check that PSBT is correctly marked as incomplete after invalid modification") + node = self.nodes[2] + wallet = node.get_wallet_rpc(self.default_wallet_name) + address = wallet.getnewaddress() + wallet.sendtoaddress(address=address, amount=1.0) + self.generate(node, nblocks=1, sync_fun=lambda: self.sync_all(self.nodes[:2])) + + utxos = wallet.listunspent(addresses=[address]) + psbt = wallet.createpsbt([{"txid": utxos[0]["txid"], "vout": utxos[0]["vout"]}], [{wallet.getnewaddress(): 0.9999}]) + signed_psbt = wallet.walletprocesspsbt(psbt)["psbt"] + + # Modify the raw transaction by changing the output address, so the signature is no longer valid + signed_psbt_obj = PSBT.from_base64(signed_psbt) + substitute_addr = wallet.getnewaddress() + raw = wallet.createrawtransaction([{"txid": utxos[0]["txid"], "vout": utxos[0]["vout"]}], [{substitute_addr: 0.9999}]) + signed_psbt_obj.g.map[PSBT_GLOBAL_UNSIGNED_TX] = bytes.fromhex(raw) + + # Check that the walletprocesspsbt call succeeds but also recognizes that the transaction is not complete + signed_psbt_incomplete = wallet.walletprocesspsbt(signed_psbt_obj.to_base64(), finalize=False) + assert signed_psbt_incomplete["complete"] is False + def test_utxo_conversion(self): self.log.info("Check that non-witness UTXOs are removed for segwit v1+ inputs") mining_node = self.nodes[2] @@ -589,6 +611,7 @@ class PSBTTest(BitcoinTestFramework): if self.options.descriptors: self.test_utxo_conversion() + self.test_psbt_incomplete_after_invalid_modification() self.test_input_confs_control()