Add a --descriptors option to various tests
ClosedPublic
Actions

Authored by PiRK on Sep 29 2021, 13:57.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABCe854278f6db0: Add a --descriptors option to various tests

Summary

Adds a --descriptors option globally to the test framework. This will
make the test create and use descriptor wallets. However some tests may
not work with this.

Some tests are modified to work with --descriptors and run with that
option in test_runer:

wallet_encryption.py

wallet_keypool.py

wallet_keypool_topup.py

wallet_labels.py

wallet_avoidreuse.py

This is a backport of core#16528 [43a/43]
https://github.com/bitcoin/bitcoin/pull/16528/commits/223588b1bbc63dc57098bbd0baa48635e0cc0b82

I'm splitting this commit into multiple commits to make review easier.
This first part contains everything that works more or less out of the
box and does not require significant changes to work with our codebase.

All the deviations from the original PR in this revision are explained by out of order
backports and lack of segwit and bech32. For instance, changes to key.py
were already included in D8475 & D9934. Deviations in RPCOverloadWrapper.createwallet
are explained by D10185 and D9101.

I excluded specifically 3 tests that require changes to transaction
amounts and fees when descriptors are used. With our codebase,
descriptor wallets generate different fees, because in some situations
the legacy wallet generates legacy p2pkh outputs and descriptor wallets generate
scriptPubKey / p2sh outputs. I will address these in one or two
separate diffs.

These excluded tests are:

rpc_psbt.py
wallet_basic.py
wallet_keypool.py

Test Plan

ninja check-functional

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16870
Build 33579: Build Diff	build-debug · build-diff · build-without-wallet · build-clang-tidy · build-clang
Build 33578: arc lint + arc unit

Event Timeline

PiRK created this revision.Sep 29 2021, 13:57

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 29 2021, 13:57

PiRK requested review of this revision.Sep 29 2021, 13:57

teamcity edited the summary of this revision. (Show Details)Sep 29 2021, 13:58

Harbormaster completed remote builds in B16810: Diff 30203.Sep 29 2021, 14:11

PiRK added a child revision: D10220: Add a --descriptors option to more tests.Sep 30 2021, 08:36

PiRK added a child revision: D10229: wallet: Remove path checking code from loadwallet RPC.Oct 4 2021, 06:15

Fabien requested changes to this revision.Oct 4 2021, 08:00

Fabien added a subscriber: Fabien.

Fabien added inline comments.

test/functional/rpc_createmultisig.py
79 ↗	(On Diff #30203)	The PR uses node0. Note that this should not impact the behavior because the test is skipped if there is no wallet, but better stick to the original material
84 ↗	(On Diff #30203)	Not from this diff, but the comment is wrong
87 ↗	(On Diff #30203)	And the test does nothing
test/functional/wallet_hd.py
100 ↗	(On Diff #30203)	Please move the os.path.join and comment change above to their own diff (the comment is from PR16681, I did not search for the os.path.join)

This revision now requires changes to proceed.Oct 4 2021, 08:01

address review:

use node0 when creating multisig address in rpc_createmultisig (like in the original PR)
remove test related to segwit, that in our codebase just duplicates the previous line of test (needlesly added in D6552)
undo a couple of changes that don't belong in this backport ("regtest" -> "chain" in a comment and style change)