Skip to content

Commit d6c85c5

Browse files
apoelstraapoelstra
committed
Merge 4b15ffe into merged_master (Bitcoin PR #20832)
2 parents 2f2c49c + 4b15ffe commit d6c85c5

File tree

7 files changed

+156
-22
lines changed

7 files changed

+156
-22
lines changed

‎src/key_io.cpp‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414
#include <assert.h>
1515
#include <string.h>
1616

17+
/// Maximum witness length for Bech32 addresses.
18+
static constexpr std::size_t BECH32_WITNESS_PROG_MAX_LEN = 40;
19+
1720
namespace {
1821
class DestinationEncoder
1922
{
@@ -119,11 +122,12 @@ class DestinationEncoder
119122
std::string operator()(const NullData& null) const { return {}; }
120123
};
121124

122-
CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, const bool for_parent)
125+
CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, const bool for_parent, std::string& error_str)
123126
{
124127
std::vector<unsigned char> data;
125128
size_t pk_size = CPubKey::COMPRESSED_SIZE;
126129
uint160 hash;
130+
error_str = "";
127131
if (DecodeBase58Check(str, data, 55)) {
128132
// base58-encoded Bitcoin addresses.
129133
// Public-key-hash-addresses have version 0 (or 111 testnet).
@@ -163,11 +167,22 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
163167
std::copy(payload_start + pk_size, data.end(), hash.begin());
164168
return ScriptHash(CScriptID(hash), pubkey);
165169
}
170+
171+
// Set potential error message.
172+
// This message may be changed if the address can also be interpreted as a Bech32 address.
173+
error_str = "Invalid prefix for Base58-encoded address";
166174
}
167175
data.clear();
168176
auto bech = bech32::Decode(str);
169177
const std::string& hrp = for_parent ? params.ParentBech32HRP() : params.Bech32HRP();
170-
if (bech.second.size() > 0 && bech.first == hrp) {
178+
if (bech.second.size() > 0) {
179+
error_str = "";
180+
181+
if (bech.first != hrp) {
182+
error_str = "Invalid prefix for Bech32 address";
183+
return CNoDestination();
184+
}
185+
171186
// Bech32 decoding
172187
int version = bech.second[0]; // The first 5 bit symbol is the witness version (0-16)
173188
// The rest of the symbols are converted witness program bytes.
@@ -188,11 +203,21 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
188203
return scriptid;
189204
}
190205
}
206+
207+
error_str = "Invalid Bech32 v0 address data size";
191208
return CNoDestination();
192209
}
193-
if (version > 16 || data.size() < 2 || data.size() > 40) {
210+
211+
if (version > 16) {
212+
error_str = "Invalid Bech32 address witness version";
194213
return CNoDestination();
195214
}
215+
216+
if (data.size() < 2 || data.size() > BECH32_WITNESS_PROG_MAX_LEN) {
217+
error_str = "Invalid Bech32 address data size";
218+
return CNoDestination();
219+
}
220+
196221
WitnessUnknown unk;
197222
unk.version = version;
198223
std::copy(data.begin(), data.end(), unk.program);
@@ -250,6 +275,9 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
250275
}
251276
}
252277

278+
// Set error message if address can't be interpreted as Base58 or Bech32.
279+
if (error_str.empty()) error_str = "Invalid address format";
280+
253281
return CNoDestination();
254282
}
255283
} // namespace
@@ -337,19 +365,27 @@ std::string EncodeDestination(const CTxDestination& dest)
337365
return std::visit(DestinationEncoder(Params(), false), dest);
338366
}
339367

368+
CTxDestination DecodeDestination(const std::string& str, std::string& error_msg)
369+
{
370+
return DecodeDestination(str, Params(), false, error_msg);
371+
}
372+
340373
CTxDestination DecodeDestination(const std::string& str)
341374
{
342-
return DecodeDestination(str, Params(), false);
375+
std::string error_msg;
376+
return DecodeDestination(str, Params(), false, error_msg);
343377
}
344378

345379
bool IsValidDestinationString(const std::string& str, const CChainParams& params)
346380
{
347-
return IsValidDestination(DecodeDestination(str, params, false));
381+
std::string error_msg;
382+
return IsValidDestination(DecodeDestination(str, params, false, error_msg));
348383
}
349384

350385
bool IsValidDestinationString(const std::string& str)
351386
{
352-
return IsValidDestination(DecodeDestination(str, Params(), false));
387+
std::string error_msg;
388+
return IsValidDestination(DecodeDestination(str, Params(), false, error_msg));
353389
}
354390

355391
//
@@ -360,9 +396,9 @@ std::string EncodeParentDestination(const CTxDestination& dest)
360396
return std::visit(DestinationEncoder(Params(), true), dest);
361397
}
362398

363-
CTxDestination DecodeParentDestination(const std::string& str)
399+
CTxDestination DecodeParentDestination(const std::string& str, std::string& error_msg)
364400
{
365-
return DecodeDestination(str, Params(), true);
401+
return DecodeDestination(str, Params(), true, error_msg);
366402
}
367403

