Merge #13072: Update createmultisig RPC to support segwit

f40b3b82df [tests] functional test for createmultisig RPC (Anthony Towns)
b9024fdda3 segwit support for createmultisig RPC (Anthony Towns)
d58055d25f Move AddAndGetDestinationForScript from wallet to outputype module (Anthony Towns)
9a44db2e46 Add outputtype module (Anthony Towns)

Pull request description:

  Adds an "address_type" parameter that accepts "legacy", "p2sh-segwit", and "bech32" to choose the type of address created. Defaults to "legacy" rather than the value of the `-address-type` option for backwards compatibility.

  As part of implementing this, OutputType is moved from wallet into its own module, and `AddAndGetDestinationForScript` is changed to apply to a `CKeyStore` rather than a wallet, and to invoke `keystore.AddCScript(script)` itself rather than expecting the caller to have done that.

  Fixes #12502

Tree-SHA512: a08c1cfa89976e4fd7d29caa90919ebd34a446354d17abb862e99f2ee60ed9bc19d8a21a18547c51dc3812cb9fbed86af0bef2f1e971f62bf95cade4a7d86237

10 files changed, 271 insertions, 122 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index 5d7eafb3f1..1dd202085a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -139,6 +139,7 @@ BITCOIN_CORE_H = \
   netbase.h \
   netmessagemaker.h \
   noui.h \
+  outputtype.h \
   policy/feerate.h \
   policy/fees.h \
   policy/policy.h \
@@ -230,6 +231,7 @@ libbitcoin_server_a_SOURCES = \
   net.cpp \
   net_processing.cpp \
   noui.cpp \
+  outputtype.cpp \
   policy/fees.cpp \
   policy/policy.cpp \
   policy/rbf.cpp \
