backport: Merge bitcoin#20998, 20832, 21077, 20663, 20915

src/bench/rpc_blockchain.cpp

@@ -10,11 +10,14 @@
 #include <llmq/instantsend.h>
 #include <rpc/blockchain.h>
 #include <streams.h>
+#include <test/util/setup_common.h>
 #include <validation.h>
 
 #include <univalue.h>
 
 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

src/init.cpp

@@ -591,14 +591,14 @@ void SetupServerArgs(NodeContext& node)
     argsman.AddArg("-onlynet=<net>", "Make outgoing connections only through network <net> (" + 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=<n>", 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=<n>", 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=<port>", strprintf("Listen for connections on <port>. 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=<ip:port>", "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=<ip>", "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=<mode>", "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=<n>", strprintf("Specify connection timeout in milliseconds (minimum: 1, default: %d)", DEFAULT_CONNECT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    argsman.AddArg("-timeout=<n>", 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=<ip>:<port>", strprintf("Tor control port to use if onion listening enabled (default: %s)", DEFAULT_TOR_CONTROL), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     argsman.AddArg("-torpassword=<pass>", "Tor control port password (default: empty)", ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE, OptionsCategory::CONNECTION);
 #ifdef USE_UPNP

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<unsigned char> 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)

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);
 

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;
 }
 

src/test/fuzz/fuzz.cpp

@@ -13,15 +13,15 @@
 
 const std::function<void(const std::string&)> G_TEST_LOG_FUN{};
 
-std::map<std::string_view, std::tuple<TypeTestOneInput, TypeInitialize>>& FuzzTargets()
+std::map<std::string_view, std::tuple<TypeTestOneInput, TypeInitialize, TypeHidden>>& FuzzTargets()
 {
-    static std::map<std::string_view, std::tuple<TypeTestOneInput, TypeInitialize>> g_fuzz_targets;
+    static std::map<std::string_view, std::tuple<TypeTestOneInput, TypeInitialize, TypeHidden>> 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);

src/test/fuzz/fuzz.h

@@ -18,22 +18,26 @@ using FuzzBufferType = Span<const uint8_t>;
 
 using TypeTestOneInput = std::function<void(FuzzBufferType)>;
 using TypeInitialize = std::function<void()>;
+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

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<const TestingSetup>();
     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);

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);
 

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()

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',