diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp index 7ed0c6d2d1c8..808486412bc6 100644 --- a/src/bench/rpc_blockchain.cpp +++ b/src/bench/rpc_blockchain.cpp @@ -10,11 +10,14 @@ #include #include #include +#include #include #include static void BlockToJsonVerbose(benchmark::Bench& bench) { + TestingSetup test_setup{}; + CDataStream stream(benchmark::data::block813851, SER_NETWORK, PROTOCOL_VERSION); char a = '\0'; stream.write(&a, 1); // Prevent compaction diff --git a/src/init.cpp b/src/init.cpp index fc7e6ae3e8aa..8c8a4afd382b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -591,14 +591,14 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-onlynet=", "Make outgoing connections only through network (" + Join(GetNetworkNames(), ", ") + "). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks. Warning: if it is used with non-onion networks and the -onion or -proxy option is set, then outbound onion connections will still be made; use -noonion or -onion=0 to disable outbound onion connections in this case.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-peertimeout=", strprintf("Specify p2p connection timeout in seconds. This option determines the amount of time a peer may be inactive before the connection to it is dropped. (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-peertimeout=", strprintf("Specify a p2p connection timeout delay in seconds. After connecting to a peer, wait this amount of time before considering disconnection based on inactivity (minimum: 1, default: %d)", DEFAULT_PEER_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-port=", strprintf("Listen for connections on . Nodes not using the default ports (default: %u, testnet: %u, regtest: %u) are unlikely to get incoming connections. Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-proxy=", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-proxyrandomize", strprintf("Randomize credentials for every proxy connection. This enables Tor stream isolation (default: %u)", DEFAULT_PROXYRANDOMIZE), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-seednode=", "Connect to a node to retrieve peer addresses, and disconnect. This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-socketevents=", "Socket events mode, which must be one of 'select', 'poll', 'epoll' or 'kqueue', depending on your system (default: Linux - 'epoll', FreeBSD/Apple - 'kqueue', Windows - 'select')", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-timeout=", strprintf("Specify connection timeout in milliseconds (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-timeout=", strprintf("Specify socket connection timeout in milliseconds. If an initial attempt to connect is unsuccessful after this amount of time, drop it (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torcontrol=:", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-torpassword=", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION); #ifdef USE_UPNP diff --git a/src/key_io.cpp b/src/key_io.cpp index f1f1752ad189..18b77f004f37 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -38,10 +38,11 @@ class DestinationEncoder 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 data; uint160 hash; + error_str = ""; if (DecodeBase58Check(str, data, 21)) { // base58-encoded Dash addresses. // Public-key-hash-addresses have version 76 (or 140 testnet). @@ -58,7 +59,13 @@ 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. + error_str = "Invalid prefix for Base58-encoded address"; } + // Set error message if address can't be interpreted as Base58. + if (error_str.empty()) error_str = "Invalid address format"; + return CNoDestination(); } } // namespace @@ -148,14 +155,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 edd0a0b7bc98..ba71a2c9445a 100644 --- a/src/key_io.h +++ b/src/key_io.h @@ -24,6 +24,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/rpc/misc.cpp b/src/rpc/misc.cpp index 84515dc3aec8..93046785d499 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -223,10 +223,11 @@ static UniValue validateaddress(const JSONRPCRequest& request) 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 dash 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::STR, "error", /* optional */ true, "Error message, if any"}, } }, RPCExamples{ @@ -235,13 +236,14 @@ static UniValue validateaddress(const JSONRPCRequest& request) }, }.Check(request); - 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); @@ -250,7 +252,10 @@ static UniValue validateaddress(const JSONRPCRequest& request) UniValue detail = DescribeAddress(dest); ret.pushKVs(detail); + } else { + ret.pushKV("error", error_msg); } + return ret; } diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index 9bd8cbe37881..2c7964e798fc 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -13,15 +13,15 @@ const std::function G_TEST_LOG_FUN{}; -std::map>& FuzzTargets() +std::map>& FuzzTargets() { - static std::map> g_fuzz_targets; + static std::map> g_fuzz_targets; return g_fuzz_targets; } -void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, TypeInitialize init) +void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, TypeInitialize init, TypeHidden hidden) { - const auto it_ins = FuzzTargets().try_emplace(name, std::move(target), std::move(init)); + const auto it_ins = FuzzTargets().try_emplace(name, std::move(target), std::move(init), hidden); Assert(it_ins.second); } @@ -31,6 +31,7 @@ void initialize() { if (std::getenv("PRINT_ALL_FUZZ_TARGETS_AND_ABORT")) { for (const auto& t : FuzzTargets()) { + if (std::get<2>(t.second)) continue; std::cout << t.first << std::endl; } Assert(false); diff --git a/src/test/fuzz/fuzz.h b/src/test/fuzz/fuzz.h index 7969211188a9..fc676a061d46 100644 --- a/src/test/fuzz/fuzz.h +++ b/src/test/fuzz/fuzz.h @@ -18,22 +18,26 @@ using FuzzBufferType = Span; using TypeTestOneInput = std::function; using TypeInitialize = std::function; +using TypeHidden = bool; -void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, TypeInitialize init); +void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, TypeInitialize init, TypeHidden hidden); -inline void FuzzFrameworkEmptyFun() {} +inline void FuzzFrameworkEmptyInitFun() {} #define FUZZ_TARGET(name) \ - FUZZ_TARGET_INIT(name, FuzzFrameworkEmptyFun) - -#define FUZZ_TARGET_INIT(name, init_fun) \ - void name##_fuzz_target(FuzzBufferType); \ - struct name##_Before_Main { \ - name##_Before_Main() \ - { \ - FuzzFrameworkRegisterTarget(#name, name##_fuzz_target, init_fun); \ - } \ - } const static g_##name##_before_main; \ + FUZZ_TARGET_INIT(name, FuzzFrameworkEmptyInitFun) + +#define FUZZ_TARGET_INIT(name, init_fun) \ + FUZZ_TARGET_INIT_HIDDEN(name, init_fun, false) + +#define FUZZ_TARGET_INIT_HIDDEN(name, init_fun, hidden) \ + void name##_fuzz_target(FuzzBufferType); \ + struct name##_Before_Main { \ + name##_Before_Main() \ + { \ + FuzzFrameworkRegisterTarget(#name, name##_fuzz_target, init_fun, hidden); \ + } \ + } const static g_##name##_before_main; \ void name##_fuzz_target(FuzzBufferType buffer) #endif // BITCOIN_TEST_FUZZ_FUZZ_H diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 94ec19bf188d..6fe278449bc2 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -41,8 +41,27 @@ namespace { const TestingSetup* g_setup; } // namespace +size_t& GetNumMsgTypes() +{ + static size_t g_num_msg_types{0}; + return g_num_msg_types; +} +#define FUZZ_TARGET_MSG(msg_type) \ + struct msg_type##_Count_Before_Main { \ + msg_type##_Count_Before_Main() \ + { \ + ++GetNumMsgTypes(); \ + } \ + } const static g_##msg_type##_count_before_main; \ + FUZZ_TARGET_INIT(process_message_##msg_type, initialize_process_message) \ + { \ + fuzz_target(buffer, #msg_type); \ + } + void initialize_process_message() { + Assert(GetNumMsgTypes() == getAllNetMessageTypes().size()); // If this fails, add or remove the message type below + static const auto testing_setup = MakeNoLogFileContext(); g_setup = testing_setup.get(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { @@ -83,27 +102,37 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE } FUZZ_TARGET_INIT(process_message, initialize_process_message) { fuzz_target(buffer, ""); } -FUZZ_TARGET_INIT(process_message_addr, initialize_process_message) { fuzz_target(buffer, "addr"); } -FUZZ_TARGET_INIT(process_message_block, initialize_process_message) { fuzz_target(buffer, "block"); } -FUZZ_TARGET_INIT(process_message_blocktxn, initialize_process_message) { fuzz_target(buffer, "blocktxn"); } -FUZZ_TARGET_INIT(process_message_cmpctblock, initialize_process_message) { fuzz_target(buffer, "cmpctblock"); } -FUZZ_TARGET_INIT(process_message_feefilter, initialize_process_message) { fuzz_target(buffer, "feefilter"); } -FUZZ_TARGET_INIT(process_message_filteradd, initialize_process_message) { fuzz_target(buffer, "filteradd"); } -FUZZ_TARGET_INIT(process_message_filterclear, initialize_process_message) { fuzz_target(buffer, "filterclear"); } -FUZZ_TARGET_INIT(process_message_filterload, initialize_process_message) { fuzz_target(buffer, "filterload"); } -FUZZ_TARGET_INIT(process_message_getaddr, initialize_process_message) { fuzz_target(buffer, "getaddr"); } -FUZZ_TARGET_INIT(process_message_getblocks, initialize_process_message) { fuzz_target(buffer, "getblocks"); } -FUZZ_TARGET_INIT(process_message_getblocktxn, initialize_process_message) { fuzz_target(buffer, "getblocktxn"); } -FUZZ_TARGET_INIT(process_message_getdata, initialize_process_message) { fuzz_target(buffer, "getdata"); } -FUZZ_TARGET_INIT(process_message_getheaders, initialize_process_message) { fuzz_target(buffer, "getheaders"); } -FUZZ_TARGET_INIT(process_message_headers, initialize_process_message) { fuzz_target(buffer, "headers"); } -FUZZ_TARGET_INIT(process_message_inv, initialize_process_message) { fuzz_target(buffer, "inv"); } -FUZZ_TARGET_INIT(process_message_mempool, initialize_process_message) { fuzz_target(buffer, "mempool"); } -FUZZ_TARGET_INIT(process_message_notfound, initialize_process_message) { fuzz_target(buffer, "notfound"); } -FUZZ_TARGET_INIT(process_message_ping, initialize_process_message) { fuzz_target(buffer, "ping"); } -FUZZ_TARGET_INIT(process_message_pong, initialize_process_message) { fuzz_target(buffer, "pong"); } -FUZZ_TARGET_INIT(process_message_sendcmpct, initialize_process_message) { fuzz_target(buffer, "sendcmpct"); } -FUZZ_TARGET_INIT(process_message_sendheaders, initialize_process_message) { fuzz_target(buffer, "sendheaders"); } -FUZZ_TARGET_INIT(process_message_tx, initialize_process_message) { fuzz_target(buffer, "tx"); } -FUZZ_TARGET_INIT(process_message_verack, initialize_process_message) { fuzz_target(buffer, "verack"); } -FUZZ_TARGET_INIT(process_message_version, initialize_process_message) { fuzz_target(buffer, "version"); } +FUZZ_TARGET_MSG(addr); +FUZZ_TARGET_MSG(addrv2); +FUZZ_TARGET_MSG(block); +FUZZ_TARGET_MSG(blocktxn); +FUZZ_TARGET_MSG(cfcheckpt); +FUZZ_TARGET_MSG(cfheaders); +FUZZ_TARGET_MSG(cfilter); +FUZZ_TARGET_MSG(cmpctblock); +FUZZ_TARGET_MSG(feefilter); +FUZZ_TARGET_MSG(filteradd); +FUZZ_TARGET_MSG(filterclear); +FUZZ_TARGET_MSG(filterload); +FUZZ_TARGET_MSG(getaddr); +FUZZ_TARGET_MSG(getblocks); +FUZZ_TARGET_MSG(getblocktxn); +FUZZ_TARGET_MSG(getcfcheckpt); +FUZZ_TARGET_MSG(getcfheaders); +FUZZ_TARGET_MSG(getcfilters); +FUZZ_TARGET_MSG(getdata); +FUZZ_TARGET_MSG(getheaders); +FUZZ_TARGET_MSG(headers); +FUZZ_TARGET_MSG(inv); +FUZZ_TARGET_MSG(mempool); +FUZZ_TARGET_MSG(merkleblock); +FUZZ_TARGET_MSG(notfound); +FUZZ_TARGET_MSG(ping); +FUZZ_TARGET_MSG(pong); +FUZZ_TARGET_MSG(sendaddrv2); +FUZZ_TARGET_MSG(sendcmpct); +FUZZ_TARGET_MSG(sendheaders); +FUZZ_TARGET_MSG(tx); +FUZZ_TARGET_MSG(verack); +FUZZ_TARGET_MSG(version); +FUZZ_TARGET_MSG(wtxidrelay); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 55d9810745ee..8f16e41ef455 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3788,13 +3788,19 @@ UniValue getaddressinfo(const JSONRPCRequest& request) 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 000000000000..bb0180a0d372 --- /dev/null +++ b/test/functional/rpc_invalid_address_message.py @@ -0,0 +1,57 @@ +#!/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, +) + + +BASE58_VALID = 'yjQ5gLvGRtmq1cwc4kePLCrzQ8GVCh9Gaz' +BASE58_INVALID_PREFIX = 'XpG61qAVhdyN7AqVZQsHfJL7AEk4dPVinc' + +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] + + # 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 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 f4c5bc1d4580..b43262a75035 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -140,6 +140,7 @@ 'feature_fee_estimation.py', 'interface_zmq_dash.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 1b04c8f6015d..e597c5fcffd0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -602,7 +602,7 @@ def run_test(self): 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("yjQ5gLvGRtmq1cwc4kePLCrzQ8GVCh9Gaz") assert_equal(address_info['address'], "yjQ5gLvGRtmq1cwc4kePLCrzQ8GVCh9Gaz") assert_equal(address_info["scriptPubKey"], "76a914fd2b4d101724a76374fccbc5b6df7670a75d7cd088ac")