diff options
48 files changed, 508 insertions, 351 deletions
diff --git a/build-aux/m4/l_atomic.m4 b/build-aux/m4/l_atomic.m4 index 5201a8cc7c..40639dfe61 100644 --- a/build-aux/m4/l_atomic.m4 +++ b/build-aux/m4/l_atomic.m4 @@ -12,11 +12,17 @@ dnl warranty. m4_define([_CHECK_ATOMIC_testbody], [[ #include <atomic> #include <cstdint> + #include <chrono> + + using namespace std::chrono_literals; int main() { std::atomic<bool> lock{true}; std::atomic_exchange(&lock, false); + std::atomic<std::chrono::seconds> t{0s}; + t.store(2s); + std::atomic<int64_t> a{}; int64_t v = 5; diff --git a/doc/build-osx.md b/doc/build-osx.md index ab298f5f2c..467feff410 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -138,6 +138,14 @@ Skip if you don't intend to use the GUI. brew install qt@5 ``` +Ensure that the `qt@5` package is installed, not the `qt` package. +If 'qt' is installed, the build process will fail. +if installed, remove the `qt` package with the following command: + +``` bash +brew uninstall qt +``` + Note: Building with Qt binaries downloaded from the Qt website is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714). diff --git a/doc/release-notes.md b/doc/release-notes.md index 5c70bc91db..53106c9f82 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -108,6 +108,12 @@ Updated RPCs Respectively, these new fields indicate the duration of a ban and the time remaining until a ban expires, both in seconds. Additionally, the `ban_created` field is repositioned to come before `banned_until`. (#21602) +- The `getnodeaddresses` RPC now returns a "network" field indicating the + network type (ipv4, ipv6, onion, or i2p) for each address. (#21594) + +- `getnodeaddresses` now also accepts a "network" argument (ipv4, ipv6, onion, + or i2p) to return only addresses of the specified network. (#21843) + Changes to Wallet or GUI related RPCs can be found in the GUI or Wallet section below. New RPCs @@ -130,9 +136,6 @@ Changes to Wallet or GUI related settings can be found in the GUI or Wallet sect - Passing an invalid `-rpcauth` argument now cause bitcoind to fail to start. (#20461) -- The `getnodeaddresses` RPC now returns a "network" field indicating the - network type (ipv4, ipv6, onion, or i2p) for each address. (#21594) - Tools and Utilities ------------------- diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index 74cf6734d6..30edb1e82d 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -358,7 +358,7 @@ $(srcdir)/qt/bitcoinstrings.cpp: FORCE translate: $(srcdir)/qt/bitcoinstrings.cpp $(QT_FORMS_UI) $(QT_FORMS_UI) $(BITCOIN_QT_BASE_CPP) qt/bitcoin.cpp $(BITCOIN_QT_WINDOWS_CPP) $(BITCOIN_QT_WALLET_CPP) $(BITCOIN_QT_H) $(BITCOIN_MM) @test -n $(LUPDATE) || echo "lupdate is required for updating translations" - $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LUPDATE) $^ -locations relative -no-obsolete -ts $(srcdir)/qt/locale/bitcoin_en.ts + $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LUPDATE) -no-obsolete -I $(srcdir) -locations relative $^ -ts $(srcdir)/qt/locale/bitcoin_en.ts @test -n $(LCONVERT) || echo "lconvert is required for updating translations" $(AM_V_GEN) QT_SELECT=$(QT_SELECT) $(LCONVERT) -o $(srcdir)/qt/locale/bitcoin_en.xlf -i $(srcdir)/qt/locale/bitcoin_en.ts diff --git a/src/addrman.cpp b/src/addrman.cpp index f91121f156..ceab1689d7 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -7,9 +7,11 @@ #include <hash.h> #include <logging.h> +#include <netaddress.h> #include <serialize.h> #include <cmath> +#include <optional> int CAddrInfo::GetTriedBucket(const uint256& nKey, const std::vector<bool> &asmap) const { @@ -481,7 +483,7 @@ int CAddrMan::Check_() } #endif -void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct) +void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) { size_t nNodes = vRandom.size(); if (max_pct != 0) { @@ -492,6 +494,7 @@ void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size } // gather a list of random nodes, skipping those of low quality + const int64_t now{GetAdjustedTime()}; for (unsigned int n = 0; n < vRandom.size(); n++) { if (vAddr.size() >= nNodes) break; @@ -501,8 +504,14 @@ void CAddrMan::GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size assert(mapInfo.count(vRandom[n]) == 1); const CAddrInfo& ai = mapInfo[vRandom[n]]; - if (!ai.IsTerrible()) - vAddr.push_back(ai); + + // Filter by network (optional) + if (network != std::nullopt && ai.GetNetClass() != network) continue; + + // Filter for quality + if (ai.IsTerrible(now)) continue; + + vAddr.push_back(ai); } } diff --git a/src/addrman.h b/src/addrman.h index 92a5570953..eaedfd318c 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -20,6 +20,7 @@ #include <hash.h> #include <iostream> #include <map> +#include <optional> #include <set> #include <stdint.h> #include <streams.h> @@ -278,8 +279,15 @@ protected: int Check_() EXCLUSIVE_LOCKS_REQUIRED(cs); #endif - //! Select several addresses at once. - void GetAddr_(std::vector<CAddress> &vAddr, size_t max_addresses, size_t max_pct) EXCLUSIVE_LOCKS_REQUIRED(cs); + /** + * Return all or many randomly selected addresses, optionally by network. + * + * @param[out] vAddr Vector of randomly selected addresses from vRandom. + * @param[in] max_addresses Maximum number of addresses to return (0 = all). + * @param[in] max_pct Maximum percentage of addresses to return (0 = all). + * @param[in] network Select only addresses of this network (nullopt = all). + */ + void GetAddr_(std::vector<CAddress>& vAddr, size_t max_addresses, size_t max_pct, std::optional<Network> network) EXCLUSIVE_LOCKS_REQUIRED(cs); /** We have successfully connected to this peer. Calling this function * updates the CAddress's nTime, which is used in our IsTerrible() @@ -715,14 +723,20 @@ public: return addrRet; } - //! Return a bunch of addresses, selected at random. - std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct) + /** + * Return all or many randomly selected addresses, optionally by network. + * + * @param[in] max_addresses Maximum number of addresses to return (0 = all). + * @param[in] max_pct Maximum percentage of addresses to return (0 = all). + * @param[in] network Select only addresses of this network (nullopt = all). + */ + std::vector<CAddress> GetAddr(size_t max_addresses, size_t max_pct, std::optional<Network> network) { Check(); std::vector<CAddress> vAddr; { LOCK(cs); - GetAddr_(vAddr, max_addresses, max_pct); + GetAddr_(vAddr, max_addresses, max_pct, network); } Check(); return vAddr; diff --git a/src/bench/addrman.cpp b/src/bench/addrman.cpp index ebdad5a4b8..b7bd8a3261 100644 --- a/src/bench/addrman.cpp +++ b/src/bench/addrman.cpp @@ -7,6 +7,7 @@ #include <random.h> #include <util/time.h> +#include <optional> #include <vector> /* A "source" is a source address from which we have received a bunch of other addresses. */ @@ -98,7 +99,7 @@ static void AddrManGetAddr(benchmark::Bench& bench) FillAddrMan(addrman); bench.run([&] { - const auto& addresses = addrman.GetAddr(2500, 23); + const auto& addresses = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt); assert(addresses.size() > 0); }); } diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index d7cc167885..362b7c1e15 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -22,8 +22,7 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b CWallet wallet{test_setup->m_node.chain.get(), "", CreateMockWalletDatabase()}; { wallet.SetupLegacyScriptPubKeyMan(); - bool first_run; - if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false); + if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); } auto handler = test_setup->m_node.chain->handleNotifications({&wallet, [](CWallet*) {}}); diff --git a/src/net.cpp b/src/net.cpp index 05588d7406..1322c971fb 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -16,6 +16,7 @@ #include <crypto/sha256.h> #include <i2p.h> #include <net_permissions.h> +#include <netaddress.h> #include <netbase.h> #include <node/ui_interface.h> #include <protocol.h> @@ -1005,7 +1006,7 @@ bool CConnman::AttemptToEvictConnection() LOCK(cs_vNodes); for (const CNode* node : vNodes) { - if (node->HasPermission(PF_NOBAN)) + if (node->HasPermission(NetPermissionFlags::NoBan)) continue; if (!node->IsInboundConn()) continue; @@ -1062,7 +1063,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) { const CAddress addr_bind = GetBindAddress(hSocket); - NetPermissionFlags permissionFlags = NetPermissionFlags::PF_NONE; + NetPermissionFlags permissionFlags = NetPermissionFlags::None; hListenSocket.AddSocketPermissionFlags(permissionFlags); CreateNodeFromAcceptedSocket(hSocket, permissionFlags, addr_bind, addr); @@ -1077,12 +1078,12 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, int nMaxInbound = nMaxConnections - m_max_outbound; AddWhitelistPermissionFlags(permissionFlags, addr); - if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_ISIMPLICIT)) { - NetPermissions::ClearFlag(permissionFlags, PF_ISIMPLICIT); - if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permissionFlags, PF_FORCERELAY); - if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permissionFlags, PF_RELAY); - NetPermissions::AddFlag(permissionFlags, PF_MEMPOOL); - NetPermissions::AddFlag(permissionFlags, PF_NOBAN); + if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::Implicit)) { + NetPermissions::ClearFlag(permissionFlags, NetPermissionFlags::Implicit); + if (gArgs.GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY)) NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::ForceRelay); + if (gArgs.GetBoolArg("-whitelistrelay", DEFAULT_WHITELISTRELAY)) NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::Relay); + NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::Mempool); + NetPermissions::AddFlag(permissionFlags, NetPermissionFlags::NoBan); } { @@ -1111,7 +1112,7 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, // Don't accept connections from banned peers. bool banned = m_banman && m_banman->IsBanned(addr); - if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && banned) + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && banned) { LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString()); CloseSocket(hSocket); @@ -1120,7 +1121,7 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, // Only accept connections from discouraged peers if our inbound slots aren't (almost) full. bool discouraged = m_banman && m_banman->IsDiscouraged(addr); - if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && discouraged) + if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && nInbound + 1 >= nMaxInbound && discouraged) { LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString()); CloseSocket(hSocket); @@ -1141,7 +1142,7 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); ServiceFlags nodeServices = nLocalServices; - if (NetPermissions::HasFlag(permissionFlags, PF_BLOOMFILTER)) { + if (NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::BloomFilter)) { nodeServices = static_cast<ServiceFlags>(nodeServices | NODE_BLOOM); } @@ -2253,7 +2254,7 @@ void CConnman::ThreadI2PAcceptIncoming() continue; } - CreateNodeFromAcceptedSocket(conn.sock->Release(), NetPermissionFlags::PF_NONE, + CreateNodeFromAcceptedSocket(conn.sock->Release(), NetPermissionFlags::None, CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE}); } } @@ -2411,7 +2412,7 @@ bool CConnman::Bind(const CService &addr, unsigned int flags, NetPermissionFlags return false; } - if (addr.IsRoutable() && fDiscover && !(flags & BF_DONT_ADVERTISE) && !NetPermissions::HasFlag(permissions, NetPermissionFlags::PF_NOBAN)) { + if (addr.IsRoutable() && fDiscover && !(flags & BF_DONT_ADVERTISE) && !NetPermissions::HasFlag(permissions, NetPermissionFlags::NoBan)) { AddLocal(addr, LOCAL_BIND); } @@ -2425,7 +2426,7 @@ bool CConnman::InitBinds( { bool fBound = false; for (const auto& addrBind : binds) { - fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR), NetPermissionFlags::PF_NONE); + fBound |= Bind(addrBind, (BF_EXPLICIT | BF_REPORT_ERROR), NetPermissionFlags::None); } for (const auto& addrBind : whiteBinds) { fBound |= Bind(addrBind.m_service, (BF_EXPLICIT | BF_REPORT_ERROR), addrBind.m_flags); @@ -2434,12 +2435,12 @@ bool CConnman::InitBinds( struct in_addr inaddr_any; inaddr_any.s_addr = htonl(INADDR_ANY); struct in6_addr inaddr6_any = IN6ADDR_ANY_INIT; - fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::PF_NONE); - fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::PF_NONE); + fBound |= Bind(CService(inaddr6_any, GetListenPort()), BF_NONE, NetPermissionFlags::None); + fBound |= Bind(CService(inaddr_any, GetListenPort()), !fBound ? BF_REPORT_ERROR : BF_NONE, NetPermissionFlags::None); } for (const auto& addr_bind : onion_binds) { - fBound |= Bind(addr_bind, BF_EXPLICIT | BF_DONT_ADVERTISE, NetPermissionFlags::PF_NONE); + fBound |= Bind(addr_bind, BF_EXPLICIT | BF_DONT_ADVERTISE, NetPermissionFlags::None); } return fBound; @@ -2669,9 +2670,9 @@ CConnman::~CConnman() Stop(); } -std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct) const +std::vector<CAddress> CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network) const { - std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct); + std::vector<CAddress> addresses = addrman.GetAddr(max_addresses, max_pct, network); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), @@ -2691,7 +2692,7 @@ std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addres auto r = m_addr_response_caches.emplace(cache_id, CachedAddrResponse{}); CachedAddrResponse& cache_entry = r.first->second; if (cache_entry.m_cache_entry_expiration < current_time) { // If emplace() added new one it has expiration 0. - cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct); + cache_entry.m_addrs_response_cache = GetAddresses(max_addresses, max_pct, /* network */ std::nullopt); // Choosing a proper cache lifetime is a trade-off between the privacy leak minimization // and the usefulness of ADDR responses to honest users. // @@ -402,7 +402,7 @@ public: std::unique_ptr<TransportDeserializer> m_deserializer; std::unique_ptr<TransportSerializer> m_serializer; - NetPermissionFlags m_permissionFlags{PF_NONE}; + NetPermissionFlags m_permissionFlags{NetPermissionFlags::None}; std::atomic<ServiceFlags> nServices{NODE_NONE}; SOCKET hSocket GUARDED_BY(cs_hSocket); /** Total size of all vSendMsg entries */ @@ -923,7 +923,14 @@ public: }; // Addrman functions - std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct) const; + /** + * Return all or many randomly selected addresses, optionally by network. + * + * @param[in] max_addresses Maximum number of addresses to return (0 = all). + * @param[in] max_pct Maximum percentage of addresses to return (0 = all). + * @param[in] network Select only addresses of this network (nullopt = all). + */ + std::vector<CAddress> GetAddresses(size_t max_addresses, size_t max_pct, std::optional<Network> network) const; /** * Cache is used to minimize topology leaks, so it should * be used for all non-trusted calls, for example, p2p. diff --git a/src/net_permissions.cpp b/src/net_permissions.cpp index 1fdcd97593..d0a45f90fa 100644 --- a/src/net_permissions.cpp +++ b/src/net_permissions.cpp @@ -20,15 +20,15 @@ const std::vector<std::string> NET_PERMISSIONS_DOC{ namespace { -// The parse the following format "perm1,perm2@xxxxxx" -bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, size_t& readen, bilingual_str& error) +// Parse the following format: "perm1,perm2@xxxxxx" +bool TryParsePermissionFlags(const std::string& str, NetPermissionFlags& output, size_t& readen, bilingual_str& error) { - NetPermissionFlags flags = PF_NONE; + NetPermissionFlags flags = NetPermissionFlags::None; const auto atSeparator = str.find('@'); // if '@' is not found (ie, "xxxxx"), the caller should apply implicit permissions if (atSeparator == std::string::npos) { - NetPermissions::AddFlag(flags, PF_ISIMPLICIT); + NetPermissions::AddFlag(flags, NetPermissionFlags::Implicit); readen = 0; } // else (ie, "perm1,perm2@xxxxx"), let's enumerate the permissions by splitting by ',' and calculate the flags @@ -44,14 +44,14 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, readen += len; // We read "perm1" if (commaSeparator != std::string::npos) readen++; // We read "," - if (permission == "bloomfilter" || permission == "bloom") NetPermissions::AddFlag(flags, PF_BLOOMFILTER); - else if (permission == "noban") NetPermissions::AddFlag(flags, PF_NOBAN); - else if (permission == "forcerelay") NetPermissions::AddFlag(flags, PF_FORCERELAY); - else if (permission == "mempool") NetPermissions::AddFlag(flags, PF_MEMPOOL); - else if (permission == "download") NetPermissions::AddFlag(flags, PF_DOWNLOAD); - else if (permission == "all") NetPermissions::AddFlag(flags, PF_ALL); - else if (permission == "relay") NetPermissions::AddFlag(flags, PF_RELAY); - else if (permission == "addr") NetPermissions::AddFlag(flags, PF_ADDR); + if (permission == "bloomfilter" || permission == "bloom") NetPermissions::AddFlag(flags, NetPermissionFlags::BloomFilter); + else if (permission == "noban") NetPermissions::AddFlag(flags, NetPermissionFlags::NoBan); + else if (permission == "forcerelay") NetPermissions::AddFlag(flags, NetPermissionFlags::ForceRelay); + else if (permission == "mempool") NetPermissions::AddFlag(flags, NetPermissionFlags::Mempool); + else if (permission == "download") NetPermissions::AddFlag(flags, NetPermissionFlags::Download); + else if (permission == "all") NetPermissions::AddFlag(flags, NetPermissionFlags::All); + else if (permission == "relay") NetPermissions::AddFlag(flags, NetPermissionFlags::Relay); + else if (permission == "addr") NetPermissions::AddFlag(flags, NetPermissionFlags::Addr); else if (permission.length() == 0); // Allow empty entries else { error = strprintf(_("Invalid P2P permission: '%s'"), permission); @@ -71,17 +71,17 @@ bool TryParsePermissionFlags(const std::string str, NetPermissionFlags& output, std::vector<std::string> NetPermissions::ToStrings(NetPermissionFlags flags) { std::vector<std::string> strings; - if (NetPermissions::HasFlag(flags, PF_BLOOMFILTER)) strings.push_back("bloomfilter"); - if (NetPermissions::HasFlag(flags, PF_NOBAN)) strings.push_back("noban"); - if (NetPermissions::HasFlag(flags, PF_FORCERELAY)) strings.push_back("forcerelay"); - if (NetPermissions::HasFlag(flags, PF_RELAY)) strings.push_back("relay"); - if (NetPermissions::HasFlag(flags, PF_MEMPOOL)) strings.push_back("mempool"); - if (NetPermissions::HasFlag(flags, PF_DOWNLOAD)) strings.push_back("download"); - if (NetPermissions::HasFlag(flags, PF_ADDR)) strings.push_back("addr"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::BloomFilter)) strings.push_back("bloomfilter"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::NoBan)) strings.push_back("noban"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::ForceRelay)) strings.push_back("forcerelay"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Relay)) strings.push_back("relay"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Mempool)) strings.push_back("mempool"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Download)) strings.push_back("download"); + if (NetPermissions::HasFlag(flags, NetPermissionFlags::Addr)) strings.push_back("addr"); return strings; } -bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error) +bool NetWhitebindPermissions::TryParse(const std::string& str, NetWhitebindPermissions& output, bilingual_str& error) { NetPermissionFlags flags; size_t offset; @@ -104,7 +104,7 @@ bool NetWhitebindPermissions::TryParse(const std::string str, NetWhitebindPermis return true; } -bool NetWhitelistPermissions::TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error) +bool NetWhitelistPermissions::TryParse(const std::string& str, NetWhitelistPermissions& output, bilingual_str& error) { NetPermissionFlags flags; size_t offset; diff --git a/src/net_permissions.h b/src/net_permissions.h index 142b317bf6..c00689465e 100644 --- a/src/net_permissions.h +++ b/src/net_permissions.h @@ -5,6 +5,7 @@ #include <netaddress.h> #include <string> +#include <type_traits> #include <vector> #ifndef BITCOIN_NET_PERMISSIONS_H @@ -14,66 +15,73 @@ struct bilingual_str; extern const std::vector<std::string> NET_PERMISSIONS_DOC; -enum NetPermissionFlags { - PF_NONE = 0, +enum class NetPermissionFlags : uint32_t { + None = 0, // Can query bloomfilter even if -peerbloomfilters is false - PF_BLOOMFILTER = (1U << 1), + BloomFilter = (1U << 1), // Relay and accept transactions from this peer, even if -blocksonly is true // This peer is also not subject to limits on how many transaction INVs are tracked - PF_RELAY = (1U << 3), + Relay = (1U << 3), // Always relay transactions from this peer, even if already in mempool // Keep parameter interaction: forcerelay implies relay - PF_FORCERELAY = (1U << 2) | PF_RELAY, + ForceRelay = (1U << 2) | Relay, // Allow getheaders during IBD and block-download after maxuploadtarget limit - PF_DOWNLOAD = (1U << 6), + Download = (1U << 6), // Can't be banned/disconnected/discouraged for misbehavior - PF_NOBAN = (1U << 4) | PF_DOWNLOAD, + NoBan = (1U << 4) | Download, // Can query the mempool - PF_MEMPOOL = (1U << 5), + Mempool = (1U << 5), // Can request addrs without hitting a privacy-preserving cache - PF_ADDR = (1U << 7), + Addr = (1U << 7), // True if the user did not specifically set fine grained permissions - PF_ISIMPLICIT = (1U << 31), - PF_ALL = PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY | PF_NOBAN | PF_MEMPOOL | PF_DOWNLOAD | PF_ADDR, + Implicit = (1U << 31), + All = BloomFilter | ForceRelay | Relay | NoBan | Mempool | Download | Addr, }; +static inline constexpr NetPermissionFlags operator|(NetPermissionFlags a, NetPermissionFlags b) +{ + using t = typename std::underlying_type<NetPermissionFlags>::type; + return static_cast<NetPermissionFlags>(static_cast<t>(a) | static_cast<t>(b)); +} class NetPermissions { public: NetPermissionFlags m_flags; static std::vector<std::string> ToStrings(NetPermissionFlags flags); - static inline bool HasFlag(const NetPermissionFlags& flags, NetPermissionFlags f) + static inline bool HasFlag(NetPermissionFlags flags, NetPermissionFlags f) { - return (flags & f) == f; + using t = typename std::underlying_type<NetPermissionFlags>::type; + return (static_cast<t>(flags) & static_cast<t>(f)) == static_cast<t>(f); } static inline void AddFlag(NetPermissionFlags& flags, NetPermissionFlags f) { - flags = static_cast<NetPermissionFlags>(flags | f); + flags = flags | f; } - //! ClearFlag is only called with `f` == NetPermissionFlags::PF_ISIMPLICIT. + //! ClearFlag is only called with `f` == NetPermissionFlags::Implicit. //! If that should change in the future, be aware that ClearFlag should not - //! be called with a subflag of a multiflag, e.g. NetPermissionFlags::PF_RELAY - //! or NetPermissionFlags::PF_DOWNLOAD, as that would leave `flags` in an + //! be called with a subflag of a multiflag, e.g. NetPermissionFlags::Relay + //! or NetPermissionFlags::Download, as that would leave `flags` in an //! invalid state corresponding to none of the existing flags. static inline void ClearFlag(NetPermissionFlags& flags, NetPermissionFlags f) { - assert(f == NetPermissionFlags::PF_ISIMPLICIT); - flags = static_cast<NetPermissionFlags>(flags & ~f); + assert(f == NetPermissionFlags::Implicit); + using t = typename std::underlying_type<NetPermissionFlags>::type; + flags = static_cast<NetPermissionFlags>(static_cast<t>(flags) & ~static_cast<t>(f)); } }; class NetWhitebindPermissions : public NetPermissions { public: - static bool TryParse(const std::string str, NetWhitebindPermissions& output, bilingual_str& error); + static bool TryParse(const std::string& str, NetWhitebindPermissions& output, bilingual_str& error); CService m_service; }; class NetWhitelistPermissions : public NetPermissions { public: - static bool TryParse(const std::string str, NetWhitelistPermissions& output, bilingual_str& error); + static bool TryParse(const std::string& str, NetWhitelistPermissions& output, bilingual_str& error); CSubNet m_subnet; }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 27ad9eefb5..cee2419610 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -124,11 +124,11 @@ static constexpr auto AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24h; /** Average delay between peer address broadcasts */ static constexpr auto AVG_ADDRESS_BROADCAST_INTERVAL = 30s; /** Average delay between trickled inventory transmissions for inbound peers. - * Blocks and peers with noban permission bypass this. */ + * Blocks and peers with NetPermissionFlags::NoBan permission bypass this. */ static constexpr auto INBOUND_INVENTORY_BROADCAST_INTERVAL = 5s; /** Average delay between trickled inventory transmissions for outbound peers. * Use a smaller delay as there is less privacy concern for them. - * Blocks and peers with noban permission bypass this. */ + * Blocks and peers with NetPermissionFlags::NoBan permission bypass this. */ static constexpr auto OUTBOUND_INVENTORY_BROADCAST_INTERVAL = 2s; /** Maximum rate of inventory items to send per second. * Limits the impact of low-fee transaction floods. */ @@ -183,7 +183,7 @@ struct Peer { Mutex m_misbehavior_mutex; /** Accumulated misbehavior score for this peer */ int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0}; - /** Whether this peer should be disconnected and marked as discouraged (unless it has the noban permission). */ + /** Whether this peer should be disconnected and marked as discouraged (unless it has NetPermissionFlags::NoBan permission). */ bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false}; /** Protects block inventory data members */ @@ -680,7 +680,7 @@ static void UpdatePreferredDownload(const CNode& node, CNodeState* state) EXCLUS nPreferredDownload -= state->fPreferredDownload; // Whether this node should be marked as a preferred download node. - state->fPreferredDownload = (!node.IsInboundConn() || node.HasPermission(PF_NOBAN)) && !node.IsAddrFetchConn() && !node.fClient; + state->fPreferredDownload = (!node.IsInboundConn() || node.HasPermission(NetPermissionFlags::NoBan)) && !node.IsAddrFetchConn() && !node.fClient; nPreferredDownload += state->fPreferredDownload; } @@ -960,24 +960,24 @@ void PeerManagerImpl::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, { AssertLockHeld(::cs_main); // For m_txrequest NodeId nodeid = node.GetId(); - if (!node.HasPermission(PF_RELAY) && m_txrequest.Count(nodeid) >= MAX_PEER_TX_ANNOUNCEMENTS) { + if (!node.HasPermission(NetPermissionFlags::Relay) && m_txrequest.Count(nodeid) >= MAX_PEER_TX_ANNOUNCEMENTS) { // Too many queued announcements from this peer return; } const CNodeState* state = State(nodeid); // Decide the TxRequestTracker parameters for this announcement: - // - "preferred": if fPreferredDownload is set (= outbound, or PF_NOBAN permission) + // - "preferred": if fPreferredDownload is set (= outbound, or NetPermissionFlags::NoBan permission) // - "reqtime": current time plus delays for: // - NONPREF_PEER_TX_DELAY for announcements from non-preferred connections // - TXID_RELAY_DELAY for txid announcements while wtxid peers are available // - OVERLOADED_PEER_TX_DELAY for announcements from peers which have at least - // MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight (and don't have PF_RELAY). + // MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight (and don't have NetPermissionFlags::Relay). auto delay = std::chrono::microseconds{0}; const bool preferred = state->fPreferredDownload; if (!preferred) delay += NONPREF_PEER_TX_DELAY; if (!gtxid.IsWtxid() && m_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY; - const bool overloaded = !node.HasPermission(PF_RELAY) && + const bool overloaded = !node.HasPermission(NetPermissionFlags::Relay) && m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT; if (overloaded) delay += OVERLOADED_PEER_TX_DELAY; m_txrequest.ReceivedInv(nodeid, gtxid, preferred, current_time + delay); @@ -1637,14 +1637,14 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& // disconnect node in case we have reached the outbound limit for serving historical blocks if (m_connman.OutboundTargetReached(true) && (((pindexBestHeader != nullptr) && (pindexBestHeader->GetBlockTime() - pindex->GetBlockTime() > HISTORICAL_BLOCK_AGE)) || inv.IsMsgFilteredBlk()) && - !pfrom.HasPermission(PF_DOWNLOAD) // nodes with the download permission may exceed target + !pfrom.HasPermission(NetPermissionFlags::Download) // nodes with the download permission may exceed target ) { LogPrint(BCLog::NET, "historical block serving limit reached, disconnect peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; return; } // Avoid leaking prune-height by never sending blocks below the NODE_NETWORK_LIMITED threshold - if (!pfrom.HasPermission(PF_NOBAN) && ( + if (!pfrom.HasPermission(NetPermissionFlags::NoBan) && ( (((pfrom.GetLocalServices() & NODE_NETWORK_LIMITED) == NODE_NETWORK_LIMITED) && ((pfrom.GetLocalServices() & NODE_NETWORK) != NODE_NETWORK) && (m_chainman.ActiveChain().Tip()->nHeight - pindex->nHeight > (int)NODE_NETWORK_LIMITED_MIN_BLOCKS + 2 /* add two blocks buffer extension for possible races */) ) )) { LogPrint(BCLog::NET, "Ignore block request below NODE_NETWORK_LIMITED threshold, disconnect peer=%d\n", pfrom.GetId()); @@ -2738,7 +2738,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, bool fBlocksOnly = m_ignore_incoming_txs || (pfrom.m_tx_relay == nullptr); // Allow peers with relay permission to send data other than blocks in blocks only mode - if (pfrom.HasPermission(PF_RELAY)) { + if (pfrom.HasPermission(NetPermissionFlags::Relay)) { fBlocksOnly = false; } @@ -2952,7 +2952,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } LOCK(cs_main); - if (m_chainman.ActiveChainstate().IsInitialBlockDownload() && !pfrom.HasPermission(PF_DOWNLOAD)) { + if (m_chainman.ActiveChainstate().IsInitialBlockDownload() && !pfrom.HasPermission(NetPermissionFlags::Download)) { LogPrint(BCLog::NET, "Ignoring getheaders from peer=%d because node is in initial block download\n", pfrom.GetId()); return; } @@ -3011,7 +3011,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // Stop processing the transaction early if // 1) We are in blocks only mode and peer has no relay permission // 2) This peer is a block-relay-only peer - if ((m_ignore_incoming_txs && !pfrom.HasPermission(PF_RELAY)) || (pfrom.m_tx_relay == nullptr)) + if ((m_ignore_incoming_txs && !pfrom.HasPermission(NetPermissionFlags::Relay)) || (pfrom.m_tx_relay == nullptr)) { LogPrint(BCLog::NET, "transaction sent in violation of protocol peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; @@ -3056,7 +3056,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. if (AlreadyHaveTx(GenTxid(/* is_wtxid=*/true, wtxid))) { - if (pfrom.HasPermission(PF_FORCERELAY)) { + if (pfrom.HasPermission(NetPermissionFlags::ForceRelay)) { // Always relay transactions received from peers with forcerelay // permission, even if they were already in the mempool, allowing // the node to function as a gateway for nodes hidden behind it. @@ -3585,8 +3585,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, pfrom.vAddrToSend.clear(); std::vector<CAddress> vAddr; - if (pfrom.HasPermission(PF_ADDR)) { - vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); + if (pfrom.HasPermission(NetPermissionFlags::Addr)) { + vAddr = m_connman.GetAddresses(MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND, /* network */ std::nullopt); } else { vAddr = m_connman.GetAddresses(pfrom, MAX_ADDR_TO_SEND, MAX_PCT_ADDR_TO_SEND); } @@ -3598,9 +3598,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } if (msg_type == NetMsgType::MEMPOOL) { - if (!(pfrom.GetLocalServices() & NODE_BLOOM) && !pfrom.HasPermission(PF_MEMPOOL)) + if (!(pfrom.GetLocalServices() & NODE_BLOOM) && !pfrom.HasPermission(NetPermissionFlags::Mempool)) { - if (!pfrom.HasPermission(PF_NOBAN)) + if (!pfrom.HasPermission(NetPermissionFlags::NoBan)) { LogPrint(BCLog::NET, "mempool request with bloom filters disabled, disconnect peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; @@ -3608,9 +3608,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - if (m_connman.OutboundTargetReached(false) && !pfrom.HasPermission(PF_MEMPOOL)) + if (m_connman.OutboundTargetReached(false) && !pfrom.HasPermission(NetPermissionFlags::Mempool)) { - if (!pfrom.HasPermission(PF_NOBAN)) + if (!pfrom.HasPermission(NetPermissionFlags::NoBan)) { LogPrint(BCLog::NET, "mempool request with bandwidth limit reached, disconnect peer=%d\n", pfrom.GetId()); pfrom.fDisconnect = true; @@ -3825,8 +3825,8 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer) peer.m_should_discourage = false; } // peer.m_misbehavior_mutex - if (pnode.HasPermission(PF_NOBAN)) { - // We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag + if (pnode.HasPermission(NetPermissionFlags::NoBan)) { + // We never disconnect or discourage peers for bad behavior if they have NetPermissionFlags::NoBan permission LogPrintf("Warning: not punishing noban peer %d!\n", peer.m_id); return false; } @@ -4463,7 +4463,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (pto->m_tx_relay != nullptr) { LOCK(pto->m_tx_relay->cs_tx_inventory); // Check whether periodic sends should happen - bool fSendTrickle = pto->HasPermission(PF_NOBAN); + bool fSendTrickle = pto->HasPermission(NetPermissionFlags::NoBan); if (pto->m_tx_relay->nNextInvSend < current_time) { fSendTrickle = true; if (pto->IsInboundConn()) { @@ -4620,12 +4620,12 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Detect whether this is a stalling initial-headers-sync peer if (pindexBestHeader->GetBlockTime() <= GetAdjustedTime() - 24 * 60 * 60) { if (current_time > state.m_headers_sync_timeout && nSyncStarted == 1 && (nPreferredDownload - state.fPreferredDownload >= 1)) { - // Disconnect a peer (without the noban permission) if it is our only sync peer, + // Disconnect a peer (without NetPermissionFlags::NoBan permission) if it is our only sync peer, // and we have others we could be using instead. // Note: If all our peers are inbound, then we won't // disconnect our sync peer for stalling; we have bigger // problems if we can't get any outbound peers. - if (!pto->HasPermission(PF_NOBAN)) { + if (!pto->HasPermission(NetPermissionFlags::NoBan)) { LogPrintf("Timeout downloading headers from peer=%d, disconnecting\n", pto->GetId()); pto->fDisconnect = true; return true; @@ -4712,7 +4712,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) !m_ignore_incoming_txs && pto->GetCommonVersion() >= FEEFILTER_VERSION && gArgs.GetBoolArg("-feefilter", DEFAULT_FEEFILTER) && - !pto->HasPermission(PF_FORCERELAY) // peers with the forcerelay permission should not filter txs to us + !pto->HasPermission(NetPermissionFlags::ForceRelay) // peers with the forcerelay permission should not filter txs to us ) { CAmount currentFilter = m_mempool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK(); static FeeFilterRounder g_filter_rounder{CFeeRate{DEFAULT_MIN_RELAY_TX_FEE}}; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 112e216c09..352abae298 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -558,7 +558,7 @@ static std::string IPv4ToString(Span<const uint8_t> a) // Return an IPv6 address text representation with zero compression as described in RFC 5952 // ("A Recommendation for IPv6 Address Text Representation"). -static std::string IPv6ToString(Span<const uint8_t> a) +static std::string IPv6ToString(Span<const uint8_t> a, uint32_t scope_id) { assert(a.size() == ADDR_IPV6_SIZE); const std::array groups{ @@ -606,6 +606,10 @@ static std::string IPv6ToString(Span<const uint8_t> a) r += strprintf("%s%x", ((!r.empty() && r.back() != ':') ? ":" : ""), groups[i]); } + if (scope_id != 0) { + r += strprintf("%%%u", scope_id); + } + return r; } @@ -615,7 +619,7 @@ std::string CNetAddr::ToStringIP() const case NET_IPV4: return IPv4ToString(m_addr); case NET_IPV6: { - return IPv6ToString(m_addr); + return IPv6ToString(m_addr, m_scope_id); } case NET_ONION: switch (m_addr.size()) { @@ -639,7 +643,7 @@ std::string CNetAddr::ToStringIP() const case NET_I2P: return EncodeBase32(m_addr, false /* don't pad with = */) + ".b32.i2p"; case NET_CJDNS: - return IPv6ToString(m_addr); + return IPv6ToString(m_addr, 0); case NET_INTERNAL: return EncodeBase32(m_addr) + ".internal"; case NET_UNROUTABLE: // m_net is never and should not be set to NET_UNROUTABLE diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 998d38e10d..4691937380 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -29,6 +29,7 @@ #include <shlwapi.h> #endif +#include <QAbstractButton> #include <QAbstractItemView> #include <QApplication> #include <QClipboard> @@ -121,6 +122,11 @@ void setupAddressWidget(QValidatedLineEdit *widget, QWidget *parent) widget->setCheckValidator(new BitcoinAddressCheckValidator(parent)); } +void AddButtonShortcut(QAbstractButton* button, const QKeySequence& shortcut) +{ + QObject::connect(new QShortcut(shortcut, button), &QShortcut::activated, [button]() { button->animateClick(); }); +} + bool parseBitcoinURI(const QUrl &uri, SendCoinsRecipient *out) { // return if URI is not valid or is no bitcoin: URI diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index a1cf274354..9c2ad74e1e 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -36,10 +36,12 @@ namespace interfaces } QT_BEGIN_NAMESPACE +class QAbstractButton; class QAbstractItemView; class QAction; class QDateTime; class QFont; +class QKeySequence; class QLineEdit; class QMenu; class QPoint; @@ -65,6 +67,14 @@ namespace GUIUtil // Set up widget for address void setupAddressWidget(QValidatedLineEdit *widget, QWidget *parent); + /** + * Connects an additional shortcut to a QAbstractButton. Works around the + * one shortcut limitation of the button's shortcut property. + * @param[in] button QAbstractButton to assign shortcut to + * @param[in] shortcut QKeySequence to use as shortcut + */ + void AddButtonShortcut(QAbstractButton* button, const QKeySequence& shortcut); + // Parse "bitcoin:" URI into recipient object, return true on successful parsing bool parseBitcoinURI(const QUrl &uri, SendCoinsRecipient *out); bool parseBitcoinURI(QString uri, SendCoinsRecipient *out); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 731d6a4eef..afbdc07ba0 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -495,8 +495,18 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty ui->openDebugLogfileButton->setIcon(platformStyle->SingleColorIcon(":/icons/export")); } ui->clearButton->setIcon(platformStyle->SingleColorIcon(":/icons/remove")); + ui->fontBiggerButton->setIcon(platformStyle->SingleColorIcon(":/icons/fontbigger")); + //: Main shortcut to increase the RPC console font size. + ui->fontBiggerButton->setShortcut(tr("Ctrl++")); + //: Secondary shortcut to increase the RPC console font size. + GUIUtil::AddButtonShortcut(ui->fontBiggerButton, tr("Ctrl+=")); + ui->fontSmallerButton->setIcon(platformStyle->SingleColorIcon(":/icons/fontsmaller")); + //: Main shortcut to decrease the RPC console font size. + ui->fontSmallerButton->setShortcut(tr("Ctrl+-")); + //: Secondary shortcut to decrease the RPC console font size. + GUIUtil::AddButtonShortcut(ui->fontSmallerButton, tr("Ctrl+_")); // Install event filter for up and down arrow ui->lineEdit->installEventFilter(this); @@ -806,20 +816,23 @@ void RPCConsole::clear(bool keep_prompt) ).arg(fixedFontInfo.family(), QString("%1pt").arg(consoleFontSize)) ); -#ifdef Q_OS_MAC - QString clsKey = "(⌘)-L"; -#else - QString clsKey = "Ctrl-L"; -#endif - - message(CMD_REPLY, (tr("Welcome to the %1 RPC console.").arg(PACKAGE_NAME) + "<br>" + - tr("Use up and down arrows to navigate history, and %1 to clear screen.").arg("<b>"+clsKey+"</b>") + "<br>" + - tr("Type %1 for an overview of available commands.").arg("<b>help</b>") + "<br>" + - tr("For more information on using this console type %1.").arg("<b>help-console</b>") + - "<br><span class=\"secwarning\"><br>" + - tr("WARNING: Scammers have been active, telling users to type commands here, stealing their wallet contents. Do not use this console without fully understanding the ramifications of a command.") + - "</span>"), - true); + message(CMD_REPLY, + tr("Welcome to the %1 RPC console.").arg(PACKAGE_NAME) + + "<br>" + + tr("Use up and down arrows to navigate history, and %1 to clear screen.") + .arg("<b>" + ui->clearButton->shortcut().toString(QKeySequence::NativeText) + "</b>") + + "<br>" + + tr("Use %1 and %2 to increase or decrease the font size.") + .arg("<b>" + ui->fontBiggerButton->shortcut().toString(QKeySequence::NativeText) + "</b>") + .arg("<b>" + ui->fontSmallerButton->shortcut().toString(QKeySequence::NativeText) + "</b>") + + "<br>" + + tr("Type %1 for an overview of available commands.").arg("<b>help</b>") + + "<br>" + + tr("For more information on using this console type %1.").arg("<b>help-console</b>") + + "<br><span class=\"secwarning\"><br>" + + tr("WARNING: Scammers have been active, telling users to type commands here, stealing their wallet contents. Do not use this console without fully understanding the ramifications of a command.") + + "</span>", + true); } void RPCConsole::keyPressEvent(QKeyEvent *event) @@ -1062,7 +1075,7 @@ void RPCConsole::updateDetailWidget() ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer)); ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type, /* prepend_direction */ true)); ui->peerNetwork->setText(GUIUtil::NetworkToQString(stats->nodeStats.m_network)); - if (stats->nodeStats.m_permissionFlags == PF_NONE) { + if (stats->nodeStats.m_permissionFlags == NetPermissionFlags::None) { ui->peerPermissions->setText(ts.na); } else { QStringList permissions; diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index a026069232..39c69fe184 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -63,8 +63,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node) node.setContext(&test.m_node); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); auto build_address = [&wallet]() { CKey key; diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 8dffd2f59f..36b9024541 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -20,9 +20,9 @@ #endif #include <QAction> -#include <QEventLoop> #include <QLineEdit> #include <QScopedPointer> +#include <QSignalSpy> #include <QTest> #include <QTextEdit> #include <QtGlobal> @@ -33,13 +33,14 @@ namespace { //! Call getblockchaininfo RPC and check first field of JSON output. void TestRpcCommand(RPCConsole* console) { - QEventLoop loop; QTextEdit* messagesWidget = console->findChild<QTextEdit*>("messagesWidget"); - QObject::connect(messagesWidget, &QTextEdit::textChanged, &loop, &QEventLoop::quit); QLineEdit* lineEdit = console->findChild<QLineEdit*>("lineEdit"); + QSignalSpy mw_spy(messagesWidget, &QTextEdit::textChanged); + QVERIFY(mw_spy.isValid()); QTest::keyClicks(lineEdit, "getblockchaininfo"); QTest::keyClick(lineEdit, Qt::Key_Return); - loop.exec(); + QVERIFY(mw_spy.wait(1000)); + QCOMPARE(mw_spy.count(), 2); QString output = messagesWidget->toPlainText(); UniValue value; value.read(output.right(output.size() - output.lastIndexOf(QChar::ObjectReplacementCharacter) - 1).toStdString()); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 03460cd6eb..febfead6ad 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -140,8 +140,7 @@ void TestGUI(interfaces::Node& node) } node.setContext(&test.m_node); std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(node.context()->chain.get(), "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f2b99579b7..63826c49e1 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1099,7 +1099,7 @@ static RPCHelpMan gettxoutsetinfo() "Note this call may take some time if you are not using coinstatsindex.\n", { {"hash_type", RPCArg::Type::STR, RPCArg::Default{"hash_serialized_2"}, "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'muhash', 'none'."}, - {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The block hash or height of the target height (only available with coinstatsindex).", "", {"", "string or numeric"}}, + {"hash_or_height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "The block hash or height of the target height (only available with coinstatsindex).", "", {"", "string or numeric"}}, {"use_index", RPCArg::Type::BOOL, RPCArg::Default{true}, "Use coinstatsindex, if available."}, }, RPCResult{ diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 1f6b6e8d7e..4999eefc24 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -28,6 +28,8 @@ #include <version.h> #include <warnings.h> +#include <optional> + #include <univalue.h> const std::vector<std::string> CONNECTION_TYPE_DOC{ @@ -159,7 +161,7 @@ static RPCHelpMan getpeerinfo() "When a message type is not listed in this json object, the bytes sent are 0.\n" "Only known message types can appear as keys in the object."} }}, - {RPCResult::Type::OBJ, "bytesrecv_per_msg", "", + {RPCResult::Type::OBJ_DYN, "bytesrecv_per_msg", "", { {RPCResult::Type::NUM, "msg", "The total bytes received aggregated by message type\n" "When a message type is not listed in this json object, the bytes received are 0.\n" @@ -851,6 +853,7 @@ static RPCHelpMan getnodeaddresses() "\nReturn known addresses, which can potentially be used to find new nodes in the network.\n", { {"count", RPCArg::Type::NUM, RPCArg::Default{1}, "The maximum number of addresses to return. Specify 0 to return all known addresses."}, + {"network", RPCArg::Type::STR, RPCArg::DefaultHint{"all networks"}, "Return only addresses of the specified network. Can be one of: " + Join(GetNetworkNames(), ", ") + "."}, }, RPCResult{ RPCResult::Type::ARR, "", "", @@ -867,7 +870,10 @@ static RPCHelpMan getnodeaddresses() }, RPCExamples{ HelpExampleCli("getnodeaddresses", "8") - + HelpExampleRpc("getnodeaddresses", "8") + + HelpExampleCli("getnodeaddresses", "4 \"i2p\"") + + HelpExampleCli("-named getnodeaddresses", "network=onion count=12") + + HelpExampleRpc("getnodeaddresses", "8") + + HelpExampleRpc("getnodeaddresses", "4, \"i2p\"") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { @@ -877,8 +883,13 @@ static RPCHelpMan getnodeaddresses() const int count{request.params[0].isNull() ? 1 : request.params[0].get_int()}; if (count < 0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Address count out of range"); + const std::optional<Network> network{request.params[1].isNull() ? std::nullopt : std::optional<Network>{ParseNetwork(request.params[1].get_str())}}; + if (network == NET_UNROUTABLE) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Network not recognized: %s", request.params[1].get_str())); + } + // returns a shuffled list of CAddress - const std::vector<CAddress> vAddr{connman.GetAddresses(count, /* max_pct */ 0)}; + const std::vector<CAddress> vAddr{connman.GetAddresses(count, /* max_pct */ 0, network)}; UniValue ret(UniValue::VARR); for (const CAddress& addr : vAddr) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index f1ab1b4687..adb8ac0595 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1434,7 +1434,7 @@ static RPCHelpMan createpsbt() "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" " accepted as second parameter.", { - {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", { {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT}, }, diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index d438537606..49b40924e0 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -12,6 +12,7 @@ #include <boost/test/unit_test.hpp> +#include <optional> #include <string> class CAddrManTest : public CAddrMan @@ -392,7 +393,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) // Test: Sanity check, GetAddr should never return anything if addrman // is empty. BOOST_CHECK_EQUAL(addrman.size(), 0U); - std::vector<CAddress> vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */0); + std::vector<CAddress> vAddr1 = addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt); BOOST_CHECK_EQUAL(vAddr1.size(), 0U); CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); @@ -415,15 +416,15 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) BOOST_CHECK(addrman.Add(addr4, source2)); BOOST_CHECK(addrman.Add(addr5, source1)); - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U); // Net processing asks for 23% of addresses. 23% of 5 is 1 rounded down. - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt).size(), 1U); // Test: Ensure GetAddr works with new and tried addresses. addrman.Good(CAddress(addr1, NODE_NONE)); addrman.Good(CAddress(addr2, NODE_NONE)); - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0).size(), 5U); - BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23).size(), 1U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 0, /* max_pct */ 0, /* network */ std::nullopt).size(), 5U); + BOOST_CHECK_EQUAL(addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt).size(), 1U); // Test: Ensure GetAddr still returns 23% when addrman has many addrs. for (unsigned int i = 1; i < (8 * 256); i++) { @@ -438,7 +439,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) if (i % 8 == 0) addrman.Good(addr); } - std::vector<CAddress> vAddr = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23); + std::vector<CAddress> vAddr = addrman.GetAddr(/* max_addresses */ 2500, /* max_pct */ 23, /* network */ std::nullopt); size_t percent23 = (addrman.size() * 23) / 100; BOOST_CHECK_EQUAL(vAddr.size(), percent23); diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 0baf30aef6..98ae32a8d0 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -60,7 +60,10 @@ FUZZ_TARGET_INIT(addrman, initialize_addrman) (void)addr_man.Select(fuzzed_data_provider.ConsumeBool()); }, [&] { - (void)addr_man.GetAddr(fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096)); + (void)addr_man.GetAddr( + /* max_addresses */ fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096), + /* max_pct */ fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, 4096), + /* network */ std::nullopt); }, [&] { const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 21dc80cc8d..878b5a27da 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -6,6 +6,7 @@ #include <chainparams.h> #include <chainparamsbase.h> #include <coins.h> +#include <consensus/tx_check.h> #include <consensus/tx_verify.h> #include <consensus/validation.h> #include <key.h> @@ -230,6 +231,11 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view) // consensus/tx_verify.cpp:171: bool Consensus::CheckTxInputs(const CTransaction &, TxValidationState &, const CCoinsViewCache &, int, CAmount &): Assertion `!coin.IsSpent()' failed. return; } + TxValidationState dummy; + if (!CheckTransaction(transaction, dummy)) { + // It is not allowed to call CheckTxInputs if CheckTransaction failed + return; + } (void)Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee_out); assert(MoneyRange(tx_fee_out)); }, diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index e07f25dedf..3e9998af30 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -71,10 +71,16 @@ FUZZ_TARGET_INIT(connman, initialize_connman) (void)connman.ForNode(fuzzed_data_provider.ConsumeIntegral<NodeId>(), [&](auto) { return fuzzed_data_provider.ConsumeBool(); }); }, [&] { - (void)connman.GetAddresses(fuzzed_data_provider.ConsumeIntegral<size_t>(), fuzzed_data_provider.ConsumeIntegral<size_t>()); + (void)connman.GetAddresses( + /* max_addresses */ fuzzed_data_provider.ConsumeIntegral<size_t>(), + /* max_pct */ fuzzed_data_provider.ConsumeIntegral<size_t>(), + /* network */ std::nullopt); }, [&] { - (void)connman.GetAddresses(random_node, fuzzed_data_provider.ConsumeIntegral<size_t>(), fuzzed_data_provider.ConsumeIntegral<size_t>()); + (void)connman.GetAddresses( + /* requestor */ random_node, + /* max_addresses */ fuzzed_data_provider.ConsumeIntegral<size_t>(), + /* max_pct */ fuzzed_data_provider.ConsumeIntegral<size_t>()); }, [&] { (void)connman.GetDeterministicRandomizer(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); diff --git a/src/test/fuzz/i2p.cpp b/src/test/fuzz/i2p.cpp index 345d68502a..e0d62fb493 100644 --- a/src/test/fuzz/i2p.cpp +++ b/src/test/fuzz/i2p.cpp @@ -37,7 +37,7 @@ FUZZ_TARGET_INIT(i2p, initialize_i2p) if (sess.Listen(conn)) { if (sess.Accept(conn)) { try { - conn.sock->RecvUntilTerminator('\n', 10ms, interrupt, i2p::sam::MAX_MSG_SIZE); + (void)conn.sock->RecvUntilTerminator('\n', 10ms, interrupt, i2p::sam::MAX_MSG_SIZE); } catch (const std::runtime_error&) { } } diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index 6fdf4b653c..6ea79464d0 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -25,7 +25,7 @@ FUZZ_TARGET(net_permissions) (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags); (void)NetPermissions::AddFlag(net_whitebind_permissions.m_flags, net_permission_flags); assert(NetPermissions::HasFlag(net_whitebind_permissions.m_flags, net_permission_flags)); - (void)NetPermissions::ClearFlag(net_whitebind_permissions.m_flags, NetPermissionFlags::PF_ISIMPLICIT); + (void)NetPermissions::ClearFlag(net_whitebind_permissions.m_flags, NetPermissionFlags::Implicit); (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags); } @@ -35,7 +35,7 @@ FUZZ_TARGET(net_permissions) (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags); (void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags); assert(NetPermissions::HasFlag(net_whitelist_permissions.m_flags, net_permission_flags)); - (void)NetPermissions::ClearFlag(net_whitelist_permissions.m_flags, NetPermissionFlags::PF_ISIMPLICIT); + (void)NetPermissions::ClearFlag(net_whitelist_permissions.m_flags, NetPermissionFlags::Implicit); (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags); } } diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index 17e4405a13..797b9cea3e 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -61,8 +61,11 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) return; } - TxValidationState state_with_dupe_check; - (void)CheckTransaction(tx, state_with_dupe_check); + { + TxValidationState state_with_dupe_check; + const bool res{CheckTransaction(tx, state_with_dupe_check)}; + Assert(res == state_with_dupe_check.IsValid()); + } const CFeeRate dust_relay_fee{DUST_RELAY_TX_FEE}; std::string reason; diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 1c397481dc..7a122bd8b0 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -300,13 +300,17 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic) // IPv6, scoped/link-local. See https://tools.ietf.org/html/rfc4007 // We support non-negative decimal integers (uint32_t) as zone id indices. - // Test with a fairly-high value, e.g. 32, to avoid locally reserved ids. + // Normal link-local scoped address functionality is to append "%" plus the + // zone id, for example, given a link-local address of "fe80::1" and a zone + // id of "32", return the address as "fe80::1%32". const std::string link_local{"fe80::1"}; const std::string scoped_addr{link_local + "%32"}; BOOST_REQUIRE(LookupHost(scoped_addr, addr, false)); BOOST_REQUIRE(addr.IsValid()); BOOST_REQUIRE(addr.IsIPv6()); BOOST_CHECK(!addr.IsBindAny()); + BOOST_CHECK_EQUAL(addr.ToString(), scoped_addr); + // Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope. BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false)); BOOST_REQUIRE(addr.IsValid()); diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index b316a37c6e..3c47cf83e2 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -381,27 +381,27 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) // If no permission flags, assume backward compatibility BOOST_CHECK(NetWhitebindPermissions::TryParse("1.2.3.4:32", whitebindPermissions, error)); BOOST_CHECK(error.empty()); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_ISIMPLICIT); - BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT)); - NetPermissions::ClearFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT); - BOOST_CHECK(!NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); - NetPermissions::AddFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT); - BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, PF_ISIMPLICIT)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::Implicit); + BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, NetPermissionFlags::Implicit)); + NetPermissions::ClearFlag(whitebindPermissions.m_flags, NetPermissionFlags::Implicit); + BOOST_CHECK(!NetPermissions::HasFlag(whitebindPermissions.m_flags, NetPermissionFlags::Implicit)); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::None); + NetPermissions::AddFlag(whitebindPermissions.m_flags, NetPermissionFlags::Implicit); + BOOST_CHECK(NetPermissions::HasFlag(whitebindPermissions.m_flags, NetPermissionFlags::Implicit)); // Can set one permission BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::BloomFilter); BOOST_CHECK(NetWhitebindPermissions::TryParse("@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::None); NetWhitebindPermissions noban, noban_download, download_noban, download; // "noban" implies "download" BOOST_REQUIRE(NetWhitebindPermissions::TryParse("noban@1.2.3.4:32", noban, error)); - BOOST_CHECK_EQUAL(noban.m_flags, NetPermissionFlags::PF_NOBAN); - BOOST_CHECK(NetPermissions::HasFlag(noban.m_flags, NetPermissionFlags::PF_DOWNLOAD)); - BOOST_CHECK(NetPermissions::HasFlag(noban.m_flags, NetPermissionFlags::PF_NOBAN)); + BOOST_CHECK_EQUAL(noban.m_flags, NetPermissionFlags::NoBan); + BOOST_CHECK(NetPermissions::HasFlag(noban.m_flags, NetPermissionFlags::Download)); + BOOST_CHECK(NetPermissions::HasFlag(noban.m_flags, NetPermissionFlags::NoBan)); // "noban,download" is equivalent to "noban" BOOST_REQUIRE(NetWhitebindPermissions::TryParse("noban,download@1.2.3.4:32", noban_download, error)); @@ -413,31 +413,31 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) // "download" excludes (does not imply) "noban" BOOST_REQUIRE(NetWhitebindPermissions::TryParse("download@1.2.3.4:32", download, error)); - BOOST_CHECK_EQUAL(download.m_flags, NetPermissionFlags::PF_DOWNLOAD); - BOOST_CHECK(NetPermissions::HasFlag(download.m_flags, NetPermissionFlags::PF_DOWNLOAD)); - BOOST_CHECK(!NetPermissions::HasFlag(download.m_flags, NetPermissionFlags::PF_NOBAN)); + BOOST_CHECK_EQUAL(download.m_flags, NetPermissionFlags::Download); + BOOST_CHECK(NetPermissions::HasFlag(download.m_flags, NetPermissionFlags::Download)); + BOOST_CHECK(!NetPermissions::HasFlag(download.m_flags, NetPermissionFlags::NoBan)); // Happy path, can parse flags BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,forcerelay@1.2.3.4:32", whitebindPermissions, error)); // forcerelay should also activate the relay permission - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_FORCERELAY | PF_RELAY); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::BloomFilter | NetPermissionFlags::ForceRelay | NetPermissionFlags::Relay); BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,noban@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::BloomFilter | NetPermissionFlags::Relay | NetPermissionFlags::NoBan); BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,forcerelay,noban@1.2.3.4:32", whitebindPermissions, error)); BOOST_CHECK(NetWhitebindPermissions::TryParse("all@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_ALL); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::All); // Allow dups BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,noban,noban@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN | PF_DOWNLOAD); // "noban" implies "download" + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::BloomFilter | NetPermissionFlags::Relay | NetPermissionFlags::NoBan | NetPermissionFlags::Download); // "noban" implies "download" // Allow empty BOOST_CHECK(NetWhitebindPermissions::TryParse("bloom,relay,,noban@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_BLOOMFILTER | PF_RELAY | PF_NOBAN); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::BloomFilter | NetPermissionFlags::Relay | NetPermissionFlags::NoBan); BOOST_CHECK(NetWhitebindPermissions::TryParse(",@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::None); BOOST_CHECK(NetWhitebindPermissions::TryParse(",,@1.2.3.4:32", whitebindPermissions, error)); - BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, PF_NONE); + BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::None); // Detect invalid flag BOOST_CHECK(!NetWhitebindPermissions::TryParse("bloom,forcerelay,oopsie@1.2.3.4:32", whitebindPermissions, error)); @@ -449,16 +449,16 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) // Happy path for whitelist parsing BOOST_CHECK(NetWhitelistPermissions::TryParse("noban@1.2.3.4", whitelistPermissions, error)); - BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, PF_NOBAN); - BOOST_CHECK(NetPermissions::HasFlag(whitelistPermissions.m_flags, NetPermissionFlags::PF_NOBAN)); + BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, NetPermissionFlags::NoBan); + BOOST_CHECK(NetPermissions::HasFlag(whitelistPermissions.m_flags, NetPermissionFlags::NoBan)); BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay@1.2.3.4/32", whitelistPermissions, error)); - BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, PF_BLOOMFILTER | PF_FORCERELAY | PF_NOBAN | PF_RELAY); + BOOST_CHECK_EQUAL(whitelistPermissions.m_flags, NetPermissionFlags::BloomFilter | NetPermissionFlags::ForceRelay | NetPermissionFlags::NoBan | NetPermissionFlags::Relay); BOOST_CHECK(error.empty()); BOOST_CHECK_EQUAL(whitelistPermissions.m_subnet.ToString(), "1.2.3.4/32"); BOOST_CHECK(NetWhitelistPermissions::TryParse("bloom,forcerelay,noban,relay,mempool@1.2.3.4/32", whitelistPermissions, error)); - const auto strings = NetPermissions::ToStrings(PF_ALL); + const auto strings = NetPermissions::ToStrings(NetPermissionFlags::All); BOOST_CHECK_EQUAL(strings.size(), 7U); BOOST_CHECK(std::find(strings.begin(), strings.end(), "bloomfilter") != strings.end()); BOOST_CHECK(std::find(strings.begin(), strings.end(), "forcerelay") != strings.end()); diff --git a/src/test/sock_tests.cpp b/src/test/sock_tests.cpp index 400de875b7..9e98f4f0b1 100644 --- a/src/test/sock_tests.cpp +++ b/src/test/sock_tests.cpp @@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(wait) Sock sock0(s[0]); Sock sock1(s[1]); - std::thread waiter([&sock0]() { sock0.Wait(24h, Sock::RECV); }); + std::thread waiter([&sock0]() { (void)sock0.Wait(24h, Sock::RECV); }); BOOST_REQUIRE_EQUAL(sock1.Send("a", 1, 0), 1); @@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(recv_until_terminator_limit) // 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); + (void)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); } diff --git a/src/test/util/net.h b/src/test/util/net.h index 9268d60a1e..71685d437a 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -46,16 +46,16 @@ constexpr ServiceFlags ALL_SERVICE_FLAGS[]{ }; constexpr NetPermissionFlags ALL_NET_PERMISSION_FLAGS[]{ - NetPermissionFlags::PF_NONE, - NetPermissionFlags::PF_BLOOMFILTER, - NetPermissionFlags::PF_RELAY, - NetPermissionFlags::PF_FORCERELAY, - NetPermissionFlags::PF_NOBAN, - NetPermissionFlags::PF_MEMPOOL, - NetPermissionFlags::PF_ADDR, - NetPermissionFlags::PF_DOWNLOAD, - NetPermissionFlags::PF_ISIMPLICIT, - NetPermissionFlags::PF_ALL, + NetPermissionFlags::None, + NetPermissionFlags::BloomFilter, + NetPermissionFlags::Relay, + NetPermissionFlags::ForceRelay, + NetPermissionFlags::NoBan, + NetPermissionFlags::Mempool, + NetPermissionFlags::Addr, + NetPermissionFlags::Download, + NetPermissionFlags::Implicit, + NetPermissionFlags::All, }; constexpr ConnectionType ALL_CONNECTION_TYPES[]{ diff --git a/src/util/sock.cpp b/src/util/sock.cpp index 0bc9795db3..b6c2a47434 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -179,7 +179,7 @@ void Sock::SendComplete(const std::string& data, // Wait for a short while (or the socket to become ready for sending) before retrying // if nothing was sent. const auto wait_time = std::min(deadline - now, std::chrono::milliseconds{MAX_WAIT_FOR_IO}); - Wait(wait_time, SEND); + (void)Wait(wait_time, SEND); } } @@ -262,7 +262,7 @@ std::string Sock::RecvUntilTerminator(uint8_t terminator, // Wait for a short while (or the socket to become ready for reading) before retrying. const auto wait_time = std::min(deadline - now, std::chrono::milliseconds{MAX_WAIT_FOR_IO}); - Wait(wait_time, RECV); + (void)Wait(wait_time, RECV); } } diff --git a/src/util/sock.h b/src/util/sock.h index a4df7cd21b..59cc8c0b1d 100644 --- a/src/util/sock.h +++ b/src/util/sock.h @@ -64,7 +64,7 @@ public: * Get the value of the contained socket. * @return socket or INVALID_SOCKET if empty */ - virtual SOCKET Get() const; + [[nodiscard]] virtual SOCKET Get() const; /** * Get the value of the contained socket and drop ownership. It will not be closed by the @@ -82,26 +82,29 @@ public: * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ - virtual ssize_t Send(const void* data, size_t len, int flags) const; + [[nodiscard]] virtual ssize_t Send(const void* data, size_t len, int flags) const; /** * recv(2) wrapper. Equivalent to `recv(this->Get(), buf, len, flags);`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ - virtual ssize_t Recv(void* buf, size_t len, int flags) const; + [[nodiscard]] virtual ssize_t Recv(void* buf, size_t len, int flags) const; /** * connect(2) wrapper. Equivalent to `connect(this->Get(), addr, addrlen)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ - virtual int Connect(const sockaddr* addr, socklen_t addr_len) const; + [[nodiscard]] virtual int Connect(const sockaddr* addr, socklen_t addr_len) const; /** * getsockopt(2) wrapper. Equivalent to * `getsockopt(this->Get(), level, opt_name, opt_val, opt_len)`. Code that uses this * wrapper can be unit tested if this method is overridden by a mock Sock implementation. */ - virtual int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const; + [[nodiscard]] virtual int GetSockOpt(int level, + int opt_name, + void* opt_val, + socklen_t* opt_len) const; using Event = uint8_t; @@ -124,9 +127,9 @@ public: * value of `true` and `occurred` being set to 0. * @return true on success and false otherwise */ - virtual bool Wait(std::chrono::milliseconds timeout, - Event requested, - Event* occurred = nullptr) const; + [[nodiscard]] virtual bool Wait(std::chrono::milliseconds timeout, + Event requested, + Event* occurred = nullptr) const; /* Higher level, convenience, methods. These may throw. */ @@ -154,17 +157,17 @@ public: * @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, - size_t max_data) const; + [[nodiscard]] virtual std::string RecvUntilTerminator(uint8_t terminator, + std::chrono::milliseconds timeout, + CThreadInterrupt& interrupt, + size_t max_data) const; /** * Check if still connected. * @param[out] errmsg The error string, if the socket has been disconnected. * @return true if connected */ - virtual bool IsConnected(std::string& errmsg) const; + [[nodiscard]] virtual bool IsConnected(std::string& errmsg) const; protected: /** diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 4734de3e0b..f514613f0d 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -593,13 +593,14 @@ std::string Capitalize(std::string str) std::string HexStr(const Span<const uint8_t> s) { - std::string rv; + std::string rv(s.size() * 2, '\0'); static constexpr char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - rv.reserve(s.size() * 2); - for (uint8_t v: s) { - rv.push_back(hexmap[v >> 4]); - rv.push_back(hexmap[v & 15]); + auto it = rv.begin(); + for (uint8_t v : s) { + *it++ = hexmap[v >> 4]; + *it++ = hexmap[v & 15]; } + assert(it == rv.end()); return rv; } diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index e314107988..c39c0c7e73 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -194,8 +194,7 @@ bool CreateFromDump(const std::string& name, const fs::path& wallet_path, biling std::shared_ptr<CWallet> wallet(new CWallet(nullptr /* chain */, name, std::move(database)), WalletToolReleaseWallet); { LOCK(wallet->cs_wallet); - bool first_run = true; - DBErrors load_wallet_ret = wallet->LoadWallet(first_run); + DBErrors load_wallet_ret = wallet->LoadWallet(); if (load_wallet_ret != DBErrors::LOAD_OK) { error = strprintf(_("Error creating %s"), name); return false; diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 342a165f39..e0df96666f 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -105,7 +105,8 @@ bool LoadWallets(interfaces::Chain& chain) if (!database && status == DatabaseStatus::FAILED_NOT_FOUND) { continue; } - std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(chain, name, std::move(database), options.create_flags, error, warnings) : nullptr; + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(&chain, name, std::move(database), options.create_flags, error, warnings) : nullptr; if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n"))); if (!pwallet) { chain.initError(error); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5567d183b6..4e3c8ce49d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -854,7 +854,7 @@ static RPCHelpMan sendmany() HELP_REQUIRING_PASSPHRASE, { {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", "\"\""}, - {"amounts", RPCArg::Type::OBJ, RPCArg::Optional::NO, "The addresses and amounts", + {"amounts", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::NO, "The addresses and amounts", { {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The bitcoin address is the key, the numeric amount (can be string) in " + CURRENCY_UNIT + " is the value"}, }, @@ -2568,7 +2568,7 @@ static RPCHelpMan loadwallet() "\napplied to the new wallet (eg -rescan, etc).\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Default{UniValue::VNULL}, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2696,10 +2696,10 @@ static RPCHelpMan createwallet() {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name for the new wallet. If this is a path, the wallet will be created at the path location."}, {"disable_private_keys", RPCArg::Type::BOOL, RPCArg::Default{false}, "Disable the possibility of private keys (only watchonlys are possible in this mode)."}, {"blank", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."}, - {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."}, + {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation"}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Default{UniValue::VNULL}, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."}, }, RPCResult{ @@ -2790,7 +2790,7 @@ static RPCHelpMan unloadwallet() "Specifying the wallet name on a wallet endpoint is invalid.", { {"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Default{UniValue::VNULL}, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, @@ -4021,7 +4021,7 @@ static RPCHelpMan send() "That is, each address can only appear once and there can only be one 'data' object.\n" "For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.", { - {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", { {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, }, @@ -4370,7 +4370,7 @@ static RPCHelpMan walletcreatefundedpsbt() "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" "accepted as second parameter.", { - {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", { {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, }, diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index e245a277e4..2e60aca017 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -16,6 +16,7 @@ #include <sqlite3.h> #include <stdint.h> +#include <optional> #include <utility> #include <vector> @@ -35,6 +36,36 @@ static void ErrorLogCallback(void* arg, int code, const char* msg) LogPrintf("SQLite Error. Code: %d. Message: %s\n", code, msg); } +static std::optional<int> ReadPragmaInteger(sqlite3* db, const std::string& key, const std::string& description, bilingual_str& error) +{ + std::string stmt_text = strprintf("PRAGMA %s", key); + sqlite3_stmt* pragma_read_stmt{nullptr}; + int ret = sqlite3_prepare_v2(db, stmt_text.c_str(), -1, &pragma_read_stmt, nullptr); + if (ret != SQLITE_OK) { + sqlite3_finalize(pragma_read_stmt); + error = Untranslated(strprintf("SQLiteDatabase: Failed to prepare the statement to fetch %s: %s", description, sqlite3_errstr(ret))); + return std::nullopt; + } + ret = sqlite3_step(pragma_read_stmt); + if (ret != SQLITE_ROW) { + sqlite3_finalize(pragma_read_stmt); + error = Untranslated(strprintf("SQLiteDatabase: Failed to fetch %s: %s", description, sqlite3_errstr(ret))); + return std::nullopt; + } + int result = sqlite3_column_int(pragma_read_stmt, 0); + sqlite3_finalize(pragma_read_stmt); + return result; +} + +static void SetPragma(sqlite3* db, const std::string& key, const std::string& value, const std::string& err_msg) +{ + std::string stmt_text = strprintf("PRAGMA %s = %s", key, value); + int ret = sqlite3_exec(db, stmt_text.c_str(), nullptr, nullptr, nullptr); + if (ret != SQLITE_OK) { + throw std::runtime_error(strprintf("SQLiteDatabase: %s: %s\n", err_msg, sqlite3_errstr(ret))); + } +} + SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, bool mock) : WalletDatabase(), m_mock(mock), m_dir_path(dir_path.string()), m_file_path(file_path.string()) { @@ -114,21 +145,9 @@ bool SQLiteDatabase::Verify(bilingual_str& error) assert(m_db); // Check the application ID matches our network magic - sqlite3_stmt* app_id_stmt{nullptr}; - int ret = sqlite3_prepare_v2(m_db, "PRAGMA application_id", -1, &app_id_stmt, nullptr); - if (ret != SQLITE_OK) { - sqlite3_finalize(app_id_stmt); - error = strprintf(_("SQLiteDatabase: Failed to prepare the statement to fetch the application id: %s"), sqlite3_errstr(ret)); - return false; - } - ret = sqlite3_step(app_id_stmt); - if (ret != SQLITE_ROW) { - sqlite3_finalize(app_id_stmt); - error = strprintf(_("SQLiteDatabase: Failed to fetch the application id: %s"), sqlite3_errstr(ret)); - return false; - } - uint32_t app_id = static_cast<uint32_t>(sqlite3_column_int(app_id_stmt, 0)); - sqlite3_finalize(app_id_stmt); + auto read_result = ReadPragmaInteger(m_db, "application_id", "the application id", error); + if (!read_result.has_value()) return false; + uint32_t app_id = static_cast<uint32_t>(read_result.value()); uint32_t net_magic = ReadBE32(Params().MessageStart()); if (app_id != net_magic) { error = strprintf(_("SQLiteDatabase: Unexpected application id. Expected %u, got %u"), net_magic, app_id); @@ -136,28 +155,16 @@ bool SQLiteDatabase::Verify(bilingual_str& error) } // Check our schema version - sqlite3_stmt* user_ver_stmt{nullptr}; - ret = sqlite3_prepare_v2(m_db, "PRAGMA user_version", -1, &user_ver_stmt, nullptr); - if (ret != SQLITE_OK) { - sqlite3_finalize(user_ver_stmt); - error = strprintf(_("SQLiteDatabase: Failed to prepare the statement to fetch sqlite wallet schema version: %s"), sqlite3_errstr(ret)); - return false; - } - ret = sqlite3_step(user_ver_stmt); - if (ret != SQLITE_ROW) { - sqlite3_finalize(user_ver_stmt); - error = strprintf(_("SQLiteDatabase: Failed to fetch sqlite wallet schema version: %s"), sqlite3_errstr(ret)); - return false; - } - int32_t user_ver = sqlite3_column_int(user_ver_stmt, 0); - sqlite3_finalize(user_ver_stmt); + read_result = ReadPragmaInteger(m_db, "user_version", "sqlite wallet schema version", error); + if (!read_result.has_value()) return false; + int32_t user_ver = read_result.value(); if (user_ver != WALLET_SCHEMA_VERSION) { error = strprintf(_("SQLiteDatabase: Unknown sqlite wallet schema version %d. Only version %d is supported"), user_ver, WALLET_SCHEMA_VERSION); return false; } sqlite3_stmt* stmt{nullptr}; - ret = sqlite3_prepare_v2(m_db, "PRAGMA integrity_check", -1, &stmt, nullptr); + int ret = sqlite3_prepare_v2(m_db, "PRAGMA integrity_check", -1, &stmt, nullptr); if (ret != SQLITE_OK) { sqlite3_finalize(stmt); error = strprintf(_("SQLiteDatabase: Failed to prepare statement to verify database: %s"), sqlite3_errstr(ret)); @@ -213,12 +220,9 @@ void SQLiteDatabase::Open() // Acquire an exclusive lock on the database // First change the locking mode to exclusive - int ret = sqlite3_exec(m_db, "PRAGMA locking_mode = exclusive", nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error(strprintf("SQLiteDatabase: Unable to change database locking mode to exclusive: %s\n", sqlite3_errstr(ret))); - } + SetPragma(m_db, "locking_mode", "exclusive", "Unable to change database locking mode to exclusive"); // Now begin a transaction to acquire the exclusive lock. This lock won't be released until we close because of the exclusive locking mode. - ret = sqlite3_exec(m_db, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr, nullptr); + int ret = sqlite3_exec(m_db, "BEGIN EXCLUSIVE TRANSACTION", nullptr, nullptr, nullptr); if (ret != SQLITE_OK) { throw std::runtime_error("SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another bitcoind?\n"); } @@ -228,18 +232,12 @@ void SQLiteDatabase::Open() } // Enable fullfsync for the platforms that use it - ret = sqlite3_exec(m_db, "PRAGMA fullfsync = true", nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error(strprintf("SQLiteDatabase: Failed to enable fullfsync: %s\n", sqlite3_errstr(ret))); - } + SetPragma(m_db, "fullfsync", "true", "Failed to enable fullfsync"); if (gArgs.GetBoolArg("-unsafesqlitesync", false)) { // Use normal synchronous mode for the journal LogPrintf("WARNING SQLite is configured to not wait for data to be flushed to disk. Data loss and corruption may occur.\n"); - ret = sqlite3_exec(m_db, "PRAGMA synchronous = OFF", nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error(strprintf("SQLiteDatabase: Failed to set synchronous mode to OFF: %s\n", sqlite3_errstr(ret))); - } + SetPragma(m_db, "synchronous", "OFF", "Failed to set synchronous mode to OFF"); } // Make the table for our key-value pairs @@ -271,18 +269,12 @@ void SQLiteDatabase::Open() // Set the application id uint32_t app_id = ReadBE32(Params().MessageStart()); - std::string set_app_id = strprintf("PRAGMA application_id = %d", static_cast<int32_t>(app_id)); - ret = sqlite3_exec(m_db, set_app_id.c_str(), nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error(strprintf("SQLiteDatabase: Failed to set the application id: %s\n", sqlite3_errstr(ret))); - } + SetPragma(m_db, "application_id", strprintf("%d", static_cast<int32_t>(app_id)), + "Failed to set the application id"); // Set the user version - std::string set_user_ver = strprintf("PRAGMA user_version = %d", WALLET_SCHEMA_VERSION); - ret = sqlite3_exec(m_db, set_user_ver.c_str(), nullptr, nullptr, nullptr); - if (ret != SQLITE_OK) { - throw std::runtime_error(strprintf("SQLiteDatabase: Failed to set the wallet schema version: %s\n", sqlite3_errstr(ret))); - } + SetPragma(m_db, "user_version", strprintf("%d", WALLET_SCHEMA_VERSION), + "Failed to set the wallet schema version"); } } diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 7eff6e592d..7bca385deb 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -297,8 +297,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) empty_wallet(); { std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); wallet->SetupLegacyScriptPubKeyMan(); LOCK(wallet->cs_wallet); add_coin(*wallet, 5 * CENT, 6 * 24, false, 0, true); diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index badf2eb459..fc744ebe5b 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -8,8 +8,7 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), m_wallet(m_node.chain.get(), "", CreateMockWalletDatabase()) { - bool fFirstRun; - m_wallet.LoadWallet(fFirstRun); + m_wallet.LoadWallet(); m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); m_wallet_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index c8e3c8f819..512374c34f 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -38,7 +38,7 @@ static_assert(WALLET_INCREMENTAL_RELAY_FEE >= DEFAULT_INCREMENTAL_RELAY_FEE, "wa BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static std::shared_ptr<CWallet> TestLoadWallet(interfaces::Chain& chain) +static std::shared_ptr<CWallet> TestLoadWallet(interfaces::Chain* chain) { DatabaseOptions options; DatabaseStatus status; @@ -46,7 +46,9 @@ static std::shared_ptr<CWallet> TestLoadWallet(interfaces::Chain& chain) std::vector<bilingual_str> warnings; auto database = MakeWalletDatabase("", options, status, error); auto wallet = CWallet::Create(chain, "", std::move(database), options.create_flags, error, warnings); - wallet->postInitProcess(); + if (chain) { + wallet->postInitProcess(); + } return wallet; } @@ -483,8 +485,7 @@ public: LOCK2(wallet->cs_wallet, ::cs_main); wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); } - bool firstRun; - wallet->LoadWallet(firstRun); + wallet->LoadWallet(); AddKey(*wallet, coinbaseKey); WalletRescanReserver reserver(*wallet); reserver.reserve(); @@ -690,7 +691,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); // Create new wallet with known key and unload it. - auto wallet = TestLoadWallet(*m_node.chain); + auto wallet = TestLoadWallet(m_node.chain.get()); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); @@ -730,7 +731,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // Reload wallet and make sure new transactions are detected despite events // being blocked - wallet = TestLoadWallet(*m_node.chain); + wallet = TestLoadWallet(m_node.chain.get()); BOOST_CHECK(rescan_completed); BOOST_CHECK_EQUAL(addtx_count, 2); { @@ -770,7 +771,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet); ENTER_CRITICAL_SECTION(cs_wallets); }); - wallet = TestLoadWallet(*m_node.chain); + wallet = TestLoadWallet(m_node.chain.get()); BOOST_CHECK_EQUAL(addtx_count, 4); { LOCK(wallet->cs_wallet); @@ -782,10 +783,17 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) TestUnloadWallet(std::move(wallet)); } +BOOST_FIXTURE_TEST_CASE(CreateWalletWithoutChain, BasicTestingSetup) +{ + auto wallet = TestLoadWallet(nullptr); + BOOST_CHECK(wallet); + UnloadWallet(std::move(wallet)); +} + BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { gArgs.ForceSetArg("-unsafesqlitesync", "1"); - auto wallet = TestLoadWallet(*m_node.chain); + auto wallet = TestLoadWallet(m_node.chain.get()); CKey key; key.MakeNewKey(true); AddKey(*wallet, key); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 456c26ea31..7cdf2fcda0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -213,7 +213,8 @@ std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const std: return nullptr; } - std::shared_ptr<CWallet> wallet = CWallet::Create(chain, name, std::move(database), options.create_flags, error, warnings); + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr<CWallet> wallet = CWallet::Create(&chain, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -292,7 +293,8 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin } // Make the wallet - std::shared_ptr<CWallet> wallet = CWallet::Create(chain, name, std::move(database), wallet_creation_flags, error, warnings); + chain.initMessage(_("Loading wallet...").translated); + std::shared_ptr<CWallet> wallet = CWallet::Create(&chain, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -3247,11 +3249,10 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve } } -DBErrors CWallet::LoadWallet(bool& fFirstRunRet) +DBErrors CWallet::LoadWallet() { LOCK(cs_wallet); - fFirstRunRet = false; DBErrors nLoadWalletRet = WalletBatch(GetDatabase()).LoadWallet(this); if (nLoadWalletRet == DBErrors::NEED_REWRITE) { @@ -3263,9 +3264,7 @@ DBErrors CWallet::LoadWallet(bool& fFirstRunRet) } } - // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys - fFirstRunRet = m_spk_managers.empty() && !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); - if (fFirstRunRet) { + if (m_spk_managers.empty()) { assert(m_external_spk_managers.empty()); assert(m_internal_spk_managers.empty()); } @@ -3886,18 +3885,15 @@ std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, cons return MakeDatabase(wallet_path, options, status, error_string); } -std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) +std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) { const std::string& walletFile = database->Filename(); - chain.initMessage(_("Loading wallet…").translated); - int64_t nStart = GetTimeMillis(); - bool fFirstRun = true; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - std::shared_ptr<CWallet> walletInstance(new CWallet(&chain, name, std::move(database)), ReleaseWallet); - DBErrors nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); + std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet); + DBErrors nLoadWalletRet = walletInstance->LoadWallet(); if (nLoadWalletRet != DBErrors::LOAD_OK) { if (nLoadWalletRet == DBErrors::CORRUPT) { error = strprintf(_("Error loading %s: Wallet corrupted"), walletFile); @@ -3924,6 +3920,10 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st } } + // This wallet is in its first run if there are no ScriptPubKeyMans and it isn't blank or no privkeys + const bool fFirstRun = walletInstance->m_spk_managers.empty() && + !walletInstance->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && + !walletInstance->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET); if (fFirstRun) { // ensure this wallet.dat can only be opened by clients supporting HD with chain split and expects no default key @@ -3952,7 +3952,9 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st } } - walletInstance->chainStateFlushed(chain.getTipLocator()); + if (chain) { + walletInstance->chainStateFlushed(chain->getTipLocator()); + } } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { // Make it impossible to disable private keys after creation error = strprintf(_("Error loading %s: Private keys can only be disabled during creation"), walletFile); @@ -4049,9 +4051,9 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st _("This is the transaction fee you will pay if you send a transaction.")); } walletInstance->m_pay_tx_fee = CFeeRate(nFeePerK, 1000); - if (walletInstance->m_pay_tx_fee < chain.relayMinFee()) { + if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { error = strprintf(_("Invalid amount for -paytxfee=<amount>: '%s' (must be at least %s)"), - gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString()); + gArgs.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); return nullptr; } } @@ -4065,15 +4067,15 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st if (nMaxFee > HIGH_MAX_TX_FEE) { warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); } - if (CFeeRate(nMaxFee, 1000) < chain.relayMinFee()) { + if (chain && CFeeRate(nMaxFee, 1000) < chain->relayMinFee()) { error = strprintf(_("Invalid amount for -maxtxfee=<amount>: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString()); + gArgs.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); return nullptr; } walletInstance->m_default_max_tx_fee = nMaxFee; } - if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { + if (chain && chain->relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-minrelaytxfee") + Untranslated(" ") + _("The wallet will avoid paying less than the minimum relay fee.")); } @@ -4089,6 +4091,35 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st LOCK(walletInstance->cs_wallet); + if (chain && !AttachChain(walletInstance, *chain, error, warnings)) { + return nullptr; + } + + { + LOCK(cs_wallets); + for (auto& load_wallet : g_load_wallet_fns) { + load_wallet(interfaces::MakeWallet(walletInstance)); + } + } + + walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); + + { + walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); + walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); + walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); + } + + return walletInstance; +} + +bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interfaces::Chain& chain, bilingual_str& error, std::vector<bilingual_str>& warnings) +{ + LOCK(walletInstance->cs_wallet); + // allow setting the chain if it hasn't been set already but prevent changing it + assert(!walletInstance->m_chain || walletInstance->m_chain == &chain); + walletInstance->m_chain = &chain; + // Register wallet with validationinterface. It's done before rescan to avoid // missing block connections between end of rescan and validation subscribing. // Because of wallet lock being hold, block connection notifications are going to @@ -4122,21 +4153,21 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st if (tip_height && *tip_height != rescan_height) { - // We can't rescan beyond non-pruned blocks, stop and throw an error. - // This might happen if a user uses an old wallet within a pruned node - // or if they ran -disablewallet for a longer time, then decided to re-enable if (chain.havePruned()) { - // Exit early and print an error. - // If a block is pruned after this check, we will load the wallet, - // but fail the rescan with a generic error. int block_height = *tip_height; while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { --block_height; } if (rescan_height != block_height) { + // We can't rescan beyond non-pruned blocks, stop and throw an error. + // This might happen if a user uses an old wallet within a pruned node + // or if they ran -disablewallet for a longer time, then decided to re-enable + // Exit early and print an error. + // If a block is pruned after this check, we will load the wallet, + // but fail the rescan with a generic error. error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"); - return nullptr; + return false; } } @@ -4158,29 +4189,14 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st WalletRescanReserver reserver(*walletInstance); if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) { error = _("Failed to rescan the wallet during initialization"); - return nullptr; + return false; } } walletInstance->chainStateFlushed(chain.getTipLocator()); walletInstance->GetDatabase().IncrementUpdateCounter(); } - { - LOCK(cs_wallets); - for (auto& load_wallet : g_load_wallet_fns) { - load_wallet(interfaces::MakeWallet(walletInstance)); - } - } - - walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); - - { - walletInstance->WalletLogPrintf("setKeyPool.size() = %u\n", walletInstance->GetKeyPoolSize()); - walletInstance->WalletLogPrintf("mapWallet.size() = %u\n", walletInstance->mapWallet.size()); - walletInstance->WalletLogPrintf("m_address_book.size() = %u\n", walletInstance->m_address_book.size()); - } - - return walletInstance; + return true; } const CAddressBookData* CWallet::FindAddressBookEntry(const CTxDestination& dest, bool allow_change) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5a36d92784..fc4edd8d20 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -763,6 +763,13 @@ private: bool CreateTransactionInternal(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign); + /** + * Catch wallet up to current chain, scanning new blocks, updating the best + * block locator and m_last_block_processed, and registering for + * notifications about new blocks and transactions. + */ + static bool AttachChain(const std::shared_ptr<CWallet>& wallet, interfaces::Chain& chain, bilingual_str& error, std::vector<bilingual_str>& warnings); + public: /** * Main wallet lock. @@ -1126,7 +1133,7 @@ public: CAmount GetChange(const CTransaction& tx) const; void chainStateFlushed(const CBlockLocator& loc) override; - DBErrors LoadWallet(bool& fFirstRunRet); + DBErrors LoadWallet(); DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& purpose); @@ -1202,7 +1209,7 @@ public: bool MarkReplaced(const uint256& originalHash, const uint256& newHash); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ - static std::shared_ptr<CWallet> Create(interfaces::Chain& chain, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings); + static std::shared_ptr<CWallet> Create(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings); /** * Wallet post-init setup diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index b2cb0bf479..50b6c9d29f 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -54,8 +54,7 @@ static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::pa std::shared_ptr<CWallet> wallet_instance{new CWallet(nullptr /* chain */, name, std::move(database)), WalletToolReleaseWallet}; DBErrors load_wallet_ret; try { - bool first_run; - load_wallet_ret = wallet_instance->LoadWallet(first_run); + load_wallet_ret = wallet_instance->LoadWallet(); } catch (const std::runtime_error&) { tfm::format(std::cerr, "Error loading %s. Is wallet being used by another process?\n", name); return nullptr; diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 16d7958712..2a58f8b3f7 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -187,43 +187,54 @@ class NetTest(BitcoinTestFramework): def test_getnodeaddresses(self): self.log.info("Test getnodeaddresses") self.nodes[0].add_p2p_connection(P2PInterface()) + services = NODE_NETWORK | NODE_WITNESS - # Add some addresses to the Address Manager over RPC. Due to the way - # bucket and bucket position are calculated, some of these addresses - # will collide. + # Add an IPv6 address to the address manager. + ipv6_addr = "1233:3432:2434:2343:3234:2345:6546:4534" + self.nodes[0].addpeeraddress(address=ipv6_addr, port=8333) + + # Add 10,000 IPv4 addresses to the address manager. Due to the way bucket + # and bucket positions are calculated, some of these addresses will collide. imported_addrs = [] for i in range(10000): first_octet = i >> 8 second_octet = i % 256 - a = "{}.{}.1.1".format(first_octet, second_octet) # IPV4 + a = f"{first_octet}.{second_octet}.1.1" imported_addrs.append(a) self.nodes[0].addpeeraddress(a, 8333) - # Obtain addresses via rpc call and check they were ones sent in before. - # - # Maximum possible addresses in addrman is 10000, although actual - # number will usually be less due to bucket and bucket position - # collisions. - node_addresses = self.nodes[0].getnodeaddresses(0) + # Fetch the addresses via the RPC and test the results. + assert_equal(len(self.nodes[0].getnodeaddresses()), 1) # default count is 1 + assert_equal(len(self.nodes[0].getnodeaddresses(count=2)), 2) + assert_equal(len(self.nodes[0].getnodeaddresses(network="ipv4", count=8)), 8) + + # Maximum possible addresses in AddrMan is 10000. The actual number will + # usually be less due to bucket and bucket position collisions. + node_addresses = self.nodes[0].getnodeaddresses(0, "ipv4") assert_greater_than(len(node_addresses), 5000) assert_greater_than(10000, len(node_addresses)) for a in node_addresses: assert_greater_than(a["time"], 1527811200) # 1st June 2018 - assert_equal(a["services"], NODE_NETWORK | NODE_WITNESS) + assert_equal(a["services"], services) assert a["address"] in imported_addrs assert_equal(a["port"], 8333) assert_equal(a["network"], "ipv4") - node_addresses = self.nodes[0].getnodeaddresses(1) - assert_equal(len(node_addresses), 1) + # Test the IPv6 address. + res = self.nodes[0].getnodeaddresses(0, "ipv6") + assert_equal(len(res), 1) + assert_equal(res[0]["address"], ipv6_addr) + assert_equal(res[0]["network"], "ipv6") + assert_equal(res[0]["port"], 8333) + assert_equal(res[0]["services"], services) - assert_raises_rpc_error(-8, "Address count out of range", self.nodes[0].getnodeaddresses, -1) + # Test for the absence of onion and I2P addresses. + for network in ["onion", "i2p"]: + assert_equal(self.nodes[0].getnodeaddresses(0, network), []) - # addrman's size cannot be known reliably after insertion, as hash collisions may occur - # so only test that requesting a large number of addresses returns less than that - LARGE_REQUEST_COUNT = 10000 - node_addresses = self.nodes[0].getnodeaddresses(LARGE_REQUEST_COUNT) - assert_greater_than(LARGE_REQUEST_COUNT, len(node_addresses)) + # Test invalid arguments. + assert_raises_rpc_error(-8, "Address count out of range", self.nodes[0].getnodeaddresses, -1) + assert_raises_rpc_error(-8, "Network not recognized: Foo", self.nodes[0].getnodeaddresses, 1, "Foo") if __name__ == '__main__': |