diff options
author | MeshCollider <dobsonsa68@gmail.com> | 2019-06-21 19:43:13 +1200 |
---|---|---|
committer | MeshCollider <dobsonsa68@gmail.com> | 2019-06-21 19:44:08 +1200 |
commit | 303ec103bacf2c9498d567210dd77c1305bddf81 (patch) | |
tree | a4aaae2a989201ec7a9a22ad351ce69d9768057b | |
parent | 23815ee74dfd08c746a70eeee4eeb1ad3af052f1 (diff) | |
parent | a49503402b6bc21e3878e151c07529941d36aed0 (diff) | |
download | bitcoin-303ec103bacf2c9498d567210dd77c1305bddf81.tar.xz |
Merge #16026: Ensure that uncompressed public keys in a multisig always returns a legacy address
a49503402b6bc21e3878e151c07529941d36aed0 Make and get the multisig redeemscript and destination in one function instead of two (Andrew Chow)
Pull request description:
`CreateMultisigRedeemscript()` is changed to `AddAndGetMultisigDestination()` so that the process of constructing the redeemScript and then getting the `CTxDestination` are done in the same function. This allows that function to see what the keys in the multisig are so that the correct address type is returned from `AddAndGetDestinationForScript()`.
This only effects the `createmultisig` and `addmultisigaddress` RPCs and does not change signing logic as #16022 does.
Alternative to #16022 and #16012
Fixes #16011
ACKs for commit a49503:
Tree-SHA512: 5b0154a714deea3b2cc3a54beb420c95eeeacf4ca30c40ca80940d9d640f8b03611b0fc14c2f0710bfd8a79e8d27ad7d9ae380b4b83d52b40ab201624f2a63f0
-rw-r--r-- | src/rpc/misc.cpp | 4 | ||||
-rw-r--r-- | src/rpc/util.cpp | 24 | ||||
-rw-r--r-- | src/rpc/util.h | 3 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 4 | ||||
-rwxr-xr-x | test/functional/rpc_createmultisig.py | 30 |
5 files changed, 53 insertions, 12 deletions
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 2b29fd543b..7a1bdec7b9 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -122,9 +122,9 @@ static UniValue createmultisig(const JSONRPCRequest& request) } // Construct using pay-to-script-hash: - const CScript inner = CreateMultisigRedeemscript(required, pubkeys); CBasicKeyStore keystore; - const CTxDestination dest = AddAndGetDestinationForScript(keystore, inner, output_type); + CScript inner; + const CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, keystore, inner); UniValue result(UniValue::VOBJ); result.pushKV("address", EncodeDestination(dest)); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 9cdb22001f..4642cf16b1 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -4,6 +4,7 @@ #include <key_io.h> #include <keystore.h> +#include <outputtype.h> #include <rpc/util.h> #include <tinyformat.h> #include <util/strencodings.h> @@ -150,8 +151,8 @@ CPubKey AddrToPubKey(CKeyStore* const keystore, const std::string& addr_in) return vchPubKey; } -// Creates a multisig redeemscript from a given list of public keys and number required. -CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey>& pubkeys) +// Creates a multisig address from a given list of public keys, number of signatures required, and the address type +CTxDestination AddAndGetMultisigDestination(const int required, const std::vector<CPubKey>& pubkeys, OutputType type, CKeyStore& keystore, CScript& script_out) { // Gather public keys if (required < 1) { @@ -164,13 +165,24 @@ CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey throw JSONRPCError(RPC_INVALID_PARAMETER, "Number of keys involved in the multisignature address creation > 16\nReduce the number"); } - CScript result = GetScriptForMultisig(required, pubkeys); + script_out = 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))); + if (script_out.size() > MAX_SCRIPT_ELEMENT_SIZE) { + throw JSONRPCError(RPC_INVALID_PARAMETER, (strprintf("redeemScript exceeds size limit: %d > %d", script_out.size(), MAX_SCRIPT_ELEMENT_SIZE))); } - return result; + // Check if any keys are uncompressed. If so, the type is legacy + for (const CPubKey& pk : pubkeys) { + if (!pk.IsCompressed()) { + type = OutputType::LEGACY; + break; + } + } + + // Make the address + CTxDestination dest = AddAndGetDestinationForScript(keystore, script_out, type); + + return dest; } class DescribeAddressVisitor : public boost::static_visitor<UniValue> diff --git a/src/rpc/util.h b/src/rpc/util.h index e4fa8fc3d7..0eb2fef5c3 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -6,6 +6,7 @@ #define BITCOIN_RPC_UTIL_H #include <node/transaction.h> +#include <outputtype.h> #include <pubkey.h> #include <rpc/protocol.h> #include <script/standard.h> @@ -70,7 +71,7 @@ extern std::string HelpExampleRpc(const std::string& methodname, const std::stri 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<CPubKey>& pubkeys); +CTxDestination AddAndGetMultisigDestination(const int required, const std::vector<CPubKey>& pubkeys, OutputType type, CKeyStore& keystore, CScript& script_out); UniValue DescribeAddress(const CTxDestination& dest); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index cfa36ad40c..3232c65bca 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1036,8 +1036,8 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request) } // Construct using pay-to-script-hash: - CScript inner = CreateMultisigRedeemscript(required, pubkeys); - CTxDestination dest = AddAndGetDestinationForScript(*pwallet, inner, output_type); + CScript inner; + CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, *pwallet, inner); pwallet->SetAddressBook(dest, label, "send"); UniValue result(UniValue::VOBJ); diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 7abcd71bb8..58010f7c2e 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -7,9 +7,13 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_raises_rpc_error, + assert_equal, ) -import decimal +from test_framework.key import ECPubKey +import binascii +import decimal +import itertools class RpcCreateMultiSigTest(BitcoinTestFramework): def set_test_params(self): @@ -44,6 +48,30 @@ class RpcCreateMultiSigTest(BitcoinTestFramework): self.checkbalances() + # Test mixed compressed and uncompressed pubkeys + self.log.info('Mixed compressed and uncompressed multisigs are not allowed') + pk0 = node0.getaddressinfo(node0.getnewaddress())['pubkey'] + pk1 = node1.getaddressinfo(node1.getnewaddress())['pubkey'] + pk2 = node2.getaddressinfo(node2.getnewaddress())['pubkey'] + + # decompress pk2 + pk_obj = ECPubKey() + pk_obj.set(binascii.unhexlify(pk2)) + pk_obj.compressed = False + pk2 = binascii.hexlify(pk_obj.get_bytes()).decode() + + # Check all permutations of keys because order matters apparently + for keys in itertools.permutations([pk0, pk1, pk2]): + # Results should be the same as this legacy one + legacy_addr = node0.createmultisig(2, keys, 'legacy')['address'] + assert_equal(legacy_addr, node0.addmultisigaddress(2, keys, '', 'legacy')['address']) + + # Generate addresses with the segwit types. These should all make legacy addresses + assert_equal(legacy_addr, node0.createmultisig(2, keys, 'bech32')['address']) + assert_equal(legacy_addr, node0.createmultisig(2, keys, 'p2sh-segwit')['address']) + assert_equal(legacy_addr, node0.addmultisigaddress(2, keys, '', 'bech32')['address']) + assert_equal(legacy_addr, node0.addmultisigaddress(2, keys, '', 'p2sh-segwit')['address']) + def check_addmultisigaddress_errors(self): self.log.info('Check that addmultisigaddress fails when the private keys are missing') addresses = [self.nodes[1].getnewaddress(address_type='legacy') for _ in range(2)] |