test: refactor, multiple cleanups in rpc_createmultisig.py

Cleaning up the test in the following ways:

* Generate priv-pub key pairs used for testing only once (instead of doing it 4 times).
* Simplifies 'wmulti' wallet creation, load and unload process.
* Removes confusing class members initialized and updated inside a nested for-loop.
* Simplifies do_multisig() outpoint detection:
  The outpoint index information is already contained in MiniWallet's
  `send_to` return value dictionary as "sent_vout".

Co-authored-by: Sebastian Falbesoner <[email protected]>

Author: furszy

test/functional/rpc_createmultisig.py

@@ -10,9 +10,9 @@
 
 from test_framework.address import address_to_scriptpubkey
 from test_framework.blocktools import COINBASE_MATURITY
-from test_framework.authproxy import JSONRPCException
 from test_framework.descriptors import descsum_create, drop_origins
 from test_framework.key import ECPubKey
+from test_framework.messages import COIN
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_raises_rpc_error,
@@ -34,19 +34,22 @@ def set_test_params(self):
         self.supports_cli = False
         self.enable_wallet_if_possible()
 
-    def get_keys(self):
+    def create_keys(self, num_keys):
         self.pub = []
         self.priv = []
-        node0, node1, node2 = self.nodes
-        for _ in range(self.nkeys):
+        for _ in range(num_keys):
             privkey, pubkey = generate_keypair(wif=True)
             self.pub.append(pubkey.hex())
             self.priv.append(privkey)
         if self.is_bdb_compiled():
-            self.final = node2.getnewaddress()
+            self.final = self.nodes[2].getnewaddress()
         else:
             self.final = getnewdestination('bech32')[2]
 
+    def create_wallet(self, node, wallet_name):
+        node.createwallet(wallet_name=wallet_name, disable_private_keys=True)
+        return node.get_wallet_rpc(wallet_name)
+
     def run_test(self):
         node0, node1, node2 = self.nodes
         self.wallet = MiniWallet(test_node=node0)
@@ -57,12 +60,15 @@ def run_test(self):
         self.log.info('Generating blocks ...')
         self.generate(self.wallet, 149)
 
+        wallet_multi = self.create_wallet(node1, 'wmulti') if self._requires_wallet else None
         self.moved = 0
-        for self.nkeys in [3, 5]:
-            for self.nsigs in [2, 3]:
-                for self.output_type in ["bech32", "p2sh-segwit", "legacy"]:
-                    self.get_keys()
-                    self.do_multisig()
+        self.create_keys(5)
+        for nkeys in [3, 5]:
+            for nsigs in [2, 3]:
+                for output_type in ["bech32", "p2sh-segwit", "legacy"]:
+                    self.do_multisig(nkeys, nsigs, output_type, wallet_multi)
+        if wallet_multi is not None:
+            wallet_multi.unloadwallet()
         if self.is_bdb_compiled():
             self.checkbalances()
 
@@ -149,101 +155,85 @@ def checkbalances(self):
         assert bal2 == self.moved
         assert_equal(bal0 + bal1 + bal2 + balw, total)
 
-    def do_multisig(self):
+    def do_multisig(self, nkeys, nsigs, output_type, wallet_multi):
         node0, node1, node2 = self.nodes
-
-        if self.is_bdb_compiled():
-            if 'wmulti' not in node1.listwallets():
-                try:
-                    node1.loadwallet('wmulti')
-                except JSONRPCException as e:
-                    path = self.nodes[1].wallets_path / "wmulti"
-                    if e.error['code'] == -18 and "Wallet file verification failed. Failed to load database path '{}'. Path does not exist.".format(path) in e.error['message']:
-                        node1.createwallet(wallet_name='wmulti', disable_private_keys=True)
-                    else:
-                        raise
-            wmulti = node1.get_wallet_rpc('wmulti')
+        pub_keys = self.pub[0: nkeys]
+        priv_keys = self.priv[0: nkeys]
 
         # Construct the expected descriptor
-        desc = 'multi({},{})'.format(self.nsigs, ','.join(self.pub))
-        if self.output_type == 'legacy':
+        desc = 'multi({},{})'.format(nsigs, ','.join(pub_keys))
+        if output_type == 'legacy':
             desc = 'sh({})'.format(desc)
-        elif self.output_type == 'p2sh-segwit':
+        elif output_type == 'p2sh-segwit':
             desc = 'sh(wsh({}))'.format(desc)
-        elif self.output_type == 'bech32':
+        elif output_type == 'bech32':
             desc = 'wsh({})'.format(desc)
         desc = descsum_create(desc)
 
-        msig = node2.createmultisig(self.nsigs, self.pub, self.output_type)
+        msig = node2.createmultisig(nsigs, pub_keys, output_type)
         assert 'warnings' not in msig
         madd = msig["address"]
         mredeem = msig["redeemScript"]
         assert_equal(desc, msig['descriptor'])
-        if self.output_type == 'bech32':
+        if output_type == 'bech32':
             assert madd[0:4] == "bcrt"  # actually a bech32 address
 
-        if self.is_bdb_compiled():
+        if wallet_multi is not None:
             # compare against addmultisigaddress
