From e194e3e93dd0665181bafeb162bf4c9f3621d6f1 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sat, 4 Mar 2023 12:35:19 +0100 Subject: [PATCH 1/3] test: PSBT: eliminate magic numbers for global unsigned tx key (0) --- test/functional/test_framework/psbt.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/functional/test_framework/psbt.py b/test/functional/test_framework/psbt.py index 3a5b4ec74db..1eff4a250ee 100644 --- a/test/functional/test_framework/psbt.py +++ b/test/functional/test_framework/psbt.py @@ -105,8 +105,8 @@ class PSBT: def deserialize(self, f): assert f.read(5) == b"psbt\xff" self.g = from_binary(PSBTMap, f) - assert 0 in self.g.map - self.tx = from_binary(CTransaction, self.g.map[0]) + assert PSBT_GLOBAL_UNSIGNED_TX in self.g.map + self.tx = from_binary(CTransaction, self.g.map[PSBT_GLOBAL_UNSIGNED_TX]) self.i = [from_binary(PSBTMap, f) for _ in self.tx.vin] self.o = [from_binary(PSBTMap, f) for _ in self.tx.vout] return self @@ -115,8 +115,8 @@ class PSBT: assert isinstance(self.g, PSBTMap) assert isinstance(self.i, list) and all(isinstance(x, PSBTMap) for x in self.i) assert isinstance(self.o, list) and all(isinstance(x, PSBTMap) for x in self.o) - assert 0 in self.g.map - tx = from_binary(CTransaction, self.g.map[0]) + assert PSBT_GLOBAL_UNSIGNED_TX in self.g.map + tx = from_binary(CTransaction, self.g.map[PSBT_GLOBAL_UNSIGNED_TX]) assert len(tx.vin) == len(self.i) assert len(tx.vout) == len(self.o) @@ -130,7 +130,7 @@ class PSBT: for m in self.i + self.o: m.map.clear() - self.g = PSBTMap(map={0: self.g.map[0]}) + self.g = PSBTMap(map={PSBT_GLOBAL_UNSIGNED_TX: self.g.map[PSBT_GLOBAL_UNSIGNED_TX]}) def to_base64(self): return base64.b64encode(self.serialize()).decode("utf8") From dd78e3fa439d57e148a2a5e312021da962c4a394 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sun, 5 Mar 2023 01:45:23 +0100 Subject: [PATCH 2/3] test: speedup rpc_psbt.py by whitelisting peers (immediate tx relay) master branch: 0m36.86s real 0m03.26s user 0m01.69s system 0m35.71s real 0m03.78s user 0m01.64s system 0m45.76s real 0m03.12s user 0m01.27s system PR branch: 0m13.04s real 0m02.66s user 0m00.93s system 0m14.08s real 0m02.81s user 0m00.82s system 0m14.05s real 0m02.50s user 0m00.93s system --- test/functional/rpc_psbt.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 58a80e37a20..dff2c80d928 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -59,6 +59,9 @@ class PSBTTest(BitcoinTestFramework): ["-walletrbf=0", "-changetype=legacy"], [] ] + # whitelist peers to speed up tx relay / mempool sync + for args in self.extra_args: + args.append("-whitelist=noban@127.0.0.1") self.supports_cli = False def skip_test_if_missing_module(self): From 3dd2f6461b4bb28b2b212c691a3df28ac793ad91 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Sun, 5 Mar 2023 02:43:39 +0100 Subject: [PATCH 3/3] test: psbt: check non-witness UTXO removal for segwit v1 input --- test/functional/rpc_psbt.py | 39 ++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index dff2c80d928..76886cc0ea5 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -4,7 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the Partially Signed Transaction RPCs. """ - from decimal import Decimal from itertools import product @@ -27,6 +26,7 @@ from test_framework.psbt import ( PSBT_IN_SHA256, PSBT_IN_HASH160, PSBT_IN_HASH256, + PSBT_IN_NON_WITNESS_UTXO, PSBT_IN_WITNESS_UTXO, PSBT_OUT_TAP_TREE, ) @@ -67,8 +67,8 @@ class PSBTTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() - # TODO: Re-enable this test with segwit v1 def test_utxo_conversion(self): + self.log.info("Check that non-witness UTXOs are removed for segwit v1+ inputs") mining_node = self.nodes[2] offline_node = self.nodes[0] online_node = self.nodes[1] @@ -80,34 +80,41 @@ class PSBTTest(BitcoinTestFramework): # Create watchonly on online_node online_node.createwallet(wallet_name='wonline', disable_private_keys=True) wonline = online_node.get_wallet_rpc('wonline') - w2 = online_node.get_wallet_rpc('') + w2 = online_node.get_wallet_rpc(self.default_wallet_name) # Mine a transaction that credits the offline address - offline_addr = offline_node.getnewaddress(address_type="p2sh-segwit") - online_addr = w2.getnewaddress(address_type="p2sh-segwit") + offline_addr = offline_node.getnewaddress(address_type="bech32m") + online_addr = w2.getnewaddress(address_type="bech32m") wonline.importaddress(offline_addr, "", False) - mining_node.sendtoaddress(address=offline_addr, amount=1.0) - self.generate(mining_node, nblocks=1) + mining_wallet = mining_node.get_wallet_rpc(self.default_wallet_name) + mining_wallet.sendtoaddress(address=offline_addr, amount=1.0) + self.generate(mining_node, nblocks=1, sync_fun=lambda: self.sync_all([online_node, mining_node])) - # Construct an unsigned PSBT on the online node (who doesn't know the output is Segwit, so will include a non-witness UTXO) + # Construct an unsigned PSBT on the online node utxos = wonline.listunspent(addresses=[offline_addr]) raw = wonline.createrawtransaction([{"txid":utxos[0]["txid"], "vout":utxos[0]["vout"]}],[{online_addr:0.9999}]) psbt = wonline.walletprocesspsbt(online_node.converttopsbt(raw))["psbt"] - assert "non_witness_utxo" in mining_node.decodepsbt(psbt)["inputs"][0] + assert not "not_witness_utxo" in mining_node.decodepsbt(psbt)["inputs"][0] - # Have the offline node sign the PSBT (which will update the UTXO to segwit) - signed_psbt = offline_node.walletprocesspsbt(psbt)["psbt"] - assert "witness_utxo" in mining_node.decodepsbt(signed_psbt)["inputs"][0] + # add non-witness UTXO manually + psbt_new = PSBT.from_base64(psbt) + prev_tx = wonline.gettransaction(utxos[0]["txid"])["hex"] + psbt_new.i[0].map[PSBT_IN_NON_WITNESS_UTXO] = bytes.fromhex(prev_tx) + assert "non_witness_utxo" in mining_node.decodepsbt(psbt_new.to_base64())["inputs"][0] + + # Have the offline node sign the PSBT (which will remove the non-witness UTXO) + signed_psbt = offline_node.walletprocesspsbt(psbt_new.to_base64())["psbt"] + assert not "non_witness_utxo" in mining_node.decodepsbt(signed_psbt)["inputs"][0] # Make sure we can mine the resulting transaction txid = mining_node.sendrawtransaction(mining_node.finalizepsbt(signed_psbt)["hex"]) - self.generate(mining_node, 1) + self.generate(mining_node, nblocks=1, sync_fun=lambda: self.sync_all([online_node, mining_node])) assert_equal(online_node.gettxout(txid,0)["confirmations"], 1) wonline.unloadwallet() # Reconnect - self.connect_nodes(0, 1) + self.connect_nodes(1, 0) self.connect_nodes(0, 2) def test_input_confs_control(self): @@ -574,8 +581,8 @@ class PSBTTest(BitcoinTestFramework): for i, signer in enumerate(signers): self.nodes[2].unloadwallet("wallet{}".format(i)) - # TODO: Re-enable this for segwit v1 - # self.test_utxo_conversion() + if self.options.descriptors: + self.test_utxo_conversion() self.test_input_confs_control()