diff --git a/src/outputtype.cpp b/src/outputtype.cpp
new file mode 100644
index 0000000000..3ff28bf9c2
--- /dev/null
+++ b/src/outputtype.cpp
@@ -0,0 +1,101 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2017 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <outputtype.h>
+
+#include <keystore.h>
+#include <pubkey.h>
+#include <script/script.h>
+#include <script/standard.h>
+
+#include <assert.h>
+#include <string>
+
+static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy";
+static const std::string OUTPUT_TYPE_STRING_P2SH_SEGWIT = "p2sh-segwit";
+static const std::string OUTPUT_TYPE_STRING_BECH32 = "bech32";
+
+bool ParseOutputType(const std::string& type, OutputType& output_type)
+{
+    if (type == OUTPUT_TYPE_STRING_LEGACY) {
+        output_type = OutputType::LEGACY;
+        return true;
+    } else if (type == OUTPUT_TYPE_STRING_P2SH_SEGWIT) {
+        output_type = OutputType::P2SH_SEGWIT;
+        return true;
+    } else if (type == OUTPUT_TYPE_STRING_BECH32) {
+        output_type = OutputType::BECH32;
+        return true;
+    }
+    return false;
+}
+
+const std::string& FormatOutputType(OutputType type)
+{
+    switch (type) {
+    case OutputType::LEGACY: return OUTPUT_TYPE_STRING_LEGACY;
+    case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT;
+    case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32;
+    default: assert(false);
+    }
+}
+
+CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type)
+{
+    switch (type) {
+    case OutputType::LEGACY: return key.GetID();
+    case OutputType::P2SH_SEGWIT:
+    case OutputType::BECH32: {
+        if (!key.IsCompressed()) return key.GetID();
+        CTxDestination witdest = WitnessV0KeyHash(key.GetID());
+        CScript witprog = GetScriptForDestination(witdest);
+        if (type == OutputType::P2SH_SEGWIT) {
+            return CScriptID(witprog);
+        } else {
+            return witdest;
+        }
+    }
+    default: assert(false);
+    }
+}
+
+std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key)
+{
+    CKeyID keyid = key.GetID();
+    if (key.IsCompressed()) {
+        CTxDestination segwit = WitnessV0KeyHash(keyid);
+        CTxDestination p2sh = CScriptID(GetScriptForDestination(segwit));
+        return std::vector<CTxDestination>{std::move(keyid), std::move(p2sh), std::move(segwit)};
+    } else {
+        return std::vector<CTxDestination>{std::move(keyid)};
+    }
+}
+
+CTxDestination AddAndGetDestinationForScript(CKeyStore& keystore, const CScript& script, OutputType type)
+{
+    // Add script to keystore
+    keystore.AddCScript(script);
+    // Note that scripts over 520 bytes are not yet supported.
+    switch (type) {
+    case OutputType::LEGACY:
+        return CScriptID(script);
+    case OutputType::P2SH_SEGWIT:
+    case OutputType::BECH32: {
+        CTxDestination witdest = WitnessV0ScriptHash(script);
+        CScript witprog = GetScriptForDestination(witdest);
+        // Check if the resulting program is solvable (i.e. doesn't use an uncompressed key)
+        if (!IsSolvable(keystore, witprog)) return CScriptID(script);
+        // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours.
+        keystore.AddCScript(witprog);
+        if (type == OutputType::BECH32) {
+            return witdest;
+        } else {
+            return CScriptID(witprog);
+        }
+    }
+    default: assert(false);
+    }
+}
+
diff --git a/src/outputtype.h b/src/outputtype.h
new file mode 100644
index 0000000000..21623e3b49
--- /dev/null
+++ b/src/outputtype.h
@@ -0,0 +1,49 @@
+// Copyright (c) 2009-2010 Satoshi Nakamoto
+// Copyright (c) 2009-2017 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_OUTPUTTYPE_H
+#define BITCOIN_OUTPUTTYPE_H
+
+#include <keystore.h>
+#include <script/standard.h>
+
+#include <string>
+#include <vector>
+
+enum class OutputType {
+    LEGACY,
+    P2SH_SEGWIT,
+    BECH32,
+
+    /**
+     * Special output type for change outputs only. Automatically choose type
+     * based on address type setting and the types other of non-change outputs
+     * (see -changetype option documentation and implementation in
+     * CWallet::TransactionChangeType for details).
+     */
+    CHANGE_AUTO,
+};
+
+bool ParseOutputType(const std::string& str, OutputType& output_type);
+const std::string& FormatOutputType(OutputType type);
+
+/**
+ * Get a destination of the requested type (if possible) to the specified key.
+ * The caller must make sure LearnRelatedScripts has been called beforehand.
+ */
+CTxDestination GetDestinationForKey(const CPubKey& key, OutputType);
+
+/** Get all destinations (potentially) supported by the wallet for the given key. */
+std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key);
+
+/**
+ * Get a destination of the requested type (if possible) to the specified script.
+ * This function will automatically add the script (and any other
+ * necessary scripts) to the keystore.
+ */
+CTxDestination AddAndGetDestinationForScript(CKeyStore& keystore, const CScript& script, OutputType);
+
+#endif // BITCOIN_OUTPUTTYPE_H
+
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index 4eeb7f29d2..09812bb980 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -12,6 +12,7 @@
 #include <httpserver.h>
 #include <net.h>
 #include <netbase.h>
+#include <outputtype.h>
 #include <rpc/blockchain.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