-            msigw = wmulti.addmultisigaddress(self.nsigs, self.pub, None, self.output_type)
+            msigw = wallet_multi.addmultisigaddress(nsigs, pub_keys, None, output_type)
             maddw = msigw["address"]
             mredeemw = msigw["redeemScript"]
             assert_equal(desc, drop_origins(msigw['descriptor']))
             # addmultisigiaddress and createmultisig work the same
             assert maddw == madd
             assert mredeemw == mredeem
-            wmulti.unloadwallet()
 
         spk = address_to_scriptpubkey(madd)
-        txid = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=spk, amount=1300)["txid"]
-        tx = node0.getrawtransaction(txid, True)
-        vout = [v["n"] for v in tx["vout"] if madd == v["scriptPubKey"]["address"]]
-        assert len(vout) == 1
-        vout = vout[0]
-        scriptPubKey = tx["vout"][vout]["scriptPubKey"]["hex"]
-        value = tx["vout"][vout]["value"]
-        prevtxs = [{"txid": txid, "vout": vout, "scriptPubKey": scriptPubKey, "redeemScript": mredeem, "amount": value}]
+        value = decimal.Decimal("0.00001300")
+        tx = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=spk, amount=int(value * COIN))
+        prevtxs = [{"txid": tx["txid"], "vout": tx["sent_vout"], "scriptPubKey": spk.hex(), "redeemScript": mredeem, "amount": value}]
 
         self.generate(node0, 1)
 
         outval = value - decimal.Decimal("0.00001000")
-        rawtx = node2.createrawtransaction([{"txid": txid, "vout": vout}], [{self.final: outval}])
+        rawtx = node2.createrawtransaction([{"txid": tx["txid"], "vout": tx["sent_vout"]}], [{self.final: outval}])
 
         prevtx_err = dict(prevtxs[0])
         del prevtx_err["redeemScript"]
 
-        assert_raises_rpc_error(-8, "Missing redeemScript/witnessScript", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
+        assert_raises_rpc_error(-8, "Missing redeemScript/witnessScript", node2.signrawtransactionwithkey, rawtx, priv_keys[0:nsigs-1], [prevtx_err])
 
         # if witnessScript specified, all ok
         prevtx_err["witnessScript"] = prevtxs[0]["redeemScript"]
-        node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
+        node2.signrawtransactionwithkey(rawtx, priv_keys[0:nsigs-1], [prevtx_err])
 
         # both specified, also ok
         prevtx_err["redeemScript"] = prevtxs[0]["redeemScript"]
-        node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
+        node2.signrawtransactionwithkey(rawtx, priv_keys[0:nsigs-1], [prevtx_err])
 
         # redeemScript mismatch to witnessScript
         prevtx_err["redeemScript"] = "6a" # OP_RETURN
-        assert_raises_rpc_error(-8, "redeemScript does not correspond to witnessScript", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
+        assert_raises_rpc_error(-8, "redeemScript does not correspond to witnessScript", node2.signrawtransactionwithkey, rawtx, priv_keys[0:nsigs-1], [prevtx_err])
 
         # redeemScript does not match scriptPubKey
         del prevtx_err["witnessScript"]
-        assert_raises_rpc_error(-8, "redeemScript/witnessScript does not match scriptPubKey", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
+        assert_raises_rpc_error(-8, "redeemScript/witnessScript does not match scriptPubKey", node2.signrawtransactionwithkey, rawtx, priv_keys[0:nsigs-1], [prevtx_err])
 
         # witnessScript does not match scriptPubKey
         prevtx_err["witnessScript"] = prevtx_err["redeemScript"]
         del prevtx_err["redeemScript"]
-        assert_raises_rpc_error(-8, "redeemScript/witnessScript does not match scriptPubKey", node2.signrawtransactionwithkey, rawtx, self.priv[0:self.nsigs-1], [prevtx_err])
+        assert_raises_rpc_error(-8, "redeemScript/witnessScript does not match scriptPubKey", node2.signrawtransactionwithkey, rawtx, priv_keys[0:nsigs-1], [prevtx_err])
 
-        rawtx2 = node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs - 1], prevtxs)
-        rawtx3 = node2.signrawtransactionwithkey(rawtx2["hex"], [self.priv[-1]], prevtxs)
+        rawtx2 = node2.signrawtransactionwithkey(rawtx, priv_keys[0:nsigs - 1], prevtxs)
+        rawtx3 = node2.signrawtransactionwithkey(rawtx2["hex"], [priv_keys[-1]], prevtxs)
 
         self.moved += outval
         tx = node0.sendrawtransaction(rawtx3["hex"], 0)
         blk = self.generate(node0, 1)[0]
         assert tx in node0.getblock(blk)["tx"]
 
         txinfo = node0.getrawtransaction(tx, True, blk)
-        self.log.info("n/m=%d/%d %s size=%d vsize=%d weight=%d" % (self.nsigs, self.nkeys, self.output_type, txinfo["size"], txinfo["vsize"], txinfo["weight"]))
+        self.log.info("n/m=%d/%d %s size=%d vsize=%d weight=%d" % (nsigs, nkeys, output_type, txinfo["size"], txinfo["vsize"], txinfo["weight"]))
 
 
 if __name__ == '__main__':