diff options
author | Andrew Chow <github@achow101.com> | 2023-03-16 14:36:34 -0400 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2023-03-16 14:48:10 -0400 |
commit | 09e86d7a1a695b0ad54b6c55d1ca2be9d2058d4b (patch) | |
tree | d5c304e5158c83299943bec22650aa4817365470 /test | |
parent | e695d8536e534f1e59de4a62a0d87a93e7a73456 (diff) | |
parent | 3dd2f6461b4bb28b2b212c691a3df28ac793ad91 (diff) | |
download | bitcoin-09e86d7a1a695b0ad54b6c55d1ca2be9d2058d4b.tar.xz |
Merge bitcoin/bitcoin#27200: test: psbt: check non-witness UTXO removal for segwit v1 input
3dd2f6461b4bb28b2b212c691a3df28ac793ad91 test: psbt: check non-witness UTXO removal for segwit v1 input (Sebastian Falbesoner)
dd78e3fa439d57e148a2a5e312021da962c4a394 test: speedup rpc_psbt.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
e194e3e93dd0665181bafeb162bf4c9f3621d6f1 test: PSBT: eliminate magic numbers for global unsigned tx key (0) (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for dropping non-witness UTXOs from PSBTs for segwit v1+ inputs (see commit 103c6fd2791f7e73eeab7f3900fbedd5b550211d). The formerly [disabled](https://github.com/bitcoin/bitcoin/commit/46004790588c24174a0bec49b540d158ce163ffd) method `test_utxo_conversion` is re-enabled and adapted to spend a Taproot (`bech32m`) instead of a wrapped SegWit (`p2sh-segwit`) output. Note that in contrast to the original test, we have to add the non-witness UTXO manually here using the test framework's PSBT module, since the constructing node knows that the output is segwit v1 and hence doesn't add the non-witness UTXO in the first place (see also [BIP371]( https://github.com/bitcoin/bips/blob/master/bip-0371.mediawiki#user-content-UTXO_Types)).
I strongly assume that most wallets would behave the same as Bitcoin Core here and wouldn't create PSBTs with non-witness UTXOs for Taproot inputs, but it's still good to test everything works as expected if it's still done and that the non-witness UTXO is simply dropped in that case.
The first two commits contain a small refactor (magic number elimination in PSBT module) and test speedup of ~2-3x (using whitelisting peers / immediate tx relay).
ACKs for top commit:
achow101:
ACK 3dd2f6461b4bb28b2b212c691a3df28ac793ad91
instagibbs:
ACK 3dd2f6461b4bb28b2b212c691a3df28ac793ad91
Tree-SHA512: b8d7f7ea5d7d21def024b70dfca61991cc96a4193be8857018b4d7cf3ca1465d185619fd4a77623803d9da309aa489c53273e9b7683d970ce12e2399b5b50031
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/rpc_psbt.py | 42 | ||||
-rw-r--r-- | test/functional/test_framework/psbt.py | 10 |
2 files changed, 31 insertions, 21 deletions
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 58a80e37a2..76886cc0ea 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, ) @@ -59,13 +59,16 @@ 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): 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] @@ -77,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] + + # 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 update the UTXO to segwit) - signed_psbt = offline_node.walletprocesspsbt(psbt)["psbt"] - assert "witness_utxo" in mining_node.decodepsbt(signed_psbt)["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): @@ -571,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() diff --git a/test/functional/test_framework/psbt.py b/test/functional/test_framework/psbt.py index 3a5b4ec74d..1eff4a250e 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") |