@@ -91,9 +92,9 @@ class CWallet;
 
 static UniValue createmultisig(const JSONRPCRequest& request)
 {
-    if (request.fHelp || request.params.size() < 2 || request.params.size() > 2)
+    if (request.fHelp || request.params.size() < 2 || request.params.size() > 3)
     {
-        std::string msg = "createmultisig nrequired [\"key\",...]\n"
+        std::string msg = "createmultisig nrequired [\"key\",...] ( \"address_type\" )\n"
             "\nCreates a multi-signature address with n signature of m keys required.\n"
             "It returns a json object with the address and redeemScript.\n"
             "\nArguments:\n"
@@ -103,6 +104,7 @@ static UniValue createmultisig(const JSONRPCRequest& request)
             "       \"key\"                    (string) The hex-encoded public key\n"
             "       ,...\n"
             "     ]\n"
+            "3. \"address_type\"               (string, optional) The address type to use. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\". Default is legacy.\n"
 
             "\nResult:\n"
             "{\n"
@@ -133,12 +135,21 @@ static UniValue createmultisig(const JSONRPCRequest& request)
         }
     }
 
+    // Get the output type
+    OutputType output_type = OutputType::LEGACY;
+    if (!request.params[2].isNull()) {
+        if (!ParseOutputType(request.params[2].get_str(), output_type)) {
+            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[2].get_str()));
+        }
+    }
+
     // Construct using pay-to-script-hash:
-    CScript inner = CreateMultisigRedeemscript(required, pubkeys);
-    CScriptID innerID(inner);
+    const CScript inner = CreateMultisigRedeemscript(required, pubkeys);
+    CBasicKeyStore keystore;
+    const CTxDestination dest = AddAndGetDestinationForScript(keystore, inner, output_type);
 
     UniValue result(UniValue::VOBJ);
-    result.pushKV("address", EncodeDestination(innerID));
+    result.pushKV("address", EncodeDestination(dest));
     result.pushKV("redeemScript", HexStr(inner.begin(), inner.end()));
 
     return result;
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index 74312b7124..076134cdd1 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -7,6 +7,7 @@
 #include <init.h>
 #include <net.h>
 #include <scheduler.h>
+#include <outputtype.h>
 #include <util.h>
 #include <utilmoneystr.h>
 #include <validation.h>
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 3530030e37..893310cf2f 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -11,6 +11,7 @@
 #include <validation.h>
 #include <key_io.h>
 #include <net.h>
+#include <outputtype.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
 #include <policy/policy.h>
@@ -1369,8 +1370,7 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request)
 
     // Construct using pay-to-script-hash:
     CScript inner = CreateMultisigRedeemscript(required, pubkeys);
-    pwallet->AddCScript(inner);
-    CTxDestination dest = pwallet->AddAndGetDestinationForScript(inner, output_type);
+    CTxDestination dest = AddAndGetDestinationForScript(*pwallet, inner, output_type);
     pwallet->SetAddressBook(dest, label, "send");
 
     UniValue result(UniValue::VOBJ);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index c9553d0d07..38f4de9633 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4362,35 +4362,6 @@ bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState&
     return ret;
 }
 
-static const std::string OUTPUT_TYPE_STRING_LEGACY = "legacy";
-static const std::string OUTPUT_TYPE_STRING_P2SH_SEGWIT = "p2sh-segwit";
-static const std::string OUTPUT_TYPE_STRING_BECH32 = "bech32";
-
-bool ParseOutputType(const std::string& type, OutputType& output_type)
-{
-    if (type == OUTPUT_TYPE_STRING_LEGACY) {
-        output_type = OutputType::LEGACY;
-        return true;
-    } else if (type == OUTPUT_TYPE_STRING_P2SH_SEGWIT) {
-        output_type = OutputType::P2SH_SEGWIT;
-        return true;
-    } else if (type == OUTPUT_TYPE_STRING_BECH32) {
-        output_type = OutputType::BECH32;
-        return true;
-    }
-    return false;
-}
-
-const std::string& FormatOutputType(OutputType type)
-{
-    switch (type) {
-    case OutputType::LEGACY: return OUTPUT_TYPE_STRING_LEGACY;
-    case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT;
-    case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32;
-    default: assert(false);
-    }
-}
-
 void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type)
 {
     if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT || type == OutputType::BECH32)) {
@@ -4408,57 +4379,3 @@ void CWallet::LearnAllRelatedScripts(const CPubKey& key)
     LearnRelatedScripts(key, OutputType::P2SH_SEGWIT);
 }
 
-CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type)
-{
-    switch (type) {
-    case OutputType::LEGACY: return key.GetID();
-    case OutputType::P2SH_SEGWIT:
-    case OutputType::BECH32: {
-        if (!key.IsCompressed()) return key.GetID();
-        CTxDestination witdest = WitnessV0KeyHash(key.GetID());
-        CScript witprog = GetScriptForDestination(witdest);
-        if (type == OutputType::P2SH_SEGWIT) {
-            return CScriptID(witprog);
-        } else {
-            return witdest;
-        }
-    }
-    default: assert(false);
-    }
-}
-
-std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key)
-{
-    CKeyID keyid = key.GetID();
-    if (key.IsCompressed()) {
-        CTxDestination segwit = WitnessV0KeyHash(keyid);
-        CTxDestination p2sh = CScriptID(GetScriptForDestination(segwit));
-        return std::vector<CTxDestination>{std::move(keyid), std::move(p2sh), std::move(segwit)};
-    } else {
-        return std::vector<CTxDestination>{std::move(keyid)};
-    }
-}
-
-CTxDestination CWallet::AddAndGetDestinationForScript(const CScript& script, OutputType type)
-{
-    // Note that scripts over 520 bytes are not yet supported.
-    switch (type) {
-    case OutputType::LEGACY:
-        return CScriptID(script);
-    case OutputType::P2SH_SEGWIT:
-    case OutputType::BECH32: {
-        CTxDestination witdest = WitnessV0ScriptHash(script);
-        CScript witprog = GetScriptForDestination(witdest);
-        // Check if the resulting program is solvable (i.e. doesn't use an uncompressed key)
-        if (!IsSolvable(*this, witprog)) return CScriptID(script);
-        // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours.
-        AddCScript(witprog);
-        if (type == OutputType::BECH32) {
-            return witdest;
-        } else {
-            return CScriptID(witprog);
-        }
-    }
-    default: assert(false);
-    }
-}
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 3cb396d7d2..2e53ca0c55 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -7,6 +7,7 @@
 #define BITCOIN_WALLET_WALLET_H
 
 #include <amount.h>
+#include <outputtype.h>
 #include <policy/feerate.h>
 #include <streams.h>
 #include <tinyformat.h>
@@ -93,20 +94,6 @@ enum WalletFeature
     FEATURE_LATEST = FEATURE_PRE_SPLIT_KEYPOOL
 };
 
-enum class OutputType {
-    LEGACY,
-    P2SH_SEGWIT,
-    BECH32,
-
-    /**
-     * Special output type for change outputs only. Automatically choose type
-     * based on address type setting and the types other of non-change outputs
-     * (see -changetype option documentation and implementation in
-     * CWallet::TransactionChangeType for details).
-     */
-    CHANGE_AUTO,
-};
-
 //! Default for -addresstype
 constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::P2SH_SEGWIT};
 
@@ -1196,12 +1183,6 @@ public:
      */
     void LearnAllRelatedScripts(const CPubKey& key);
 
-    /**
-     * Get a destination of the requested type (if possible) to the specified script.
-     * This function will automatically add the necessary scripts to the wallet.
-     */
-    CTxDestination AddAndGetDestinationForScript(const CScript& script, OutputType);
-
     /** Whether a given output is spendable by this wallet */
     bool OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibility_filter) const;
 };
@@ -1268,18 +1249,6 @@ public:
     }
 };
 
