test, doc: basic M-of-N multisig minor cleanup and clarifications

wallet_multisig_descriptor_psbt.py is refactored in this commit. While
behavior doesn't change we do cleanup the way wallets are accessed
throughout the test as this is done a lot for the various signers
and their multisigs. We also get rid of some shallow methods and
instead inline them for improved readability.

descriptors.md is improved to be more explicit about which wallet
(ie the signer or multisig) is required for each step.

1 files changed, 47 insertions, 43 deletions

diff --git a/test/functional/wallet_multisig_descriptor_psbt.py b/test/functional/wallet_multisig_descriptor_psbt.py
index 59a0b94a9f..68c206b038 100755
--- a/test/functional/wallet_multisig_descriptor_psbt.py
+++ b/test/functional/wallet_multisig_descriptor_psbt.py
@@ -26,13 +26,15 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         self.skip_if_no_wallet()
         self.skip_if_no_sqlite()
 
-    def _get_xpub(self, wallet):
+    @staticmethod
+    def _get_xpub(wallet):
         """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]
 
-    def _check_psbt(self, psbt, to, value, multisig):
-        """Helper method for any of the N participants to check the psbt with decodepsbt and verify it is OK before signing."""
+    @staticmethod
+    def _check_psbt(psbt, to, value, multisig):
+        """Helper function for any of the N participants to check the psbt with decodepsbt and verify it is OK before signing."""
         tx = multisig.decodepsbt(psbt)["tx"]
         amount = 0
         for vout in tx["vout"]:
@@ -42,13 +44,7 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
                 amount += vout["value"]
         assert_approx(amount, float(value), vspan=0.001)
 
-    def generate_and_exchange_xpubs(self, participants):
-        """Every participant generates an xpub. The most straightforward way is to create a new descriptor wallet. Avoid reusing this wallet for any other purpose.."""
-        for i, node in enumerate(participants):
-            node.createwallet(wallet_name=f"participant_{i}", descriptors=True)
-            yield self._get_xpub(node.get_wallet_rpc(f"participant_{i}"))
-
-    def participants_import_descriptors(self, participants, xpubs):
+    def participants_create_multisigs(self, 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)
@@ -56,11 +52,11 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         for xpub in xpubs:
             base58_to_byte(xpub)
 
-        for i, node in enumerate(participants):
+        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'/0/*,'.join(xpubs)}/0/*))")
+            internal = multisig.getdescriptorinfo(f"wsh(sortedmulti({self.M},{f'/1/*,'.join(xpubs)}/1/*))")
             result = multisig.importdescriptors([
                 {  # receiving addresses (internal: False)
                     "desc": external["descriptor"],
@@ -76,18 +72,7 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
                 },
             ])
             assert all(r["success"] for r in result)
-
-    def get_multisig_receiving_address(self):
-        """We will send funds to the resulting address (every participant should get the same addresses)."""
-        multisig = self.nodes[0].get_wallet_rpc(f"{self.name}_{0}")
-        receiving_address = multisig.getnewaddress()
-        for i in range(1, self.N):
-            assert_equal(receiving_address, self.nodes[i].get_wallet_rpc(f"{self.name}_{i}").getnewaddress())
-        return receiving_address
-
-    def make_sending_transaction(self, to, value):
-        """Make a sending transaction, created using walletcreatefundedpsbt (anyone can initiate this)."""
-        return self.nodes[0].get_wallet_rpc(f"{self.name}_{0}").walletcreatefundedpsbt(inputs=[], outputs={to: value}, options={"feeRate": 0.00010})
+            yield multisig
 
     def run_test(self):
         self.M = 2
@@ -95,40 +80,57 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         self.name = f"{self.M}_of_{self.N}_multisig"
         self.log.info(f"Testing {self.name}...")
 
+        participants = {
+            # Every participant generates an xpub. The most straightforward way is to create a new descriptor wallet.
+            # This wallet will be the participant's `signer` for the resulting multisig. Avoid reusing this wallet for any other purpose (for privacy reasons).
+            "signers": [node.get_wallet_rpc(node.createwallet(wallet_name=f"participant_{self.nodes.index(node)}", descriptors=True)["name"]) for node in self.nodes],
+            # After participants generate and exchange their xpubs they will each create their own watch-only multisig.
+            # Note: these multisigs are all the same, this justs highlights that each participant can independently verify everything on their own node.
+            "multisigs": []
+        }
+
         self.log.info("Generate and exchange xpubs...")
-        xpubs = list(self.generate_and_exchange_xpubs(self.nodes))
+        xpubs = [self._get_xpub(signer) for signer in participants["signers"]]
 
         self.log.info("Every participant imports the following descriptors to create the watch-only multisig...")
-        self.participants_import_descriptors(self.nodes, xpubs)
+        participants["multisigs"] = list(self.participants_create_multisigs(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
+            receive_addresses = [multisig.getnewaddress() for multisig in participants["multisigs"]]
+            all(address == receive_addresses[0] for address in receive_addresses)
+            change_addresses = [multisig.getrawchangeaddress() for multisig in participants["multisigs"]]
+            all(address == change_addresses[0] for address in change_addresses)
 
         self.log.info("Get a mature utxo to send to the multisig...")
-        coordinator_wallet = self.nodes[0].get_wallet_rpc(f"participant_{0}")
+        coordinator_wallet = participants["signers"][0]
         coordinator_wallet.generatetoaddress(101, coordinator_wallet.getnewaddress())
 
         deposit_amount = 6.15
-        multisig_receiving_address = self.get_multisig_receiving_address()
+        multisig_receiving_address = participants["multisigs"][0].getnewaddress()
         self.log.info("Send funds to the resulting multisig receiving address...")
         coordinator_wallet.sendtoaddress(multisig_receiving_address, deposit_amount)
         self.nodes[0].generate(1)
         self.sync_all()
-        for n in range(self.N):
-            assert_approx(self.nodes[n].get_wallet_rpc(f"{self.name}_{n}").getbalance(), deposit_amount, vspan=0.001)
+        for participant in participants["multisigs"]:
+            assert_approx(participant.getbalance(), deposit_amount, vspan=0.001)
 
         self.log.info("Send a transaction from the multisig!")
-        to = self.nodes[self.N - 1].get_wallet_rpc(f"participant_{self.N - 1}").getnewaddress()
+        to = participants["signers"][self.N - 1].getnewaddress()
         value = 1
-        psbt = self.make_sending_transaction(to, value)
+        self.log.info("First, make a sending transaction, created using `walletcreatefundedpsbt` (anyone can initiate this)...")
+        psbt = participants["multisigs"][0].walletcreatefundedpsbt(inputs=[], outputs={to: value}, options={"feeRate": 0.00010})
 
         psbts = []
-        self.log.info("At least M users check the psbt with decodepsbt and (if OK) signs it with walletprocesspsbt...")
+        self.log.info("Now at least M users check the psbt with decodepsbt and (if OK) signs it with walletprocesspsbt...")
         for m in range(self.M):
-            signers_multisig = self.nodes[m].get_wallet_rpc(f"{self.name}_{m}")
+            signers_multisig = participants["multisigs"][m]
             self._check_psbt(psbt["psbt"], to, value, signers_multisig)
-            signing_wallet = self.nodes[m].get_wallet_rpc(f"participant_{m}")
+            signing_wallet = participants["signers"][m]
             partially_signed_psbt = signing_wallet.walletprocesspsbt(psbt["psbt"])
             psbts.append(partially_signed_psbt["psbt"])
 
-        self.log.info("Collect the signed PSBTs with combinepsbt, finalizepsbt, then broadcast the resulting transaction...")
+        self.log.info("Finally, collect the signed PSBTs with combinepsbt, finalizepsbt, then broadcast the resulting transaction...")
         combined = coordinator_wallet.combinepsbt(psbts)
         finalized = coordinator_wallet.finalizepsbt(combined)
         coordinator_wallet.sendrawtransaction(finalized["hex"])
@@ -136,13 +138,15 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         self.log.info("Check that balances are correct after the transaction has been included in a block.")
         self.nodes[0].generate(1)
         self.sync_all()
-        assert_approx(self.nodes[0].get_wallet_rpc(f"{self.name}_{0}").getbalance(), deposit_amount - value, vspan=0.001)
-        assert_equal(self.nodes[self.N - 1].get_wallet_rpc(f"participant_{self.N - 1}").getbalance(), value)
+        assert_approx(participants["multisigs"][0].getbalance(), deposit_amount - value, vspan=0.001)
+        assert_equal(participants["signers"][self.N - 1].getbalance(), value)
 
         self.log.info("Send another transaction from the multisig, this time with a daisy chained signing flow (one after another in series)!")
-        psbt = self.make_sending_transaction(to, value)
+        psbt = participants["multisigs"][0].walletcreatefundedpsbt(inputs=[], outputs={to: value}, options={"feeRate": 0.00010})
         for m in range(self.M):
-            signing_wallet = self.nodes[m].get_wallet_rpc(f"participant_{m}")
+            signers_multisig = participants["multisigs"][m]
+            self._check_psbt(psbt["psbt"], to, value, signers_multisig)
+            signing_wallet = participants["signers"][m]
             psbt = signing_wallet.walletprocesspsbt(psbt["psbt"])
             assert_equal(psbt["complete"], m == self.M - 1)
         finalized = coordinator_wallet.finalizepsbt(psbt["psbt"])
@@ -151,8 +155,8 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework):
         self.log.info("Check that balances are correct after the transaction has been included in a block.")
         self.nodes[0].generate(1)
         self.sync_all()
-        assert_approx(self.nodes[0].get_wallet_rpc(f"{self.name}_{0}").getbalance(), deposit_amount - (value * 2), vspan=0.001)
-        assert_equal(self.nodes[self.N - 1].get_wallet_rpc(f"participant_{self.N - 1}").getbalance(), value * 2)
+        assert_approx(participants["multisigs"][0].getbalance(), deposit_amount - (value * 2), vspan=0.001)
+        assert_equal(participants["signers"][self.N - 1].getbalance(), value * 2)
 
 
 if __name__ == "__main__":