From 1df206f854d222230dcffd58e1b496567e570978 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Fri, 29 Sep 2017 00:21:28 -0400 Subject: [PATCH] Disallow using addresses in createmultisig Make createmultisig only accept public keys with the old functionality marked as deprecated. Splits _createmultisig_redeemscript into two functions, one for getting public keys from UniValue and one for getting addresses from UniValue and then their respective public keys. The one for retrieving address's public keys is located in rpcwallet.cpp Changes addwitnessaddress's output to be a JSON object with two fields, address and redeemscript. Adds a test to deprecated_rpc.py for testing the deprecation. Update the tests to use addwitnessaddress or give only public keys to createmultisig. Anything that used addwitnessaddress was also updated to reflect the new API. --- src/Makefile.am | 2 + src/rpc/misc.cpp | 109 ++++++++------------------ src/rpc/util.cpp | 68 ++++++++++++++++ src/rpc/util.h | 19 +++++ src/wallet/rpcwallet.cpp | 48 +++++++++--- test/functional/address_types.py | 16 ++-- test/functional/deprecated_rpc.py | 6 +- test/functional/fundrawtransaction.py | 6 +- test/functional/importmulti.py | 8 +- test/functional/listtransactions.py | 3 +- test/functional/nulldummy.py | 2 +- test/functional/rawtransactions.py | 13 ++- test/functional/segwit.py | 26 +++--- test/functional/wallet-accounts.py | 2 +- test/functional/wallet-dump.py | 2 +- 15 files changed, 204 insertions(+), 126 deletions(-) create mode 100644 src/rpc/util.cpp create mode 100644 src/rpc/util.h diff --git a/src/Makefile.am b/src/Makefile.am index 4b65774fc6f58..4fbd605d9e5c9 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -133,6 +133,7 @@ BITCOIN_CORE_H = \ rpc/safemode.h \ rpc/server.h \ rpc/register.h \ + rpc/util.h \ scheduler.h \ script/sigcache.h \ script/sign.h \ @@ -352,6 +353,7 @@ libbitcoin_util_a_SOURCES = \ fs.cpp \ random.cpp \ rpc/protocol.cpp \ + rpc/util.cpp \ support/cleanse.cpp \ sync.cpp \ threadinterrupt.cpp \ diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 95a27d474bc59..62bcdea31c4ec 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -254,88 +255,21 @@ UniValue validateaddress(const JSONRPCRequest& request) // Needed even with !ENABLE_WALLET, to pass (ignored) pointers around class CWallet; -/** - * Used by addmultisigaddress / createmultisig: - */ -CScript _createmultisig_redeemScript(CWallet * const pwallet, const UniValue& params) -{ - int nRequired = params[0].get_int(); - const UniValue& keys = params[1].get_array(); - - // Gather public keys - if (nRequired < 1) - throw std::runtime_error("a multisignature address must require at least one key to redeem"); - if ((int)keys.size() < nRequired) - throw std::runtime_error( - strprintf("not enough keys supplied " - "(got %u keys, but need at least %d to redeem)", keys.size(), nRequired)); - if (keys.size() > 16) - throw std::runtime_error("Number of addresses involved in the multisignature address creation > 16\nReduce the number"); - std::vector pubkeys; - pubkeys.resize(keys.size()); - for (unsigned int i = 0; i < keys.size(); i++) - { - const std::string& ks = keys[i].get_str(); -#ifdef ENABLE_WALLET - // Case 1: Bitcoin address and we have full public key: - CTxDestination dest = DecodeDestination(ks); - if (pwallet && IsValidDestination(dest)) { - CKeyID key = GetKeyForDestination(*pwallet, dest); - if (key.IsNull()) { - throw std::runtime_error(strprintf("%s does not refer to a key", ks)); - } - CPubKey vchPubKey; - if (!pwallet->GetPubKey(key, vchPubKey)) { - throw std::runtime_error(strprintf("no full public key for address %s", ks)); - } - if (!vchPubKey.IsFullyValid()) - throw std::runtime_error(" Invalid public key: "+ks); - pubkeys[i] = vchPubKey; - } - - // Case 2: hex public key - else -#endif - if (IsHex(ks)) - { - CPubKey vchPubKey(ParseHex(ks)); - if (!vchPubKey.IsFullyValid()) - throw std::runtime_error(" Invalid public key: "+ks); - pubkeys[i] = vchPubKey; - } - else - { - throw std::runtime_error(" Invalid public key: "+ks); - } - } - CScript result = GetScriptForMultisig(nRequired, pubkeys); - - if (result.size() > MAX_SCRIPT_ELEMENT_SIZE) - throw std::runtime_error( - strprintf("redeemScript exceeds size limit: %d > %d", result.size(), MAX_SCRIPT_ELEMENT_SIZE)); - - return result; -} - UniValue createmultisig(const JSONRPCRequest& request) { -#ifdef ENABLE_WALLET - CWallet * const pwallet = GetWalletForJSONRPCRequest(request); -#else - CWallet * const pwallet = nullptr; -#endif - if (request.fHelp || request.params.size() < 2 || request.params.size() > 2) { std::string msg = "createmultisig nrequired [\"key\",...]\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" - + "DEPRECATION WARNING: Using addresses with createmultisig is deprecated. Clients must\n" + "transition to using addmultisigaddress to create multisig addresses with addresses known\n" + "to the wallet before upgrading to v0.17. To use the deprecated functionality, start bitcoind with -deprecatedrpc=createmultisig\n" "\nArguments:\n" - "1. nrequired (numeric, required) The number of required signatures out of the n keys or addresses.\n" - "2. \"keys\" (string, required) A json array of keys which are bitcoin addresses or hex-encoded public keys\n" + "1. nrequired (numeric, required) The number of required signatures out of the n keys or addresses.\n" + "2. \"keys\" (string, required) A json array of hex-encoded public keys\n" " [\n" - " \"key\" (string) bitcoin address or hex-encoded public key\n" + " \"key\" (string) The hex-encoded public key\n" " ,...\n" " ]\n" @@ -346,16 +280,37 @@ UniValue createmultisig(const JSONRPCRequest& request) "}\n" "\nExamples:\n" - "\nCreate a multisig address from 2 addresses\n" - + HelpExampleCli("createmultisig", "2 \"[\\\"16sSauSf5pF2UkUwvKGq4qjNRzBZYqgEL5\\\",\\\"171sgjn4YtPu27adkKGrdDwzRTxnRkBfKV\\\"]\"") + + "\nCreate a multisig address from 2 public keys\n" + + HelpExampleCli("createmultisig", "2 \"[\\\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\\\",\\\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\\\"]\"") + "\nAs a json rpc call\n" - + HelpExampleRpc("createmultisig", "2, \"[\\\"16sSauSf5pF2UkUwvKGq4qjNRzBZYqgEL5\\\",\\\"171sgjn4YtPu27adkKGrdDwzRTxnRkBfKV\\\"]\"") + + HelpExampleRpc("createmultisig", "2, \"[\\\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\\\",\\\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\\\"]\"") ; throw std::runtime_error(msg); } + int required = request.params[0].get_int(); + + // Get the public keys + const UniValue& keys = request.params[1].get_array(); + std::vector pubkeys; + for (unsigned int i = 0; i < keys.size(); ++i) { + if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) { + pubkeys.push_back(HexToPubKey(keys[i].get_str())); + } else { +#ifdef ENABLE_WALLET + CWallet* const pwallet = GetWalletForJSONRPCRequest(request); + if (IsDeprecatedRPCEnabled("createmultisig") && EnsureWalletIsAvailable(pwallet, false)) { + pubkeys.push_back(AddrToPubKey(pwallet, keys[i].get_str())); + } else +#endif + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\nNote that from v0.16, createmultisig no longer accepts addresses." + " Clients must transition to using addmultisigaddress to create multisig addresses with addresses known to the wallet before upgrading to v0.17." + " To use the deprecated functionality, start bitcoind with -deprecatedrpc=createmultisig", keys[i].get_str())); + } + } + // Construct using pay-to-script-hash: - CScript inner = _createmultisig_redeemScript(pwallet, request.params); + CScript inner = CreateMultisigRedeemscript(required, pubkeys); CScriptID innerID(inner); UniValue result(UniValue::VOBJ); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp new file mode 100644 index 0000000000000..09ded4e46e73b --- /dev/null +++ b/src/rpc/util.cpp @@ -0,0 +1,68 @@ +// Copyright (c) 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 +#include +#include +#include +#include +#include +#include + +// Converts a hex string to a public key if possible +CPubKey HexToPubKey(const std::string& hex_in) +{ + if (!IsHex(hex_in)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + } + CPubKey vchPubKey(ParseHex(hex_in)); + if (!vchPubKey.IsFullyValid()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + } + return vchPubKey; +} + +// Retrieves a public key for an address from the given CKeyStore +CPubKey AddrToPubKey(CKeyStore* const keystore, const std::string& addr_in) +{ + CTxDestination dest = DecodeDestination(addr_in); + if (!IsValidDestination(dest)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address: " + addr_in); + } + CKeyID key = GetKeyForDestination(*keystore, dest); + if (key.IsNull()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("%s does not refer to a key", addr_in)); + } + CPubKey vchPubKey; + if (!keystore->GetPubKey(key, vchPubKey)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("no full public key for address %s", addr_in)); + } + if (!vchPubKey.IsFullyValid()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet contains an invalid public key"); + } + return vchPubKey; +} + +// Creates a multisig redeemscript from a given list of public keys and number required. +CScript CreateMultisigRedeemscript(const int required, const std::vector& pubkeys) +{ + // Gather public keys + if (required < 1) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "a multisignature address must require at least one key to redeem"); + } + if ((int)pubkeys.size() < required) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("not enough keys supplied (got %u keys, but need at least %d to redeem)", pubkeys.size(), required)); + } + if (pubkeys.size() > 16) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Number of keys involved in the multisignature address creation > 16\nReduce the number"); + } + + CScript result = GetScriptForMultisig(required, pubkeys); + + if (result.size() > MAX_SCRIPT_ELEMENT_SIZE) { + throw JSONRPCError(RPC_INVALID_PARAMETER, (strprintf("redeemScript exceeds size limit: %d > %d", result.size(), MAX_SCRIPT_ELEMENT_SIZE))); + } + + return result; +} diff --git a/src/rpc/util.h b/src/rpc/util.h new file mode 100644 index 0000000000000..568a4260ba3dc --- /dev/null +++ b/src/rpc/util.h @@ -0,0 +1,19 @@ +// Copyright (c) 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_RPC_UTIL_H +#define BITCOIN_RPC_UTIL_H + +#include +#include + +class CKeyStore; +class CPubKey; +class CScript; + +CPubKey HexToPubKey(const std::string& hex_in); +CPubKey AddrToPubKey(CKeyStore* const keystore, const std::string& addr_in); +CScript CreateMultisigRedeemscript(const int required, const std::vector& pubkeys); + +#endif // BITCOIN_RPC_UTIL_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e307623fd5d8c..bd8c9ecb0cd8d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include