Update basic multisig test/docs to use multipath descriptor

mjdietzx commented at 2:22 pm on October 19, 2021: contributor

This is on top of #22838, only my last commit should be considered here. This highlights how a basic multisig like our example can be cleaned up a bit with multipath descriptors. Also serves as some light additional testing for #22838

laanwj added the label Docs on Oct 19, 2021

DrahtBot commented at 2:43 am on October 20, 2021: contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#23512 (policy: Treat taproot as always active by MarcoFalke)
#23417 (wallet, spkm: Move key management from DescriptorScriptPubKeyMan to wallet level KeyManager by achow101)
#23171 (qa: test descriptors with mixed xpubs and const pubkeys by darosior)
#19602 (wallet: Migrate legacy wallets to descriptor wallets 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.

Rspigler commented at 10:13 pm on October 22, 2021: contributor

Concept ACK on doc changes. Thanks for working on this! Some questions though:

Should these doc changes also go in the descriptor BIP in addition to doc/descriptors.md? @achow101

mjdietzx force-pushed on Oct 23, 2021

mjdietzx commented at 9:57 pm on October 23, 2021: contributor

Rebased

descriptors: Add PubkeyProvider::Clone 3bc62dd98e

descriptors: Add DescriptorImpl::Clone f1d8e25ed8

descriptors: Change ParseScript to return pair of descriptors

To prepare for returning multipath descriptors which will be a shorthand
for specifying 2 descriptors, change ParseScript's signature to return a
pair.

7071f955e0

descriptors: Have ParsePubkey return a pair

To prepare for multipath descriptors, have ParsePubkey return a pair of
PubkeyProviders and have ParseScript be able to handle them.

cfc49e4437

descriptors: Have ParseKeypath handle multipath specifiers

Multipath specifiers are derivation path indexes of the form `<r;c>`
used for specifying derivation paths for receiving and change
descriptors. Only one multipath specifier is allowed per PubkeyProvider,
and it must only contain two path indexes. This is syntactic sugar which
is parsed into two distinct descriptors. One descriptor will have all of
the `r` paths, the other all of the `c` paths.

ParseKeypath will always return a pair of keypaths. If no multipath
descriptor is provided, the second one will be a std::nullopt. The
callers of this function are updated to deal with this case and return
multiple PubkeyProviders. Their callers have also been updated to handle
pairs of PubkeyProviders.

d6da820cb9

descriptors: Change Parse to return pair of descriptors

When given a descriptor which contins a multipath derivation specifier,
a pair of descriptors will be returned.

39074b7a05

tests: Add unit tests for multipath descriptors 236e6354db

tests: Multipath descriptors for getdescriptorinfo af5d62df27

rpc: Have deriveaddresses derive receiving and change

When given a multipath descriptor, derive both the receiving and change
descriptors. The derived addresses will be returned in an object
consisting of 2 arrays. For compatibility, when given a single path
descriptor, the addresses are provided in a single array as before.

eaa4c9ebd5

tests: Multipath descriptors for scantxoutset and deriveaddresses 70b570d958

wallet: Move internal to be per key when importing

Instead of applying internal-ness to all keys being imported at the same
time, apply it on a per key basis. So each key that is imported will
carry with it whether it is for the change keypool.

77c9a45694

rpc: Allow importmulti to import multipath descriptors correctly

Multipath descriptors will be imported as two separate descriptors with
the first one for receiving addreses, and the second for change
addresses. When importing a multipath descriptor, 'internal' cannot be
specified because of the implicit receive and change descriptors.

7d913bcf19

wallet, rpc: Allow importdescriptors to import multipath descriptors

Multipath descriptors will be imported as two separate descriptors with
the first for receiving addresses and the second for change. This
mirrors importmulti.

01c928b00d

tests: Test importing of multipath descriptors

Test that both importmulti and importdescriptors behave as expected when
importing a multipath descriptor.

0d0aa1bf9d

doc: Mention multipath specifier a6ff1c7246

test, doc: use multipath descriptor for basic multisig 688c692a42

mjdietzx force-pushed on Oct 24, 2021

mjdietzx marked this as ready for review on Oct 25, 2021

DrahtBot commented at 7:42 am on November 25, 2021: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

DrahtBot added the label Needs rebase on Nov 25, 2021

Rspigler commented at 0:33 am on December 15, 2021: contributor

Should https://github.com/bitcoin/bitcoin/blob/master/doc/multisig-tutorial.md and https://github.com/bitcoin/bitcoin/blob/master/doc/managing-wallets.md also be updated?

DrahtBot commented at 12:15 pm on March 21, 2022: contributor

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

DrahtBot commented at 7:03 am on July 25, 2022: contributor

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

MarcoFalke commented at 7:13 am on July 25, 2022: member

Closing for now. This can be reopened or a new pull be created.

MarcoFalke closed this on Jul 25, 2022

bitcoin locked this on Jul 25, 2023

Update basic multisig test/docs to use multipath descriptor #23308

Conflicts