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

pull theStack wants to merge 3 commits into bitcoin:master from theStack:202303-test-psbt_non_witness_utxo_drop changing 2 files +31 −21

theStack commented at 2:29 AM on March 5, 2023: contributor

This PR adds missing test coverage for dropping non-witness UTXOs from PSBTs for segwit v1+ inputs (see commit 103c6fd2791f7e73eeab7f3900fbedd5b550211d). The formerly disabled 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).

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).
test: PSBT: eliminate magic numbers for global unsigned tx key (0) e194e3e93d

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

dd78e3fa43

test: psbt: check non-witness UTXO removal for segwit v1 input 3dd2f6461b
theStack force-pushed on Mar 5, 2023
fanquake requested review from achow101 on Mar 5, 2023
fanquake requested review from instagibbs on Mar 5, 2023
DrahtBot commented at 2:17 PM on March 5, 2023: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK instagibbs, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #21283 (Implement BIP 370 PSBTv2 by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot added the label Tests on Mar 6, 2023
in test/functional/rpc_psbt.py:62 in dd78e3fa43 outdated
```
  58 | @@ -59,6 +59,9 @@ def set_test_params(self):
  59 |              ["-walletrbf=0", "-changetype=legacy"],
  60 |              []
  61 |          ]
  62 | +        # whitelist peers to speed up tx relay / mempool sync
```
instagibbs commented at 2:44 PM on March 6, 2023:

feel like there should be a subroutine to do this for any given test

instagibbs commented at 2:46 PM on March 6, 2023:

also above: "#TODO: Remove address type restrictions once taproot has psbt extensions" can this be dropped now?
instagibbs approved
instagibbs commented at 2:49 PM on March 6, 2023: member

ACK 3dd2f6461b4bb28b2b212c691a3df28ac793ad91

Documenting the expected behavior is good. I wonder if some signers could have trouble if they haven't implemented taproot support, and thus will expect non witness utxos?

Apparently this hasn't caused a problem yet, so maybe not.
achow101 commented at 6:36 PM on March 16, 2023: member

ACK 3dd2f6461b4bb28b2b212c691a3df28ac793ad91
DrahtBot removed review request from achow101 on Mar 16, 2023
achow101 merged this on Mar 16, 2023
achow101 closed this on Mar 16, 2023
sidhujag referenced this in commit 6e5c7d5f2e on Mar 16, 2023
sidhujag referenced this in commit 49d45e212c on Mar 16, 2023
theStack deleted the branch on Mar 17, 2023
bitcoin locked this on Mar 16, 2024

Type	Reviewers
ACK	instagibbs, achow101

Contributors

Labels

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

Reviews

Conflicts