Merge #11415: [RPC] Disallow using addresses in createmultisig

1df206f Disallow using addresses in createmultisig (Andrew Chow)

Pull request description:

  This PR should be the last part of #7965.

  This PR makes createmultisig only accept public keys and marks the old functionality of accepting addresses as deprecated.

  It also splits `_createmultisig_redeemscript` into two functions, `_createmultisig_getpubkeys` and `_createmultisig_getaddr_pubkeys`. `_createmultisig_getpubkeys` retrieves public keys from the RPC parameters and `_createmultisig_getaddr_pubkeys` retrieves addresses' public keys from the wallet. `_createmultisig_getaddr_pubkeys` requires the wallet and is only used by `addwitnessaddress` (except when `createmultisig` is used in deprecated mode).

  `addwitnessaddress`'s API is also changed. Instead of returning just an address, it now returns the same thing as `createmultisig`: a JSON object with two fields, address and redeemscript.

Tree-SHA512: a5796e41935ad5e47d8165ff996a8b20d5112b5fc1a06a6d3c7f5513c13e7628a4fd37ec30fde05d8b15abfed51bc250710140f6834b13f64d0a0e47a3817969

10 files changed, 47 insertions, 37 deletions

diff --git a/test/functional/address_types.py b/test/functional/address_types.py
index cb119c04b0..9984e171f0 100755
--- a/test/functional/address_types.py
+++ b/test/functional/address_types.py
@@ -117,14 +117,14 @@ class AddressTypeTest(BitcoinTestFramework):
 
         # addmultisigaddress with at least 1 uncompressed key should return a legacy address.
         for node in range(4):
-            self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, uncompressed_2]), True, 'legacy')
-            self.test_address(node, self.nodes[node].addmultisigaddress(2, [compressed_1, uncompressed_2]), True, 'legacy')
-            self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, compressed_2]), True, 'legacy')
+            self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, uncompressed_2])['address'], True, 'legacy')
+            self.test_address(node, self.nodes[node].addmultisigaddress(2, [compressed_1, uncompressed_2])['address'], True, 'legacy')
+            self.test_address(node, self.nodes[node].addmultisigaddress(2, [uncompressed_1, compressed_2])['address'], True, 'legacy')
         # addmultisigaddress with all compressed keys should return the appropriate address type (even when the keys are not ours).
-        self.test_address(0, self.nodes[0].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'legacy')
-        self.test_address(1, self.nodes[1].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'p2sh-segwit')
-        self.test_address(2, self.nodes[2].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'p2sh-segwit')
-        self.test_address(3, self.nodes[3].addmultisigaddress(2, [compressed_1, compressed_2]), True, 'bech32')
+        self.test_address(0, self.nodes[0].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'legacy')
+        self.test_address(1, self.nodes[1].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'p2sh-segwit')
+        self.test_address(2, self.nodes[2].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'p2sh-segwit')
+        self.test_address(3, self.nodes[3].addmultisigaddress(2, [compressed_1, compressed_2])['address'], True, 'bech32')
 
         for explicit_type, multisig, from_node in itertools.product([False, True], [False, True], range(4)):
             address_type = None
@@ -155,7 +155,7 @@ class AddressTypeTest(BitcoinTestFramework):
                 else:
                     addr1 = self.nodes[to_node].getnewaddress()
                     addr2 = self.nodes[to_node].getnewaddress()
-                    address = self.nodes[to_node].addmultisigaddress(2, [addr1, addr2])
+                    address = self.nodes[to_node].addmultisigaddress(2, [addr1, addr2])['address']
 
                 # Do some sanity checking on the created address
                 if address_type is not None:
diff --git a/test/functional/deprecated_rpc.py b/test/functional/deprecated_rpc.py
index 19fd24edb9..d6f25158ef 100755
--- a/test/functional/deprecated_rpc.py
+++ b/test/functional/deprecated_rpc.py
@@ -10,7 +10,7 @@ class DeprecatedRpcTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.setup_clean_chain = True
-        self.extra_args = [[], ["-deprecatedrpc=estimatefee"]]
+        self.extra_args = [[], ["-deprecatedrpc=estimatefee", "-deprecatedrpc=createmultisig"]]
 
     def run_test(self):
         self.log.info("estimatefee: Shows deprecated message")
@@ -19,5 +19,9 @@ class DeprecatedRpcTest(BitcoinTestFramework):
         self.log.info("Using -deprecatedrpc=estimatefee bypasses the error")
         self.nodes[1].estimatefee(1)
 
+        self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses")
+        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()])
+        self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])
+
 if __name__ == '__main__':
     DeprecatedRpcTest().main()
diff --git a/test/functional/fundrawtransaction.py b/test/functional/fundrawtransaction.py
index 4739e73c39..b3d6549229 100755
--- a/test/functional/fundrawtransaction.py
+++ b/test/functional/fundrawtransaction.py
@@ -358,7 +358,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         addr1Obj = self.nodes[1].validateaddress(addr1)
         addr2Obj = self.nodes[1].validateaddress(addr2)
 