368404
// ELEMENTS

‎src/key_io.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@ std::string EncodeExtPubKey(const CExtPubKey& extpubkey);
2323

2424
std::string EncodeDestination(const CTxDestination& dest);
2525
CTxDestination DecodeDestination(const std::string& str);
26+
CTxDestination DecodeDestination(const std::string& str, std::string& error_msg);
2627
bool IsValidDestinationString(const std::string& str);
2728
bool IsValidDestinationString(const std::string& str, const CChainParams& params);
2829

2930
// ELEMENTS
3031
std::string EncodeParentDestination(const CTxDestination& dest);
31-
CTxDestination DecodeParentDestination(const std::string& str);
32+
CTxDestination DecodeParentDestination(const std::string& str, std::string& error_msg);
3233

3334
#endif // BITCOIN_KEY_IO_H

‎src/rpc/misc.cpp‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ static RPCHelpMan validateaddress()
4343
RPCResult{
4444
RPCResult::Type::OBJ, "", "",
4545
{
46-
{RPCResult::Type::BOOL, "isvalid", "If the address is valid or not. If not, this is the only property returned."},
47-
{RPCResult::Type::BOOL, "isvalid_parent", "If the address is valid or not for parent chain. Valid or not, no additional details will be appended unlike `isvalid`"},
48-
{RPCResult::Type::STR, "address", "The address validated"},
46+
{RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"},
47+
{RPCResult::Type::BOOL, "isvalid_parent", "If the address is valid or not for parent chain"},
48+
{RPCResult::Type::STR, "address", "The bitcoin address validated"},
4949
{RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded scriptPubKey generated by the address"},
5050
{RPCResult::Type::BOOL, "isscript", "If the key is a script"},
5151
{RPCResult::Type::BOOL, "iswitness", "If the address is a witness address"},
@@ -58,6 +58,8 @@ static RPCHelpMan validateaddress()
5858
{RPCResult::Type::STR, "address", ""},
5959
{RPCResult::Type::STR_HEX, "scriptPubKey", ""},
6060
}},
61+
{RPCResult::Type::STR, "error", /* optional */ true, "Error message, if any"},
62+
{RPCResult::Type::STR, "error_parent", /* optional */ true, "Error message, if any"},
6163
}
6264
},
6365
RPCExamples{
@@ -66,15 +68,19 @@ static RPCHelpMan validateaddress()
6668
},
6769
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
6870
{
69-
CTxDestination dest = DecodeDestination(request.params[0].get_str());
70-
CTxDestination parent_dest = DecodeParentDestination(request.params[0].get_str());
71+
std::string error_msg;
72+
CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg);
73+
std::string error_msg_parent;
74+
CTxDestination parent_dest = DecodeParentDestination(request.params[0].get_str(), error_msg_parent);
7175
bool isValid = IsValidDestination(dest);
7276
bool is_valid_parent = IsValidDestination(parent_dest);
77+
CHECK_NONFATAL(isValid == error_msg.empty());
78+
CHECK_NONFATAL(is_valid_parent == error_msg_parent.empty());
79+
7380
UniValue ret(UniValue::VOBJ);
7481
ret.pushKV("isvalid", isValid);
7582
ret.pushKV("isvalid_parent", is_valid_parent);
76-
if (isValid)
77-
{
83+
if (isValid) {
7884
std::string currentAddress = EncodeDestination(dest);
7985
ret.pushKV("address", currentAddress);
8086

@@ -100,6 +106,11 @@ static RPCHelpMan validateaddress()
100106
parent_info.pushKVs(blind_detail);
101107
ret.pushKV("parent_address_info", parent_info);
102108
}
109+
if (!isValid && !is_valid_parent) {
110+
ret.pushKV("error", error_msg);
111+
ret.pushKV("error_parent", error_msg_parent);
112+
}
113+
103114
return ret;
104115
},
105116
};

‎src/wallet/rpcwallet.cpp‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4211,13 +4211,19 @@ RPCHelpMan getaddressinfo()
42114211

42124212
LOCK(pwallet->cs_wallet);
42134213

4214-
UniValue ret(UniValue::VOBJ);
4215-
CTxDestination dest = DecodeDestination(request.params[0].get_str());
4214+
std::string error_msg;
4215+
CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg);
4216+
42164217
// Make sure the destination is valid
42174218
if (!IsValidDestination(dest)) {
4218-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
4219+
// Set generic error message in case 'DecodeDestination' didn't set it
4220+
if (error_msg.empty()) error_msg = "Invalid address";
4221+
4222+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error_msg);
42194223
}
42204224

4225+
UniValue ret(UniValue::VOBJ);
4226+
42214227
std::string currentAddress = EncodeDestination(dest);
42224228
ret.pushKV("address", currentAddress);
42234229

@@ -5547,9 +5553,10 @@ static RPCHelpMan sendtomainchain_base()
55475553

55485554
EnsureWalletIsUnlocked(pwallet);
55495555

