diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index f9343f48a897e..474d9076be031 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -124,11 +124,7 @@ static RPCHelpMan createmultisig() 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 { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\n.", keys[i].get_str())); - } + pubkeys.push_back(HexToPubKey(keys[i].get_str())); } // Get the output type diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 22ec4e696748b..9a7c731afe903 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -194,11 +194,14 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList& CPubKey HexToPubKey(const std::string& hex_in) { if (!IsHex(hex_in)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be a hex string"); + } + if (hex_in.length() != 66 && hex_in.length() != 130) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must have a length of either 33 or 65 bytes"); } CPubKey vchPubKey(ParseHex(hex_in)); if (!vchPubKey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be cryptographically valid."); } return vchPubKey; } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index c05600484eaee..8d3eea59ee0c6 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -456,12 +456,7 @@ RPCHelpMan importpubkey() throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } - if (!IsHex(request.params[0].get_str())) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string"); - std::vector data(ParseHex(request.params[0].get_str())); - CPubKey pubKey(data); - if (!pubKey.IsFullyValid()) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); + CPubKey pubKey = HexToPubKey(request.params[0].get_str()); { LOCK(pwallet->cs_wallet); @@ -983,15 +978,7 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map(parsed_witnessscript.begin(), parsed_witnessscript.end()); } for (size_t i = 0; i < pubKeys.size(); ++i) { - const auto& str = pubKeys[i].get_str(); - if (!IsHex(str)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" must be a hex string"); - } - auto parsed_pubkey = ParseHex(str); - CPubKey pubkey(parsed_pubkey); - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" is not a valid public key"); - } + CPubKey pubkey = HexToPubKey(pubKeys[i].get_str()); pubkey_map.emplace(pubkey.GetID(), pubkey); ordered_pubkeys.push_back(pubkey.GetID()); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 6060f017ce994..1a364a75edcd2 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -627,15 +627,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact const UniValue solving_data = options["solving_data"].get_obj(); if (solving_data.exists("pubkeys")) { for (const UniValue& pk_univ : solving_data["pubkeys"].get_array().getValues()) { - const std::string& pk_str = pk_univ.get_str(); - if (!IsHex(pk_str)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not hex", pk_str)); - } - const std::vector data(ParseHex(pk_str)); - const CPubKey pubkey(data.begin(), data.end()); - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not a valid public key", pk_str)); - } + const CPubKey pubkey = HexToPubKey(pk_univ.get_str()); coinControl.m_external_provider.pubkeys.emplace(pubkey.GetID(), pubkey); // Add witness script for pubkeys const CScript wit_script = GetScriptForDestination(WitnessV0KeyHash(pubkey)); diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 12697e9d0c3c1..3978c80ddee14 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -490,11 +490,11 @@ def raw_multisig_transaction_legacy_tests(self): addr2Obj = self.nodes[2].getaddressinfo(addr2) # Tests for createmultisig and addmultisigaddress - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) + assert_raises_rpc_error(-5, 'Pubkey "01020304" must have a length of either 33 or 65 bytes', self.nodes[0].createmultisig, 1, ["01020304"]) # createmultisig can only take public keys self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) + assert_raises_rpc_error(-5, f'Pubkey "{addr1}" must be a hex string', self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 56228d2bada65..1b2b8ec1f3c8b 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -461,10 +461,13 @@ def run_test(self): assert_raises_rpc_error(-5, "Invalid Bitcoin address or script", self.nodes[0].importaddress, "invalid") # This will raise an exception for attempting to import a pubkey that isn't in hex - assert_raises_rpc_error(-5, "Pubkey must be a hex string", self.nodes[0].importpubkey, "not hex") + assert_raises_rpc_error(-5, 'Pubkey "not hex" must be a hex string', self.nodes[0].importpubkey, "not hex") - # This will raise an exception for importing an invalid pubkey - assert_raises_rpc_error(-5, "Pubkey is not a valid public key", self.nodes[0].importpubkey, "5361746f736869204e616b616d6f746f") + # This will raise exceptions for importing a pubkeys with invalid length / invalid coordinates + too_short_pubkey = "5361746f736869204e616b616d6f746f" + assert_raises_rpc_error(-5, f'Pubkey "{too_short_pubkey}" must have a length of either 33 or 65 bytes', self.nodes[0].importpubkey, too_short_pubkey) + not_on_curve_pubkey = bytes([4] + [0]*64).hex() # pubkey with coordinates (0,0) is not on curve + assert_raises_rpc_error(-5, f'Pubkey "{not_on_curve_pubkey}" must be cryptographically valid', self.nodes[0].importpubkey, not_on_curve_pubkey) # Bech32m addresses cannot be imported into a legacy wallet assert_raises_rpc_error(-5, "Bech32m addresses cannot be imported into legacy wallets", self.nodes[0].importaddress, "bcrt1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqc8gma6") diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index ff4648e638e47..71c883f166bf3 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -1054,8 +1054,8 @@ def test_external_inputs(self): assert_raises_rpc_error(-4, "Not solvable pre-selected input COutPoint(%s, %s)" % (ext_utxo["txid"][0:10], ext_utxo["vout"]), wallet.fundrawtransaction, raw_tx) # Error conditions - assert_raises_rpc_error(-5, "'not a pubkey' is not hex", wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["not a pubkey"]}) - assert_raises_rpc_error(-5, "'01234567890a0b0c0d0e0f' is not a valid public key", wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["01234567890a0b0c0d0e0f"]}) + assert_raises_rpc_error(-5, 'Pubkey "not a pubkey" must be a hex string', wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["not a pubkey"]}) + assert_raises_rpc_error(-5, 'Pubkey "01234567890a0b0c0d0e0f" must have a length of either 33 or 65 bytes', wallet.fundrawtransaction, raw_tx, solving_data={"pubkeys":["01234567890a0b0c0d0e0f"]}) assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, solving_data={"scripts":["not a script"]}) assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, solving_data={"descriptors":["not a descriptor"]}) assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", wallet.fundrawtransaction, raw_tx, input_weights=[{"txid": ext_utxo["txid"]}])