-        mSigObj = self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
+        mSigObj = self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
 
         inputs = []
         outputs = {mSigObj:1.1}
@@ -391,7 +391,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         addr4Obj = self.nodes[1].validateaddress(addr4)
         addr5Obj = self.nodes[1].validateaddress(addr5)
 
-        mSigObj = self.nodes[1].addmultisigaddress(4, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey'], addr4Obj['pubkey'], addr5Obj['pubkey']])
+        mSigObj = self.nodes[1].addmultisigaddress(4, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey'], addr4Obj['pubkey'], addr5Obj['pubkey']])['address']
 
         inputs = []
         outputs = {mSigObj:1.1}
@@ -418,7 +418,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         addr1Obj = self.nodes[2].validateaddress(addr1)
         addr2Obj = self.nodes[2].validateaddress(addr2)
 
-        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
 
 
         # send 1.2 BTC to msig addr
diff --git a/test/functional/importmulti.py b/test/functional/importmulti.py
index ab4ca48e48..be9be83839 100755
--- a/test/functional/importmulti.py
+++ b/test/functional/importmulti.py
@@ -228,7 +228,7 @@ class ImportMultiTest (BitcoinTestFramework):
         sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
-        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']])
+        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']])
         self.nodes[1].generate(100)
         transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
         self.nodes[1].generate(1)
@@ -255,7 +255,7 @@ class ImportMultiTest (BitcoinTestFramework):
         sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
-        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']])
+        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']])
         self.nodes[1].generate(100)
         transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
         self.nodes[1].generate(1)
@@ -282,7 +282,7 @@ class ImportMultiTest (BitcoinTestFramework):
         sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
-        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']])
+        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']])
         self.nodes[1].generate(100)
         transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
         self.nodes[1].generate(1)
@@ -309,7 +309,7 @@ class ImportMultiTest (BitcoinTestFramework):
         sig_address_1 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_2 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
         sig_address_3 = self.nodes[0].validateaddress(self.nodes[0].getnewaddress())
-        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['address'], sig_address_2['address'], sig_address_3['pubkey']])
+        multi_sig_script = self.nodes[0].createmultisig(2, [sig_address_1['pubkey'], sig_address_2['pubkey'], sig_address_3['pubkey']])
         self.nodes[1].generate(100)
         transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00)
         self.nodes[1].generate(1)
diff --git a/test/functional/listtransactions.py b/test/functional/listtransactions.py
index cbed065928..ba71ac0967 100755
--- a/test/functional/listtransactions.py
+++ b/test/functional/listtransactions.py
@@ -81,7 +81,8 @@ class ListTransactionsTest(BitcoinTestFramework):
                            {"category":"receive","amount":Decimal("0.44")},
                            {"txid":txid, "account" : "toself"} )
 
-        multisig = self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])
+        pubkey = self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['pubkey']
+        multisig = self.nodes[1].createmultisig(1, [pubkey])
         self.nodes[0].importaddress(multisig["redeemScript"], "watchonly", False, True)
         txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1)
         self.nodes[1].generate(1)
diff --git a/test/functional/nulldummy.py b/test/functional/nulldummy.py
index cbdcfdcf5f..fac620bc8d 100755
--- a/test/functional/nulldummy.py
+++ b/test/functional/nulldummy.py
@@ -46,7 +46,7 @@ class NULLDUMMYTest(BitcoinTestFramework):
 
     def run_test(self):
         self.address = self.nodes[0].getnewaddress()
-        self.ms_address = self.nodes[0].addmultisigaddress(1,[self.address])
+        self.ms_address = self.nodes[0].addmultisigaddress(1,[self.address])['address']
         self.wit_address = self.nodes[0].addwitnessaddress(self.address)
         self.wit_ms_address = self.nodes[0].addwitnessaddress(self.ms_address)
 
diff --git a/test/functional/rawtransactions.py b/test/functional/rawtransactions.py
index fe749adb49..d39d86b310 100755
--- a/test/functional/rawtransactions.py
+++ b/test/functional/rawtransactions.py
@@ -145,7 +145,12 @@ class RawTransactionsTest(BitcoinTestFramework):
         addr1Obj = self.nodes[2].validateaddress(addr1)
         addr2Obj = self.nodes[2].validateaddress(addr2)
 
-        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
+        # Tests for createmultisig and addmultisigaddress
+        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"])
+        self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys
+        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here.
+
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address']
 
         #use balance deltas instead of absolute values
         bal = self.nodes[2].getbalance()
@@ -168,7 +173,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         addr2Obj = self.nodes[2].validateaddress(addr2)
         addr3Obj = self.nodes[2].validateaddress(addr3)
 
-        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address']
 
         txId = self.nodes[0].sendtoaddress(mSigObj, 2.2)
         decTx = self.nodes[0].gettransaction(txId)
@@ -213,8 +218,8 @@ class RawTransactionsTest(BitcoinTestFramework):
         addr1Obj = self.nodes[1].validateaddress(addr1)
         addr2Obj = self.nodes[2].validateaddress(addr2)
 