5550-
CTxDestination parent_address = DecodeParentDestination(request.params[0].get_str());
5556+
std::string error_str;
5557+
CTxDestination parent_address = DecodeParentDestination(request.params[0].get_str(), error_str);
55515558
if (!IsValidDestination(parent_address))
5552-
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
5559+
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid Bitcoin address: %s", error_str));
55535560

55545561
CAmount nAmount = AmountFromValue(request.params[1]);
55555562
if (nAmount <= 0)

‎test/functional/rpc_invalid_address_message.py‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2020 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""Test error messages for 'getaddressinfo' and 'validateaddress' RPC commands."""
6+
7+
from test_framework.test_framework import BitcoinTestFramework
8+
9+
from test_framework.util import (
10+
assert_equal,
11+
assert_raises_rpc_error,
12+
)
13+
14+
BECH32_VALID = 'ert1qtmp74ayg7p24uslctssvjm06q5phz4yr7gdkdv'
15+
BECH32_INVALID_SIZE = 'ert1sqqpx5djq'
16+
BECH32_INVALID_PREFIX = 'bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4'
17+
18+
BASE58_VALID = '2dcjQH4DQC3pMcSQkMkSQyPPEr7rZ6Ga4GR'
19+
BASE58_INVALID_PREFIX = '17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem'
20+
21+
INVALID_ADDRESS = 'asfah14i8fajz0123f'
22+
23+
class InvalidAddressErrorMessageTest(BitcoinTestFramework):
24+
def set_test_params(self):
25+
self.setup_clean_chain = True
26+
self.num_nodes = 1
27+
28+
def skip_test_if_missing_module(self):
29+
self.skip_if_no_wallet()
30+
31+
def test_validateaddress(self):
32+
node = self.nodes[0]
33+
34+
# Bech32
35+
info = node.validateaddress(BECH32_INVALID_SIZE)
36+
assert not info['isvalid']
37+
assert_equal(info['error'], 'Invalid Bech32 address data size')
38+
39+
info = node.validateaddress(BECH32_INVALID_PREFIX)
40+
assert not info['isvalid']
41+
assert_equal(info['error'], 'Invalid prefix for Bech32 address')
42+
43+
info = node.validateaddress(BECH32_VALID)
44+
assert info['isvalid']
45+
assert 'error' not in info
46+
47+
# Base58
48+
info = node.validateaddress(BASE58_INVALID_PREFIX)
49+
assert not info['isvalid']
50+
assert_equal(info['error'], 'Invalid prefix for Base58-encoded address')
51+
52+
info = node.validateaddress(BASE58_VALID)
53+
assert info['isvalid']
54+
assert 'error' not in info
55+
56+
# Invalid address format
57+
info = node.validateaddress(INVALID_ADDRESS)
58+
assert not info['isvalid']
59+
assert_equal(info['error'], 'Invalid address format')
60+
61+
def test_getaddressinfo(self):
62+
node = self.nodes[0]
63+
64+
assert_raises_rpc_error(-5, "Invalid Bech32 address data size", node.getaddressinfo, BECH32_INVALID_SIZE)
65+
66+
assert_raises_rpc_error(-5, "Invalid prefix for Bech32 address", node.getaddressinfo, BECH32_INVALID_PREFIX)
67+
68+
assert_raises_rpc_error(-5, "Invalid prefix for Base58-encoded address", node.getaddressinfo, BASE58_INVALID_PREFIX)
69+
70+
assert_raises_rpc_error(-5, "Invalid address format", node.getaddressinfo, INVALID_ADDRESS)
71+
72+
def run_test(self):
73+
self.test_validateaddress()
74+
self.test_getaddressinfo()
75+
76+
77+
if __name__ == '__main__':
78+
InvalidAddressErrorMessageTest().main()

‎test/functional/test_runner.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@
157157
'wallet_keypool_topup.py',
158158
'wallet_keypool_topup.py --descriptors',
159159
'interface_zmq.py',
160+
'rpc_invalid_address_message.py',
160161
'interface_bitcoin_cli.py',
161162
'mempool_resurrect.py',
162163
'wallet_txn_doublespend.py --mineblock',

‎test/functional/wallet_basic.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ def run_test(self):
592592
assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999)))
593593

594594
# Test getaddressinfo on external address. Note that these addresses are taken from disablewallet.py
595-
assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy")
595+
assert_raises_rpc_error(-5, "Invalid prefix for Base58-encoded address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy")
596596
address_info = self.nodes[0].getaddressinfo("CTEuA2rzWUDTGt6jFwmAtGtWWqtRgP7NwY4qbGGyht8QpgLvpQmbRHtzeF1yTV1rmJ9GiHhJoqnrbUYT")
597597
assert_equal(address_info['address'], "CTEuA2rzWUDTGt6jFwmAtGtWWqtRgP7NwY4qbGGyht8QpgLvpQmbRHtzeF1yTV1rmJ9GiHhJoqnrbUYT")
598598
assert_equal(address_info["scriptPubKey"], "76a914dc389a2145ec3cd4fe37bd6a11653456168cfa2c88ac")

0 commit comments

Comments
 (0)