aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/key_io.cpp46
-rw-r--r--src/key_io.h1
-rw-r--r--src/qt/bitcoingui.cpp2
-rw-r--r--src/rpc/misc.cpp15
-rw-r--r--src/sync.cpp3
-rw-r--r--src/test/sync_tests.cpp6
-rw-r--r--src/wallet/ismine.h22
-rw-r--r--src/wallet/rpcwallet.cpp12
-rwxr-xr-xtest/functional/rpc_invalid_address_message.py78
-rwxr-xr-xtest/functional/test_runner.py1
-rwxr-xr-xtest/functional/wallet_basic.py2
-rwxr-xr-xtest/fuzz/test_runner.py27
12 files changed, 180 insertions, 35 deletions
diff --git a/src/key_io.cpp b/src/key_io.cpp
index a270ede864..e27673fd16 100644
--- a/src/key_io.cpp
+++ b/src/key_io.cpp
@@ -12,6 +12,9 @@
#include <assert.h>
#include <string.h>
+/// Maximum witness length for Bech32 addresses.
+static constexpr std::size_t BECH32_WITNESS_PROG_MAX_LEN = 40;
+
namespace {
class DestinationEncoder
{
@@ -65,10 +68,11 @@ public:
std::string operator()(const CNoDestination& no) const { return {}; }
};
-CTxDestination DecodeDestination(const std::string& str, const CChainParams& params)
+CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str)
{
std::vector<unsigned char> data;
uint160 hash;
+ error_str = "";
if (DecodeBase58Check(str, data, 21)) {
// base58-encoded Bitcoin addresses.
// Public-key-hash-addresses have version 0 (or 111 testnet).
@@ -85,10 +89,21 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
std::copy(data.begin() + script_prefix.size(), data.end(), hash.begin());
return ScriptHash(hash);
}
+
+ // Set potential error message.
+ // This message may be changed if the address can also be interpreted as a Bech32 address.
+ error_str = "Invalid prefix for Base58-encoded address";
}
data.clear();
auto bech = bech32::Decode(str);
- if (bech.second.size() > 0 && bech.first == params.Bech32HRP()) {
+ if (bech.second.size() > 0) {
+ error_str = "";
+
+ if (bech.first != params.Bech32HRP()) {
+ error_str = "Invalid prefix for Bech32 address";
+ return CNoDestination();
+ }
+
// Bech32 decoding
int version = bech.second[0]; // The first 5 bit symbol is the witness version (0-16)
// The rest of the symbols are converted witness program bytes.
@@ -109,11 +124,21 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
return scriptid;
}
}
+
+ error_str = "Invalid Bech32 v0 address data size";
+ return CNoDestination();
+ }
+
+ if (version > 16) {
+ error_str = "Invalid Bech32 address witness version";
return CNoDestination();
}
- if (version > 16 || data.size() < 2 || data.size() > 40) {
+
+ if (data.size() < 2 || data.size() > BECH32_WITNESS_PROG_MAX_LEN) {
+ error_str = "Invalid Bech32 address data size";
return CNoDestination();
}
+
WitnessUnknown unk;
unk.version = version;
std::copy(data.begin(), data.end(), unk.program);
@@ -121,6 +146,10 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
return unk;
}
}
+
+ // Set error message if address can't be interpreted as Base58 or Bech32.
+ if (error_str.empty()) error_str = "Invalid address format";
+
return CNoDestination();
}
} // namespace
@@ -208,14 +237,21 @@ std::string EncodeDestination(const CTxDestination& dest)
return std::visit(DestinationEncoder(Params()), dest);
}
+CTxDestination DecodeDestination(const std::string& str, std::string& error_msg)
+{
+ return DecodeDestination(str, Params(), error_msg);
+}
+
CTxDestination DecodeDestination(const std::string& str)
{
- return DecodeDestination(str, Params());
+ std::string error_msg;
+ return DecodeDestination(str, error_msg);
}
bool IsValidDestinationString(const std::string& str, const CChainParams& params)
{
- return IsValidDestination(DecodeDestination(str, params));
+ std::string error_msg;
+ return IsValidDestination(DecodeDestination(str, params, error_msg));
}
bool IsValidDestinationString(const std::string& str)
diff --git a/src/key_io.h b/src/key_io.h
index d80c08f49c..bd81f7847e 100644
--- a/src/key_io.h
+++ b/src/key_io.h
@@ -23,6 +23,7 @@ std::string EncodeExtPubKey(const CExtPubKey& extpubkey);
std::string EncodeDestination(const CTxDestination& dest);
CTxDestination DecodeDestination(const std::string& str);
+CTxDestination DecodeDestination(const std::string& str, std::string& error_msg);
bool IsValidDestinationString(const std::string& str);
bool IsValidDestinationString(const std::string& str, const CChainParams& params);
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index f2a49e5a76..a367f91cc3 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -845,7 +845,7 @@ void BitcoinGUI::showDebugWindowActivateConsole()
void BitcoinGUI::showHelpMessageClicked()
{
- helpMessageDialog->show();
+ GUIUtil::bringToFront(helpMessageDialog);
}
#ifdef ENABLE_WALLET
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index b3102a236d..215e48ca26 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -39,13 +39,14 @@ static RPCHelpMan validateaddress()
RPCResult{
RPCResult::Type::OBJ, "", "",
{
- {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not. If not, this is the only property returned."},
+ {RPCResult::Type::BOOL, "isvalid", "If the address is valid or not"},
{RPCResult::Type::STR, "address", "The bitcoin address validated"},
{RPCResult::Type::STR_HEX, "scriptPubKey", "The hex-encoded scriptPubKey generated by the address"},
{RPCResult::Type::BOOL, "isscript", "If the key is a script"},
{RPCResult::Type::BOOL, "iswitness", "If the address is a witness address"},
{RPCResult::Type::NUM, "witness_version", /* optional */ true, "The version number of the witness program"},
{RPCResult::Type::STR_HEX, "witness_program", /* optional */ true, "The hex value of the witness program"},
+ {RPCResult::Type::STR, "error", /* optional */ true, "Error message, if any"},
}
},
RPCExamples{
@@ -54,13 +55,14 @@ static RPCHelpMan validateaddress()
},
[&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
{
- CTxDestination dest = DecodeDestination(request.params[0].get_str());
- bool isValid = IsValidDestination(dest);
+ std::string error_msg;
+ CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg);
+ const bool isValid = IsValidDestination(dest);
+ CHECK_NONFATAL(isValid == error_msg.empty());
UniValue ret(UniValue::VOBJ);
ret.pushKV("isvalid", isValid);
- if (isValid)
- {
+ if (isValid) {
std::string currentAddress = EncodeDestination(dest);
ret.pushKV("address", currentAddress);
@@ -69,7 +71,10 @@ static RPCHelpMan validateaddress()
UniValue detail = DescribeAddress(dest);
ret.pushKVs(detail);
+ } else {
+ ret.pushKV("error", error_msg);
}
+
return ret;
},
};
diff --git a/src/sync.cpp b/src/sync.cpp
index acfbe8fe29..a2b62c2286 100644
--- a/src/sync.cpp
+++ b/src/sync.cpp
@@ -13,8 +13,6 @@
#include <util/strencodings.h>
#include <util/threadnames.h>
-#include <boost/thread/mutex.hpp>
-
#include <map>
#include <mutex>
#include <set>
@@ -224,7 +222,6 @@ template void EnterCritical(const char*, const char*, int, Mutex*, bool);
template void EnterCritical(const char*, const char*, int, RecursiveMutex*, bool);
template void EnterCritical(const char*, const char*, int, std::mutex*, bool);
template void EnterCritical(const char*, const char*, int, std::recursive_mutex*, bool);
-template void EnterCritical(const char*, const char*, int, boost::mutex*, bool);
void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
{
diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp
index 71275f69d9..3e4d1dac9e 100644
--- a/src/test/sync_tests.cpp
+++ b/src/test/sync_tests.cpp
@@ -6,7 +6,6 @@
#include <test/util/setup_common.h>
#include <boost/test/unit_test.hpp>
-#include <boost/thread/mutex.hpp>
#include <mutex>
@@ -110,11 +109,6 @@ BOOST_AUTO_TEST_CASE(double_lock_mutex)
TestDoubleLock<Mutex>(true /* should throw */);
}
-BOOST_AUTO_TEST_CASE(double_lock_boost_mutex)
-{
- TestDoubleLock<boost::mutex>(true /* should throw */);
-}
-
BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex)
{
TestDoubleLock<RecursiveMutex>(false /* should not throw */);
diff --git a/src/wallet/ismine.h b/src/wallet/ismine.h
index 5cdd7dff80..38ed7e7770 100644
--- a/src/wallet/ismine.h
+++ b/src/wallet/ismine.h
@@ -14,7 +14,27 @@
class CWallet;
class CScript;
-/** IsMine() return codes */
+/**
+ * IsMine() return codes, which depend on ScriptPubKeyMan implementation.
+ * Not every ScriptPubKeyMan covers all types, please refer to
+ * https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.21.0.md#ismine-semantics
+ * for better understanding.
+ *
+ * For LegacyScriptPubKeyMan,
+ * ISMINE_NO: the scriptPubKey is not in the wallet;
+ * ISMINE_WATCH_ONLY: the scriptPubKey has been imported into the wallet;
+ * ISMINE_SPENDABLE: the scriptPubKey corresponds to an address owned by the wallet user (can spend with the private key);
+ * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user;
+ * ISMINE_ALL: all ISMINE flags except for USED;
+ * ISMINE_ALL_USED: all ISMINE flags including USED;
+ * ISMINE_ENUM_ELEMENTS: the number of isminetype enum elements.
+ *
+ * For DescriptorScriptPubKeyMan and future ScriptPubKeyMan,
+ * ISMINE_NO: the scriptPubKey is not in the wallet;
+ * ISMINE_SPENDABLE: the scriptPubKey matches a scriptPubKey in the wallet.
+ * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user.
+ *
+ */
enum isminetype : unsigned int
{
ISMINE_NO = 0,
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 9db327c913..b865130642 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -3820,13 +3820,19 @@ RPCHelpMan getaddressinfo()
LOCK(pwallet->cs_wallet);
- UniValue ret(UniValue::VOBJ);
- CTxDestination dest = DecodeDestination(request.params[0].get_str());
+ std::string error_msg;
+ CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg);
+
// Make sure the destination is valid
if (!IsValidDestination(dest)) {
- throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address");
+ // Set generic error message in case 'DecodeDestination' didn't set it
+ if (error_msg.empty()) error_msg = "Invalid address";
+
+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error_msg);
}
+ UniValue ret(UniValue::VOBJ);
+
std::string currentAddress = EncodeDestination(dest);
ret.pushKV("address", currentAddress);
diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py
new file mode 100755
index 0000000000..469d6bdb05
--- /dev/null
+++ b/test/functional/rpc_invalid_address_message.py
@@ -0,0 +1,78 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 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 error messages for 'getaddressinfo' and 'validateaddress' RPC commands."""
+
+from test_framework.test_framework import BitcoinTestFramework
+
+from test_framework.util import (
+ assert_equal,
+ assert_raises_rpc_error,
+)
+
+BECH32_VALID = 'bcrt1qtmp74ayg7p24uslctssvjm06q5phz4yrxucgnv'
+BECH32_INVALID_SIZE = 'bcrt1sqqpl9r5c'
+BECH32_INVALID_PREFIX = 'bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4'
+
+BASE58_VALID = 'mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn'
+BASE58_INVALID_PREFIX = '17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem'
+
+INVALID_ADDRESS = 'asfah14i8fajz0123f'
+
+class InvalidAddressErrorMessageTest(BitcoinTestFramework):
+ def set_test_params(self):
+ self.setup_clean_chain = True
+ self.num_nodes = 1
+
+ def skip_test_if_missing_module(self):
+ self.skip_if_no_wallet()
+
+ def test_validateaddress(self):
+ node = self.nodes[0]
+
+ # Bech32
+ info = node.validateaddress(BECH32_INVALID_SIZE)
+ assert not info['isvalid']
+ assert_equal(info['error'], 'Invalid Bech32 address data size')
+
+ info = node.validateaddress(BECH32_INVALID_PREFIX)
+ assert not info['isvalid']
+ assert_equal(info['error'], 'Invalid prefix for Bech32 address')
+
+ info = node.validateaddress(BECH32_VALID)
+ assert info['isvalid']
+ assert 'error' not in info
+
+ # Base58
+ info = node.validateaddress(BASE58_INVALID_PREFIX)
+ assert not info['isvalid']
+ assert_equal(info['error'], 'Invalid prefix for Base58-encoded address')
+
+ info = node.validateaddress(BASE58_VALID)
+ assert info['isvalid']
+ assert 'error' not in info
+
+ # Invalid address format
+ info = node.validateaddress(INVALID_ADDRESS)
+ assert not info['isvalid']
+ assert_equal(info['error'], 'Invalid address format')
+
+ def test_getaddressinfo(self):
+ node = self.nodes[0]
+
+ assert_raises_rpc_error(-5, "Invalid Bech32 address data size", node.getaddressinfo, BECH32_INVALID_SIZE)
+
+ assert_raises_rpc_error(-5, "Invalid prefix for Bech32 address", node.getaddressinfo, BECH32_INVALID_PREFIX)
+
+ assert_raises_rpc_error(-5, "Invalid prefix for Base58-encoded address", node.getaddressinfo, BASE58_INVALID_PREFIX)
+
+ assert_raises_rpc_error(-5, "Invalid address format", node.getaddressinfo, INVALID_ADDRESS)
+
+ def run_test(self):
+ self.test_validateaddress()
+ self.test_getaddressinfo()
+
+
+if __name__ == '__main__':
+ InvalidAddressErrorMessageTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 9bbf862568..898d4bfe17 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -134,6 +134,7 @@ BASE_SCRIPTS = [
'wallet_keypool_topup.py --descriptors',
'feature_fee_estimation.py',
'interface_zmq.py',
+ 'rpc_invalid_address_message.py',
'interface_bitcoin_cli.py',
'mempool_resurrect.py',
'wallet_txn_doublespend.py --mineblock',
diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py
index 6bcb03e8ed..4a589f0393 100755
--- a/test/functional/wallet_basic.py
+++ b/test/functional/wallet_basic.py
@@ -586,7 +586,7 @@ class WalletTest(BitcoinTestFramework):
assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999)))
# Test getaddressinfo on external address. Note that these addresses are taken from disablewallet.py
- assert_raises_rpc_error(-5, "Invalid address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy")
+ assert_raises_rpc_error(-5, "Invalid prefix for Base58-encoded address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy")
address_info = self.nodes[0].getaddressinfo("mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ")
assert_equal(address_info['address'], "mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ")
assert_equal(address_info["scriptPubKey"], "76a9144e3854046c7bd1594ac904e4793b6a45b36dea0988ac")
diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py
index 3c743603bb..ab766b4a45 100755
--- a/test/fuzz/test_runner.py
+++ b/test/fuzz/test_runner.py
@@ -14,6 +14,14 @@ import subprocess
import sys
+def get_fuzz_env(*, target):
+ return {
+ 'FUZZ': target,
+ 'ASAN_OPTIONS': # symbolizer disabled due to https://github.com/google/sanitizers/issues/1364#issuecomment-761072085
+ 'symbolize=0:detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1',
+ }
+
+
def main():
parser = argparse.ArgumentParser(
formatter_class=argparse.ArgumentDefaultsHelpFormatter,
@@ -129,9 +137,7 @@ def main():
os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz'),
'-help=1',
],
- env={
- 'FUZZ': test_list_selection[0]
- },
+ env=get_fuzz_env(target=test_list_selection[0]),
timeout=20,
check=True,
stderr=subprocess.PIPE,
@@ -186,9 +192,7 @@ def generate_corpus_seeds(*, fuzz_pool, build_dir, seed_dir, targets):
' '.join(command),
subprocess.run(
command,
- env={
- 'FUZZ': t
- },
+ env=get_fuzz_env(target=t),
check=True,
stderr=subprocess.PIPE,
universal_newlines=True,
@@ -227,9 +231,7 @@ def merge_inputs(*, fuzz_pool, corpus, test_list, build_dir, merge_dir):
output = 'Run {} with args {}\n'.format(t, " ".join(args))
output += subprocess.run(
args,
- env={
- 'FUZZ': t
- },
+ env=get_fuzz_env(target=t),
check=True,
stderr=subprocess.PIPE,
universal_newlines=True,
@@ -257,7 +259,12 @@ def run_once(*, fuzz_pool, corpus, test_list, build_dir, use_valgrind):
def job(t, args):
output = 'Run {} with args {}'.format(t, args)
- result = subprocess.run(args, env={'FUZZ': t}, stderr=subprocess.PIPE, universal_newlines=True)
+ result = subprocess.run(
+ args,
+ env=get_fuzz_env(target=t),
+ stderr=subprocess.PIPE,
+ universal_newlines=True,
+ )
output += result.stderr
return output, result