-bool ParseOutputType(const std::string& str, OutputType& output_type);
-const std::string& FormatOutputType(OutputType type);
-
-/**
- * Get a destination of the requested type (if possible) to the specified key.
- * The caller must make sure LearnRelatedScripts has been called beforehand.
- */
-CTxDestination GetDestinationForKey(const CPubKey& key, OutputType);
-
-/** Get all destinations (potentially) supported by the wallet for the given key. */
-std::vector<CTxDestination> GetAllDestinationsForKey(const CPubKey& key);
-
 /** RAII object to check and reserve a wallet rescan */
 class WalletRescanReserver
 {
diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py
new file mode 100755
index 0000000000..97e614c888
--- /dev/null
+++ b/test/functional/rpc_createmultisig.py
@@ -0,0 +1,98 @@
+#!/usr/bin/env python3
+# Copyright (c) 2015-2017 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test transaction signing using the signrawtransaction* RPCs."""
+
+from test_framework.test_framework import BitcoinTestFramework
+import decimal
+
+class RpcCreateMultiSigTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = True
+        self.num_nodes = 3
+
+    def get_keys(self):
+        node0,node1,node2 = self.nodes
+        self.add = [node1.getnewaddress() for _ in range(self.nkeys)]
+        self.pub = [node1.getaddressinfo(a)["pubkey"] for a in self.add]
+        self.priv = [node1.dumpprivkey(a) for a in self.add]
+        self.final = node2.getnewaddress()
+
+    def run_test(self):
+        node0,node1,node2 = self.nodes
+
+        # 50 BTC each, rest will be 25 BTC each
+        node0.generate(149)
+        self.sync_all()
+
+        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.checkbalances()
+
+    def checkbalances(self):
+        node0,node1,node2 = self.nodes
+        node0.generate(100)
+        self.sync_all()
+
+        bal0 = node0.getbalance()
+        bal1 = node1.getbalance()
+        bal2 = node2.getbalance()
+
+        height = node0.getblockchaininfo()["blocks"]
+        assert 150 < height < 350
+        total = 149*50 + (height-149-100)*25
+        assert bal1 == 0
+        assert bal2 == self.moved
+        assert bal0+bal1+bal2 == total
+
+    def do_multisig(self):
+        node0,node1,node2 = self.nodes
+
+        msig = node2.createmultisig(self.nsigs, self.pub, self.output_type)
+        madd = msig["address"]
+        mredeem = msig["redeemScript"]
+        if self.output_type == 'bech32':
+            assert madd[0:4] == "bcrt"  # actually a bech32 address
+
+        # compare against addmultisigaddress
+        msigw = node1.addmultisigaddress(self.nsigs, self.pub, None, self.output_type)
+        maddw = msigw["address"]
+        mredeemw = msigw["redeemScript"]
+        # addmultisigiaddress and createmultisig work the same
+        assert maddw == madd
+        assert mredeemw == mredeem
+
+        txid = node0.sendtoaddress(madd, 40)
+
+        tx = node0.getrawtransaction(txid, True)
+        vout = [v["n"] for v in tx["vout"] if madd in v["scriptPubKey"].get("addresses",[])]
+        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}]
+
+        node0.generate(1)
+
+        outval = value - decimal.Decimal("0.00001000")
+        rawtx = node2.createrawtransaction([{"txid": txid, "vout": vout}], [{self.final: outval}])
+
+        rawtx2 = node2.signrawtransactionwithkey(rawtx, self.priv[0:self.nsigs-1], prevtxs)
+        rawtx3 = node2.signrawtransactionwithkey(rawtx2["hex"], [self.priv[-1]], prevtxs)
+
+        self.moved += outval
+        tx = node0.sendrawtransaction(rawtx3["hex"], True)
+        blk = node0.generate(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"]))
+
+if __name__ == '__main__':
+    RpcCreateMultiSigTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index c3a5468296..49833e5dd4 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -113,6 +113,7 @@ BASE_SCRIPTS = [
     'mining_prioritisetransaction.py',
     'p2p_invalid_block.py',
     'p2p_invalid_tx.py',
+    'rpc_createmultisig.py',
     'feature_versionbits_warning.py',
     'rpc_preciousblock.py',
     'wallet_importprunedfunds.py',