foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-05 14:06:27 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#27200: test: psbt: check non-witness UTXO removal for segwit v1 input

Browse source 
3dd2f6461b test: psbt: check non-witness UTXO removal for segwit v1 input (Sebastian Falbesoner)
dd78e3fa43 test: speedup rpc_psbt.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
e194e3e93d 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 103c6fd279). The formerly [disabled](4600479058) 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 3dd2f6461b
  instagibbs:
    ACK 3dd2f6461b

Tree-SHA512: b8d7f7ea5d7d21def024b70dfca61991cc96a4193be8857018b4d7cf3ca1465d185619fd4a77623803d9da309aa489c53273e9b7683d970ce12e2399b5b50031
This commit is contained in:
Andrew Chow 2023-03-16 14:36:34 -04:00
commit 09e86d7a1a
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
2 changed files with 31 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
test/functional/rpc_psbt.py
View file

@ -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]
# 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):
@ -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()

10
test/functional/test_framework/psbt.py
View file

@ -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")