-        self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
-        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
+        self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
         mSigObjValid = self.nodes[2].validateaddress(mSigObj)
 
         txId = self.nodes[0].sendtoaddress(mSigObj, 2.2)
diff --git a/test/functional/segwit.py b/test/functional/segwit.py
index 67cca5f1e7..931ebdea39 100755
--- a/test/functional/segwit.py
+++ b/test/functional/segwit.py
@@ -95,7 +95,7 @@ class SegWitTest(BitcoinTestFramework):
         for i in range(3):
             newaddress = self.nodes[i].getnewaddress()
             self.pubkey.append(self.nodes[i].validateaddress(newaddress)["pubkey"])
-            multiaddress = self.nodes[i].addmultisigaddress(1, [self.pubkey[-1]])
+            multiaddress = self.nodes[i].addmultisigaddress(1, [self.pubkey[-1]])['address']
             multiscript = CScript([OP_1, hex_str_to_bytes(self.pubkey[-1]), OP_1, OP_CHECKMULTISIG])
             p2sh_addr = self.nodes[i].addwitnessaddress(newaddress)
             bip173_addr = self.nodes[i].addwitnessaddress(newaddress, False)
@@ -290,19 +290,19 @@ class SegWitTest(BitcoinTestFramework):
         solvable_anytime = []                       # These outputs should be solvable after importpubkey
         unseen_anytime = []                         # These outputs should never be seen
 
-        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]]))
-        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]]))
-        compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]]))
-        uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], uncompressed_solvable_address[0]]))
-        compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]]))
-        compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], compressed_solvable_address[1]]))
+        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]])['address'])
+        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]])['address'])
+        compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]])['address'])
+        uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], uncompressed_solvable_address[0]])['address'])
+        compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]])['address'])
+        compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], compressed_solvable_address[1]])['address'])
         unknown_address = ["mtKKyoHabkk6e4ppT7NaM7THqPUt7AzPrT", "2NDP3jLWAFT8NDAiUa9qiE6oBt2awmMq7Dx"]
 
         # Test multisig_without_privkey
         # We have 2 public keys without private keys, use addmultisigaddress to add to wallet.
         # Money sent to P2SH of multisig of this should only be seen after importaddress with the BASE58 P2SH address.
 
-        multisig_without_privkey_address = self.nodes[0].addmultisigaddress(2, [pubkeys[3], pubkeys[4]])
+        multisig_without_privkey_address = self.nodes[0].addmultisigaddress(2, [pubkeys[3], pubkeys[4]])['address']
         script = CScript([OP_2, hex_str_to_bytes(pubkeys[3]), hex_str_to_bytes(pubkeys[4]), OP_2, OP_CHECKMULTISIG])
         solvable_after_importaddress.append(CScript([OP_HASH160, hash160(script), OP_EQUAL]))
 
@@ -463,11 +463,11 @@ class SegWitTest(BitcoinTestFramework):
         solvable_anytime = []                       # These outputs should be solvable after importpubkey
         unseen_anytime = []                         # These outputs should never be seen
 
-        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]]))
-        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]]))
-        compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]]))
-        uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], uncompressed_solvable_address[0]]))
-        compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]]))
+        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], compressed_spendable_address[0]])['address'])
+        uncompressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [uncompressed_spendable_address[0], uncompressed_spendable_address[0]])['address'])
+        compressed_spendable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_spendable_address[0]])['address'])
+        uncompressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_solvable_address[0], uncompressed_solvable_address[0]])['address'])
+        compressed_solvable_address.append(self.nodes[0].addmultisigaddress(2, [compressed_spendable_address[0], compressed_solvable_address[0]])['address'])
 
         premature_witaddress = []
 
diff --git a/test/functional/wallet-accounts.py b/test/functional/wallet-accounts.py
index 287436877f..ecd1cfc82b 100755
--- a/test/functional/wallet-accounts.py
+++ b/test/functional/wallet-accounts.py
@@ -126,7 +126,7 @@ class WalletAccountsTest(BitcoinTestFramework):
             addresses = []
             for x in range(10):
                 addresses.append(node.getnewaddress())
-            multisig_address = node.addmultisigaddress(5, addresses, account.name)
+            multisig_address = node.addmultisigaddress(5, addresses, account.name)['address']
             account.add_address(multisig_address)
             account.verify(node)
             node.sendfrom("", multisig_address, 50)
diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py
index a1a4f25921..77f90ffb81 100755
--- a/test/functional/wallet-dump.py
+++ b/test/functional/wallet-dump.py
@@ -94,7 +94,7 @@ class WalletDumpTest(BitcoinTestFramework):
 
         # Test scripts dump by adding a P2SH witness and a 1-of-1 multisig address
         witness_addr = self.nodes[0].addwitnessaddress(addrs[0]["address"], True)
-        multisig_addr = self.nodes[0].addmultisigaddress(1, [addrs[1]["address"]])
+        multisig_addr = self.nodes[0].addmultisigaddress(1, [addrs[1]["address"]])["address"]
         script_addrs = [witness_addr, multisig_addr]
 
         # dump unencrypted wallet