Merge bitcoin/bitcoin#29154: tests: improve wallet multisig descriptor test and docs

d93b79470916b1e6f85c55cc6beb1e41b382196f tests: improve wallet multisig descriptor test and docs (Michael Dietz)

Pull request description:

  It is best to store all key origin information
  (master key fingerprint and all derivation steps)
  in the multisig descriptor. Being explicit with
  this information should be beneficial if this approach is used with other wallets/signers (whether hardware or software). There is no harm including all of this with xpubs (if anything it simplifies the test code) and makes this example/docs more complete and safer incase it is referenced by others.

ACKs for top commit:
  S3RK:
    Code Review ACK d93b79470916b1e6f85c55cc6beb1e41b382196f
  achow101:
    ACK d93b79470916b1e6f85c55cc6beb1e41b382196f

Tree-SHA512: 0e5c4d13f060489405e6cf50c8a09911f5a0cee71023649235afd80a5e3aae38d52c6e12ad4660205b9357b09f45596941391bdcf6fceccbe07c4e5a1592a482

1 files changed, 10 insertions, 15 deletions

diff --git a/test/functional/wallet_multisig_descriptor_psbt.py b/test/functional/wallet_multisig_descriptor_psbt.py
index 68bf45f7e3..145912025f 100755
--- a/test/functional/wallet_multisig_descriptor_psbt.py
+++ b/test/functional/wallet_multisig_descriptor_psbt.py
@@ -7,7 +7,6 @@
 This is meant to be documentation as much as functional tests, so it is kept as simple and readable as possible.
 """
 
-from test_framework.address import base58_to_byte
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_approx,
@@ -30,10 +29,12 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         self.skip_if_no_sqlite()
 
     @staticmethod
-    def _get_xpub(wallet):
+    def _get_xpub(wallet, internal):
         """Extract the wallet's xpubs using `listdescriptors` and pick the one from the `pkh` descriptor since it's least likely to be accidentally reused (legacy addresses)."""
-        descriptor = next(filter(lambda d: d["desc"].startswith("pkh"), wallet.listdescriptors()["descriptors"]))
-        return descriptor["desc"].split("]")[-1].split("/")[0]
+        pkh_descriptor = next(filter(lambda d: d["desc"].startswith("pkh(") and d["internal"] == internal, wallet.listdescriptors()["descriptors"]))
+        # Keep all key origin information (master key fingerprint and all derivation steps) for proper support of hardware devices
+        # See section 'Key origin identification' in 'doc/descriptors.md' for more details...
+        return pkh_descriptor["desc"].split("pkh(")[1].split(")")[0]
 
     @staticmethod
     def _check_psbt(psbt, to, value, multisig):
@@ -47,19 +48,13 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
                 amount += vout["value"]
         assert_approx(amount, float(value), vspan=0.001)
 
-    def participants_create_multisigs(self, xpubs):
+    def participants_create_multisigs(self, external_xpubs, internal_xpubs):
         """The multisig is created by importing the following descriptors. The resulting wallet is watch-only and every participant can do this."""
-        # some simple validation
-        assert_equal(len(xpubs), self.N)
-        # a sanity-check/assertion, this will throw if the base58 checksum of any of the provided xpubs are invalid
-        for xpub in xpubs:
-            base58_to_byte(xpub)
-
         for i, node in enumerate(self.nodes):
             node.createwallet(wallet_name=f"{self.name}_{i}", blank=True, descriptors=True, disable_private_keys=True)
             multisig = node.get_wallet_rpc(f"{self.name}_{i}")
-            external = multisig.getdescriptorinfo(f"wsh(sortedmulti({self.M},{f'/0/*,'.join(xpubs)}/0/*))")
-            internal = multisig.getdescriptorinfo(f"wsh(sortedmulti({self.M},{f'/1/*,'.join(xpubs)}/1/*))")
+            external = multisig.getdescriptorinfo(f"wsh(sortedmulti({self.M},{f','.join(external_xpubs)}))")
+            internal = multisig.getdescriptorinfo(f"wsh(sortedmulti({self.M},{f','.join(internal_xpubs)}))")
             result = multisig.importdescriptors([
                 {  # receiving addresses (internal: False)
                     "desc": external["descriptor"],
@@ -93,10 +88,10 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         }
 
         self.log.info("Generate and exchange xpubs...")
-        xpubs = [self._get_xpub(signer) for signer in participants["signers"]]
+        external_xpubs, internal_xpubs = [[self._get_xpub(signer, internal) for signer in participants["signers"]] for internal in [False, True]]
 
         self.log.info("Every participant imports the following descriptors to create the watch-only multisig...")
-        participants["multisigs"] = list(self.participants_create_multisigs(xpubs))
+        participants["multisigs"] = list(self.participants_create_multisigs(external_xpubs, internal_xpubs))
 
         self.log.info("Check that every participant's multisig generates the same addresses...")
         for _ in range(10):  # we check that the first 10 generated addresses are the same for all participant's multisigs