diff options
52 files changed, 800 insertions, 576 deletions
diff --git a/.appveyor.yml b/.appveyor.yml index 7250d4ad94..097874b17a 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -10,7 +10,7 @@ environment: QT_DOWNLOAD_URL: 'https://github.com/sipsorcery/qt_win_binary/releases/download/qt598x64_vs2019_v1681/qt598_x64_vs2019_1681.zip' QT_DOWNLOAD_HASH: '00cf7327818c07d74e0b1a4464ffe987c2728b00d49d4bf333065892af0515c3' QT_LOCAL_PATH: 'C:\Qt5.9.8_x64_static_vs2019' - VCPKG_TAG: '2020.11-1' + VCPKG_TAG: '75522bb1f2e7d863078bcd06322348f053a9e33f' install: # Disable zmq test for now since python zmq library on Windows would cause Access violation sometimes. # - cmd: pip install zmq diff --git a/ci/test/00_setup_env_mac.sh b/ci/test/00_setup_env_mac.sh index 645dcffd02..6ce3812447 100644 --- a/ci/test/00_setup_env_mac.sh +++ b/ci/test/00_setup_env_mac.sh @@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_macos_cross export DOCKER_NAME_TAG=ubuntu:20.04 # Check that Focal can cross-compile to macos (Focal is used in the gitian build as well) export HOST=x86_64-apple-darwin18 -export PACKAGES="cmake imagemagick librsvg2-bin libz-dev libtiff-tools libtinfo5 python3-dev python3-setuptools xorriso" +export PACKAGES="cmake imagemagick librsvg2-bin libz-dev libtiff-tools libtinfo5 python3-setuptools xorriso" export XCODE_VERSION=11.3.1 export XCODE_BUILD_ID=11C505 export RUN_UNIT_TESTS=false diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml index 98c3888b45..c37ad5b842 100644 --- a/contrib/gitian-descriptors/gitian-osx.yml +++ b/contrib/gitian-descriptors/gitian-osx.yml @@ -23,7 +23,6 @@ packages: - "imagemagick" - "libz-dev" - "python3" -- "python3-dev" - "python3-setuptools" - "fonts-tuffy" - "xorriso" diff --git a/doc/external-signer.md b/doc/external-signer.md index c91ea9bab7..2b3b378bee 100644 --- a/doc/external-signer.md +++ b/doc/external-signer.md @@ -4,7 +4,7 @@ Bitcoin Core can be launched with `-signer=<cmd>` where `<cmd>` is an external t ## Example usage -The following example is based on the [HWI](https://github.com/bitcoin-core/HWI) tool. Although this tool is hosted under the Bitcoin Core GitHub organization and maintained by Bitcoin Core developers, it should be used with caution. It is considered experimental and has far less review than Bitcoin Core itself. Be particularly careful when running tools such as these on a computer with private keys on it. +The following example is based on the [HWI](https://github.com/bitcoin-core/HWI) tool. Version 2.0 or newer is required. Although this tool is hosted under the Bitcoin Core GitHub organization and maintained by Bitcoin Core developers, it should be used with caution. It is considered experimental and has far less review than Bitcoin Core itself. Be particularly careful when running tools such as these on a computer with private keys on it. When using a hardware wallet, consult the manufacturer website for (alternative) software they recommend. As long as their software conforms to the standard below, it should be able to work with Bitcoin Core. diff --git a/doc/fuzzing.md b/doc/fuzzing.md index 87df2bbbb9..4d8825f4c2 100644 --- a/doc/fuzzing.md +++ b/doc/fuzzing.md @@ -108,33 +108,32 @@ Full configure that was tested on macOS Catalina with `brew` installed `llvm`: Read the [libFuzzer documentation](https://llvm.org/docs/LibFuzzer.html) for more information. This [libFuzzer tutorial](https://github.com/google/fuzzing/blob/master/tutorial/libFuzzerTutorial.md) might also be of interest. -# Fuzzing Bitcoin Core using american fuzzy lop (`afl-fuzz`) +# Fuzzing Bitcoin Core using afl++ ## Quickstart guide -To quickly get started fuzzing Bitcoin Core using [`afl-fuzz`](https://github.com/google/afl): +To quickly get started fuzzing Bitcoin Core using [afl++](https://github.com/AFLplusplus/AFLplusplus): ```sh $ git clone https://github.com/bitcoin/bitcoin $ cd bitcoin/ -$ git clone https://github.com/google/afl -$ make -C afl/ -$ make -C afl/llvm_mode/ +$ git clone https://github.com/AFLplusplus/AFLplusplus +$ make -C AFLplusplus/ source-only $ ./autogen.sh -# It is possible to compile with afl-gcc and afl-g++ instead of afl-clang. However, running afl-fuzz -# may require more memory via the -m flag. -$ CC=$(pwd)/afl/afl-clang-fast CXX=$(pwd)/afl/afl-clang-fast++ ./configure --enable-fuzz +# If afl-clang-lto is not available, see +# https://github.com/AFLplusplus/AFLplusplus#a-selecting-the-best-afl-compiler-for-instrumenting-the-target +$ CC=$(pwd)/AFLplusplus/afl-clang-lto CXX=$(pwd)/AFLplusplus/afl-clang-lto++ ./configure --enable-fuzz $ make # For macOS you may need to ignore x86 compilation checks when running "make". If so, # try compiling using: AFL_NO_X86=1 make $ mkdir -p inputs/ outputs/ $ echo A > inputs/thin-air-input -$ FUZZ=bech32 afl/afl-fuzz -i inputs/ -o outputs/ -- src/test/fuzz/fuzz +$ FUZZ=bech32 AFLplusplus/afl-fuzz -i inputs/ -o outputs/ -- src/test/fuzz/fuzz # You may have to change a few kernel parameters to test optimally - afl-fuzz # will print an error and suggestion if so. ``` -Read the [`afl-fuzz` documentation](https://github.com/google/afl) for more information. +Read the [afl++ documentation](https://github.com/AFLplusplus/AFLplusplus) for more information. # Fuzzing Bitcoin Core using Honggfuzz diff --git a/src/Makefile.am b/src/Makefile.am index 07d046c32d..9903c2e9b3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -552,7 +552,6 @@ libbitcoin_util_a_SOURCES = \ support/lockedpool.cpp \ chainparamsbase.cpp \ clientversion.cpp \ - compat/glibc_sanity.cpp \ compat/glibcxx_sanity.cpp \ compat/strnlen.cpp \ fs.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 133277baa6..24b2879789 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -252,6 +252,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/net.cpp \ test/fuzz/net_permissions.cpp \ test/fuzz/netaddress.cpp \ + test/fuzz/netbase_dns_lookup.cpp \ test/fuzz/node_eviction.cpp \ test/fuzz/p2p_transport_deserializer.cpp \ test/fuzz/parse_hd_keypath.cpp \ @@ -296,7 +297,8 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/transaction.cpp \ test/fuzz/tx_in.cpp \ test/fuzz/tx_out.cpp \ - test/fuzz/txrequest.cpp + test/fuzz/txrequest.cpp \ + test/fuzz/validation_load_mempool.cpp endif # ENABLE_FUZZ_BINARY nodist_test_test_bitcoin_SOURCES = $(GENERATED_TEST_FILES) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 376e70ffba..80acdec044 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -49,7 +49,10 @@ static void CoinSelection(benchmark::Bench& bench) } const CoinEligibilityFilter filter_standard(1, 6, 0); - const CoinSelectionParams coin_selection_params(true, 34, 148, CFeeRate(0), 0, false); + const CoinSelectionParams coin_selection_params(/* use_bnb= */ true, /* change_output_size= */ 34, + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); bench.run([&] { std::set<CInputCoin> setCoinsRet; CAmount nValueRet; diff --git a/src/compat/glibc_compat.cpp b/src/compat/glibc_compat.cpp index 8a51f310f7..ff581d4a9e 100644 --- a/src/compat/glibc_compat.cpp +++ b/src/compat/glibc_compat.cpp @@ -9,13 +9,6 @@ #include <cstddef> #include <cstdint> -// Prior to GLIBC_2.14, memcpy was aliased to memmove. -extern "C" void* memmove(void* a, const void* b, size_t c); -extern "C" void* memcpy(void* a, const void* b, size_t c) -{ - return memmove(a, b, c); -} - #if defined(__i386__) || defined(__arm__) extern "C" int64_t __udivmoddi4(uint64_t u, uint64_t v, uint64_t* rp); diff --git a/src/compat/glibc_sanity.cpp b/src/compat/glibc_sanity.cpp deleted file mode 100644 index 06d0dd6fba..0000000000 --- a/src/compat/glibc_sanity.cpp +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright (c) 2009-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. - -#if defined(HAVE_CONFIG_H) -#include <config/bitcoin-config.h> -#endif - -#include <cstddef> - -extern "C" void* memcpy(void* a, const void* b, size_t c); -void* memcpy_int(void* a, const void* b, size_t c) -{ - return memcpy(a, b, c); -} - -namespace -{ -// trigger: Use the memcpy_int wrapper which calls our internal memcpy. -// A direct call to memcpy may be optimized away by the compiler. -// test: Fill an array with a sequence of integers. memcpy to a new empty array. -// Verify that the arrays are equal. Use an odd size to decrease the odds of -// the call being optimized away. -template <unsigned int T> -bool sanity_test_memcpy() -{ - unsigned int memcpy_test[T]; - unsigned int memcpy_verify[T] = {}; - for (unsigned int i = 0; i != T; ++i) - memcpy_test[i] = i; - - memcpy_int(memcpy_verify, memcpy_test, sizeof(memcpy_test)); - - for (unsigned int i = 0; i != T; ++i) { - if (memcpy_verify[i] != i) - return false; - } - return true; -} -} // namespace - -bool glibc_sanity_test() -{ - return sanity_test_memcpy<1025>(); -} diff --git a/src/compat/sanity.h b/src/compat/sanity.h index 909c4f6da8..8efa416102 100644 --- a/src/compat/sanity.h +++ b/src/compat/sanity.h @@ -5,7 +5,6 @@ #ifndef BITCOIN_COMPAT_SANITY_H #define BITCOIN_COMPAT_SANITY_H -bool glibc_sanity_test(); bool glibcxx_sanity_test(); #endif // BITCOIN_COMPAT_SANITY_H diff --git a/src/i2p.cpp b/src/i2p.cpp index 42270deaeb..d16c620d88 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -153,7 +153,7 @@ bool Session::Accept(Connection& conn) } const std::string& peer_dest = - conn.sock.RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt); + conn.sock.RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE); conn.peer = CService(DestB64ToAddr(peer_dest), Params().GetDefaultPort()); @@ -252,7 +252,7 @@ Session::Reply Session::SendRequestAndGetReply(const Sock& sock, // signaled. static constexpr auto recv_timeout = 3min; - reply.full = sock.RecvUntilTerminator('\n', recv_timeout, *m_interrupt); + reply.full = sock.RecvUntilTerminator('\n', recv_timeout, *m_interrupt, MAX_MSG_SIZE); for (const auto& kv : spanparsing::Split(reply.full, ' ')) { const auto& pos = std::find(kv.begin(), kv.end(), '='); @@ -41,6 +41,14 @@ struct Connection { namespace sam { /** + * The maximum size of an incoming message from the I2P SAM proxy (in bytes). + * Used to avoid a runaway proxy from sending us an "unlimited" amount of data without a terminator. + * The longest known message is ~1400 bytes, so this is high enough not to be triggered during + * normal operation, yet low enough to avoid a malicious proxy from filling our memory. + */ +static constexpr size_t MAX_MSG_SIZE{65536}; + +/** * I2P SAM session. */ class Session diff --git a/src/init.cpp b/src/init.cpp index b91a94bc25..7d5420e3be 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -585,6 +585,7 @@ void SetupServerArgs(NodeContext& node) argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS); #else hidden_args.emplace_back("-daemon"); + hidden_args.emplace_back("-daemonwait"); #endif // Add the hidden options @@ -772,7 +773,7 @@ static bool InitSanityCheck() return InitError(Untranslated("Elliptic curve cryptography sanity check failure. Aborting.")); } - if (!glibc_sanity_test() || !glibcxx_sanity_test()) + if (!glibcxx_sanity_test()) return false; if (!Random_SanityCheck()) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b600cc3a1a..8ffbd45034 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -576,16 +576,16 @@ struct CNodeState { */ struct ChainSyncTimeoutState { //! A timeout used for checking whether our peer has sufficiently synced - int64_t m_timeout; + int64_t m_timeout{0}; //! A header with the work we require on our peer's chain - const CBlockIndex * m_work_header; + const CBlockIndex* m_work_header{nullptr}; //! After timeout is reached, set to true after sending getheaders - bool m_sent_getheaders; + bool m_sent_getheaders{false}; //! Whether this peer is protected from disconnection due to a bad/slow chain - bool m_protect; + bool m_protect{false}; }; - ChainSyncTimeoutState m_chain_sync{0, nullptr, false, false}; + ChainSyncTimeoutState m_chain_sync; //! Time of last new block announcement int64_t m_last_block_announcement{0}; @@ -599,11 +599,7 @@ struct CNodeState { //! Whether this peer relays txs via wtxid bool m_wtxid_relay{false}; - CNodeState(bool is_inbound) - : m_is_inbound(is_inbound) - { - m_recently_announced_invs.reset(); - } + CNodeState(bool is_inbound) : m_is_inbound(is_inbound) {} }; /** Map maintaining per-node state. */ diff --git a/src/netbase.cpp b/src/netbase.cpp index 88c36ed86c..ac2392ebed 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -42,6 +42,50 @@ bool fNameLookup = DEFAULT_NAME_LOOKUP; int g_socks5_recv_timeout = 20 * 1000; static std::atomic<bool> interruptSocks5Recv(false); +std::vector<CNetAddr> WrappedGetAddrInfo(const std::string& name, bool allow_lookup) +{ + addrinfo ai_hint{}; + // We want a TCP port, which is a streaming socket type + ai_hint.ai_socktype = SOCK_STREAM; + ai_hint.ai_protocol = IPPROTO_TCP; + // We don't care which address family (IPv4 or IPv6) is returned + ai_hint.ai_family = AF_UNSPEC; + // If we allow lookups of hostnames, use the AI_ADDRCONFIG flag to only + // return addresses whose family we have an address configured for. + // + // If we don't allow lookups, then use the AI_NUMERICHOST flag for + // getaddrinfo to only decode numerical network addresses and suppress + // hostname lookups. + ai_hint.ai_flags = allow_lookup ? AI_ADDRCONFIG : AI_NUMERICHOST; + + addrinfo* ai_res{nullptr}; + const int n_err{getaddrinfo(name.c_str(), nullptr, &ai_hint, &ai_res)}; + if (n_err != 0) { + return {}; + } + + // Traverse the linked list starting with ai_trav. + addrinfo* ai_trav{ai_res}; + std::vector<CNetAddr> resolved_addresses; + while (ai_trav != nullptr) { + if (ai_trav->ai_family == AF_INET) { + assert(ai_trav->ai_addrlen >= sizeof(sockaddr_in)); + resolved_addresses.emplace_back(reinterpret_cast<sockaddr_in*>(ai_trav->ai_addr)->sin_addr); + } + if (ai_trav->ai_family == AF_INET6) { + assert(ai_trav->ai_addrlen >= sizeof(sockaddr_in6)); + const sockaddr_in6* s6{reinterpret_cast<sockaddr_in6*>(ai_trav->ai_addr)}; + resolved_addresses.emplace_back(s6->sin6_addr, s6->sin6_scope_id); + } + ai_trav = ai_trav->ai_next; + } + freeaddrinfo(ai_res); + + return resolved_addresses; +} + +DNSLookupFn g_dns_lookup{WrappedGetAddrInfo}; + enum Network ParseNetwork(const std::string& net_in) { std::string net = ToLower(net_in); if (net == "ipv4") return NET_IPV4; @@ -87,7 +131,7 @@ std::vector<std::string> GetNetworkNames(bool append_unroutable) return names; } -bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup) +static bool LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { vIP.clear(); @@ -109,73 +153,20 @@ bool static LookupIntern(const std::string& name, std::vector<CNetAddr>& vIP, un } } - struct addrinfo aiHint; - memset(&aiHint, 0, sizeof(struct addrinfo)); - - // We want a TCP port, which is a streaming socket type - aiHint.ai_socktype = SOCK_STREAM; - aiHint.ai_protocol = IPPROTO_TCP; - // We don't care which address family (IPv4 or IPv6) is returned - aiHint.ai_family = AF_UNSPEC; - // If we allow lookups of hostnames, use the AI_ADDRCONFIG flag to only - // return addresses whose family we have an address configured for. - // - // If we don't allow lookups, then use the AI_NUMERICHOST flag for - // getaddrinfo to only decode numerical network addresses and suppress - // hostname lookups. - aiHint.ai_flags = fAllowLookup ? AI_ADDRCONFIG : AI_NUMERICHOST; - struct addrinfo *aiRes = nullptr; - int nErr = getaddrinfo(name.c_str(), nullptr, &aiHint, &aiRes); - if (nErr) - return false; - - // Traverse the linked list starting with aiTrav, add all non-internal - // IPv4,v6 addresses to vIP while respecting nMaxSolutions. - struct addrinfo *aiTrav = aiRes; - while (aiTrav != nullptr && (nMaxSolutions == 0 || vIP.size() < nMaxSolutions)) - { - CNetAddr resolved; - if (aiTrav->ai_family == AF_INET) - { - assert(aiTrav->ai_addrlen >= sizeof(sockaddr_in)); - resolved = CNetAddr(((struct sockaddr_in*)(aiTrav->ai_addr))->sin_addr); - } - - if (aiTrav->ai_family == AF_INET6) - { - assert(aiTrav->ai_addrlen >= sizeof(sockaddr_in6)); - struct sockaddr_in6* s6 = (struct sockaddr_in6*) aiTrav->ai_addr; - resolved = CNetAddr(s6->sin6_addr, s6->sin6_scope_id); + for (const CNetAddr& resolved : dns_lookup_function(name, fAllowLookup)) { + if (nMaxSolutions > 0 && vIP.size() >= nMaxSolutions) { + break; } /* Never allow resolving to an internal address. Consider any such result invalid */ if (!resolved.IsInternal()) { vIP.push_back(resolved); } - - aiTrav = aiTrav->ai_next; } - freeaddrinfo(aiRes); - return (vIP.size() > 0); } -/** - * Resolve a host string to its corresponding network addresses. - * - * @param name The string representing a host. Could be a name or a numerical - * IP address (IPv6 addresses in their bracketed form are - * allowed). - * @param[out] vIP The resulting network addresses to which the specified host - * string resolved. - * - * @returns Whether or not the specified host string successfully resolved to - * any resulting network addresses. - * - * @see Lookup(const char *, std::vector<CService>&, int, bool, unsigned int) - * for additional parameter descriptions. - */ -bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup) +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(name)) { return false; @@ -187,49 +178,23 @@ bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned in strHost = strHost.substr(1, strHost.size() - 2); } - return LookupIntern(strHost, vIP, nMaxSolutions, fAllowLookup); + return LookupIntern(strHost, vIP, nMaxSolutions, fAllowLookup, dns_lookup_function); } - /** - * Resolve a host string to its first corresponding network address. - * - * @see LookupHost(const std::string&, std::vector<CNetAddr>&, unsigned int, bool) for - * additional parameter descriptions. - */ -bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup) +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(name)) { return false; } std::vector<CNetAddr> vIP; - LookupHost(name, vIP, 1, fAllowLookup); + LookupHost(name, vIP, 1, fAllowLookup, dns_lookup_function); if(vIP.empty()) return false; addr = vIP.front(); return true; } -/** - * Resolve a service string to its corresponding service. - * - * @param name The string representing a service. Could be a name or a - * numerical IP address (IPv6 addresses should be in their - * disambiguated bracketed form), optionally followed by a port - * number. (e.g. example.com:8333 or - * [2001:db8:85a3:8d3:1319:8a2e:370:7348]:420) - * @param[out] vAddr The resulting services to which the specified service string - * resolved. - * @param portDefault The default port for resulting services if not specified - * by the service string. - * @param fAllowLookup Whether or not hostname lookups are permitted. If yes, - * external queries may be performed. - * @param nMaxSolutions The maximum number of results we want, specifying 0 - * means "as many solutions as we get." - * - * @returns Whether or not the service string successfully resolved to any - * resulting services. - */ -bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions) +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function) { if (name.empty() || !ValidAsCString(name)) { return false; @@ -239,7 +204,7 @@ bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefau SplitHostPort(name, port, hostname); std::vector<CNetAddr> vIP; - bool fRet = LookupIntern(hostname, vIP, nMaxSolutions, fAllowLookup); + bool fRet = LookupIntern(hostname, vIP, nMaxSolutions, fAllowLookup, dns_lookup_function); if (!fRet) return false; vAddr.resize(vIP.size()); @@ -248,36 +213,20 @@ bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefau return true; } -/** - * Resolve a service string to its first corresponding service. - * - * @see Lookup(const char *, std::vector<CService>&, int, bool, unsigned int) - * for additional parameter descriptions. - */ -bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup) +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(name)) { return false; } std::vector<CService> vService; - bool fRet = Lookup(name, vService, portDefault, fAllowLookup, 1); + bool fRet = Lookup(name, vService, portDefault, fAllowLookup, 1, dns_lookup_function); if (!fRet) return false; addr = vService[0]; return true; } -/** - * Resolve a service string with a numeric IP to its first corresponding - * service. - * - * @returns The resulting CService if the resolution was successful, [::]:0 - * otherwise. - * - * @see Lookup(const char *, CService&, int, bool) for additional parameter - * descriptions. - */ -CService LookupNumeric(const std::string& name, int portDefault) +CService LookupNumeric(const std::string& name, int portDefault, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(name)) { return {}; @@ -285,7 +234,7 @@ CService LookupNumeric(const std::string& name, int portDefault) CService addr; // "1.2:345" will fail to resolve the ip, but will still set the port. // If the ip fails to resolve, re-init the result. - if(!Lookup(name, addr, portDefault, false)) + if(!Lookup(name, addr, portDefault, false, dns_lookup_function)) addr = CService(); return addr; } @@ -414,24 +363,6 @@ static std::string Socks5ErrorString(uint8_t err) } } -/** - * Connect to a specified destination service through an already connected - * SOCKS5 proxy. - * - * @param strDest The destination fully-qualified domain name. - * @param port The destination port. - * @param auth The credentials with which to authenticate with the specified - * SOCKS5 proxy. - * @param sock The SOCKS5 proxy socket. - * - * @returns Whether or not the operation succeeded. - * - * @note The specified SOCKS5 proxy socket must already be connected to the - * SOCKS5 proxy. - * - * @see <a href="https://www.ietf.org/rfc/rfc1928.txt">RFC1928: SOCKS Protocol - * Version 5</a> - */ bool Socks5(const std::string& strDest, int port, const ProxyCredentials* auth, const Sock& sock) { IntrRecvError recvr; @@ -606,18 +537,6 @@ static void LogConnectFailure(bool manual_connection, const char* fmt, const Arg } } -/** - * Try to connect to the specified service on the specified socket. - * - * @param addrConnect The service to which to connect. - * @param hSocket The socket on which to connect. - * @param nTimeout Wait this many milliseconds for the connection to be - * established. - * @param manual_connection Whether or not the connection was manually requested - * (e.g. through the addnode RPC) - * - * @returns Whether or not a connection was successfully made. - */ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout, bool manual_connection) { // Create a sockaddr from the specified service. @@ -716,22 +635,6 @@ bool GetProxy(enum Network net, proxyType &proxyInfoOut) { return true; } -/** - * Set the name proxy to use for all connections to nodes specified by a - * hostname. After setting this proxy, connecting to a node specified by a - * hostname won't result in a local lookup of said hostname, rather, connect to - * the node by asking the name proxy for a proxy connection to the hostname, - * effectively delegating the hostname lookup to the specified proxy. - * - * This delegation increases privacy for those who set the name proxy as they no - * longer leak their external hostname queries to their DNS servers. - * - * @returns Whether or not the operation succeeded. - * - * @note SOCKS5's support for UDP-over-SOCKS5 has been considered, but no SOCK5 - * server in common use (most notably Tor) actually implements UDP - * support, and a DNS resolver is beyond the scope of this project. - */ bool SetNameProxy(const proxyType &addrProxy) { if (!addrProxy.IsValid()) return false; @@ -762,21 +665,6 @@ bool IsProxy(const CNetAddr &addr) { return false; } -/** - * Connect to a specified destination service through a SOCKS5 proxy by first - * connecting to the SOCKS5 proxy. - * - * @param proxy The SOCKS5 proxy. - * @param strDest The destination service to which to connect. - * @param port The destination port. - * @param sock The socket on which to connect to the SOCKS5 proxy. - * @param nTimeout Wait this many milliseconds for the connection to the SOCKS5 - * proxy to be established. - * @param[out] outProxyConnectionFailed Whether or not the connection to the - * SOCKS5 proxy failed. - * - * @returns Whether or not the operation succeeded. - */ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed) { // first connect to proxy server @@ -800,18 +688,7 @@ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int return true; } -/** - * Parse and resolve a specified subnet string into the appropriate internal - * representation. - * - * @param strSubnet A string representation of a subnet of the form `network - * address [ "/", ( CIDR-style suffix | netmask ) ]`(e.g. - * `2001:db8::/32`, `192.0.2.0/255.255.255.0`, or `8.8.8.8`). - * @param ret The resulting internal representation of a subnet. - * - * @returns Whether the operation succeeded or not. - */ -bool LookupSubNet(const std::string& strSubnet, CSubNet& ret) +bool LookupSubNet(const std::string& strSubnet, CSubNet& ret, DNSLookupFn dns_lookup_function) { if (!ValidAsCString(strSubnet)) { return false; @@ -822,7 +699,7 @@ bool LookupSubNet(const std::string& strSubnet, CSubNet& ret) std::string strAddress = strSubnet.substr(0, slash); // TODO: Use LookupHost(const std::string&, CNetAddr&, bool) instead to just get // one CNetAddr. - if (LookupHost(strAddress, vIP, 1, false)) + if (LookupHost(strAddress, vIP, 1, false, dns_lookup_function)) { CNetAddr network = vIP[0]; if (slash != strSubnet.npos) @@ -837,7 +714,7 @@ bool LookupSubNet(const std::string& strSubnet, CSubNet& ret) else // If not a valid number, try full netmask syntax { // Never allow lookup for netmask - if (LookupHost(strNetmask, vIP, 1, false)) { + if (LookupHost(strNetmask, vIP, 1, false, dns_lookup_function)) { ret = CSubNet(network, vIP[0]); return ret.IsValid(); } diff --git a/src/netbase.h b/src/netbase.h index 751f7eb3f0..e98a21ce1f 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -64,6 +64,11 @@ struct ProxyCredentials std::string password; }; +/** + * Wrapper for getaddrinfo(3). Do not use directly: call Lookup/LookupHost/LookupNumeric/LookupSubNet. + */ +std::vector<CNetAddr> WrappedGetAddrInfo(const std::string& name, bool allow_lookup); + enum Network ParseNetwork(const std::string& net); std::string GetNetworkName(enum Network net); /** Return a vector of publicly routable Network names; optionally append NET_UNROUTABLE. */ @@ -71,15 +76,107 @@ std::vector<std::string> GetNetworkNames(bool append_unroutable = false); bool SetProxy(enum Network net, const proxyType &addrProxy); bool GetProxy(enum Network net, proxyType &proxyInfoOut); bool IsProxy(const CNetAddr &addr); +/** + * Set the name proxy to use for all connections to nodes specified by a + * hostname. After setting this proxy, connecting to a node specified by a + * hostname won't result in a local lookup of said hostname, rather, connect to + * the node by asking the name proxy for a proxy connection to the hostname, + * effectively delegating the hostname lookup to the specified proxy. + * + * This delegation increases privacy for those who set the name proxy as they no + * longer leak their external hostname queries to their DNS servers. + * + * @returns Whether or not the operation succeeded. + * + * @note SOCKS5's support for UDP-over-SOCKS5 has been considered, but no SOCK5 + * server in common use (most notably Tor) actually implements UDP + * support, and a DNS resolver is beyond the scope of this project. + */ bool SetNameProxy(const proxyType &addrProxy); bool HaveNameProxy(); bool GetNameProxy(proxyType &nameProxyOut); -bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup); -bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup); -bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup); -bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions); -CService LookupNumeric(const std::string& name, int portDefault = 0); -bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet); + +using DNSLookupFn = std::function<std::vector<CNetAddr>(const std::string&, bool)>; +extern DNSLookupFn g_dns_lookup; + +/** + * Resolve a host string to its corresponding network addresses. + * + * @param name The string representing a host. Could be a name or a numerical + * IP address (IPv6 addresses in their bracketed form are + * allowed). + * @param[out] vIP The resulting network addresses to which the specified host + * string resolved. + * + * @returns Whether or not the specified host string successfully resolved to + * any resulting network addresses. + * + * @see Lookup(const std::string&, std::vector<CService>&, int, bool, unsigned int, DNSLookupFn) + * for additional parameter descriptions. + */ +bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); + +/** + * Resolve a host string to its first corresponding network address. + * + * @see LookupHost(const std::string&, std::vector<CNetAddr>&, unsigned int, bool, DNSLookupFn) + * for additional parameter descriptions. + */ +bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); + +/** + * Resolve a service string to its corresponding service. + * + * @param name The string representing a service. Could be a name or a + * numerical IP address (IPv6 addresses should be in their + * disambiguated bracketed form), optionally followed by a port + * number. (e.g. example.com:8333 or + * [2001:db8:85a3:8d3:1319:8a2e:370:7348]:420) + * @param[out] vAddr The resulting services to which the specified service string + * resolved. + * @param portDefault The default port for resulting services if not specified + * by the service string. + * @param fAllowLookup Whether or not hostname lookups are permitted. If yes, + * external queries may be performed. + * @param nMaxSolutions The maximum number of results we want, specifying 0 + * means "as many solutions as we get." + * + * @returns Whether or not the service string successfully resolved to any + * resulting services. + */ +bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup); + +/** + * Resolve a service string to its first corresponding service. + * + * @see Lookup(const std::string&, std::vector<CService>&, int, bool, unsigned int, DNSLookupFn) + * for additional parameter descriptions. + */ +bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup); + +/** + * Resolve a service string with a numeric IP to its first corresponding + * service. + * + * @returns The resulting CService if the resolution was successful, [::]:0 otherwise. + * + * @see Lookup(const std::string&, std::vector<CService>&, int, bool, unsigned int, DNSLookupFn) + * for additional parameter descriptions. + */ +CService LookupNumeric(const std::string& name, int portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup); + +/** + * Parse and resolve a specified subnet string into the appropriate internal + * representation. + * + * @param strSubnet A string representation of a subnet of the form `network + * address [ "/", ( CIDR-style suffix | netmask ) ]`(e.g. + * `2001:db8::/32`, `192.0.2.0/255.255.255.0`, or `8.8.8.8`). + * @param ret The resulting internal representation of a subnet. + * + * @returns Whether the operation succeeded or not. + */ +bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet, DNSLookupFn dns_lookup_function = g_dns_lookup); /** * Create a TCP socket in the given address family. @@ -93,14 +190,61 @@ std::unique_ptr<Sock> CreateSockTCP(const CService& address_family); */ extern std::function<std::unique_ptr<Sock>(const CService&)> CreateSock; -bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocketRet, int nTimeout, bool manual_connection); +/** + * Try to connect to the specified service on the specified socket. + * + * @param addrConnect The service to which to connect. + * @param hSocket The socket on which to connect. + * @param nTimeout Wait this many milliseconds for the connection to be + * established. + * @param manual_connection Whether or not the connection was manually requested + * (e.g. through the addnode RPC) + * + * @returns Whether or not a connection was successfully made. + */ +bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout, bool manual_connection); + +/** + * Connect to a specified destination service through a SOCKS5 proxy by first + * connecting to the SOCKS5 proxy. + * + * @param proxy The SOCKS5 proxy. + * @param strDest The destination service to which to connect. + * @param port The destination port. + * @param sock The socket on which to connect to the SOCKS5 proxy. + * @param nTimeout Wait this many milliseconds for the connection to the SOCKS5 + * proxy to be established. + * @param[out] outProxyConnectionFailed Whether or not the connection to the + * SOCKS5 proxy failed. + * + * @returns Whether or not the operation succeeded. + */ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed); + /** Disable or enable blocking-mode for a socket */ bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking); /** Set the TCP_NODELAY flag on a socket */ bool SetSocketNoDelay(const SOCKET& hSocket); void InterruptSocks5(bool interrupt); +/** + * Connect to a specified destination service through an already connected + * SOCKS5 proxy. + * + * @param strDest The destination fully-qualified domain name. + * @param port The destination port. + * @param auth The credentials with which to authenticate with the specified + * SOCKS5 proxy. + * @param sock The SOCKS5 proxy socket. + * + * @returns Whether or not the operation succeeded. + * + * @note The specified SOCKS5 proxy socket must already be connected to the + * SOCKS5 proxy. + * + * @see <a href="https://www.ietf.org/rfc/rfc1928.txt">RFC1928: SOCKS Protocol + * Version 5</a> + */ bool Socks5(const std::string& strDest, int port, const ProxyCredentials* auth, const Sock& socket); #endif // BITCOIN_NETBASE_H diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index ab6168a541..9927e925ac 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -295,7 +295,7 @@ void AddressBookPage::on_exportButton_clicked() // CSV is currently the only supported format QString filename = GUIUtil::getSaveFileName(this, tr("Export Address List"), QString(), - tr("Comma separated file (*.csv)"), nullptr); + tr("Comma separated file", "Name of CSV file format") + QLatin1String(" (*.csv)"), nullptr); if (filename.isNull()) return; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index f25b415820..791a29f7a0 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -52,7 +52,6 @@ #include <QThread> #include <QTimer> #include <QTranslator> -#include <QtGlobal> #if defined(QT_STATICPLUGIN) #include <QtPlugin> @@ -467,13 +466,6 @@ int GuiMain(int argc, char* argv[]) QApplication::setAttribute(Qt::AA_UseHighDpiPixmaps); QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling); -#if (QT_VERSION <= QT_VERSION_CHECK(5, 9, 8)) && defined(Q_OS_MACOS) - const auto os_name = QSysInfo::prettyProductName(); - if (os_name.startsWith("macOS 11") || os_name.startsWith("macOS 10.16")) { - QApplication::setStyle("fusion"); - } -#endif - BitcoinApplication app; QFontDatabase::addApplicationFont(":/fonts/monospace"); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 89d6deb70d..32890ed0fe 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -683,13 +683,13 @@ QString formatDurationStr(int secs) int seconds = secs % 60; if (days) - strList.append(QString(QObject::tr("%1 d")).arg(days)); + strList.append(QObject::tr("%1 d").arg(days)); if (hours) - strList.append(QString(QObject::tr("%1 h")).arg(hours)); + strList.append(QObject::tr("%1 h").arg(hours)); if (mins) - strList.append(QString(QObject::tr("%1 m")).arg(mins)); + strList.append(QObject::tr("%1 m").arg(mins)); if (seconds || (!days && !hours && !mins)) - strList.append(QString(QObject::tr("%1 s")).arg(seconds)); + strList.append(QObject::tr("%1 s").arg(seconds)); return strList.join(" "); } @@ -712,12 +712,12 @@ QString formatPingTime(std::chrono::microseconds ping_time) { return (ping_time == std::chrono::microseconds::max() || ping_time == 0us) ? QObject::tr("N/A") : - QString(QObject::tr("%1 ms")).arg(QString::number((int)(count_microseconds(ping_time) / 1000), 10)); + QObject::tr("%1 ms").arg(QString::number((int)(count_microseconds(ping_time) / 1000), 10)); } QString formatTimeOffset(int64_t nTimeOffset) { - return QString(QObject::tr("%1 s")).arg(QString::number((int)nTimeOffset, 10)); + return QObject::tr("%1 s").arg(QString::number((int)nTimeOffset, 10)); } QString formatNiceTimeOffset(qint64 secs) @@ -760,13 +760,13 @@ QString formatNiceTimeOffset(qint64 secs) QString formatBytes(uint64_t bytes) { if(bytes < 1024) - return QString(QObject::tr("%1 B")).arg(bytes); + return QObject::tr("%1 B").arg(bytes); if(bytes < 1024 * 1024) - return QString(QObject::tr("%1 KB")).arg(bytes / 1024); + return QObject::tr("%1 KB").arg(bytes / 1024); if(bytes < 1024 * 1024 * 1024) - return QString(QObject::tr("%1 MB")).arg(bytes / 1024 / 1024); + return QObject::tr("%1 MB").arg(bytes / 1024 / 1024); - return QString(QObject::tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); + return QObject::tr("%1 GB").arg(bytes / 1024 / 1024 / 1024); } qreal calculateIdealFontSize(int width, const QString& text, QFont font, qreal minPointSize, qreal font_size) { diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index d1f8d56100..17746b395b 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -141,7 +141,7 @@ void PSBTOperationsDialog::saveTransaction() { filename_suggestion.append(".psbt"); QString filename = GUIUtil::getSaveFileName(this, tr("Save Transaction Data"), filename_suggestion, - tr("Partially Signed Transaction (Binary) (*.psbt)"), &selected_filter); + tr("Partially Signed Transaction (Binary)", "Name of binary PSBT file format") + QLatin1String(" (*.psbt)"), &selected_filter); if (filename.isEmpty()) { return; } diff --git a/src/qt/qrimagewidget.cpp b/src/qt/qrimagewidget.cpp index 490826cbbb..a71c8831e9 100644 --- a/src/qt/qrimagewidget.cpp +++ b/src/qt/qrimagewidget.cpp @@ -120,7 +120,9 @@ void QRImageWidget::saveImage() { if (!GUIUtil::HasPixmap(this)) return; - QString fn = GUIUtil::getSaveFileName(this, tr("Save QR Code"), QString(), tr("PNG Image (*.png)"), nullptr); + QString fn = GUIUtil::getSaveFileName( + this, tr("Save QR Code"), QString(), + tr("PNG Image", "Name of PNG file format") + QLatin1String(" (*.png)"), nullptr); if (!fn.isEmpty()) { exportImage().save(fn); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index e401f46696..95e1ce2210 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -430,7 +430,7 @@ void SendCoinsDialog::on_sendButton_clicked() fileNameSuggestion.append(".psbt"); QString filename = GUIUtil::getSaveFileName(this, tr("Save Transaction Data"), fileNameSuggestion, - tr("Partially Signed Transaction (Binary) (*.psbt)"), &selectedFilter); + tr("Partially Signed Transaction (Binary)", "Name of binary PSBT file format") + QLatin1String(" (*.psbt)"), &selectedFilter); if (filename.isEmpty()) { return; } diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index f99a3e286d..42e08c6af7 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -357,7 +357,7 @@ void TransactionView::exportClicked() // CSV is currently the only supported format QString filename = GUIUtil::getSaveFileName(this, tr("Export Transaction History"), QString(), - tr("Comma separated file (*.csv)"), nullptr); + tr("Comma separated file", "Name of CSV file format") + QLatin1String(" (*.csv)"), nullptr); if (filename.isNull()) return; diff --git a/src/qt/walletview.cpp b/src/qt/walletview.cpp index b1e6b43e60..8612893683 100644 --- a/src/qt/walletview.cpp +++ b/src/qt/walletview.cpp @@ -273,7 +273,7 @@ void WalletView::backupWallet() { QString filename = GUIUtil::getSaveFileName(this, tr("Backup Wallet"), QString(), - tr("Wallet Data (*.dat)"), nullptr); + tr("Wallet Data", "Name of wallet data file format") + QLatin1String(" (*.dat)"), nullptr); if (filename.isEmpty()) return; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 99182a3767..b90dc9bf42 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1100,30 +1100,29 @@ static RPCHelpMan gettxoutsetinfo() static RPCHelpMan gettxout() { return RPCHelpMan{"gettxout", - "\nReturns details about an unspent transaction output.\n", - { - {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, - {"n", RPCArg::Type::NUM, RPCArg::Optional::NO, "vout number"}, - {"include_mempool", RPCArg::Type::BOOL, /* default */ "true", "Whether to include the mempool. Note that an unspent output that is spent in the mempool won't appear."}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at the tip of the chain"}, - {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, - {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT}, - {RPCResult::Type::OBJ, "scriptPubKey", "", - { - {RPCResult::Type::STR_HEX, "asm", ""}, - {RPCResult::Type::STR_HEX, "hex", ""}, - {RPCResult::Type::NUM, "reqSigs", "Number of required signatures"}, - {RPCResult::Type::STR_HEX, "type", "The type, eg pubkeyhash"}, - {RPCResult::Type::ARR, "addresses", "array of bitcoin addresses", - {{RPCResult::Type::STR, "address", "bitcoin address"}}}, - }}, - {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"}, - }}, - RPCExamples{ + "\nReturns details about an unspent transaction output.\n", + { + {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, + {"n", RPCArg::Type::NUM, RPCArg::Optional::NO, "vout number"}, + {"include_mempool", RPCArg::Type::BOOL, /* default */ "true", "Whether to include the mempool. Note that an unspent output that is spent in the mempool won't appear."}, + }, + { + RPCResult{"If the UTXO was not found", RPCResult::Type::NONE, "", ""}, + RPCResult{"Otherwise", RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at the tip of the chain"}, + {RPCResult::Type::NUM, "confirmations", "The number of confirmations"}, + {RPCResult::Type::STR_AMOUNT, "value", "The transaction value in " + CURRENCY_UNIT}, + {RPCResult::Type::OBJ, "scriptPubKey", "", { + {RPCResult::Type::STR_HEX, "asm", ""}, + {RPCResult::Type::STR_HEX, "hex", ""}, + {RPCResult::Type::NUM, "reqSigs", "Number of required signatures"}, + {RPCResult::Type::STR_HEX, "type", "The type, eg pubkeyhash"}, + {RPCResult::Type::ARR, "addresses", "array of bitcoin addresses", {{RPCResult::Type::STR, "address", "bitcoin address"}}}, + }}, + {RPCResult::Type::BOOL, "coinbase", "Coinbase or not"}, + }}, + }, + RPCExamples{ "\nGet unspent transactions\n" + HelpExampleCli("listunspent", "") + "\nView the details\n" @@ -2137,59 +2136,63 @@ public: static RPCHelpMan scantxoutset() { return RPCHelpMan{"scantxoutset", - "\nEXPERIMENTAL warning: this call may be removed or changed in future releases.\n" - "\nScans the unspent transaction output set for entries that match certain output descriptors.\n" - "Examples of output descriptors are:\n" - " addr(<address>) Outputs whose scriptPubKey corresponds to the specified address (does not include P2PK)\n" - " raw(<hex script>) Outputs whose scriptPubKey equals the specified hex scripts\n" - " combo(<pubkey>) P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH outputs for the given pubkey\n" - " pkh(<pubkey>) P2PKH outputs for the given pubkey\n" - " sh(multi(<n>,<pubkey>,<pubkey>,...)) P2SH-multisig outputs for the given threshold and pubkeys\n" - "\nIn the above, <pubkey> either refers to a fixed public key in hexadecimal notation, or to an xpub/xprv optionally followed by one\n" - "or more path elements separated by \"/\", and optionally ending in \"/*\" (unhardened), or \"/*'\" or \"/*h\" (hardened) to specify all\n" - "unhardened or hardened child keys.\n" - "In the latter case, a range needs to be specified by below if different from 1000.\n" - "For more information on output descriptors, see the documentation in the doc/descriptors.md file.\n", + "\nScans the unspent transaction output set for entries that match certain output descriptors.\n" + "Examples of output descriptors are:\n" + " addr(<address>) Outputs whose scriptPubKey corresponds to the specified address (does not include P2PK)\n" + " raw(<hex script>) Outputs whose scriptPubKey equals the specified hex scripts\n" + " combo(<pubkey>) P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH outputs for the given pubkey\n" + " pkh(<pubkey>) P2PKH outputs for the given pubkey\n" + " sh(multi(<n>,<pubkey>,<pubkey>,...)) P2SH-multisig outputs for the given threshold and pubkeys\n" + "\nIn the above, <pubkey> either refers to a fixed public key in hexadecimal notation, or to an xpub/xprv optionally followed by one\n" + "or more path elements separated by \"/\", and optionally ending in \"/*\" (unhardened), or \"/*'\" or \"/*h\" (hardened) to specify all\n" + "unhardened or hardened child keys.\n" + "In the latter case, a range needs to be specified by below if different from 1000.\n" + "For more information on output descriptors, see the documentation in the doc/descriptors.md file.\n", + { + {"action", RPCArg::Type::STR, RPCArg::Optional::NO, "The action to execute\n" + "\"start\" for starting a scan\n" + "\"abort\" for aborting the current scan (returns true when abort was successful)\n" + "\"status\" for progress report (in %) of the current scan"}, + {"scanobjects", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Array of scan objects. Required for \"start\" action\n" + "Every scan object is either a string descriptor or an object:", + { + {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "An output descriptor"}, + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with output descriptor and metadata", { - {"action", RPCArg::Type::STR, RPCArg::Optional::NO, "The action to execute\n" - " \"start\" for starting a scan\n" - " \"abort\" for aborting the current scan (returns true when abort was successful)\n" - " \"status\" for progress report (in %) of the current scan"}, - {"scanobjects", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Array of scan objects. Required for \"start\" action\n" - " Every scan object is either a string descriptor or an object:", - { - {"descriptor", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "An output descriptor"}, - {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "An object with output descriptor and metadata", - { - {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, - {"range", RPCArg::Type::RANGE, /* default */ "1000", "The range of HD chain indexes to explore (either end or [begin,end])"}, - }, - }, - }, + {"desc", RPCArg::Type::STR, RPCArg::Optional::NO, "An output descriptor"}, + {"range", RPCArg::Type::RANGE, /* default */ "1000", "The range of HD chain indexes to explore (either end or [begin,end])"}, + }}, + }, "[scanobjects,...]"}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", + }, + { + RPCResult{"When action=='abort'", RPCResult::Type::BOOL, "", ""}, + RPCResult{"When action=='status' and no scan is in progress", RPCResult::Type::NONE, "", ""}, + RPCResult{"When action=='status' and scan is in progress", RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::NUM, "progress", "The scan progress"}, + }}, + RPCResult{"When action=='start'", RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::BOOL, "success", "Whether the scan was completed"}, + {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs scanned"}, + {RPCResult::Type::NUM, "height", "The current block height (index)"}, + {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at the tip of the chain"}, + {RPCResult::Type::ARR, "unspents", "", + { + {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "success", "Whether the scan was completed"}, - {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs scanned"}, - {RPCResult::Type::NUM, "height", "The current block height (index)"}, - {RPCResult::Type::STR_HEX, "bestblock", "The hash of the block at the tip of the chain"}, - {RPCResult::Type::ARR, "unspents", "", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, - {RPCResult::Type::NUM, "vout", "The vout value"}, - {RPCResult::Type::STR_HEX, "scriptPubKey", "The script key"}, - {RPCResult::Type::STR, "desc", "A specialized descriptor for the matched scriptPubKey"}, - {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " of the unspent output"}, - {RPCResult::Type::NUM, "height", "Height of the unspent transaction output"}, - }}, - }}, - {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of all found unspent outputs in " + CURRENCY_UNIT}, + {RPCResult::Type::STR_HEX, "txid", "The transaction id"}, + {RPCResult::Type::NUM, "vout", "The vout value"}, + {RPCResult::Type::STR_HEX, "scriptPubKey", "The script key"}, + {RPCResult::Type::STR, "desc", "A specialized descriptor for the matched scriptPubKey"}, + {RPCResult::Type::STR_AMOUNT, "amount", "The total amount in " + CURRENCY_UNIT + " of the unspent output"}, + {RPCResult::Type::NUM, "height", "Height of the unspent transaction output"}, }}, - RPCExamples{""}, + }}, + {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount of all found unspent outputs in " + CURRENCY_UNIT}, + }}, + }, + RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR}); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 62b9d1138c..fd780ba782 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -505,83 +505,84 @@ static std::string gbt_vb_name(const Consensus::DeploymentPos pos) { static RPCHelpMan getblocktemplate() { return RPCHelpMan{"getblocktemplate", - "\nIf the request parameters include a 'mode' key, that is used to explicitly select between the default 'template' request or a 'proposal'.\n" - "It returns data needed to construct a block to work on.\n" - "For full specification, see BIPs 22, 23, 9, and 145:\n" - " https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki\n" - " https://github.com/bitcoin/bips/blob/master/bip-0023.mediawiki\n" - " https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate_changes\n" - " https://github.com/bitcoin/bips/blob/master/bip-0145.mediawiki\n", + "\nIf the request parameters include a 'mode' key, that is used to explicitly select between the default 'template' request or a 'proposal'.\n" + "It returns data needed to construct a block to work on.\n" + "For full specification, see BIPs 22, 23, 9, and 145:\n" + " https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki\n" + " https://github.com/bitcoin/bips/blob/master/bip-0023.mediawiki\n" + " https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate_changes\n" + " https://github.com/bitcoin/bips/blob/master/bip-0145.mediawiki\n", + { + {"template_request", RPCArg::Type::OBJ, "{}", "Format of the template", + { + {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, + {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings", { - {"template_request", RPCArg::Type::OBJ, "{}", "Format of the template", - { - {"mode", RPCArg::Type::STR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "This must be set to \"template\", \"proposal\" (see BIP 23), or omitted"}, - {"capabilities", RPCArg::Type::ARR, /* treat as named arg */ RPCArg::Optional::OMITTED_NAMED_ARG, "A list of strings", - { - {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, - }, - }, - {"rules", RPCArg::Type::ARR, RPCArg::Optional::NO, "A list of strings", - { - {"segwit", RPCArg::Type::STR, RPCArg::Optional::NO, "(literal) indicates client side segwit support"}, - {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "other client side supported softfork deployment"}, - }, - }, - }, + {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "client side supported feature, 'longpoll', 'coinbasevalue', 'proposal', 'serverlist', 'workid'"}, + }}, + {"rules", RPCArg::Type::ARR, RPCArg::Optional::NO, "A list of strings", + { + {"segwit", RPCArg::Type::STR, RPCArg::Optional::NO, "(literal) indicates client side segwit support"}, + {"str", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "other client side supported softfork deployment"}, + }}, + }, "\"template_request\""}, - }, - RPCResult{ - RPCResult::Type::OBJ, "", "", + }, + { + RPCResult{"If the proposal was accepted with mode=='proposal'", RPCResult::Type::NONE, "", ""}, + RPCResult{"If the proposal was not accepted with mode=='proposal'", RPCResult::Type::STR, "", "According to BIP22"}, + RPCResult{"Otherwise", RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::NUM, "version", "The preferred block version"}, + {RPCResult::Type::ARR, "rules", "specific block rules that are to be enforced", + { + {RPCResult::Type::STR, "", "name of a rule the client must understand to some extent; see BIP 9 for format"}, + }}, + {RPCResult::Type::OBJ_DYN, "vbavailable", "set of pending, supported versionbit (BIP 9) softfork deployments", + { + {RPCResult::Type::NUM, "rulename", "identifies the bit number as indicating acceptance and readiness for the named softfork rule"}, + }}, + {RPCResult::Type::NUM, "vbrequired", "bit mask of versionbits the server requires set in submissions"}, + {RPCResult::Type::STR, "previousblockhash", "The hash of current highest block"}, + {RPCResult::Type::ARR, "transactions", "contents of non-coinbase transactions that should be included in the next block", + { + {RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::NUM, "version", "The preferred block version"}, - {RPCResult::Type::ARR, "rules", "specific block rules that are to be enforced", - { - {RPCResult::Type::STR, "", "name of a rule the client must understand to some extent; see BIP 9 for format"}, - }}, - {RPCResult::Type::OBJ_DYN, "vbavailable", "set of pending, supported versionbit (BIP 9) softfork deployments", - { - {RPCResult::Type::NUM, "rulename", "identifies the bit number as indicating acceptance and readiness for the named softfork rule"}, - }}, - {RPCResult::Type::NUM, "vbrequired", "bit mask of versionbits the server requires set in submissions"}, - {RPCResult::Type::STR, "previousblockhash", "The hash of current highest block"}, - {RPCResult::Type::ARR, "transactions", "contents of non-coinbase transactions that should be included in the next block", - { - {RPCResult::Type::OBJ, "", "", - { - {RPCResult::Type::STR_HEX, "data", "transaction data encoded in hexadecimal (byte-for-byte)"}, - {RPCResult::Type::STR_HEX, "txid", "transaction id encoded in little-endian hexadecimal"}, - {RPCResult::Type::STR_HEX, "hash", "hash encoded in little-endian hexadecimal (including witness data)"}, - {RPCResult::Type::ARR, "depends", "array of numbers", - { - {RPCResult::Type::NUM, "", "transactions before this one (by 1-based index in 'transactions' list) that must be present in the final block if this one is"}, - }}, - {RPCResult::Type::NUM, "fee", "difference in value between transaction inputs and outputs (in satoshis); for coinbase transactions, this is a negative Number of the total collected block fees (ie, not including the block subsidy); if key is not present, fee is unknown and clients MUST NOT assume there isn't one"}, - {RPCResult::Type::NUM, "sigops", "total SigOps cost, as counted for purposes of block limits; if key is not present, sigop cost is unknown and clients MUST NOT assume it is zero"}, - {RPCResult::Type::NUM, "weight", "total transaction weight, as counted for purposes of block limits"}, - }}, - }}, - {RPCResult::Type::OBJ_DYN, "coinbaseaux", "data that should be included in the coinbase's scriptSig content", + {RPCResult::Type::STR_HEX, "data", "transaction data encoded in hexadecimal (byte-for-byte)"}, + {RPCResult::Type::STR_HEX, "txid", "transaction id encoded in little-endian hexadecimal"}, + {RPCResult::Type::STR_HEX, "hash", "hash encoded in little-endian hexadecimal (including witness data)"}, + {RPCResult::Type::ARR, "depends", "array of numbers", { - {RPCResult::Type::STR_HEX, "key", "values must be in the coinbase (keys may be ignored)"}, + {RPCResult::Type::NUM, "", "transactions before this one (by 1-based index in 'transactions' list) that must be present in the final block if this one is"}, }}, - {RPCResult::Type::NUM, "coinbasevalue", "maximum allowable input to coinbase transaction, including the generation award and transaction fees (in satoshis)"}, - {RPCResult::Type::STR, "longpollid", "an id to include with a request to longpoll on an update to this template"}, - {RPCResult::Type::STR, "target", "The hash target"}, - {RPCResult::Type::NUM_TIME, "mintime", "The minimum timestamp appropriate for the next block time, expressed in " + UNIX_EPOCH_TIME}, - {RPCResult::Type::ARR, "mutable", "list of ways the block template may be changed", - { - {RPCResult::Type::STR, "value", "A way the block template may be changed, e.g. 'time', 'transactions', 'prevblock'"}, - }}, - {RPCResult::Type::STR_HEX, "noncerange", "A range of valid nonces"}, - {RPCResult::Type::NUM, "sigoplimit", "limit of sigops in blocks"}, - {RPCResult::Type::NUM, "sizelimit", "limit of block size"}, - {RPCResult::Type::NUM, "weightlimit", "limit of block weight"}, - {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME}, - {RPCResult::Type::STR, "bits", "compressed target of next block"}, - {RPCResult::Type::NUM, "height", "The height of the next block"}, - {RPCResult::Type::STR, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"} + {RPCResult::Type::NUM, "fee", "difference in value between transaction inputs and outputs (in satoshis); for coinbase transactions, this is a negative Number of the total collected block fees (ie, not including the block subsidy); if key is not present, fee is unknown and clients MUST NOT assume there isn't one"}, + {RPCResult::Type::NUM, "sigops", "total SigOps cost, as counted for purposes of block limits; if key is not present, sigop cost is unknown and clients MUST NOT assume it is zero"}, + {RPCResult::Type::NUM, "weight", "total transaction weight, as counted for purposes of block limits"}, }}, - RPCExamples{ + }}, + {RPCResult::Type::OBJ_DYN, "coinbaseaux", "data that should be included in the coinbase's scriptSig content", + { + {RPCResult::Type::STR_HEX, "key", "values must be in the coinbase (keys may be ignored)"}, + }}, + {RPCResult::Type::NUM, "coinbasevalue", "maximum allowable input to coinbase transaction, including the generation award and transaction fees (in satoshis)"}, + {RPCResult::Type::STR, "longpollid", "an id to include with a request to longpoll on an update to this template"}, + {RPCResult::Type::STR, "target", "The hash target"}, + {RPCResult::Type::NUM_TIME, "mintime", "The minimum timestamp appropriate for the next block time, expressed in " + UNIX_EPOCH_TIME}, + {RPCResult::Type::ARR, "mutable", "list of ways the block template may be changed", + { + {RPCResult::Type::STR, "value", "A way the block template may be changed, e.g. 'time', 'transactions', 'prevblock'"}, + }}, + {RPCResult::Type::STR_HEX, "noncerange", "A range of valid nonces"}, + {RPCResult::Type::NUM, "sigoplimit", "limit of sigops in blocks"}, + {RPCResult::Type::NUM, "sizelimit", "limit of block size"}, + {RPCResult::Type::NUM, "weightlimit", "limit of block weight"}, + {RPCResult::Type::NUM_TIME, "curtime", "current timestamp in " + UNIX_EPOCH_TIME}, + {RPCResult::Type::STR, "bits", "compressed target of next block"}, + {RPCResult::Type::NUM, "height", "The height of the next block"}, + {RPCResult::Type::STR, "default_witness_commitment", /* optional */ true, "a valid witness commitment for the unmodified block template"}, + }}, + }, + RPCExamples{ HelpExampleCli("getblocktemplate", "'{\"rules\": [\"segwit\"]}'") + HelpExampleRpc("getblocktemplate", "{\"rules\": [\"segwit\"]}") }, @@ -940,14 +941,17 @@ static RPCHelpMan submitblock() { // We allow 2 arguments for compliance with BIP22. Argument 2 is ignored. return RPCHelpMan{"submitblock", - "\nAttempts to submit new block to network.\n" - "See https://en.bitcoin.it/wiki/BIP_0022 for full specification.\n", - { - {"hexdata", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hex-encoded block data to submit"}, - {"dummy", RPCArg::Type::STR, /* default */ "ignored", "dummy value, for compatibility with BIP22. This value is ignored."}, - }, - RPCResult{RPCResult::Type::NONE, "", "Returns JSON Null when valid, a string according to BIP22 otherwise"}, - RPCExamples{ + "\nAttempts to submit new block to network.\n" + "See https://en.bitcoin.it/wiki/BIP_0022 for full specification.\n", + { + {"hexdata", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hex-encoded block data to submit"}, + {"dummy", RPCArg::Type::STR, /* default */ "ignored", "dummy value, for compatibility with BIP22. This value is ignored."}, + }, + { + RPCResult{"If the block was accepted", RPCResult::Type::NONE, "", ""}, + RPCResult{"Otherwise", RPCResult::Type::STR, "", "According to BIP22"}, + }, + RPCExamples{ HelpExampleCli("submitblock", "\"mydata\"") + HelpExampleRpc("submitblock", "\"mydata\"") }, diff --git a/src/rpc/request.h b/src/rpc/request.h index de3a4ae840..27d06f3c92 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -34,19 +34,19 @@ public: UniValue id; std::string strMethod; UniValue params; - bool fHelp; + enum Mode { EXECUTE, GET_HELP, GET_ARGS } mode = EXECUTE; std::string URI; std::string authUser; std::string peerAddr; const util::Ref& context; - explicit JSONRPCRequest(const util::Ref& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {} + explicit JSONRPCRequest(const util::Ref& context) : id(NullUniValue), params(NullUniValue), context(context) {} //! Initializes request information from another request object and the //! given context. The implementation should be updated if any members are //! added or removed above. JSONRPCRequest(const JSONRPCRequest& other, const util::Ref& context) - : id(other.id), strMethod(other.strMethod), params(other.params), fHelp(other.fHelp), URI(other.URI), + : id(other.id), strMethod(other.strMethod), params(other.params), mode(other.mode), URI(other.URI), authUser(other.authUser), peerAddr(other.peerAddr), context(context) { } diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9a9b3713f3..39938f4eb9 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -88,7 +88,7 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& sort(vCommands.begin(), vCommands.end()); JSONRPCRequest jreq(helpreq); - jreq.fHelp = true; + jreq.mode = JSONRPCRequest::GET_HELP; jreq.params = UniValue(); for (const std::pair<std::string, const CRPCCommand*>& command : vCommands) @@ -149,7 +149,7 @@ static RPCHelpMan help() } if (strCommand == "dump_all_command_conversions") { // Used for testing only, undocumented - return tableRPC.dumpArgMap(); + return tableRPC.dumpArgMap(jsonRequest); } return tableRPC.help(strCommand, jsonRequest); @@ -437,6 +437,16 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c return out; } +static bool ExecuteCommands(const std::vector<const CRPCCommand*>& commands, const JSONRPCRequest& request, UniValue& result) +{ + for (const auto& command : commands) { + if (ExecuteCommand(*command, request, result, &command == &commands.back())) { + return true; + } + } + return false; +} + UniValue CRPCTable::execute(const JSONRPCRequest &request) const { // Return immediately if in warmup @@ -450,10 +460,8 @@ UniValue CRPCTable::execute(const JSONRPCRequest &request) const auto it = mapCommands.find(request.strMethod); if (it != mapCommands.end()) { UniValue result; - for (const auto& command : it->second) { - if (ExecuteCommand(*command, request, result, &command == &it->second.back())) { - return result; - } + if (ExecuteCommands(it->second, request, result)) { + return result; } } throw JSONRPCError(RPC_METHOD_NOT_FOUND, "Method not found"); @@ -484,13 +492,18 @@ std::vector<std::string> CRPCTable::listCommands() const return commandList; } -UniValue CRPCTable::dumpArgMap() const +UniValue CRPCTable::dumpArgMap(const JSONRPCRequest& args_request) const { + JSONRPCRequest request(args_request); + request.mode = JSONRPCRequest::GET_ARGS; + UniValue ret{UniValue::VARR}; for (const auto& cmd : mapCommands) { - for (const auto& c : cmd.second) { - const auto help = RpcMethodFnType(c->unique_id)(); - help.AppendArgMap(ret); + UniValue result; + if (ExecuteCommands(cmd.second, request, result)) { + for (const auto& values : result.getValues()) { + ret.push_back(values); + } } } return ret; diff --git a/src/rpc/server.h b/src/rpc/server.h index fe5a791e1e..03967020c2 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -148,7 +148,7 @@ public: /** * Return all named arguments that need to be converted by the client from string to another JSON type */ - UniValue dumpArgMap() const; + UniValue dumpArgMap(const JSONRPCRequest& request) const; /** * Appends a CRPCCommand to the dispatch table. diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index e890c0108a..44e58cb75f 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -459,6 +459,21 @@ std::string RPCExamples::ToDescriptionString() const return m_examples.empty() ? m_examples : "\nExamples:\n" + m_examples; } +UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) +{ + if (request.mode == JSONRPCRequest::GET_ARGS) { + return GetArgMap(); + } + /* + * Check if the given request is valid according to this command or if + * the user is asking for help information, and throw help when appropriate. + */ + if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) { + throw std::runtime_error(ToString()); + } + return m_fun(*this, request); +} + bool RPCHelpMan::IsValidNumArgs(size_t num_args) const { size_t num_required_args = 0; @@ -532,8 +547,9 @@ std::string RPCHelpMan::ToString() const return ret; } -void RPCHelpMan::AppendArgMap(UniValue& arr) const +UniValue RPCHelpMan::GetArgMap() const { + UniValue arr{UniValue::VARR}; for (int i{0}; i < int(m_args.size()); ++i) { const auto& arg = m_args.at(i); std::vector<std::string> arg_names; @@ -548,6 +564,7 @@ void RPCHelpMan::AppendArgMap(UniValue& arr) const arr.push_back(map); } } + return arr; } std::string RPCArg::GetFirstName() const diff --git a/src/rpc/util.h b/src/rpc/util.h index c54ce85f60..94c2d2d626 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -333,26 +333,12 @@ public: using RPCMethodImpl = std::function<UniValue(const RPCHelpMan&, const JSONRPCRequest&)>; RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples, RPCMethodImpl fun); + UniValue HandleRequest(const JSONRPCRequest& request); std::string ToString() const; - /** Append the named args that need to be converted from string to another JSON type */ - void AppendArgMap(UniValue& arr) const; - UniValue HandleRequest(const JSONRPCRequest& request) - { - Check(request); - return m_fun(*this, request); - } + /** Return the named args that need to be converted from string to another JSON type */ + UniValue GetArgMap() const; /** If the supplied number of args is neither too small nor too high */ bool IsValidNumArgs(size_t num_args) const; - /** - * Check if the given request is valid according to this command or if - * the user is asking for help information, and throw help when appropriate. - */ - inline void Check(const JSONRPCRequest& request) const { - if (request.fHelp || !IsValidNumArgs(request.params.size())) { - throw std::runtime_error(ToString()); - } - } - std::vector<std::string> GetArgNames() const; const std::string m_name; diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp new file mode 100644 index 0000000000..7be8b13743 --- /dev/null +++ b/src/test/fuzz/netbase_dns_lookup.cpp @@ -0,0 +1,71 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <netaddress.h> +#include <netbase.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> + +#include <cstdint> +#include <string> +#include <vector> + +FUZZ_TARGET(netbase_dns_lookup) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const std::string name = fuzzed_data_provider.ConsumeRandomLengthString(512); + const unsigned int max_results = fuzzed_data_provider.ConsumeIntegral<unsigned int>(); + const bool allow_lookup = fuzzed_data_provider.ConsumeBool(); + const int default_port = fuzzed_data_provider.ConsumeIntegral<int>(); + + auto fuzzed_dns_lookup_function = [&](const std::string&, bool) { + std::vector<CNetAddr> resolved_addresses; + while (fuzzed_data_provider.ConsumeBool()) { + resolved_addresses.push_back(ConsumeNetAddr(fuzzed_data_provider)); + } + return resolved_addresses; + }; + + { + std::vector<CNetAddr> resolved_addresses; + if (LookupHost(name, resolved_addresses, max_results, allow_lookup, fuzzed_dns_lookup_function)) { + for (const CNetAddr& resolved_address : resolved_addresses) { + assert(!resolved_address.IsInternal()); + } + } + assert(resolved_addresses.size() <= max_results || max_results == 0); + } + { + CNetAddr resolved_address; + if (LookupHost(name, resolved_address, allow_lookup, fuzzed_dns_lookup_function)) { + assert(!resolved_address.IsInternal()); + } + } + { + std::vector<CService> resolved_services; + if (Lookup(name, resolved_services, default_port, allow_lookup, max_results, fuzzed_dns_lookup_function)) { + for (const CNetAddr& resolved_service : resolved_services) { + assert(!resolved_service.IsInternal()); + } + } + assert(resolved_services.size() <= max_results || max_results == 0); + } + { + CService resolved_service; + if (Lookup(name, resolved_service, default_port, allow_lookup, fuzzed_dns_lookup_function)) { + assert(!resolved_service.IsInternal()); + } + } + { + CService resolved_service = LookupNumeric(name, default_port, fuzzed_dns_lookup_function); + assert(!resolved_service.IsInternal()); + } + { + CSubNet resolved_subnet; + if (LookupSubNet(name, resolved_subnet, fuzzed_dns_lookup_function)) { + assert(resolved_subnet.IsValid()); + } + } +} diff --git a/src/test/fuzz/parse_numbers.cpp b/src/test/fuzz/parse_numbers.cpp index ddd2bcfba3..1ad5fb6a05 100644 --- a/src/test/fuzz/parse_numbers.cpp +++ b/src/test/fuzz/parse_numbers.cpp @@ -18,6 +18,9 @@ FUZZ_TARGET(parse_numbers) double d; (void)ParseDouble(random_string, &d); + uint8_t u8; + (void)ParseUInt8(random_string, &u8); + int32_t i32; (void)ParseInt32(random_string, &i32); (void)atoi(random_string); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index d8c536e8b1..cdddad82b3 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -259,6 +259,16 @@ void SetFuzzedErrNo(FuzzedDataProvider& fuzzed_data_provider, const std::array<T errno = fuzzed_data_provider.PickValueInArray(errnos); } +/* + * Sets a fuzzed errno in the range [0, 133 (EHWPOISON)]. Can be used from functions emulating + * standard library functions that set errno, or in other contexts where the value of errno + * might be relevant for the execution path that will be taken. + */ +inline void SetFuzzedErrNo(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + errno = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 133); +} + /** * Returns a byte vector of specified size regardless of the number of remaining bytes available * from the fuzzer. Pads with zero value bytes if needed to achieve the specified size. @@ -345,6 +355,7 @@ public: FILE* open() { + SetFuzzedErrNo(m_fuzzed_data_provider); if (m_fuzzed_data_provider.ConsumeBool()) { return nullptr; } @@ -386,6 +397,7 @@ public: static ssize_t read(void* cookie, char* buf, size_t size) { FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie; + SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider); if (buf == nullptr || size == 0 || fuzzed_file->m_fuzzed_data_provider.ConsumeBool()) { return fuzzed_file->m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; } @@ -404,6 +416,7 @@ public: static ssize_t write(void* cookie, const char* buf, size_t size) { FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie; + SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider); const ssize_t n = fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange<ssize_t>(0, size); if (AdditionOverflow(fuzzed_file->m_offset, (int64_t)n)) { return fuzzed_file->m_fuzzed_data_provider.ConsumeBool() ? 0 : -1; @@ -414,8 +427,9 @@ public: static int seek(void* cookie, int64_t* offset, int whence) { - assert(whence == SEEK_SET || whence == SEEK_CUR); // SEEK_END not implemented yet. + assert(whence == SEEK_SET || whence == SEEK_CUR || whence == SEEK_END); FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie; + SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider); int64_t new_offset = 0; if (whence == SEEK_SET) { new_offset = *offset; @@ -424,6 +438,12 @@ public: return -1; } new_offset = fuzzed_file->m_offset + *offset; + } else if (whence == SEEK_END) { + const int64_t n = fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange<int64_t>(0, 4096); + if (AdditionOverflow(n, *offset)) { + return -1; + } + new_offset = n + *offset; } if (new_offset < 0) { return -1; @@ -436,6 +456,7 @@ public: static int close(void* cookie) { FuzzedFileProvider* fuzzed_file = (FuzzedFileProvider*)cookie; + SetFuzzedErrNo(fuzzed_file->m_fuzzed_data_provider); return fuzzed_file->m_fuzzed_data_provider.ConsumeIntegralInRange<int>(-1, 0); } }; diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp new file mode 100644 index 0000000000..e1a21b6c53 --- /dev/null +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -0,0 +1,34 @@ +// 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. + +#include <chainparamsbase.h> +#include <test/fuzz/FuzzedDataProvider.h> +#include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> +#include <test/util/setup_common.h> +#include <txmempool.h> +#include <util/time.h> +#include <validation.h> + +#include <cstdint> +#include <vector> + +void initialize_validation_load_mempool() +{ + static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(); +} + +FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool) +{ + FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + SetMockTime(ConsumeTime(fuzzed_data_provider)); + FuzzedFileProvider fuzzed_file_provider = ConsumeFile(fuzzed_data_provider); + + CTxMemPool pool{}; + auto fuzzed_fopen = [&](const fs::path&, const char*) { + return fuzzed_file_provider.open(); + }; + (void)LoadMempool(pool, ::ChainstateActive(), fuzzed_fopen); + (void)DumpMempool(pool, fuzzed_fopen, true); +} diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 810665877d..2e0fc7e48f 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -36,7 +36,6 @@ UniValue RPCTestingSetup::CallRPC(std::string args) JSONRPCRequest request(context); request.strMethod = strMethod; request.params = RPCConvertValues(strMethod, vArgs); - request.fHelp = false; if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished(); try { UniValue result = tableRPC.execute(request); diff --git a/src/test/sanity_tests.cpp b/src/test/sanity_tests.cpp index 3e4b963fe3..496292875d 100644 --- a/src/test/sanity_tests.cpp +++ b/src/test/sanity_tests.cpp @@ -13,7 +13,6 @@ BOOST_FIXTURE_TEST_SUITE(sanity_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(basic_sanity) { - BOOST_CHECK_MESSAGE(glibc_sanity_test() == true, "libc sanity test"); BOOST_CHECK_MESSAGE(glibcxx_sanity_test() == true, "stdlib sanity test"); BOOST_CHECK_MESSAGE(ECC_InitSanityCheck() == true, "secp256k1 sanity test"); BOOST_CHECK_MESSAGE(ChronoSanityCheck() == true, "chrono epoch test"); diff --git a/src/test/sock_tests.cpp b/src/test/sock_tests.cpp index ed9780dfb5..400de875b7 100644 --- a/src/test/sock_tests.cpp +++ b/src/test/sock_tests.cpp @@ -4,11 +4,13 @@ #include <compat.h> #include <test/util/setup_common.h> +#include <threadinterrupt.h> #include <util/sock.h> #include <util/system.h> #include <boost/test/unit_test.hpp> +#include <cassert> #include <thread> using namespace std::chrono_literals; @@ -144,6 +146,35 @@ BOOST_AUTO_TEST_CASE(wait) waiter.join(); } +BOOST_AUTO_TEST_CASE(recv_until_terminator_limit) +{ + constexpr auto timeout = 1min; // High enough so that it is never hit. + CThreadInterrupt interrupt; + int s[2]; + CreateSocketPair(s); + + Sock sock_send(s[0]); + Sock sock_recv(s[1]); + + std::thread receiver([&sock_recv, &timeout, &interrupt]() { + constexpr size_t max_data{10}; + bool threw_as_expected{false}; + // BOOST_CHECK_EXCEPTION() writes to some variables shared with the main thread which + // creates a data race. So mimic it manually. + try { + sock_recv.RecvUntilTerminator('\n', timeout, interrupt, max_data); + } catch (const std::runtime_error& e) { + threw_as_expected = HasReason("too many bytes without a terminator")(e); + } + assert(threw_as_expected); + }); + + BOOST_REQUIRE_NO_THROW(sock_send.SendComplete("1234567", timeout, interrupt)); + BOOST_REQUIRE_NO_THROW(sock_send.SendComplete("89a\n", timeout, interrupt)); + + receiver.join(); +} + #endif /* WIN32 */ BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 55f1069d0c..98a322eae5 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -38,6 +38,7 @@ #include <boost/test/unit_test.hpp> using namespace std::literals; +static const std::string STRING_WITH_EMBEDDED_NULL_CHAR{"1"s "\0" "1"s}; /* defined in logging.cpp */ namespace BCLog { @@ -1272,7 +1273,7 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) // Parsing strings with embedded NUL characters should fail BOOST_CHECK(!ParseMoney("\0-1"s, ret)); - BOOST_CHECK(!ParseMoney("\0" "1"s, ret)); + BOOST_CHECK(!ParseMoney(STRING_WITH_EMBEDDED_NULL_CHAR, ret)); BOOST_CHECK(!ParseMoney("1\0"s, ret)); } @@ -1450,9 +1451,7 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32) BOOST_CHECK(!ParseInt32("aap", &n)); BOOST_CHECK(!ParseInt32("0x1", &n)); // no hex BOOST_CHECK(!ParseInt32("0x1", &n)); // no hex - const char test_bytes[] = {'1', 0, '1'}; - std::string teststr(test_bytes, sizeof(test_bytes)); - BOOST_CHECK(!ParseInt32(teststr, &n)); // no embedded NULs + BOOST_CHECK(!ParseInt32(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); // Overflow and underflow BOOST_CHECK(!ParseInt32("-2147483649", nullptr)); BOOST_CHECK(!ParseInt32("2147483648", nullptr)); @@ -1480,9 +1479,7 @@ BOOST_AUTO_TEST_CASE(test_ParseInt64) BOOST_CHECK(!ParseInt64("1a", &n)); BOOST_CHECK(!ParseInt64("aap", &n)); BOOST_CHECK(!ParseInt64("0x1", &n)); // no hex - const char test_bytes[] = {'1', 0, '1'}; - std::string teststr(test_bytes, sizeof(test_bytes)); - BOOST_CHECK(!ParseInt64(teststr, &n)); // no embedded NULs + BOOST_CHECK(!ParseInt64(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); // Overflow and underflow BOOST_CHECK(!ParseInt64("-9223372036854775809", nullptr)); BOOST_CHECK(!ParseInt64("9223372036854775808", nullptr)); @@ -1490,6 +1487,42 @@ BOOST_AUTO_TEST_CASE(test_ParseInt64) BOOST_CHECK(!ParseInt64("32482348723847471234", nullptr)); } +BOOST_AUTO_TEST_CASE(test_ParseUInt8) +{ + uint8_t n; + // Valid values + BOOST_CHECK(ParseUInt8("255", nullptr)); + BOOST_CHECK(ParseUInt8("0", &n) && n == 0); + BOOST_CHECK(ParseUInt8("255", &n) && n == 255); + BOOST_CHECK(ParseUInt8("0255", &n) && n == 255); // no octal + BOOST_CHECK(ParseUInt8("255", &n) && n == static_cast<uint8_t>(255)); + BOOST_CHECK(ParseUInt8("+255", &n) && n == 255); + BOOST_CHECK(ParseUInt8("00000000000000000012", &n) && n == 12); + BOOST_CHECK(ParseUInt8("00000000000000000000", &n) && n == 0); + // Invalid values + BOOST_CHECK(!ParseUInt8("-00000000000000000000", &n)); + BOOST_CHECK(!ParseUInt8("", &n)); + BOOST_CHECK(!ParseUInt8(" 1", &n)); // no padding inside + BOOST_CHECK(!ParseUInt8(" -1", &n)); + BOOST_CHECK(!ParseUInt8("++1", &n)); + BOOST_CHECK(!ParseUInt8("+-1", &n)); + BOOST_CHECK(!ParseUInt8("-+1", &n)); + BOOST_CHECK(!ParseUInt8("--1", &n)); + BOOST_CHECK(!ParseUInt8("-1", &n)); + BOOST_CHECK(!ParseUInt8("1 ", &n)); + BOOST_CHECK(!ParseUInt8("1a", &n)); + BOOST_CHECK(!ParseUInt8("aap", &n)); + BOOST_CHECK(!ParseUInt8("0x1", &n)); // no hex + BOOST_CHECK(!ParseUInt8("0x1", &n)); // no hex + BOOST_CHECK(!ParseUInt8(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); + // Overflow and underflow + BOOST_CHECK(!ParseUInt8("-255", &n)); + BOOST_CHECK(!ParseUInt8("256", &n)); + BOOST_CHECK(!ParseUInt8("-123", &n)); + BOOST_CHECK(!ParseUInt8("-123", nullptr)); + BOOST_CHECK(!ParseUInt8("256", nullptr)); +} + BOOST_AUTO_TEST_CASE(test_ParseUInt32) { uint32_t n; @@ -1519,9 +1552,7 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt32) BOOST_CHECK(!ParseUInt32("aap", &n)); BOOST_CHECK(!ParseUInt32("0x1", &n)); // no hex BOOST_CHECK(!ParseUInt32("0x1", &n)); // no hex - const char test_bytes[] = {'1', 0, '1'}; - std::string teststr(test_bytes, sizeof(test_bytes)); - BOOST_CHECK(!ParseUInt32(teststr, &n)); // no embedded NULs + BOOST_CHECK(!ParseUInt32(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); // Overflow and underflow BOOST_CHECK(!ParseUInt32("-2147483648", &n)); BOOST_CHECK(!ParseUInt32("4294967296", &n)); @@ -1550,9 +1581,7 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt64) BOOST_CHECK(!ParseUInt64("1a", &n)); BOOST_CHECK(!ParseUInt64("aap", &n)); BOOST_CHECK(!ParseUInt64("0x1", &n)); // no hex - const char test_bytes[] = {'1', 0, '1'}; - std::string teststr(test_bytes, sizeof(test_bytes)); - BOOST_CHECK(!ParseUInt64(teststr, &n)); // no embedded NULs + BOOST_CHECK(!ParseUInt64(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); // Overflow and underflow BOOST_CHECK(!ParseUInt64("-9223372036854775809", nullptr)); BOOST_CHECK(!ParseUInt64("18446744073709551616", nullptr)); @@ -1582,9 +1611,7 @@ BOOST_AUTO_TEST_CASE(test_ParseDouble) BOOST_CHECK(!ParseDouble("1a", &n)); BOOST_CHECK(!ParseDouble("aap", &n)); BOOST_CHECK(!ParseDouble("0x1", &n)); // no hex - const char test_bytes[] = {'1', 0, '1'}; - std::string teststr(test_bytes, sizeof(test_bytes)); - BOOST_CHECK(!ParseDouble(teststr, &n)); // no embedded NULs + BOOST_CHECK(!ParseDouble(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); // Overflow and underflow BOOST_CHECK(!ParseDouble("-1e10000", nullptr)); BOOST_CHECK(!ParseDouble("1e10000", nullptr)); diff --git a/src/util/sock.cpp b/src/util/sock.cpp index e13c52a16a..f9ecfef5d4 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -175,7 +175,8 @@ void Sock::SendComplete(const std::string& data, std::string Sock::RecvUntilTerminator(uint8_t terminator, std::chrono::milliseconds timeout, - CThreadInterrupt& interrupt) const + CThreadInterrupt& interrupt, + size_t max_data) const { const auto deadline = GetTime<std::chrono::milliseconds>() + timeout; std::string data; @@ -190,9 +191,14 @@ std::string Sock::RecvUntilTerminator(uint8_t terminator, // at a time is about 50 times slower. for (;;) { + if (data.size() >= max_data) { + throw std::runtime_error( + strprintf("Received too many bytes without a terminator (%u)", data.size())); + } + char buf[512]; - const ssize_t peek_ret{Recv(buf, sizeof(buf), MSG_PEEK)}; + const ssize_t peek_ret{Recv(buf, std::min(sizeof(buf), max_data - data.size()), MSG_PEEK)}; switch (peek_ret) { case -1: { diff --git a/src/util/sock.h b/src/util/sock.h index ecebb84205..4b0618dcff 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -135,13 +135,16 @@ public: * @param[in] terminator Character up to which to read from the socket. * @param[in] timeout Timeout for the entire operation. * @param[in] interrupt If this is signaled then the operation is canceled. + * @param[in] max_data The maximum amount of data (in bytes) to receive. If this many bytes + * are received and there is still no terminator, then this method will throw an exception. * @return The data that has been read, without the terminating character. * @throws std::runtime_error if the operation cannot be completed. In this case some bytes may * have been consumed from the socket. */ virtual std::string RecvUntilTerminator(uint8_t terminator, std::chrono::milliseconds timeout, - CThreadInterrupt& interrupt) const; + CThreadInterrupt& interrupt, + size_t max_data) const; /** * Check if still connected. diff --git a/src/validation.cpp b/src/validation.cpp index b50edeb6fa..74df13f462 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5010,11 +5010,11 @@ CBlockFileInfo* GetBlockFileInfo(size_t n) static const uint64_t MEMPOOL_DUMP_VERSION = 1; -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate) +bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function) { const CChainParams& chainparams = Params(); int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60; - FILE* filestr = fsbridge::fopen(GetDataDir() / "mempool.dat", "rb"); + FILE* filestr{mockable_fopen_function(GetDataDir() / "mempool.dat", "rb")}; CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); if (file.IsNull()) { LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); @@ -5095,7 +5095,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate) return true; } -bool DumpMempool(const CTxMemPool& pool) +bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool skip_file_commit) { int64_t start = GetTimeMicros(); @@ -5118,7 +5118,7 @@ bool DumpMempool(const CTxMemPool& pool) int64_t mid = GetTimeMicros(); try { - FILE* filestr = fsbridge::fopen(GetDataDir() / "mempool.dat.new", "wb"); + FILE* filestr{mockable_fopen_function(GetDataDir() / "mempool.dat.new", "wb")}; if (!filestr) { return false; } @@ -5141,7 +5141,7 @@ bool DumpMempool(const CTxMemPool& pool) LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); file << unbroadcast_txids; - if (!FileCommit(file.Get())) + if (!skip_file_commit && !FileCommit(file.Get())) throw std::runtime_error("FileCommit failed"); file.fclose(); if (!RenameOver(GetDataDir() / "mempool.dat.new", GetDataDir() / "mempool.dat")) { diff --git a/src/validation.h b/src/validation.h index 1d3be63a56..2ff5f4ac87 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1012,11 +1012,13 @@ int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Para /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); +using FopenFn = std::function<FILE*(const fs::path&, const char*)>; + /** Dump the mempool to disk. */ -bool DumpMempool(const CTxMemPool& pool); +bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); /** Load the mempool from disk. */ -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate); +bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function = fsbridge::fopen); //! Check whether the block associated with this index entry is pruned or not. inline bool IsBlockPruned(const CBlockIndex* pblockindex) diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f3e24384df..fdeead1fa5 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -70,7 +70,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletbroadcast", strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-walletdir=<dir>", "Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET); #if HAVE_SYSTEM - argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID and %w is replaced by wallet name. %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-walletnotify=<cmd>", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); diff --git a/src/wallet/rpcsigner.cpp b/src/wallet/rpcsigner.cpp index 607b778c68..696c74d665 100644 --- a/src/wallet/rpcsigner.cpp +++ b/src/wallet/rpcsigner.cpp @@ -32,9 +32,6 @@ static RPCHelpMan enumeratesigners() }, RPCExamples{""}, [](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; - const std::string command = gArgs.GetArg("-signer", ""); if (command == "") throw JSONRPCError(RPC_WALLET_ERROR, "Error: restart bitcoind with -signer=<cmd>"); std::string chain = gArgs.GetChainName(); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index f6bae47365..6dc8d1de42 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -96,7 +96,7 @@ bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& request) { - CHECK_NONFATAL(!request.fHelp); + CHECK_NONFATAL(request.mode == JSONRPCRequest::EXECUTE); std::string wallet_name; if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { std::shared_ptr<CWallet> pwallet = GetWallet(wallet_name); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 0e9f3819eb..7eff6e592d 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -35,7 +35,10 @@ static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); CoinEligibilityFilter filter_confirmed(1, 1, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0); -CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0, false); +CoinSelectionParams coin_selection_params(/* use_bnb= */ false, /* change_output_size= */ 0, + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) { @@ -269,7 +272,10 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) } // Make sure that effective value is working in SelectCoinsMinConf when BnB is used - CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0, false); + CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 0, + /* change_spend_size= */ 0, /* effective_feerate= */ CFeeRate(3000), + /* long_term_feerate= */ CFeeRate(1000), /* discard_feerate= */ CFeeRate(1000), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); CoinSet setCoinsRet; CAmount nValueRet; bool bnb_used; @@ -301,7 +307,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CCoinControl coin_control; coin_control.fAllowOtherInputs = true; coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i)); - coin_selection_params_bnb.effective_fee = CFeeRate(0); + coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); BOOST_CHECK(wallet->SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); BOOST_CHECK(bnb_used); BOOST_CHECK(coin_selection_params_bnb.use_bnb); @@ -639,8 +645,14 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CAmount target = rand.randrange(balance - 1000) + 1000; // Perform selection - CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0, false); - CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0, false); + CoinSelectionParams coin_selection_params_knapsack(/* use_bnb= */ false, /* change_output_size= */ 34, + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); + CoinSelectionParams coin_selection_params_bnb(/* use_bnb= */ true, /* change_output_size= */ 34, + /* change_spend_size= */ 148, /* effective_feerate= */ CFeeRate(0), + /* long_term_feerate= */ CFeeRate(0), /* discard_feerate= */ CFeeRate(0), + /* tx_no_inputs_size= */ 0, /* avoid_partial= */ false); CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2b36da2e0e..d04279ab5e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -944,6 +944,14 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio if (!strCmd.empty()) { boost::replace_all(strCmd, "%s", hash.GetHex()); + if (confirm.status == CWalletTx::Status::CONFIRMED) + { + boost::replace_all(strCmd, "%b", confirm.hashBlock.GetHex()); + boost::replace_all(strCmd, "%h", ToString(confirm.block_height)); + } else { + boost::replace_all(strCmd, "%b", "unconfirmed"); + boost::replace_all(strCmd, "%h", "-1"); + } #ifndef WIN32 // Substituting the wallet name isn't currently supported on windows // because windows shell escaping has not been implemented yet: @@ -2391,26 +2399,20 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil nValueRet = 0; if (coin_selection_params.use_bnb) { - // Get long term estimate - FeeCalculation feeCalc; - CCoinControl temp; - temp.m_confirm_target = 1008; - CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); - // Get the feerate for effective value. // When subtracting the fee from the outputs, we want the effective feerate to be 0 CFeeRate effective_feerate{0}; if (!coin_selection_params.m_subtract_fee_outputs) { - effective_feerate = coin_selection_params.effective_fee; + effective_feerate = coin_selection_params.m_effective_feerate; } - std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, long_term_feerate, eligibility_filter, true /* positive_only */); + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, coin_selection_params.m_long_term_feerate, eligibility_filter, true /* positive_only */); // Calculate cost of change - CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); + CAmount cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.change_output_size); // Calculate the fees for things that aren't inputs - CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); + CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.tx_noinputs_size); bnb_used = true; return SelectCoinsBnB(groups, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); } else { @@ -2464,7 +2466,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm if (coin.m_input_bytes <= 0) { return false; // Not solvable, can't estimate size for fee } - coin.effective_value = coin.txout.nValue - coin_selection_params.effective_fee.GetFee(coin.m_input_bytes); + coin.effective_value = coin.txout.nValue - coin_selection_params.m_effective_feerate.GetFee(coin.m_input_bytes); if (coin_selection_params.use_bnb) { value_to_select -= coin.effective_value; } else { @@ -2832,17 +2834,28 @@ bool CWallet::CreateTransactionInternal( CTxOut change_prototype_txout(0, scriptChange); coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout); - CFeeRate discard_rate = GetDiscardRate(*this); + // Set discard feerate + coin_selection_params.m_discard_feerate = GetDiscardRate(*this); // Get the fee rate to use effective values in coin selection - CFeeRate nFeeRateNeeded = GetMinimumFeeRate(*this, coin_control, &feeCalc); + coin_selection_params.m_effective_feerate = GetMinimumFeeRate(*this, coin_control, &feeCalc); // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly // provided one - if (coin_control.m_feerate && nFeeRateNeeded > *coin_control.m_feerate) { - error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), nFeeRateNeeded.ToString(FeeEstimateMode::SAT_VB)); + if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { + error = strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); + return false; + } + if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { + // eventually allow a fallback fee + error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); return false; } + // Get long term estimate + CCoinControl cc_temp; + cc_temp.m_confirm_target = chain().estimateMaxBlocks(); + coin_selection_params.m_long_term_feerate = GetMinimumFeeRate(*this, cc_temp, nullptr); + nFeeRet = 0; bool pick_new_inputs = true; CAmount nValueIn = 0; @@ -2916,7 +2929,6 @@ bool CWallet::CreateTransactionInternal( } else { coin_selection_params.change_spend_size = (size_t)change_spend_size; } - coin_selection_params.effective_fee = nFeeRateNeeded; if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params, bnb_used)) { // If BnB was used, it was the first pass. No longer the first pass and continue loop with knapsack. @@ -2942,7 +2954,7 @@ bool CWallet::CreateTransactionInternal( // Never create dust outputs; if we would, just // add the dust to the fee. // The nChange when BnB is used is always going to go to fees. - if (IsDust(newTxOut, discard_rate) || bnb_used) + if (IsDust(newTxOut, coin_selection_params.m_discard_feerate) || bnb_used) { nChangePosInOut = -1; nFeeRet += nChange; @@ -2980,13 +2992,7 @@ bool CWallet::CreateTransactionInternal( return false; } - nFeeNeeded = GetMinimumFee(*this, nBytes, coin_control, &feeCalc); - if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) { - // eventually allow a fallback fee - error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); - return false; - } - + nFeeNeeded = coin_selection_params.m_effective_feerate.GetFee(nBytes); if (nFeeRet >= nFeeNeeded) { // Reduce fee to only the needed amount if possible. This // prevents potential overpayment in fees if the coins @@ -3000,8 +3006,8 @@ bool CWallet::CreateTransactionInternal( // change output. Only try this once. if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size - CAmount fee_needed_with_change = GetMinimumFee(*this, tx_size_with_change, coin_control, nullptr); - CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); + CAmount fee_needed_with_change = coin_selection_params.m_effective_feerate.GetFee(tx_size_with_change); + CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, coin_selection_params.m_discard_feerate); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { pick_new_inputs = false; nFeeRet = fee_needed_with_change; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 13a856afb0..6031d36b97 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -609,17 +609,22 @@ struct CoinSelectionParams bool use_bnb = true; size_t change_output_size = 0; size_t change_spend_size = 0; - CFeeRate effective_fee = CFeeRate(0); + CFeeRate m_effective_feerate; + CFeeRate m_long_term_feerate; + CFeeRate m_discard_feerate; size_t tx_noinputs_size = 0; //! Indicate that we are subtracting the fee from outputs bool m_subtract_fee_outputs = false; bool m_avoid_partial_spends = false; - CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size, bool avoid_partial) : + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_feerate, + CFeeRate long_term_feerate, CFeeRate discard_feerate, size_t tx_noinputs_size, bool avoid_partial) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), - effective_fee(effective_fee), + m_effective_feerate(effective_feerate), + m_long_term_feerate(long_term_feerate), + m_discard_feerate(discard_feerate), tx_noinputs_size(tx_noinputs_size), m_avoid_partial_spends(avoid_partial) {} diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index b068ce612c..4e2de1daf4 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -17,7 +17,7 @@ from test_framework.util import ( FILE_CHAR_START = 32 if os.name == 'nt' else 1 FILE_CHAR_END = 128 FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if os.name == 'nt' else '/' - +UNCONFIRMED_HASH_STRING = 'unconfirmed' def notify_outputname(walletname, txid): return txid if os.name == 'nt' else '{}_{}'.format(walletname, txid) @@ -43,7 +43,7 @@ class NotificationsTest(BitcoinTestFramework): "-blocknotify=echo > {}".format(os.path.join(self.blocknotify_dir, '%s')), ], [ "-rescan", - "-walletnotify=echo > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))), + "-walletnotify=echo %h_%b > {}".format(os.path.join(self.walletnotify_dir, notify_outputname('%w', '%s'))), ]] self.wallet_names = [self.default_wallet_name, self.wallet] super().setup_network() @@ -90,11 +90,9 @@ class NotificationsTest(BitcoinTestFramework): self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) # directory content should equal the generated transaction hashes - txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count))) - assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) + tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count))) self.stop_node(1) - for tx_file in os.listdir(self.walletnotify_dir): - os.remove(os.path.join(self.walletnotify_dir, tx_file)) + self.expect_wallet_notify(tx_details) self.log.info("test -walletnotify after rescan") # restart node to rescan to force wallet notifications @@ -104,10 +102,8 @@ class NotificationsTest(BitcoinTestFramework): self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) == block_count, timeout=10) # directory content should equal the generated transaction hashes - txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count))) - assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) - for tx_file in os.listdir(self.walletnotify_dir): - os.remove(os.path.join(self.walletnotify_dir, tx_file)) + tx_details = list(map(lambda t: (t['txid'], t['blockheight'], t['blockhash']), self.nodes[1].listtransactions("*", block_count))) + self.expect_wallet_notify(tx_details) # Conflicting transactions tests. # Generate spends from node 0, and check notifications @@ -122,7 +118,7 @@ class NotificationsTest(BitcoinTestFramework): tx1 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True) assert_equal(tx1 in self.nodes[0].getrawmempool(), True) self.sync_mempools() - self.expect_wallet_notify([tx1]) + self.expect_wallet_notify([(tx1, -1, UNCONFIRMED_HASH_STRING)]) # Generate bump transaction, sync mempools, and check for bump1 # notification. In the future, per @@ -131,39 +127,57 @@ class NotificationsTest(BitcoinTestFramework): bump1 = self.nodes[0].bumpfee(tx1)["txid"] assert_equal(bump1 in self.nodes[0].getrawmempool(), True) self.sync_mempools() - self.expect_wallet_notify([bump1]) + self.expect_wallet_notify([(bump1, -1, UNCONFIRMED_HASH_STRING)]) # Add bump1 transaction to new block, checking for a notification # and the correct number of confirmations. - self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE) + blockhash1 = self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)[0] + blockheight1 = self.nodes[0].getblockcount() self.sync_blocks() - self.expect_wallet_notify([bump1]) + self.expect_wallet_notify([(bump1, blockheight1, blockhash1)]) assert_equal(self.nodes[1].gettransaction(bump1)["confirmations"], 1) # Generate a second transaction to be bumped. tx2 = self.nodes[0].sendtoaddress(address=ADDRESS_BCRT1_UNSPENDABLE, amount=1, replaceable=True) assert_equal(tx2 in self.nodes[0].getrawmempool(), True) self.sync_mempools() - self.expect_wallet_notify([tx2]) + self.expect_wallet_notify([(tx2, -1, UNCONFIRMED_HASH_STRING)]) # Bump tx2 as bump2 and generate a block on node 0 while # disconnected, then reconnect and check for notifications on node 1 # about newly confirmed bump2 and newly conflicted tx2. self.disconnect_nodes(0, 1) bump2 = self.nodes[0].bumpfee(tx2)["txid"] - self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE) + blockhash2 = self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)[0] + blockheight2 = self.nodes[0].getblockcount() assert_equal(self.nodes[0].gettransaction(bump2)["confirmations"], 1) assert_equal(tx2 in self.nodes[1].getrawmempool(), True) self.connect_nodes(0, 1) self.sync_blocks() - self.expect_wallet_notify([bump2, tx2]) + self.expect_wallet_notify([(bump2, blockheight2, blockhash2), (tx2, -1, UNCONFIRMED_HASH_STRING)]) assert_equal(self.nodes[1].gettransaction(bump2)["confirmations"], 1) # TODO: add test for `-alertnotify` large fork notifications - def expect_wallet_notify(self, tx_ids): - self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_ids), timeout=10) - assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id in tx_ids), sorted(os.listdir(self.walletnotify_dir))) + def expect_wallet_notify(self, tx_details): + self.wait_until(lambda: len(os.listdir(self.walletnotify_dir)) >= len(tx_details), timeout=10) + # Should have no more and no less files than expected + assert_equal(sorted(notify_outputname(self.wallet, tx_id) for tx_id, _, _ in tx_details), sorted(os.listdir(self.walletnotify_dir))) + # Should now verify contents of each file + for tx_id, blockheight, blockhash in tx_details: + fname = os.path.join(self.walletnotify_dir, notify_outputname(self.wallet, tx_id)) + with open(fname, 'rt', encoding='utf-8') as f: + text = f.read() + # Universal newline ensures '\n' on 'nt' + assert_equal(text[-1], '\n') + text = text[:-1] + if os.name == 'nt': + # On Windows, echo as above will append a whitespace + assert_equal(text[-1], ' ') + text = text[:-1] + expected = str(blockheight) + '_' + blockhash + assert_equal(text, expected) + for tx_file in os.listdir(self.walletnotify_dir): os.remove(os.path.join(self.walletnotify_dir, tx_file)) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 6b300e7231..8c9755cc8c 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -32,6 +32,7 @@ class RawTransactionsTest(BitcoinTestFramework): # This test isn't testing tx relay. Set whitelist on the peers for # instant tx relay. self.extra_args = [['-whitelist=noban@127.0.0.1']] * self.num_nodes + self.rpc_timeout = 90 # to prevent timeouts in `test_transaction_too_large` def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -910,22 +911,23 @@ class RawTransactionsTest(BitcoinTestFramework): def test_transaction_too_large(self): self.log.info("Test fundrawtx where BnB solution would result in a too large transaction, but Knapsack would not") - self.nodes[0].createwallet("large") wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name) recipient = self.nodes[0].get_wallet_rpc("large") outputs = {} rawtx = recipient.createrawtransaction([], {wallet.getnewaddress(): 147.99899260}) - # Make 1500 0.1 BTC outputs - # The amount that we target for funding is in the BnB range when these outputs are used. - # However if these outputs are selected, the transaction will end up being too large, so it shouldn't use BnB and instead fallback to Knapsack - # but that behavior is not implemented yet. For now we just check that we get an error. - for i in range(0, 1500): + # Make 1500 0.1 BTC outputs. The amount that we target for funding is in + # the BnB range when these outputs are used. However if these outputs + # are selected, the transaction will end up being too large, so it + # shouldn't use BnB and instead fall back to Knapsack but that behavior + # is not implemented yet. For now we just check that we get an error. + for _ in range(1500): outputs[recipient.getnewaddress()] = 0.1 wallet.sendmany("", outputs) self.nodes[0].generate(10) assert_raises_rpc_error(-4, "Transaction too large", recipient.fundrawtransaction, rawtx) + if __name__ == '__main__': RawTransactionsTest().main() |