From 5be34bacf6d07fb73a0cedfb63a384968d538f3e Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Wed, 18 Sep 2024 18:33:53 +0100 Subject: [PATCH 01/36] qt: Fix linking when configured with `-DENABLE_WALLET=OFF` This change is required for Qt 6, but it is meaningful on its own. --- src/qt/rpcconsole.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index ae3f9aa686..20311cb631 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -16,7 +16,9 @@ #include #include #include +#ifdef ENABLE_WALLET #include +#endif // ENABLE_WALLET #include #include #include From fee4cba48472239f7a426db62f45c4b6af8e5ef2 Mon Sep 17 00:00:00 2001 From: pablomartin4btc Date: Sun, 15 Sep 2024 14:17:35 -0300 Subject: [PATCH 02/36] gui: Fix proxy details display in Options Dialog - Ensured that the proxy IP is displayed correctly in the UI when using an IPv6 address. No functionality impact; changes only affect UI display. --- src/qt/optionsmodel.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 0c21c6748d..d15735fa18 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -320,10 +320,15 @@ static ProxySetting ParseProxyString(const QString& proxy) if (proxy.isEmpty()) { return default_val; } - // contains IP at index 0 and port at index 1 - QStringList ip_port = GUIUtil::SplitSkipEmptyParts(proxy, ":"); - if (ip_port.size() == 2) { - return {true, ip_port.at(0), ip_port.at(1)}; + uint16_t port{0}; + std::string hostname; + if (SplitHostPort(proxy.toStdString(), port, hostname) && port != 0) { + // Valid and port within the valid range + // Check if the hostname contains a colon, indicating an IPv6 address + if (hostname.find(':') != std::string::npos) { + hostname = "[" + hostname + "]"; // Wrap IPv6 address in brackets + } + return {true, QString::fromStdString(hostname), QString::number(port)}; } else { // Invalid: return default return default_val; } From 754e4254388ec8ac1be6cf807bf300cd43fd3da5 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 28 Apr 2024 21:39:20 +0200 Subject: [PATCH 03/36] crypto: Add missing WriteBE16 function Also add fuzz test, mimicing the WriteLE16 one. --- src/crypto/common.h | 6 ++++++ src/test/fuzz/crypto_common.cpp | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/crypto/common.h b/src/crypto/common.h index 1dc4f3f55c..d45459b1f6 100644 --- a/src/crypto/common.h +++ b/src/crypto/common.h @@ -70,6 +70,12 @@ uint64_t static inline ReadBE64(const unsigned char* ptr) return be64toh_internal(x); } +void static inline WriteBE16(unsigned char* ptr, uint16_t x) +{ + uint16_t v = htobe16_internal(x); + memcpy(ptr, &v, 2); +} + void static inline WriteBE32(unsigned char* ptr, uint32_t x) { uint32_t v = htobe32_internal(x); diff --git a/src/test/fuzz/crypto_common.cpp b/src/test/fuzz/crypto_common.cpp index 8e07dfedb9..5a76d4e1a9 100644 --- a/src/test/fuzz/crypto_common.cpp +++ b/src/test/fuzz/crypto_common.cpp @@ -35,6 +35,10 @@ FUZZ_TARGET(crypto_common) WriteLE64(writele64_arr.data(), random_u64); assert(ReadLE64(writele64_arr.data()) == random_u64); + std::array writebe16_arr; + WriteBE16(writebe16_arr.data(), random_u16); + assert(ReadBE16(writebe16_arr.data()) == random_u16); + std::array writebe32_arr; WriteBE32(writebe32_arr.data(), random_u32); assert(ReadBE32(writebe32_arr.data()) == random_u32); From e02030432b77abbf27bb4f67d879d3ad6d6366e6 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Thu, 16 May 2024 10:16:41 +0200 Subject: [PATCH 04/36] net: Add netif utility This adds an utility header with two functions that will be needed for PCP, `QueryDefaultGateway` and `GetLocalAddresses`. Co-authored-by: Vasil Dimov --- src/util/CMakeLists.txt | 2 + src/util/netif.cpp | 303 ++++++++++++++++++++++++++++++++++++++++ src/util/netif.h | 20 +++ 3 files changed, 325 insertions(+) create mode 100644 src/util/netif.cpp create mode 100644 src/util/netif.h diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 26c6271f9b..d65e634c57 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -15,6 +15,7 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL fs_helpers.cpp hasher.cpp moneystr.cpp + netif.cpp rbf.cpp readwritefile.cpp serfloat.cpp @@ -42,4 +43,5 @@ target_link_libraries(bitcoin_util bitcoin_clientversion bitcoin_crypto $<$:ws2_32> + $<$:iphlpapi> ) diff --git a/src/util/netif.cpp b/src/util/netif.cpp new file mode 100644 index 0000000000..75d0cf00c5 --- /dev/null +++ b/src/util/netif.cpp @@ -0,0 +1,303 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include // IWYU pragma: keep + +#include + +#include +#include +#include +#include +#include + +#if defined(__linux__) +#include +#elif defined(__FreeBSD__) +#include +#if __FreeBSD_version >= 1400000 +// Workaround https://github.com/freebsd/freebsd-src/pull/1070. +#define typeof __typeof +#include +#include +#endif +#elif defined(WIN32) +#include +#elif defined(__APPLE__) +#include +#include +#endif + +namespace { + +// Linux and FreeBSD 14.0+. For FreeBSD 13.2 the code can be compiled but +// running it requires loading a special kernel module, otherwise socket(AF_NETLINK,...) +// will fail, so we skip that. +#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 1400000) + +std::optional QueryDefaultGatewayImpl(sa_family_t family) +{ + // Create a netlink socket. + auto sock{CreateSock(AF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE)}; + if (!sock) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "socket(AF_NETLINK): %s\n", NetworkErrorString(errno)); + return std::nullopt; + } + + // Send request. + struct { + nlmsghdr hdr; ///< Request header. + rtmsg data; ///< Request data, a "route message". + nlattr dst_hdr; ///< One attribute, conveying the route destination address. + char dst_data[16]; ///< Route destination address. To query the default route we use 0.0.0.0/0 or [::]/0. For IPv4 the first 4 bytes are used. + } request{}; + + // Whether to use the first 4 or 16 bytes from request.dst_data. + const size_t dst_data_len = family == AF_INET ? 4 : 16; + + request.hdr.nlmsg_type = RTM_GETROUTE; + request.hdr.nlmsg_flags = NLM_F_REQUEST; +#ifdef __linux__ + // Linux IPv4 / IPv6 - this must be present, otherwise no gateway is found + // FreeBSD IPv4 - does not matter, the gateway is found with or without this + // FreeBSD IPv6 - this must be absent, otherwise no gateway is found + request.hdr.nlmsg_flags |= NLM_F_DUMP; +#endif + request.hdr.nlmsg_len = NLMSG_LENGTH(sizeof(rtmsg) + sizeof(nlattr) + dst_data_len); + request.hdr.nlmsg_seq = 0; // Sequence number, used to match which reply is to which request. Irrelevant for us because we send just one request. + request.data.rtm_family = family; + request.data.rtm_dst_len = 0; // Prefix length. +#ifdef __FreeBSD__ + // Linux IPv4 / IPv6 this must be absent, otherwise no gateway is found + // FreeBSD IPv4 - does not matter, the gateway is found with or without this + // FreeBSD IPv6 - this must be present, otherwise no gateway is found + request.data.rtm_flags = RTM_F_PREFIX; +#endif + request.dst_hdr.nla_type = RTA_DST; + request.dst_hdr.nla_len = sizeof(nlattr) + dst_data_len; + + if (sock->Send(&request, request.hdr.nlmsg_len, 0) != static_cast(request.hdr.nlmsg_len)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "send() to netlink socket: %s\n", NetworkErrorString(errno)); + return std::nullopt; + } + + // Receive response. + char response[4096]; + int64_t recv_result; + do { + recv_result = sock->Recv(response, sizeof(response), 0); + } while (recv_result < 0 && (errno == EINTR || errno == EAGAIN)); + if (recv_result < 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "recv() from netlink socket: %s\n", NetworkErrorString(errno)); + return std::nullopt; + } + + for (nlmsghdr* hdr = (nlmsghdr*)response; NLMSG_OK(hdr, recv_result); hdr = NLMSG_NEXT(hdr, recv_result)) { + rtmsg* r = (rtmsg*)NLMSG_DATA(hdr); + int remaining_len = RTM_PAYLOAD(hdr); + + // Iterate over the attributes. + rtattr *rta_gateway = nullptr; + int scope_id = 0; + for (rtattr* attr = RTM_RTA(r); RTA_OK(attr, remaining_len); attr = RTA_NEXT(attr, remaining_len)) { + if (attr->rta_type == RTA_GATEWAY) { + rta_gateway = attr; + } else if (attr->rta_type == RTA_OIF && sizeof(int) == RTA_PAYLOAD(attr)) { + std::memcpy(&scope_id, RTA_DATA(attr), sizeof(scope_id)); + } + } + + // Found gateway? + if (rta_gateway != nullptr) { + if (family == AF_INET && sizeof(in_addr) == RTA_PAYLOAD(rta_gateway)) { + in_addr gw; + std::memcpy(&gw, RTA_DATA(rta_gateway), sizeof(gw)); + return CNetAddr(gw); + } else if (family == AF_INET6 && sizeof(in6_addr) == RTA_PAYLOAD(rta_gateway)) { + in6_addr gw; + std::memcpy(&gw, RTA_DATA(rta_gateway), sizeof(gw)); + return CNetAddr(gw, scope_id); + } + } + } + + return std::nullopt; +} + +#elif defined(WIN32) + +std::optional QueryDefaultGatewayImpl(sa_family_t family) +{ + NET_LUID interface_luid = {}; + SOCKADDR_INET destination_address = {}; + MIB_IPFORWARD_ROW2 best_route = {}; + SOCKADDR_INET best_source_address = {}; + DWORD best_if_idx = 0; + DWORD status = 0; + + // Pass empty destination address of the requested type (:: or 0.0.0.0) to get interface of default route. + destination_address.si_family = family; + status = GetBestInterfaceEx((sockaddr*)&destination_address, &best_if_idx); + if (status != NO_ERROR) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Could not get best interface for default route: %s\n", NetworkErrorString(status)); + return std::nullopt; + } + + // Get best route to default gateway. + // Leave interface_luid at all-zeros to use interface index instead. + status = GetBestRoute2(&interface_luid, best_if_idx, nullptr, &destination_address, 0, &best_route, &best_source_address); + if (status != NO_ERROR) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Could not get best route for default route for interface index %d: %s\n", + best_if_idx, NetworkErrorString(status)); + return std::nullopt; + } + + Assume(best_route.NextHop.si_family == family); + if (family == AF_INET) { + return CNetAddr(best_route.NextHop.Ipv4.sin_addr); + } else if(family == AF_INET6) { + return CNetAddr(best_route.NextHop.Ipv6.sin6_addr, best_route.InterfaceIndex); + } + return std::nullopt; +} + +#elif defined(__APPLE__) + +#define ROUNDUP32(a) \ + ((a) > 0 ? (1 + (((a) - 1) | (sizeof(uint32_t) - 1))) : sizeof(uint32_t)) + +std::optional FromSockAddr(const struct sockaddr* addr) +{ + // Check valid length. Note that sa_len is not part of POSIX, and exists on MacOS and some BSDs only, so we can't + // do this check in SetSockAddr. + if (!(addr->sa_family == AF_INET && addr->sa_len == sizeof(struct sockaddr_in)) && + !(addr->sa_family == AF_INET6 && addr->sa_len == sizeof(struct sockaddr_in6))) { + return std::nullopt; + } + + // Fill in a CService from the sockaddr, then drop the port part. + CService service; + if (service.SetSockAddr(addr)) { + return (CNetAddr)service; + } + return std::nullopt; +} + +//! MacOS: Get default gateway from route table. See route(4) for the format. +std::optional QueryDefaultGatewayImpl(sa_family_t family) +{ + // net.route.0.inet[6].flags.gateway + int mib[] = {CTL_NET, PF_ROUTE, 0, family, NET_RT_FLAGS, RTF_GATEWAY}; + // The size of the available data is determined by calling sysctl() with oldp=nullptr. See sysctl(3). + size_t l = 0; + if (sysctl(/*name=*/mib, /*namelen=*/sizeof(mib) / sizeof(int), /*oldp=*/nullptr, /*oldlenp=*/&l, /*newp=*/nullptr, /*newlen=*/0) < 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Could not get sysctl length of routing table: %s\n", SysErrorString(errno)); + return std::nullopt; + } + std::vector buf(l); + if (sysctl(/*name=*/mib, /*namelen=*/sizeof(mib) / sizeof(int), /*oldp=*/buf.data(), /*oldlenp=*/&l, /*newp=*/nullptr, /*newlen=*/0) < 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Could not get sysctl data of routing table: %s\n", SysErrorString(errno)); + return std::nullopt; + } + // Iterate over messages (each message is a routing table entry). + for (size_t msg_pos = 0; msg_pos < buf.size(); ) { + if ((msg_pos + sizeof(rt_msghdr)) > buf.size()) return std::nullopt; + const struct rt_msghdr* rt = (const struct rt_msghdr*)(buf.data() + msg_pos); + const size_t next_msg_pos = msg_pos + rt->rtm_msglen; + if (rt->rtm_msglen < sizeof(rt_msghdr) || next_msg_pos > buf.size()) return std::nullopt; + // Iterate over addresses within message, get destination and gateway (if present). + // Address data starts after header. + size_t sa_pos = msg_pos + sizeof(struct rt_msghdr); + std::optional dst, gateway; + for (int i = 0; i < RTAX_MAX; i++) { + if (rt->rtm_addrs & (1 << i)) { + // 2 is just sa_len + sa_family, the theoretical minimum size of a socket address. + if ((sa_pos + 2) > next_msg_pos) return std::nullopt; + const struct sockaddr* sa = (const struct sockaddr*)(buf.data() + sa_pos); + if ((sa_pos + sa->sa_len) > next_msg_pos) return std::nullopt; + if (i == RTAX_DST) { + dst = FromSockAddr(sa); + } else if (i == RTAX_GATEWAY) { + gateway = FromSockAddr(sa); + } + // Skip sockaddr entries for bit flags we're not interested in, + // move cursor. + sa_pos += ROUNDUP32(sa->sa_len); + } + } + // Found default gateway? + if (dst && gateway && dst->IsBindAny()) { // Route to 0.0.0.0 or :: ? + return *gateway; + } + // Skip to next message. + msg_pos = next_msg_pos; + } + return std::nullopt; +} + +#else + +// Dummy implementation. +std::optional QueryDefaultGatewayImpl(sa_family_t) +{ + return std::nullopt; +} + +#endif + +} + +std::optional QueryDefaultGateway(Network network) +{ + Assume(network == NET_IPV4 || network == NET_IPV6); + + sa_family_t family; + if (network == NET_IPV4) { + family = AF_INET; + } else if(network == NET_IPV6) { + family = AF_INET6; + } else { + return std::nullopt; + } + + std::optional ret = QueryDefaultGatewayImpl(family); + + // It's possible for the default gateway to be 0.0.0.0 or ::0 on at least Windows + // for some routing strategies. If so, return as if no default gateway was found. + if (ret && !ret->IsBindAny()) { + return ret; + } else { + return std::nullopt; + } +} + +std::vector GetLocalAddresses() +{ + std::vector addresses; +#ifdef WIN32 + char pszHostName[256] = ""; + if (gethostname(pszHostName, sizeof(pszHostName)) != SOCKET_ERROR) { + addresses = LookupHost(pszHostName, 0, true); + } +#elif (HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS) + struct ifaddrs* myaddrs; + if (getifaddrs(&myaddrs) == 0) { + for (struct ifaddrs* ifa = myaddrs; ifa != nullptr; ifa = ifa->ifa_next) + { + if (ifa->ifa_addr == nullptr) continue; + if ((ifa->ifa_flags & IFF_UP) == 0) continue; + if ((ifa->ifa_flags & IFF_LOOPBACK) != 0) continue; + if (ifa->ifa_addr->sa_family == AF_INET) { + struct sockaddr_in* s4 = (struct sockaddr_in*)(ifa->ifa_addr); + addresses.emplace_back(s4->sin_addr); + } else if (ifa->ifa_addr->sa_family == AF_INET6) { + struct sockaddr_in6* s6 = (struct sockaddr_in6*)(ifa->ifa_addr); + addresses.emplace_back(s6->sin6_addr); + } + } + freeifaddrs(myaddrs); + } +#endif + return addresses; +} diff --git a/src/util/netif.h b/src/util/netif.h new file mode 100644 index 0000000000..d38d94a719 --- /dev/null +++ b/src/util/netif.h @@ -0,0 +1,20 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_NETIF_H +#define BITCOIN_UTIL_NETIF_H + +#include + +#include + +//! Query the OS for the default gateway for `network`. This only makes sense for NET_IPV4 and NET_IPV6. +//! Returns std::nullopt if it cannot be found, or there is no support for this OS. +std::optional QueryDefaultGateway(Network network); + +// TODO share with Discover() +//! Return all local non-loopback IPv4 and IPv6 network addresses. +std::vector GetLocalAddresses(); + +#endif // BITCOIN_UTIL_NETIF_H From d72df63d16941576b3523cfeaa49985cf3cd4d42 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 16:14:43 +0200 Subject: [PATCH 05/36] net: Use GetLocalAddresses in Discover This has the same code, it's unnecessary to duplicate it. --- src/net.cpp | 43 ++++--------------------------------------- src/util/netif.h | 1 - 2 files changed, 4 insertions(+), 40 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index d87adcc031..e120aee2c4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -3118,46 +3119,10 @@ void Discover() if (!fDiscover) return; -#ifdef WIN32 - // Get local host IP - char pszHostName[256] = ""; - if (gethostname(pszHostName, sizeof(pszHostName)) != SOCKET_ERROR) - { - const std::vector addresses{LookupHost(pszHostName, 0, true)}; - for (const CNetAddr& addr : addresses) - { - if (AddLocal(addr, LOCAL_IF)) - LogPrintf("%s: %s - %s\n", __func__, pszHostName, addr.ToStringAddr()); - } + for (const CNetAddr &addr: GetLocalAddresses()) { + if (AddLocal(addr, LOCAL_IF)) + LogPrintf("%s: %s\n", __func__, addr.ToStringAddr()); } -#elif (HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS) - // Get local host ip - struct ifaddrs* myaddrs; - if (getifaddrs(&myaddrs) == 0) - { - for (struct ifaddrs* ifa = myaddrs; ifa != nullptr; ifa = ifa->ifa_next) - { - if (ifa->ifa_addr == nullptr) continue; - if ((ifa->ifa_flags & IFF_UP) == 0) continue; - if ((ifa->ifa_flags & IFF_LOOPBACK) != 0) continue; - if (ifa->ifa_addr->sa_family == AF_INET) - { - struct sockaddr_in* s4 = (struct sockaddr_in*)(ifa->ifa_addr); - CNetAddr addr(s4->sin_addr); - if (AddLocal(addr, LOCAL_IF)) - LogPrintf("%s: IPv4 %s: %s\n", __func__, ifa->ifa_name, addr.ToStringAddr()); - } - else if (ifa->ifa_addr->sa_family == AF_INET6) - { - struct sockaddr_in6* s6 = (struct sockaddr_in6*)(ifa->ifa_addr); - CNetAddr addr(s6->sin6_addr); - if (AddLocal(addr, LOCAL_IF)) - LogPrintf("%s: IPv6 %s: %s\n", __func__, ifa->ifa_name, addr.ToStringAddr()); - } - } - freeifaddrs(myaddrs); - } -#endif } void CConnman::SetNetworkActive(bool active) diff --git a/src/util/netif.h b/src/util/netif.h index d38d94a719..5ff473fd4f 100644 --- a/src/util/netif.h +++ b/src/util/netif.h @@ -13,7 +13,6 @@ //! Returns std::nullopt if it cannot be found, or there is no support for this OS. std::optional QueryDefaultGateway(Network network); -// TODO share with Discover() //! Return all local non-loopback IPv4 and IPv6 network addresses. std::vector GetLocalAddresses(); From 781c01f58066d375c14b1a717160f51c2f2ebe20 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 25 Sep 2024 13:10:02 +0200 Subject: [PATCH 06/36] init: Check mempool arguments in AppInitParameterInteractions This makes the handling more consistent and reports errors sooner. --- src/init.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 2a301e114e..9a687c6fe3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1072,6 +1072,13 @@ bool AppInitParameterInteraction(const ArgsManager& args) if (!blockman_result) { return InitError(util::ErrorString(blockman_result)); } + CTxMemPool::Options mempool_opts{ + .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, + }; + auto mempool_result{ApplyArgsManOptions(args, chainparams, mempool_opts)}; + if (!mempool_result) { + return InitError(util::ErrorString(mempool_result)); + } } return true; @@ -1543,10 +1550,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, .signals = &validation_signals, }; - auto result{ApplyArgsManOptions(args, chainparams, mempool_opts)}; - if (!result) { - return InitError(util::ErrorString(result)); - } + Assert(ApplyArgsManOptions(args, chainparams, mempool_opts)); // no error can happen, already checked in AppInitParameterInteraction bool do_reindex{args.GetBoolArg("-reindex", false)}; const bool do_reindex_chainstate{args.GetBoolArg("-reindex-chainstate", false)}; From c1d8870ea4155c2766d30d38fc5b1afc63dcd364 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 25 Sep 2024 13:24:10 +0200 Subject: [PATCH 07/36] refactor: Move most of init retry for loop to a function This makes it clearer which state is being mutated by the function and facilitates getting rid of the for loop in the following commit. Move creation of the required options into the function too, such that the function takes fewer arguments and is more self-contained. Co-Authored-By: Ryan Ofsky --- src/init.cpp | 209 +++++++++++++++++++++++++++------------------------ 1 file changed, 112 insertions(+), 97 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 9a687c6fe3..05a8eabc65 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -123,17 +123,19 @@ using node::ApplyArgsManOptions; using node::BlockManager; using node::CacheSizes; using node::CalculateCacheSizes; +using node::ChainstateLoadResult; +using node::ChainstateLoadStatus; using node::DEFAULT_PERSIST_MEMPOOL; using node::DEFAULT_PRINT_MODIFIED_FEE; using node::DEFAULT_STOPATHEIGHT; using node::DumpMempool; -using node::LoadMempool; +using node::ImportBlocks; using node::KernelNotifications; using node::LoadChainstate; +using node::LoadMempool; using node::MempoolPath; using node::NodeContext; using node::ShouldPersistMempool; -using node::ImportBlocks; using node::VerifyLoadedChainstate; using util::Join; using util::ReplaceAll; @@ -1183,6 +1185,104 @@ bool CheckHostPortOptions(const ArgsManager& args) { return true; } +// A GUI user may opt to retry once if there is a failure during chainstate initialization. +// The function therefore has to support re-entry. +static ChainstateLoadResult InitAndLoadChainstate( + NodeContext& node, + bool do_reindex, + const bool do_reindex_chainstate, + CacheSizes& cache_sizes, + const ArgsManager& args) +{ + const CChainParams& chainparams = Params(); + CTxMemPool::Options mempool_opts{ + .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, + .signals = node.validation_signals.get(), + }; + Assert(ApplyArgsManOptions(args, chainparams, mempool_opts)); // no error can happen, already checked in AppInitParameterInteraction + bilingual_str mempool_error; + node.mempool = std::make_unique(mempool_opts, mempool_error); + if (!mempool_error.empty()) { + return {ChainstateLoadStatus::FAILURE_FATAL, mempool_error}; + } + LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), mempool_opts.max_size_bytes * (1.0 / 1024 / 1024)); + ChainstateManager::Options chainman_opts{ + .chainparams = chainparams, + .datadir = args.GetDataDirNet(), + .notifications = *node.notifications, + .signals = node.validation_signals.get(), + }; + Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction + BlockManager::Options blockman_opts{ + .chainparams = chainman_opts.chainparams, + .blocks_dir = args.GetBlocksDirPath(), + .notifications = chainman_opts.notifications, + }; + Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction + try { + node.chainman = std::make_unique(*Assert(node.shutdown), chainman_opts, blockman_opts); + } catch (std::exception& e) { + return {ChainstateLoadStatus::FAILURE_FATAL, strprintf(Untranslated("Failed to initialize ChainstateManager: %s"), e.what())}; + } + ChainstateManager& chainman = *node.chainman; + // This is defined and set here instead of inline in validation.h to avoid a hard + // dependency between validation and index/base, since the latter is not in + // libbitcoinkernel. + chainman.snapshot_download_completed = [&node]() { + if (!node.chainman->m_blockman.IsPruneMode()) { + LogPrintf("[snapshot] re-enabling NODE_NETWORK services\n"); + node.connman->AddLocalServices(NODE_NETWORK); + } + LogPrintf("[snapshot] restarting indexes\n"); + // Drain the validation interface queue to ensure that the old indexes + // don't have any pending work. + Assert(node.validation_signals)->SyncWithValidationInterfaceQueue(); + for (auto* index : node.indexes) { + index->Interrupt(); + index->Stop(); + if (!(index->Init() && index->StartBackgroundSync())) { + LogPrintf("[snapshot] WARNING failed to restart index %s on snapshot chain\n", index->GetName()); + } + } + }; + node::ChainstateLoadOptions options; + options.mempool = Assert(node.mempool.get()); + options.wipe_block_tree_db = do_reindex; + options.wipe_chainstate_db = do_reindex || do_reindex_chainstate; + options.prune = chainman.m_blockman.IsPruneMode(); + options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); + options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL); + options.require_full_verification = args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel"); + options.coins_error_cb = [] { + uiInterface.ThreadSafeMessageBox( + _("Error reading from database, shutting down."), + "", CClientUIInterface::MSG_ERROR); + }; + uiInterface.InitMessage(_("Loading block index…").translated); + const auto load_block_index_start_time{SteadyClock::now()}; + auto catch_exceptions = [](auto&& f) { + try { + return f(); + } catch (const std::exception& e) { + LogError("%s\n", e.what()); + return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database")); + } + }; + auto [status, error] = catch_exceptions([&] { return LoadChainstate(chainman, cache_sizes, options); }); + if (status == node::ChainstateLoadStatus::SUCCESS) { + uiInterface.InitMessage(_("Verifying blocks…").translated); + if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) { + LogWarning("pruned datadir may not have more than %d blocks; only checking available blocks\n", + MIN_BLOCKS_TO_KEEP); + } + std::tie(status, error) = catch_exceptions([&] { return VerifyLoadedChainstate(chainman, options); }); + if (status == node::ChainstateLoadStatus::SUCCESS) { + LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); + } + } + return {status, error}; +}; + bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { const ArgsManager& args = *Assert(node.args); @@ -1514,20 +1614,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) node.notifications = std::make_unique(*Assert(node.shutdown), node.exit_status, *Assert(node.warnings)); ReadNotificationArgs(args, *node.notifications); - ChainstateManager::Options chainman_opts{ - .chainparams = chainparams, - .datadir = args.GetDataDirNet(), - .notifications = *node.notifications, - .signals = &validation_signals, - }; - Assert(ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction - - BlockManager::Options blockman_opts{ - .chainparams = chainman_opts.chainparams, - .blocks_dir = args.GetBlocksDirPath(), - .notifications = chainman_opts.notifications, - }; - Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction // cache size calculations CacheSizes cache_sizes = CalculateCacheSizes(args, g_enabled_filter_types.size()); @@ -1546,96 +1632,25 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) assert(!node.mempool); assert(!node.chainman); - CTxMemPool::Options mempool_opts{ - .check_ratio = chainparams.DefaultConsistencyChecks() ? 1 : 0, - .signals = &validation_signals, - }; - Assert(ApplyArgsManOptions(args, chainparams, mempool_opts)); // no error can happen, already checked in AppInitParameterInteraction - bool do_reindex{args.GetBoolArg("-reindex", false)}; const bool do_reindex_chainstate{args.GetBoolArg("-reindex-chainstate", false)}; for (bool fLoaded = false; !fLoaded && !ShutdownRequested(node);) { - bilingual_str mempool_error; - node.mempool = std::make_unique(mempool_opts, mempool_error); - if (!mempool_error.empty()) { - return InitError(mempool_error); - } - LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), mempool_opts.max_size_bytes * (1.0 / 1024 / 1024)); - - try { - node.chainman = std::make_unique(*Assert(node.shutdown), chainman_opts, blockman_opts); - } catch (std::exception& e) { - return InitError(strprintf(Untranslated("Failed to initialize ChainstateManager: %s"), e.what())); - } - ChainstateManager& chainman = *node.chainman; - - // This is defined and set here instead of inline in validation.h to avoid a hard - // dependency between validation and index/base, since the latter is not in - // libbitcoinkernel. - chainman.snapshot_download_completed = [&node]() { - if (!node.chainman->m_blockman.IsPruneMode()) { - LogPrintf("[snapshot] re-enabling NODE_NETWORK services\n"); - node.connman->AddLocalServices(NODE_NETWORK); - } - - LogPrintf("[snapshot] restarting indexes\n"); - - // Drain the validation interface queue to ensure that the old indexes - // don't have any pending work. - Assert(node.validation_signals)->SyncWithValidationInterfaceQueue(); - - for (auto* index : node.indexes) { - index->Interrupt(); - index->Stop(); - if (!(index->Init() && index->StartBackgroundSync())) { - LogPrintf("[snapshot] WARNING failed to restart index %s on snapshot chain\n", index->GetName()); - } - } - }; - - node::ChainstateLoadOptions options; - options.mempool = Assert(node.mempool.get()); - options.wipe_block_tree_db = do_reindex; - options.wipe_chainstate_db = do_reindex || do_reindex_chainstate; - options.prune = chainman.m_blockman.IsPruneMode(); - options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); - options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL); - options.require_full_verification = args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel"); - options.coins_error_cb = [] { - uiInterface.ThreadSafeMessageBox( - _("Error reading from database, shutting down."), - "", CClientUIInterface::MSG_ERROR); - }; - - uiInterface.InitMessage(_("Loading block index…").translated); - const auto load_block_index_start_time{SteadyClock::now()}; - auto catch_exceptions = [](auto&& f) { - try { - return f(); - } catch (const std::exception& e) { - LogError("%s\n", e.what()); - return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database")); - } - }; - auto [status, error] = catch_exceptions([&]{ return LoadChainstate(chainman, cache_sizes, options); }); - if (status == node::ChainstateLoadStatus::SUCCESS) { - uiInterface.InitMessage(_("Verifying blocks…").translated); - if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) { - LogWarning("pruned datadir may not have more than %d blocks; only checking available blocks\n", - MIN_BLOCKS_TO_KEEP); - } - std::tie(status, error) = catch_exceptions([&]{ return VerifyLoadedChainstate(chainman, options);}); - if (status == node::ChainstateLoadStatus::SUCCESS) { - fLoaded = true; - LogPrintf(" block index %15dms\n", Ticks(SteadyClock::now() - load_block_index_start_time)); - } - } + auto [status, error] = InitAndLoadChainstate( + node, + do_reindex, + do_reindex_chainstate, + cache_sizes, + args); if (status == node::ChainstateLoadStatus::FAILURE_FATAL || status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB || status == node::ChainstateLoadStatus::FAILURE_INSUFFICIENT_DBCACHE) { return InitError(error); } + if (status == ChainstateLoadStatus::SUCCESS) { + fLoaded = true; + } + if (!fLoaded && !ShutdownRequested(node)) { // first suggest a reindex if (!do_reindex) { From e9d60af9889c12b4d427adefa53fd234e417f8f6 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 25 Sep 2024 14:14:07 +0200 Subject: [PATCH 08/36] refactor: Replace init retry for loop with if statement The for loop has been a long standing source of confusion and bugs, both because its purpose is not clearly documented and because the body of the for loop contains a lot of logic. Co-Authored-By: Ryan Ofsky --- src/init.cpp | 56 +++++++++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 05a8eabc65..7755e7dad7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1635,42 +1635,36 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) bool do_reindex{args.GetBoolArg("-reindex", false)}; const bool do_reindex_chainstate{args.GetBoolArg("-reindex-chainstate", false)}; - for (bool fLoaded = false; !fLoaded && !ShutdownRequested(node);) { - auto [status, error] = InitAndLoadChainstate( + // Chainstate initialization and loading may be retried once with reindexing by GUI users + auto [status, error] = InitAndLoadChainstate( + node, + do_reindex, + do_reindex_chainstate, + cache_sizes, + args); + if (status == ChainstateLoadStatus::FAILURE && !do_reindex && !ShutdownRequested(node)) { + // suggest a reindex + bool do_retry = uiInterface.ThreadSafeQuestion( + error + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"), + error.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.", + "", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT); + if (!do_retry) { + LogError("Aborted block database rebuild. Exiting.\n"); + return false; + } + do_reindex = true; + if (!Assert(node.shutdown)->reset()) { + LogError("Internal error: failed to reset shutdown signal.\n"); + } + std::tie(status, error) = InitAndLoadChainstate( node, do_reindex, do_reindex_chainstate, cache_sizes, args); - - if (status == node::ChainstateLoadStatus::FAILURE_FATAL || status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB || status == node::ChainstateLoadStatus::FAILURE_INSUFFICIENT_DBCACHE) { - return InitError(error); - } - - if (status == ChainstateLoadStatus::SUCCESS) { - fLoaded = true; - } - - if (!fLoaded && !ShutdownRequested(node)) { - // first suggest a reindex - if (!do_reindex) { - bool fRet = uiInterface.ThreadSafeQuestion( - error + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"), - error.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.", - "", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT); - if (fRet) { - do_reindex = true; - if (!Assert(node.shutdown)->reset()) { - LogError("Internal error: failed to reset shutdown signal.\n"); - } - } else { - LogError("Aborted block database rebuild. Exiting.\n"); - return false; - } - } else { - return InitError(error); - } - } + } + if (status != ChainstateLoadStatus::SUCCESS && status != ChainstateLoadStatus::INTERRUPTED) { + return InitError(error); } // As LoadBlockIndex can take several minutes, it's possible the user From a7498cc7e26b4b3de976e111de2bd2d79b056b31 Mon Sep 17 00:00:00 2001 From: marcofleon Date: Thu, 26 Sep 2024 12:02:34 +0100 Subject: [PATCH 09/36] Fix bug in p2p_headers_presync harness --- src/test/fuzz/p2p_headers_presync.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/fuzz/p2p_headers_presync.cpp b/src/test/fuzz/p2p_headers_presync.cpp index e22087303a..2670aa8ee4 100644 --- a/src/test/fuzz/p2p_headers_presync.cpp +++ b/src/test/fuzz/p2p_headers_presync.cpp @@ -197,15 +197,15 @@ FUZZ_TARGET(p2p_headers_presync, .init = initialize) auto headers_msg = NetMsg::Make(NetMsgType::BLOCK, TX_WITH_WITNESS(block)); g_testing_setup->SendMessage(fuzzed_data_provider, std::move(headers_msg)); }); + } - // This is a conservative overestimate, as base is only moved forward when sending headers. In theory, - // the longest chain generated by this test is 1600 (FUZZ_MAX_HEADERS_RESULTS * 100) headers. In that case, - // this variable will accurately reflect the chain's total work. - total_work += CalculateClaimedHeadersWork(all_headers); + // This is a conservative overestimate, as base is only moved forward when sending headers. In theory, + // the longest chain generated by this test is 1600 (FUZZ_MAX_HEADERS_RESULTS * 100) headers. In that case, + // this variable will accurately reflect the chain's total work. + total_work += CalculateClaimedHeadersWork(all_headers); - // This test should never create a chain with more work than MinimumChainWork. - assert(total_work < chainman.MinimumChainWork()); - } + // This test should never create a chain with more work than MinimumChainWork. + assert(total_work < chainman.MinimumChainWork()); // The headers/blocks sent in this test should never be stored, as the chains don't have the work required // to meet the anti-DoS work threshold. So, if at any point the block index grew in size, then there's a bug From f1daa80521eccebe86af6ee6fa594edf40eaa676 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:51:36 +0100 Subject: [PATCH 10/36] guix: Drop no longer needed `PATH` modification --- contrib/guix/libexec/build.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/guix/libexec/build.sh b/contrib/guix/libexec/build.sh index 1ffc22a76b..3184cd4afe 100755 --- a/contrib/guix/libexec/build.sh +++ b/contrib/guix/libexec/build.sh @@ -230,8 +230,6 @@ case "$HOST" in *mingw*) HOST_LDFLAGS="-Wl,--no-insert-timestamp" ;; esac -# Make $HOST-specific native binaries from depends available in $PATH -export PATH="${BASEPREFIX}/${HOST}/native/bin:${PATH}" mkdir -p "$DISTSRC" ( cd "$DISTSRC" From c16ae717689295338025dde74c0a7a51965c383f Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 26 Aug 2024 19:35:00 +0200 Subject: [PATCH 11/36] test: switch MiniWallet padding unit from weight to vsize The weight unit is merely a consensus rule detail and is largely irrelevant for fee-rate calculations and mempool policy rules (e.g. for package relay and TRUC limits), so there doesn't seem to be any value of using a granularity that we can't even guarantee to reach exactly anyway. Switch to the more natural unit of vsize instead, which simplifies both the padding implementation and the current tests that take use of this padding. The rather annoying multiplications by `WITNESS_SCALE_FACTOR` can then be removed and weird-looking magic numbers like `4004` can be replaced by numbers that are more connected to actual policy limit constants from the codebase, e.g. `1001` for exceeding `TRUC_CHILD_MAX_VSIZE` by one. --- test/functional/feature_blocksxor.py | 2 +- .../feature_framework_miniwallet.py | 16 ++++----- test/functional/mempool_limit.py | 31 ++++++++-------- test/functional/mempool_package_limits.py | 17 ++++----- test/functional/mempool_truc.py | 35 +++++++++---------- test/functional/test_framework/wallet.py | 32 +++++++---------- 6 files changed, 61 insertions(+), 72 deletions(-) diff --git a/test/functional/feature_blocksxor.py b/test/functional/feature_blocksxor.py index 7698a66ec4..9824bf9715 100755 --- a/test/functional/feature_blocksxor.py +++ b/test/functional/feature_blocksxor.py @@ -31,7 +31,7 @@ def run_test(self): node = self.nodes[0] wallet = MiniWallet(node) for _ in range(5): - wallet.send_self_transfer(from_node=node, target_weight=80000) + wallet.send_self_transfer(from_node=node, target_vsize=20000) self.generate(wallet, 1) block_files = list(node.blocks_path.glob('blk[0-9][0-9][0-9][0-9][0-9].dat')) diff --git a/test/functional/feature_framework_miniwallet.py b/test/functional/feature_framework_miniwallet.py index d1aa24e7cd..f723f7f31e 100755 --- a/test/functional/feature_framework_miniwallet.py +++ b/test/functional/feature_framework_miniwallet.py @@ -9,7 +9,7 @@ from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( - assert_greater_than_or_equal, + assert_equal, ) from test_framework.wallet import ( MiniWallet, @@ -22,17 +22,15 @@ def set_test_params(self): self.num_nodes = 1 def test_tx_padding(self): - """Verify that MiniWallet's transaction padding (`target_weight` parameter) - works accurately enough (i.e. at most 3 WUs higher) with all modes.""" + """Verify that MiniWallet's transaction padding (`target_vsize` parameter) + works accurately with all modes.""" for mode_name, wallet in self.wallets: self.log.info(f"Test tx padding with MiniWallet mode {mode_name}...") utxo = wallet.get_utxo(mark_as_spent=False) - for target_weight in [1000, 2000, 5000, 10000, 20000, 50000, 100000, 200000, 4000000, - 989, 2001, 4337, 13371, 23219, 49153, 102035, 223419, 3999989]: - tx = wallet.create_self_transfer(utxo_to_spend=utxo, target_weight=target_weight)["tx"] - self.log.debug(f"-> target weight: {target_weight}, actual weight: {tx.get_weight()}") - assert_greater_than_or_equal(tx.get_weight(), target_weight) - assert_greater_than_or_equal(target_weight + 3, tx.get_weight()) + for target_vsize in [250, 500, 1250, 2500, 5000, 12500, 25000, 50000, 1000000, + 248, 501, 1085, 3343, 5805, 12289, 25509, 55855, 999998]: + tx = wallet.create_self_transfer(utxo_to_spend=utxo, target_vsize=target_vsize)["tx"] + assert_equal(tx.get_vsize(), target_vsize) def test_wallet_tagging(self): """Verify that tagged wallet instances are able to send funds.""" diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 626928a49a..a29c103c3f 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -55,12 +55,12 @@ def test_rbf_carveout_disallowed(self): self.generate(node, 1) # tx_A needs to be RBF'd, set minfee at set size - A_weight = 1000 + A_vsize = 250 mempoolmin_feerate = node.getmempoolinfo()["mempoolminfee"] tx_A = self.wallet.send_self_transfer( from_node=node, fee_rate=mempoolmin_feerate, - target_weight=A_weight, + target_vsize=A_vsize, utxo_to_spend=rbf_utxo, confirmed_only=True ) @@ -68,15 +68,15 @@ def test_rbf_carveout_disallowed(self): # RBF's tx_A, is not yet submitted tx_B = self.wallet.create_self_transfer( fee=tx_A["fee"] * 4, - target_weight=A_weight, + target_vsize=A_vsize, utxo_to_spend=rbf_utxo, confirmed_only=True ) # Spends tx_B's output, too big for cpfp carveout (because that would also increase the descendant limit by 1) - non_cpfp_carveout_weight = 40001 # EXTRA_DESCENDANT_TX_SIZE_LIMIT + 1 + non_cpfp_carveout_vsize = 10001 # EXTRA_DESCENDANT_TX_SIZE_LIMIT + 1 tx_C = self.wallet.create_self_transfer( - target_weight=non_cpfp_carveout_weight, + target_vsize=non_cpfp_carveout_vsize, fee_rate=mempoolmin_feerate, utxo_to_spend=tx_B["new_utxo"], confirmed_only=True @@ -103,14 +103,14 @@ def test_mid_package_eviction(self): # UTXOs to be spent by the ultimate child transaction parent_utxos = [] - evicted_weight = 8000 + evicted_vsize = 2000 # Mempool transaction which is evicted due to being at the "bottom" of the mempool when the # mempool overflows and evicts by descendant score. It's important that the eviction doesn't # happen in the middle of package evaluation, as it can invalidate the coins cache. mempool_evicted_tx = self.wallet.send_self_transfer( from_node=node, fee_rate=mempoolmin_feerate, - target_weight=evicted_weight, + target_vsize=evicted_vsize, confirmed_only=True ) # Already in mempool when package is submitted. @@ -132,14 +132,16 @@ def test_mid_package_eviction(self): # Series of parents that don't need CPFP and are submitted individually. Each one is large and # high feerate, which means they should trigger eviction but not be evicted. - parent_weight = 100000 + parent_vsize = 25000 num_big_parents = 3 - assert_greater_than(parent_weight * num_big_parents, current_info["maxmempool"] - current_info["bytes"]) + # Need to be large enough to trigger eviction + # (note that the mempool usage of a tx is about three times its vsize) + assert_greater_than(parent_vsize * num_big_parents * 3, current_info["maxmempool"] - current_info["bytes"]) parent_feerate = 100 * mempoolmin_feerate big_parent_txids = [] for i in range(num_big_parents): - parent = self.wallet.create_self_transfer(fee_rate=parent_feerate, target_weight=parent_weight, confirmed_only=True) + parent = self.wallet.create_self_transfer(fee_rate=parent_feerate, target_vsize=parent_vsize, confirmed_only=True) parent_utxos.append(parent["new_utxo"]) package_hex.append(parent["hex"]) big_parent_txids.append(parent["txid"]) @@ -311,8 +313,9 @@ def run_test(self): entry = node.getmempoolentry(txid) worst_feerate_btcvb = min(worst_feerate_btcvb, entry["fees"]["descendant"] / entry["descendantsize"]) # Needs to be large enough to trigger eviction - target_weight_each = 200000 - assert_greater_than(target_weight_each * 2, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["bytes"]) + # (note that the mempool usage of a tx is about three times its vsize) + target_vsize_each = 50000 + assert_greater_than(target_vsize_each * 2 * 3, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["bytes"]) # Should be a true CPFP: parent's feerate is just below mempool min feerate parent_feerate = mempoolmin_feerate - Decimal("0.000001") # 0.1 sats/vbyte below min feerate # Parent + child is above mempool minimum feerate @@ -320,8 +323,8 @@ def run_test(self): # However, when eviction is triggered, these transactions should be at the bottom. # This assertion assumes parent and child are the same size. miniwallet.rescan_utxos() - tx_parent_just_below = miniwallet.create_self_transfer(fee_rate=parent_feerate, target_weight=target_weight_each) - tx_child_just_above = miniwallet.create_self_transfer(utxo_to_spend=tx_parent_just_below["new_utxo"], fee_rate=child_feerate, target_weight=target_weight_each) + tx_parent_just_below = miniwallet.create_self_transfer(fee_rate=parent_feerate, target_vsize=target_vsize_each) + tx_child_just_above = miniwallet.create_self_transfer(utxo_to_spend=tx_parent_just_below["new_utxo"], fee_rate=child_feerate, target_vsize=target_vsize_each) # This package ranks below the lowest descendant package in the mempool package_fee = tx_parent_just_below["fee"] + tx_child_just_above["fee"] package_vsize = tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize() diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py index 6e26a684e2..3290ff43c4 100755 --- a/test/functional/mempool_package_limits.py +++ b/test/functional/mempool_package_limits.py @@ -4,9 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test logic for limiting mempool and package ancestors/descendants.""" from test_framework.blocktools import COINBASE_MATURITY -from test_framework.messages import ( - WITNESS_SCALE_FACTOR, -) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -290,19 +287,18 @@ def test_anc_size_limits(self): parent_utxos = [] target_vsize = 30_000 high_fee = 10 * target_vsize # 10 sats/vB - target_weight = target_vsize * WITNESS_SCALE_FACTOR self.log.info("Check that in-mempool and in-package ancestor size limits are calculated properly in packages") # Mempool transactions A and B for _ in range(2): - bulked_tx = self.wallet.create_self_transfer(target_weight=target_weight) + bulked_tx = self.wallet.create_self_transfer(target_vsize=target_vsize) self.wallet.sendrawtransaction(from_node=node, tx_hex=bulked_tx["hex"]) parent_utxos.append(bulked_tx["new_utxo"]) # Package transaction C - pc_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_utxos, fee_per_output=high_fee, target_weight=target_weight) + pc_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_utxos, fee_per_output=high_fee, target_vsize=target_vsize) # Package transaction D - pd_tx = self.wallet.create_self_transfer(utxo_to_spend=pc_tx["new_utxos"][0], target_weight=target_weight) + pd_tx = self.wallet.create_self_transfer(utxo_to_spend=pc_tx["new_utxos"][0], target_vsize=target_vsize) assert_equal(2, node.getmempoolinfo()["size"]) return [pc_tx["hex"], pd_tx["hex"]] @@ -321,20 +317,19 @@ def test_desc_size_limits(self): node = self.nodes[0] target_vsize = 21_000 high_fee = 10 * target_vsize # 10 sats/vB - target_weight = target_vsize * WITNESS_SCALE_FACTOR self.log.info("Check that in-mempool and in-package descendant sizes are calculated properly in packages") # Top parent in mempool, Ma - ma_tx = self.wallet.create_self_transfer_multi(num_outputs=2, fee_per_output=high_fee // 2, target_weight=target_weight) + ma_tx = self.wallet.create_self_transfer_multi(num_outputs=2, fee_per_output=high_fee // 2, target_vsize=target_vsize) self.wallet.sendrawtransaction(from_node=node, tx_hex=ma_tx["hex"]) package_hex = [] for j in range(2): # Two legs (left and right) # Mempool transaction (Mb and Mc) - mempool_tx = self.wallet.create_self_transfer(utxo_to_spend=ma_tx["new_utxos"][j], target_weight=target_weight) + mempool_tx = self.wallet.create_self_transfer(utxo_to_spend=ma_tx["new_utxos"][j], target_vsize=target_vsize) self.wallet.sendrawtransaction(from_node=node, tx_hex=mempool_tx["hex"]) # Package transaction (Pd and Pe) - package_tx = self.wallet.create_self_transfer(utxo_to_spend=mempool_tx["new_utxo"], target_weight=target_weight) + package_tx = self.wallet.create_self_transfer(utxo_to_spend=mempool_tx["new_utxo"], target_vsize=target_vsize) package_hex.append(package_tx["hex"]) assert_equal(3, node.getmempoolinfo()["size"]) diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py index 28f3256ef1..a7b5e83e89 100755 --- a/test/functional/mempool_truc.py +++ b/test/functional/mempool_truc.py @@ -6,7 +6,6 @@ from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, - WITNESS_SCALE_FACTOR, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -55,14 +54,14 @@ def check_mempool(self, txids): def test_truc_max_vsize(self): node = self.nodes[0] self.log.info("Test TRUC-specific maximum transaction vsize") - tx_v3_heavy = self.wallet.create_self_transfer(target_weight=(TRUC_MAX_VSIZE + 1) * WITNESS_SCALE_FACTOR, version=3) + tx_v3_heavy = self.wallet.create_self_transfer(target_vsize=TRUC_MAX_VSIZE + 1, version=3) assert_greater_than_or_equal(tx_v3_heavy["tx"].get_vsize(), TRUC_MAX_VSIZE) expected_error_heavy = f"TRUC-violation, version=3 tx {tx_v3_heavy['txid']} (wtxid={tx_v3_heavy['wtxid']}) is too big" assert_raises_rpc_error(-26, expected_error_heavy, node.sendrawtransaction, tx_v3_heavy["hex"]) self.check_mempool([]) # Ensure we are hitting the TRUC-specific limit and not something else - tx_v2_heavy = self.wallet.send_self_transfer(from_node=node, target_weight=(TRUC_MAX_VSIZE + 1) * WITNESS_SCALE_FACTOR, version=2) + tx_v2_heavy = self.wallet.send_self_transfer(from_node=node, target_vsize=TRUC_MAX_VSIZE + 1, version=2) self.check_mempool([tx_v2_heavy["txid"]]) @cleanup(extra_args=["-datacarriersize=1000"]) @@ -73,7 +72,7 @@ def test_truc_acceptance(self): self.check_mempool([tx_v3_parent_normal["txid"]]) tx_v3_child_heavy = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_weight=4004, + target_vsize=1001, version=3 ) assert_greater_than_or_equal(tx_v3_child_heavy["tx"].get_vsize(), 1000) @@ -88,7 +87,7 @@ def test_truc_acceptance(self): from_node=node, fee_rate=DEFAULT_FEE, utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_weight=3987, + target_vsize=997, version=3 ) assert_greater_than_or_equal(1000, tx_v3_child_almost_heavy["tx"].get_vsize()) @@ -98,7 +97,7 @@ def test_truc_acceptance(self): from_node=node, fee_rate=DEFAULT_FEE * 2, utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_weight=3500, + target_vsize=875, version=3 ) assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000) @@ -199,7 +198,7 @@ def test_truc_reorg(self): self.check_mempool([]) tx_v2_from_v3 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block["new_utxo"], version=2) tx_v3_from_v2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v2_block["new_utxo"], version=3) - tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_weight=5000, version=3) + tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_vsize=1250, version=3) assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000) self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]]) node.invalidateblock(block[0]) @@ -217,16 +216,16 @@ def test_nondefault_package_limits(self): """ node = self.nodes[0] self.log.info("Test that a decreased limitdescendantsize also applies to TRUC child") - parent_target_weight = 9990 * WITNESS_SCALE_FACTOR - child_target_weight = 500 * WITNESS_SCALE_FACTOR + parent_target_vsize = 9990 + child_target_vsize = 500 tx_v3_parent_large1 = self.wallet.send_self_transfer( from_node=node, - target_weight=parent_target_weight, + target_vsize=parent_target_vsize, version=3 ) tx_v3_child_large1 = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_large1["new_utxo"], - target_weight=child_target_weight, + target_vsize=child_target_vsize, version=3 ) @@ -244,12 +243,12 @@ def test_nondefault_package_limits(self): self.restart_node(0, extra_args=["-limitancestorsize=10", "-datacarriersize=40000"]) tx_v3_parent_large2 = self.wallet.send_self_transfer( from_node=node, - target_weight=parent_target_weight, + target_vsize=parent_target_vsize, version=3 ) tx_v3_child_large2 = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_large2["new_utxo"], - target_weight=child_target_weight, + target_vsize=child_target_vsize, version=3 ) @@ -267,12 +266,12 @@ def test_truc_ancestors_package(self): node = self.nodes[0] tx_v3_parent_normal = self.wallet.create_self_transfer( fee_rate=0, - target_weight=4004, + target_vsize=1001, version=3 ) tx_v3_parent_2_normal = self.wallet.create_self_transfer( fee_rate=0, - target_weight=4004, + target_vsize=1001, version=3 ) tx_v3_child_multiparent = self.wallet.create_self_transfer_multi( @@ -282,7 +281,7 @@ def test_truc_ancestors_package(self): ) tx_v3_child_heavy = self.wallet.create_self_transfer_multi( utxos_to_spend=[tx_v3_parent_normal["new_utxo"]], - target_weight=4004, + target_vsize=1001, fee_per_output=10000, version=3 ) @@ -294,7 +293,7 @@ def test_truc_ancestors_package(self): self.check_mempool([]) result = node.submitpackage([tx_v3_parent_normal["hex"], tx_v3_child_heavy["hex"]]) - # tx_v3_child_heavy is heavy based on weight, not sigops. + # tx_v3_child_heavy is heavy based on vsize, not sigops. assert_equal(result['package_msg'], f"TRUC-violation, version=3 child tx {tx_v3_child_heavy['txid']} (wtxid={tx_v3_child_heavy['wtxid']}) is too big: {tx_v3_child_heavy['tx'].get_vsize()} > 1000 virtual bytes") self.check_mempool([]) @@ -416,7 +415,7 @@ def test_truc_package_inheritance(self): node = self.nodes[0] tx_v3_parent = self.wallet.create_self_transfer( fee_rate=0, - target_weight=4004, + target_vsize=1001, version=3 ) tx_v2_child = self.wallet.create_self_transfer_multi( diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index f3713f297e..1cef714705 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -7,7 +7,6 @@ from copy import deepcopy from decimal import Decimal from enum import Enum -import math from typing import ( Any, Optional, @@ -35,7 +34,6 @@ CTxOut, hash256, ser_compact_size, - WITNESS_SCALE_FACTOR, ) from test_framework.script import ( CScript, @@ -119,20 +117,18 @@ def __init__(self, test_node, *, mode=MiniWalletMode.ADDRESS_OP_TRUE, tag_name=N def _create_utxo(self, *, txid, vout, value, height, coinbase, confirmations): return {"txid": txid, "vout": vout, "value": value, "height": height, "coinbase": coinbase, "confirmations": confirmations} - def _bulk_tx(self, tx, target_weight): - """Pad a transaction with extra outputs until it reaches a target weight (or higher). + def _bulk_tx(self, tx, target_vsize): + """Pad a transaction with extra outputs until it reaches a target vsize. returns the tx """ tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN]))) - # determine number of needed padding bytes by converting weight difference to vbytes - dummy_vbytes = (target_weight - tx.get_weight() + 3) // 4 + # determine number of needed padding bytes + dummy_vbytes = target_vsize - tx.get_vsize() # compensate for the increase of the compact-size encoded script length # (note that the length encoding of the unpadded output script needs one byte) dummy_vbytes -= len(ser_compact_size(dummy_vbytes)) - 1 tx.vout[-1].scriptPubKey = CScript([OP_RETURN] + [OP_1] * dummy_vbytes) - # Actual weight should be at most 3 higher than target weight - assert_greater_than_or_equal(tx.get_weight(), target_weight) - assert_greater_than_or_equal(target_weight + 3, tx.get_weight()) + assert_equal(tx.get_vsize(), target_vsize) def get_balance(self): return sum(u['value'] for u in self._utxos) @@ -309,7 +305,7 @@ def create_self_transfer_multi( locktime=0, sequence=0, fee_per_output=1000, - target_weight=0, + target_vsize=0, confirmed_only=False, ): """ @@ -338,8 +334,8 @@ def create_self_transfer_multi( self.sign_tx(tx) - if target_weight: - self._bulk_tx(tx, target_weight) + if target_vsize: + self._bulk_tx(tx, target_vsize) txid = tx.rehash() return { @@ -364,7 +360,7 @@ def create_self_transfer( fee_rate=Decimal("0.003"), fee=Decimal("0"), utxo_to_spend=None, - target_weight=0, + target_vsize=0, confirmed_only=False, **kwargs, ): @@ -379,20 +375,18 @@ def create_self_transfer( vsize = Decimal(168) # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other) else: assert False - if target_weight and not fee: # respect fee_rate if target weight is passed - # the actual weight might be off by 3 WUs, so calculate based on that (see self._bulk_tx) - max_actual_weight = target_weight + 3 - fee = get_fee(math.ceil(max_actual_weight / WITNESS_SCALE_FACTOR), fee_rate) + if target_vsize and not fee: # respect fee_rate if target vsize is passed + fee = get_fee(target_vsize, fee_rate) send_value = utxo_to_spend["value"] - (fee or (fee_rate * vsize / 1000)) # create tx tx = self.create_self_transfer_multi( utxos_to_spend=[utxo_to_spend], amount_per_output=int(COIN * send_value), - target_weight=target_weight, + target_vsize=target_vsize, **kwargs, ) - if not target_weight: + if not target_vsize: assert_equal(tx["tx"].get_vsize(), vsize) tx["new_utxo"] = tx.pop("new_utxos")[0] From 940edd6ac24e921f639b1376fe7d35dc14c1887d Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 26 Aug 2024 20:00:49 +0200 Subject: [PATCH 12/36] test: refactor: introduce and use `TRUC_CHILD_MAX_VSIZE` constant --- test/functional/mempool_truc.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py index a7b5e83e89..54a258215d 100755 --- a/test/functional/mempool_truc.py +++ b/test/functional/mempool_truc.py @@ -22,6 +22,7 @@ MAX_REPLACEMENT_CANDIDATES = 100 TRUC_MAX_VSIZE = 10000 +TRUC_CHILD_MAX_VSIZE = 1000 def cleanup(extra_args=None): def decorator(func): @@ -72,10 +73,10 @@ def test_truc_acceptance(self): self.check_mempool([tx_v3_parent_normal["txid"]]) tx_v3_child_heavy = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_vsize=1001, + target_vsize=TRUC_CHILD_MAX_VSIZE + 1, version=3 ) - assert_greater_than_or_equal(tx_v3_child_heavy["tx"].get_vsize(), 1000) + assert_greater_than_or_equal(tx_v3_child_heavy["tx"].get_vsize(), TRUC_CHILD_MAX_VSIZE) expected_error_child_heavy = f"TRUC-violation, version=3 child tx {tx_v3_child_heavy['txid']} (wtxid={tx_v3_child_heavy['wtxid']}) is too big" assert_raises_rpc_error(-26, expected_error_child_heavy, node.sendrawtransaction, tx_v3_child_heavy["hex"]) self.check_mempool([tx_v3_parent_normal["txid"]]) @@ -87,10 +88,10 @@ def test_truc_acceptance(self): from_node=node, fee_rate=DEFAULT_FEE, utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_vsize=997, + target_vsize=TRUC_CHILD_MAX_VSIZE - 3, version=3 ) - assert_greater_than_or_equal(1000, tx_v3_child_almost_heavy["tx"].get_vsize()) + assert_greater_than_or_equal(TRUC_CHILD_MAX_VSIZE, tx_v3_child_almost_heavy["tx"].get_vsize()) self.check_mempool([tx_v3_parent_normal["txid"], tx_v3_child_almost_heavy["txid"]]) assert_equal(node.getmempoolentry(tx_v3_parent_normal["txid"])["descendantcount"], 2) tx_v3_child_almost_heavy_rbf = self.wallet.send_self_transfer( @@ -100,7 +101,8 @@ def test_truc_acceptance(self): target_vsize=875, version=3 ) - assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000) + assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), + TRUC_CHILD_MAX_VSIZE) self.check_mempool([tx_v3_parent_normal["txid"], tx_v3_child_almost_heavy_rbf["txid"]]) assert_equal(node.getmempoolentry(tx_v3_parent_normal["txid"])["descendantcount"], 2) @@ -199,7 +201,7 @@ def test_truc_reorg(self): tx_v2_from_v3 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block["new_utxo"], version=2) tx_v3_from_v2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v2_block["new_utxo"], version=3) tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_vsize=1250, version=3) - assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000) + assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], TRUC_CHILD_MAX_VSIZE) self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]]) node.invalidateblock(block[0]) self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]]) @@ -231,7 +233,7 @@ def test_nondefault_package_limits(self): # Parent and child are within v3 limits, but parent's 10kvB descendant limit is exceeded assert_greater_than_or_equal(TRUC_MAX_VSIZE, tx_v3_parent_large1["tx"].get_vsize()) - assert_greater_than_or_equal(1000, tx_v3_child_large1["tx"].get_vsize()) + assert_greater_than_or_equal(TRUC_CHILD_MAX_VSIZE, tx_v3_child_large1["tx"].get_vsize()) assert_greater_than(tx_v3_parent_large1["tx"].get_vsize() + tx_v3_child_large1["tx"].get_vsize(), 10000) assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds descendant size limit for tx {tx_v3_parent_large1['txid']}", node.sendrawtransaction, tx_v3_child_large1["hex"]) @@ -254,7 +256,7 @@ def test_nondefault_package_limits(self): # Parent and child are within TRUC limits assert_greater_than_or_equal(TRUC_MAX_VSIZE, tx_v3_parent_large2["tx"].get_vsize()) - assert_greater_than_or_equal(1000, tx_v3_child_large2["tx"].get_vsize()) + assert_greater_than_or_equal(TRUC_CHILD_MAX_VSIZE, tx_v3_child_large2["tx"].get_vsize()) assert_greater_than(tx_v3_parent_large2["tx"].get_vsize() + tx_v3_child_large2["tx"].get_vsize(), 10000) assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds ancestor size limit", node.sendrawtransaction, tx_v3_child_large2["hex"]) @@ -281,7 +283,7 @@ def test_truc_ancestors_package(self): ) tx_v3_child_heavy = self.wallet.create_self_transfer_multi( utxos_to_spend=[tx_v3_parent_normal["new_utxo"]], - target_vsize=1001, + target_vsize=TRUC_CHILD_MAX_VSIZE + 1, fee_per_output=10000, version=3 ) From 9123a286e9743196307d38dd66ae0cfaf3805029 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 29 Sep 2024 12:05:12 +0100 Subject: [PATCH 13/36] qt6: Handle deprecated `QLocale::nativeCountryName` This change ensures compatibility across all supported Qt versions. --- src/qt/optionsdialog.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 4d590c3eed..45fdd13a21 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -169,8 +169,15 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet) /** check if the locale name consists of 2 parts (language_country) */ if(langStr.contains("_")) { - /** display language strings as "native language - native country (locale name)", e.g. "Deutsch - Deutschland (de)" */ - ui->lang->addItem(locale.nativeLanguageName() + QString(" - ") + locale.nativeCountryName() + QString(" (") + langStr + QString(")"), QVariant(langStr)); + /** display language strings as "native language - native country/territory (locale name)", e.g. "Deutsch - Deutschland (de)" */ + ui->lang->addItem(locale.nativeLanguageName() + QString(" - ") + +#if (QT_VERSION >= QT_VERSION_CHECK(6, 2, 0)) + locale.nativeTerritoryName() + +#else + locale.nativeCountryName() + +#endif + QString(" (") + langStr + QString(")"), QVariant(langStr)); + } else { From cb750b4b405b1036991a49b410a75da95e9d1ac0 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 29 Sep 2024 14:20:08 +0100 Subject: [PATCH 14/36] qt6, test: Use `qWarning()` instead of `QWARN()` macro The `QWARN()` macro internally uses `QTest::qWarn()`, which has been deprecated since Qt 6.3. Replacing it with `qWarning()` ensures compatibility across all Qt versions. --- src/qt/test/addressbooktests.cpp | 4 ++-- src/qt/test/apptests.cpp | 4 ++-- src/qt/test/test_main.cpp | 2 +- src/qt/test/wallettests.cpp | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index f7d66f316e..3d5cb4a863 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -222,8 +222,8 @@ void AddressBookTests::addressBookTests() // framework when it tries to look up unimplemented cocoa functions, // and fails to handle returned nulls // (https://bugreports.qt.io/browse/QTBUG-49686). - QWARN("Skipping AddressBookTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke " - "with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build."); + qWarning() << "Skipping AddressBookTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke " + "with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build."; return; } #endif diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index 10abcb00eb..73e04b55c8 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -60,8 +60,8 @@ void AppTests::appTests() // framework when it tries to look up unimplemented cocoa functions, // and fails to handle returned nulls // (https://bugreports.qt.io/browse/QTBUG-49686). - QWARN("Skipping AppTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke " - "with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build."); + qWarning() << "Skipping AppTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke " + "with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build."; return; } #endif diff --git a/src/qt/test/test_main.cpp b/src/qt/test/test_main.cpp index 958cc7ae88..5116fbacb9 100644 --- a/src/qt/test/test_main.cpp +++ b/src/qt/test/test_main.cpp @@ -58,7 +58,7 @@ int main(int argc, char* argv[]) gArgs.ForceSetArg("-natpmp", "0"); std::string error; - if (!gArgs.ReadConfigFiles(error, true)) QWARN(error.c_str()); + if (!gArgs.ReadConfigFiles(error, true)) qWarning() << error.c_str(); // Prefer the "minimal" platform for the test instead of the normal default // platform ("xcb", "windows", or "cocoa") so tests can't unintentionally diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 6a573d284c..98dfe12f08 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -475,8 +475,8 @@ void WalletTests::walletTests() // framework when it tries to look up unimplemented cocoa functions, // and fails to handle returned nulls // (https://bugreports.qt.io/browse/QTBUG-49686). - QWARN("Skipping WalletTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke " - "with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build."); + qWarning() << "Skipping WalletTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke " + "with 'QT_QPA_PLATFORM=cocoa test_bitcoin-qt' on mac, or else use a linux or windows build."; return; } #endif From 97c97177cdb2f596aa7d4a65c4bde87de50a96f2 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Thu, 16 May 2024 10:17:57 +0200 Subject: [PATCH 15/36] net: Add PCP and NATPMP implementation Add a RFC 6886 NATPMP and RFC 6887 Port Control Protocol (PCP) implementation, to replace libnatpmp. --- src/util/CMakeLists.txt | 1 + src/util/pcp.cpp | 524 ++++++++++++++++++++++++++++++++++++++++ src/util/pcp.h | 68 ++++++ 3 files changed, 593 insertions(+) create mode 100644 src/util/pcp.cpp create mode 100644 src/util/pcp.h diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index d65e634c57..f4b9b47d10 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -16,6 +16,7 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL hasher.cpp moneystr.cpp netif.cpp + pcp.cpp rbf.cpp readwritefile.cpp serfloat.cpp diff --git a/src/util/pcp.cpp b/src/util/pcp.cpp new file mode 100644 index 0000000000..b02568db35 --- /dev/null +++ b/src/util/pcp.cpp @@ -0,0 +1,524 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +// RFC6886 NAT-PMP and RFC6887 Port Control Protocol (PCP) implementation. +// NAT-PMP and PCP use network byte order (big-endian). + +// NAT-PMP (v0) protocol constants. +//! NAT-PMP uses a fixed server port number (RFC6887 section 1.1). +constexpr uint16_t NATPMP_SERVER_PORT = 5351; +//! Version byte for NATPMP (RFC6886 1.1) +constexpr uint8_t NATPMP_VERSION = 0; +//! Request opcode base (RFC6886 3). +constexpr uint8_t NATPMP_REQUEST = 0x00; +//! Response opcode base (RFC6886 3). +constexpr uint8_t NATPMP_RESPONSE = 0x80; +//! Get external address (RFC6886 3.2) +constexpr uint8_t NATPMP_OP_GETEXTERNAL = 0x00; +//! Map TCP port (RFC6886 3.3) +constexpr uint8_t NATPMP_OP_MAP_TCP = 0x02; +//! Shared request header size in bytes. +constexpr size_t NATPMP_REQUEST_HDR_SIZE = 2; +//! Shared response header (minimum) size in bytes. +constexpr size_t NATPMP_RESPONSE_HDR_SIZE = 8; +//! GETEXTERNAL request size in bytes, including header (RFC6886 3.2). +constexpr size_t NATPMP_GETEXTERNAL_REQUEST_SIZE = NATPMP_REQUEST_HDR_SIZE + 0; +//! GETEXTERNAL response size in bytes, including header (RFC6886 3.2). +constexpr size_t NATPMP_GETEXTERNAL_RESPONSE_SIZE = NATPMP_RESPONSE_HDR_SIZE + 4; +//! MAP request size in bytes, including header (RFC6886 3.3). +constexpr size_t NATPMP_MAP_REQUEST_SIZE = NATPMP_REQUEST_HDR_SIZE + 10; +//! MAP response size in bytes, including header (RFC6886 3.3). +constexpr size_t NATPMP_MAP_RESPONSE_SIZE = NATPMP_RESPONSE_HDR_SIZE + 8; + +// Shared header offsets (RFC6886 3.2, 3.3), relative to start of packet. +//! Offset of version field in packets. +constexpr size_t NATPMP_HDR_VERSION_OFS = 0; +//! Offset of opcode field in packets +constexpr size_t NATPMP_HDR_OP_OFS = 1; +//! Offset of result code in packets. Result codes are 16 bit in NAT-PMP instead of 8 bit in PCP. +constexpr size_t NATPMP_RESPONSE_HDR_RESULT_OFS = 2; + +// GETEXTERNAL response offsets (RFC6886 3.2), relative to start of packet. +//! Returned external address +constexpr size_t NATPMP_GETEXTERNAL_RESPONSE_IP_OFS = 8; + +// MAP request offsets (RFC6886 3.3), relative to start of packet. +//! Internal port to be mapped. +constexpr size_t NATPMP_MAP_REQUEST_INTERNAL_PORT_OFS = 4; +//! Suggested external port for mapping. +constexpr size_t NATPMP_MAP_REQUEST_EXTERNAL_PORT_OFS = 6; +//! Requested port mapping lifetime in seconds. +constexpr size_t NATPMP_MAP_REQUEST_LIFETIME_OFS = 8; + +// MAP response offsets (RFC6886 3.3), relative to start of packet. +//! Internal port for mapping (will match internal port of request). +constexpr size_t NATPMP_MAP_RESPONSE_INTERNAL_PORT_OFS = 8; +//! External port for mapping. +constexpr size_t NATPMP_MAP_RESPONSE_EXTERNAL_PORT_OFS = 10; +//! Created port mapping lifetime in seconds. +constexpr size_t NATPMP_MAP_RESPONSE_LIFETIME_OFS = 12; + +// Relevant NETPMP result codes (RFC6886 3.5). +//! Result code representing success status. +constexpr uint8_t NATPMP_RESULT_SUCCESS = 0; +//! Result code representing unsupported version. +constexpr uint8_t NATPMP_RESULT_UNSUPP_VERSION = 1; +//! Result code representing lack of resources. +constexpr uint8_t NATPMP_RESULT_NO_RESOURCES = 4; + +//! Mapping of NATPMP result code to string (RFC6886 3.5). Result codes <=2 match PCP. +const std::map NATPMP_RESULT_STR{ + {0, "SUCCESS"}, + {1, "UNSUPP_VERSION"}, + {2, "NOT_AUTHORIZED"}, + {3, "NETWORK_FAILURE"}, + {4, "NO_RESOURCES"}, + {5, "UNSUPP_OPCODE"}, +}; + +// PCP (v2) protocol constants. +//! Maximum packet size in bytes (RFC6887 section 7). +constexpr size_t PCP_MAX_SIZE = 1100; +//! PCP uses a fixed server port number (RFC6887 section 19.1). Shared with NAT-PMP. +constexpr uint16_t PCP_SERVER_PORT = NATPMP_SERVER_PORT; +//! Version byte. 0 is NAT-PMP (RFC6886), 1 is forbidden, 2 for PCP (RFC6887). +constexpr uint8_t PCP_VERSION = 2; +//! PCP Request Header. See RFC6887 section 7.1. Shared with NAT-PMP. +constexpr uint8_t PCP_REQUEST = NATPMP_REQUEST; // R = 0 +//! PCP Response Header. See RFC6887 section 7.2. Shared with NAT-PMP. +constexpr uint8_t PCP_RESPONSE = NATPMP_RESPONSE; // R = 1 +//! Map opcode. See RFC6887 section 19.2 +constexpr uint8_t PCP_OP_MAP = 0x01; +//! TCP protocol number (IANA). +constexpr uint16_t PCP_PROTOCOL_TCP = 6; +//! Request and response header size in bytes (RFC6887 section 7.1). +constexpr size_t PCP_HDR_SIZE = 24; +//! Map request and response size in bytes (RFC6887 section 11.1). +constexpr size_t PCP_MAP_SIZE = 36; + +// Header offsets shared between request and responses (RFC6887 7.1, 7.2), relative to start of packet. +//! Version field (1 byte). +constexpr size_t PCP_HDR_VERSION_OFS = NATPMP_HDR_VERSION_OFS; +//! Opcode field (1 byte). +constexpr size_t PCP_HDR_OP_OFS = NATPMP_HDR_OP_OFS; +//! Requested lifetime (request), granted lifetime (response) (4 bytes). +constexpr size_t PCP_HDR_LIFETIME_OFS = 4; + +// Request header offsets (RFC6887 7.1), relative to start of packet. +//! PCP client's IP address (16 bytes). +constexpr size_t PCP_REQUEST_HDR_IP_OFS = 8; + +// Response header offsets (RFC6887 7.2), relative to start of packet. +//! Result code (1 byte). +constexpr size_t PCP_RESPONSE_HDR_RESULT_OFS = 3; + +// MAP request/response offsets (RFC6887 11.1), relative to start of opcode-specific data. +//! Mapping nonce (12 bytes). +constexpr size_t PCP_MAP_NONCE_OFS = 0; +//! Protocol (1 byte). +constexpr size_t PCP_MAP_PROTOCOL_OFS = 12; +//! Internal port for mapping (2 bytes). +constexpr size_t PCP_MAP_INTERNAL_PORT_OFS = 16; +//! Suggested external port (request), assigned external port (response) (2 bytes). +constexpr size_t PCP_MAP_EXTERNAL_PORT_OFS = 18; +//! Suggested external IP (request), assigned external IP (response) (16 bytes). +constexpr size_t PCP_MAP_EXTERNAL_IP_OFS = 20; + +//! Result code representing success (RFC6887 7.4), shared with NAT-PMP. +constexpr uint8_t PCP_RESULT_SUCCESS = NATPMP_RESULT_SUCCESS; +//! Result code representing lack of resources (RFC6887 7.4). +constexpr uint8_t PCP_RESULT_NO_RESOURCES = 8; + +//! Mapping of PCP result code to string (RFC6887 7.4). Result codes <=2 match NAT-PMP. +const std::map PCP_RESULT_STR{ + {0, "SUCCESS"}, + {1, "UNSUPP_VERSION"}, + {2, "NOT_AUTHORIZED"}, + {3, "MALFORMED_REQUEST"}, + {4, "UNSUPP_OPCODE"}, + {5, "UNSUPP_OPTION"}, + {6, "MALFORMED_OPTION"}, + {7, "NETWORK_FAILURE"}, + {8, "NO_RESOURCES"}, + {9, "UNSUPP_PROTOCOL"}, + {10, "USER_EX_QUOTA"}, + {11, "CANNOT_PROVIDE_EXTERNAL"}, + {12, "ADDRESS_MISMATCH"}, + {13, "EXCESSIVE_REMOTE_PEER"}, +}; + +//! Return human-readable string from NATPMP result code. +std::string NATPMPResultString(uint8_t result_code) +{ + auto result_i = NATPMP_RESULT_STR.find(result_code); + return strprintf("%s (code %d)", result_i == NATPMP_RESULT_STR.end() ? "(unknown)" : result_i->second, result_code); +} + +//! Return human-readable string from PCP result code. +std::string PCPResultString(uint8_t result_code) +{ + auto result_i = PCP_RESULT_STR.find(result_code); + return strprintf("%s (code %d)", result_i == PCP_RESULT_STR.end() ? "(unknown)" : result_i->second, result_code); +} + +//! Wrap address in IPv6 according to RFC6887. wrapped_addr needs to be able to store 16 bytes. +[[nodiscard]] bool PCPWrapAddress(Span wrapped_addr, const CNetAddr &addr) +{ + Assume(wrapped_addr.size() == ADDR_IPV6_SIZE); + if (addr.IsIPv4()) { + struct in_addr addr4; + if (!addr.GetInAddr(&addr4)) return false; + // Section 5: "When the address field holds an IPv4 address, an IPv4-mapped IPv6 address [RFC4291] is used (::ffff:0:0/96)." + std::memcpy(wrapped_addr.data(), IPV4_IN_IPV6_PREFIX.data(), IPV4_IN_IPV6_PREFIX.size()); + std::memcpy(wrapped_addr.data() + IPV4_IN_IPV6_PREFIX.size(), &addr4, ADDR_IPV4_SIZE); + return true; + } else if (addr.IsIPv6()) { + struct in6_addr addr6; + if (!addr.GetIn6Addr(&addr6)) return false; + std::memcpy(wrapped_addr.data(), &addr6, ADDR_IPV6_SIZE); + return true; + } else { + return false; + } +} + +//! Unwrap PCP-encoded address according to RFC6887. +CNetAddr PCPUnwrapAddress(Span wrapped_addr) +{ + Assume(wrapped_addr.size() == ADDR_IPV6_SIZE); + if (util::HasPrefix(wrapped_addr, IPV4_IN_IPV6_PREFIX)) { + struct in_addr addr4; + std::memcpy(&addr4, wrapped_addr.data() + IPV4_IN_IPV6_PREFIX.size(), ADDR_IPV4_SIZE); + return CNetAddr(addr4); + } else { + struct in6_addr addr6; + std::memcpy(&addr6, wrapped_addr.data(), ADDR_IPV6_SIZE); + return CNetAddr(addr6); + } +} + +//! PCP or NAT-PMP send-receive loop. +std::optional> PCPSendRecv(Sock &sock, const std::string &protocol, Span request, int num_tries, + std::chrono::milliseconds timeout_per_try, + std::function)> check_packet) +{ + using namespace std::chrono; + // UDP is a potentially lossy protocol, so we try to send again a few times. + uint8_t response[PCP_MAX_SIZE]; + bool got_response = false; + int recvsz = 0; + for (int ntry = 0; !got_response && ntry < num_tries; ++ntry) { + if (ntry > 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "%s: Retrying (%d)\n", protocol, ntry); + } + // Dispatch packet to gateway. + if (sock.Send(request.data(), request.size(), 0) != static_cast(request.size())) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "%s: Could not send request: %s\n", protocol, NetworkErrorString(WSAGetLastError())); + return std::nullopt; // Network-level error, probably no use retrying. + } + + // Wait for response(s) until we get a valid response, a network error, or time out. + auto cur_time = time_point_cast(steady_clock::now()); + auto deadline = cur_time + timeout_per_try; + while ((cur_time = time_point_cast(steady_clock::now())) < deadline) { + Sock::Event occurred = 0; + if (!sock.Wait(deadline - cur_time, Sock::RECV, &occurred)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "%s: Could not wait on socket: %s\n", protocol, NetworkErrorString(WSAGetLastError())); + return std::nullopt; // Network-level error, probably no use retrying. + } + if (!occurred) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "%s: Timeout\n", protocol); + break; // Retry. + } + + // Receive response. + recvsz = sock.Recv(response, sizeof(response), MSG_DONTWAIT); + if (recvsz < 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "%s: Could not receive response: %s\n", protocol, NetworkErrorString(WSAGetLastError())); + return std::nullopt; // Network-level error, probably no use retrying. + } + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "%s: Received response of %d bytes: %s\n", protocol, recvsz, HexStr(Span(response, recvsz))); + + if (check_packet(Span(response, recvsz))) { + got_response = true; // Got expected response, break from receive loop as well as from retry loop. + break; + } + } + } + if (!got_response) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "%s: Giving up after %d tries\n", protocol, num_tries); + return std::nullopt; + } + return std::vector(response, response + recvsz); +} + +} + +std::variant NATPMPRequestPortMap(const CNetAddr &gateway, uint16_t port, uint32_t lifetime, int num_tries, std::chrono::milliseconds timeout_per_try) +{ + struct sockaddr_storage dest_addr; + socklen_t dest_addrlen = sizeof(struct sockaddr_storage); + + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "natpmp: Requesting port mapping port %d from gateway %s\n", port, gateway.ToStringAddr()); + + // Validate gateway, make sure it's IPv4. NAT-PMP does not support IPv6. + if (!CService(gateway, PCP_SERVER_PORT).GetSockAddr((struct sockaddr*)&dest_addr, &dest_addrlen)) return MappingError::NETWORK_ERROR; + if (dest_addr.ss_family != AF_INET) return MappingError::NETWORK_ERROR; + + // Create IPv4 UDP socket + auto sock{CreateSock(AF_INET, SOCK_DGRAM, IPPROTO_UDP)}; + if (!sock) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Could not create UDP socket: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + + // Associate UDP socket to gateway. + if (sock->Connect((struct sockaddr*)&dest_addr, dest_addrlen) != 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Could not connect to gateway: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + + // Use getsockname to get the address toward the default gateway (the internal address). + struct sockaddr_in internal; + socklen_t internal_addrlen = sizeof(struct sockaddr_in); + if (sock->GetSockName((struct sockaddr*)&internal, &internal_addrlen) != 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Could not get sock name: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + + // Request external IP address (RFC6886 section 3.2). + std::vector request(NATPMP_GETEXTERNAL_REQUEST_SIZE); + request[NATPMP_HDR_VERSION_OFS] = NATPMP_VERSION; + request[NATPMP_HDR_OP_OFS] = NATPMP_REQUEST | NATPMP_OP_GETEXTERNAL; + + auto recv_res = PCPSendRecv(*sock, "natpmp", request, num_tries, timeout_per_try, + [&](const Span response) -> bool { + if (response.size() < NATPMP_GETEXTERNAL_RESPONSE_SIZE) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Response too small\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + if (response[NATPMP_HDR_VERSION_OFS] != NATPMP_VERSION || response[NATPMP_HDR_OP_OFS] != (NATPMP_RESPONSE | NATPMP_OP_GETEXTERNAL)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Response to wrong command\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + return true; + }); + + struct in_addr external_addr; + if (recv_res) { + const std::span response = *recv_res; + + Assume(response.size() >= NATPMP_GETEXTERNAL_RESPONSE_SIZE); + uint16_t result_code = ReadBE16(response.data() + NATPMP_RESPONSE_HDR_RESULT_OFS); + if (result_code != NATPMP_RESULT_SUCCESS) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Getting external address failed with result %s\n", NATPMPResultString(result_code)); + return MappingError::PROTOCOL_ERROR; + } + + std::memcpy(&external_addr, response.data() + NATPMP_GETEXTERNAL_RESPONSE_IP_OFS, ADDR_IPV4_SIZE); + } else { + return MappingError::NETWORK_ERROR; + } + + // Create TCP mapping request (RFC6886 section 3.3). + request = std::vector(NATPMP_MAP_REQUEST_SIZE); + request[NATPMP_HDR_VERSION_OFS] = NATPMP_VERSION; + request[NATPMP_HDR_OP_OFS] = NATPMP_REQUEST | NATPMP_OP_MAP_TCP; + WriteBE16(request.data() + NATPMP_MAP_REQUEST_INTERNAL_PORT_OFS, port); + WriteBE16(request.data() + NATPMP_MAP_REQUEST_EXTERNAL_PORT_OFS, port); + WriteBE32(request.data() + NATPMP_MAP_REQUEST_LIFETIME_OFS, lifetime); + + recv_res = PCPSendRecv(*sock, "natpmp", request, num_tries, timeout_per_try, + [&](const Span response) -> bool { + if (response.size() < NATPMP_MAP_RESPONSE_SIZE) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Response too small\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + if (response[0] != NATPMP_VERSION || response[1] != (NATPMP_RESPONSE | NATPMP_OP_MAP_TCP)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Response to wrong command\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + uint16_t internal_port = ReadBE16(response.data() + NATPMP_MAP_RESPONSE_INTERNAL_PORT_OFS); + if (internal_port != port) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Response port doesn't match request\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + return true; + }); + + if (recv_res) { + const std::span response = *recv_res; + + Assume(response.size() >= NATPMP_MAP_RESPONSE_SIZE); + uint16_t result_code = ReadBE16(response.data() + NATPMP_RESPONSE_HDR_RESULT_OFS); + if (result_code != NATPMP_RESULT_SUCCESS) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "natpmp: Port mapping failed with result %s\n", NATPMPResultString(result_code)); + if (result_code == NATPMP_RESULT_NO_RESOURCES) { + return MappingError::NO_RESOURCES; + } + return MappingError::PROTOCOL_ERROR; + } + + uint32_t lifetime_ret = ReadBE32(response.data() + NATPMP_MAP_RESPONSE_LIFETIME_OFS); + uint16_t external_port = ReadBE16(response.data() + NATPMP_MAP_RESPONSE_EXTERNAL_PORT_OFS); + return MappingResult(NATPMP_VERSION, CService(internal.sin_addr, port), CService(external_addr, external_port), lifetime_ret); + } else { + return MappingError::NETWORK_ERROR; + } +} + +std::variant PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, int num_tries, std::chrono::milliseconds timeout_per_try) +{ + struct sockaddr_storage dest_addr, bind_addr; + socklen_t dest_addrlen = sizeof(struct sockaddr_storage), bind_addrlen = sizeof(struct sockaddr_storage); + + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "pcp: Requesting port mapping for addr %s port %d from gateway %s\n", bind.ToStringAddr(), port, gateway.ToStringAddr()); + + // Validate addresses, make sure they're the same network family. + if (!CService(gateway, PCP_SERVER_PORT).GetSockAddr((struct sockaddr*)&dest_addr, &dest_addrlen)) return MappingError::NETWORK_ERROR; + if (!CService(bind, 0).GetSockAddr((struct sockaddr*)&bind_addr, &bind_addrlen)) return MappingError::NETWORK_ERROR; + if (dest_addr.ss_family != bind_addr.ss_family) return MappingError::NETWORK_ERROR; + + // Create UDP socket (IPv4 or IPv6 based on provided gateway). + auto sock{CreateSock(dest_addr.ss_family, SOCK_DGRAM, IPPROTO_UDP)}; + if (!sock) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Could not create UDP socket: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + + // Make sure that we send from requested destination address, anything else will be + // rejected by a security-conscious router. + if (sock->Bind((struct sockaddr*)&bind_addr, bind_addrlen) != 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Could not bind to address: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + + // Associate UDP socket to gateway. + if (sock->Connect((struct sockaddr*)&dest_addr, dest_addrlen) != 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Could not connect to gateway: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + + // Use getsockname to get the address toward the default gateway (the internal address), + // in case we don't know what address to map + // (this is only needed if bind is INADDR_ANY, but it doesn't hurt as an extra check). + struct sockaddr_storage internal_addr; + socklen_t internal_addrlen = sizeof(struct sockaddr_storage); + if (sock->GetSockName((struct sockaddr*)&internal_addr, &internal_addrlen) != 0) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Could not get sock name: %s\n", NetworkErrorString(WSAGetLastError())); + return MappingError::NETWORK_ERROR; + } + CService internal; + if (!internal.SetSockAddr((struct sockaddr*)&internal_addr)) return MappingError::NETWORK_ERROR; + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "pcp: Internal address after connect: %s\n", internal.ToStringAddr()); + + // Build request packet. Make sure the packet is zeroed so that reserved fields are zero + // as required by the spec (and not potentially leak data). + // Make sure there's space for the request header and MAP specific request data. + std::vector request(PCP_HDR_SIZE + PCP_MAP_SIZE); + // Fill in request header, See RFC6887 Figure 2. + size_t ofs = 0; + request[ofs + PCP_HDR_VERSION_OFS] = PCP_VERSION; + request[ofs + PCP_HDR_OP_OFS] = PCP_REQUEST | PCP_OP_MAP; + WriteBE32(request.data() + ofs + PCP_HDR_LIFETIME_OFS, lifetime); + if (!PCPWrapAddress(Span(request).subspan(ofs + PCP_REQUEST_HDR_IP_OFS, ADDR_IPV6_SIZE), internal)) return MappingError::NETWORK_ERROR; + + ofs += PCP_HDR_SIZE; + + // Fill in MAP request packet, See RFC6887 Figure 9. + // Randomize mapping nonce (this is repeated in the response, to be able to + // correlate requests and responses, and used to authenticate changes to the mapping). + std::memcpy(request.data() + ofs + PCP_MAP_NONCE_OFS, nonce.data(), PCP_MAP_NONCE_SIZE); + request[ofs + PCP_MAP_PROTOCOL_OFS] = PCP_PROTOCOL_TCP; + WriteBE16(request.data() + ofs + PCP_MAP_INTERNAL_PORT_OFS, port); + WriteBE16(request.data() + ofs + PCP_MAP_EXTERNAL_PORT_OFS, port); + if (!PCPWrapAddress(Span(request).subspan(ofs + PCP_MAP_EXTERNAL_IP_OFS, ADDR_IPV6_SIZE), bind)) return MappingError::NETWORK_ERROR; + + ofs += PCP_MAP_SIZE; + Assume(ofs == request.size()); + + // Receive loop. + bool is_natpmp = false; + auto recv_res = PCPSendRecv(*sock, "pcp", request, num_tries, timeout_per_try, + [&](const Span response) -> bool { + // Unsupported version according to RFC6887 appendix A and RFC6886 section 3.5, can fall back to NAT-PMP. + if (response.size() == NATPMP_RESPONSE_HDR_SIZE && response[PCP_HDR_VERSION_OFS] == NATPMP_VERSION && response[PCP_RESPONSE_HDR_RESULT_OFS] == NATPMP_RESULT_UNSUPP_VERSION) { + is_natpmp = true; + return true; // Let it through to caller. + } + if (response.size() < (PCP_HDR_SIZE + PCP_MAP_SIZE)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Response too small\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + if (response[PCP_HDR_VERSION_OFS] != PCP_VERSION || response[PCP_HDR_OP_OFS] != (PCP_RESPONSE | PCP_OP_MAP)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Response to wrong command\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + // Handle MAP opcode response. See RFC6887 Figure 10. + // Check that returned mapping nonce matches our request. + if (!std::ranges::equal(response.subspan(PCP_HDR_SIZE + PCP_MAP_NONCE_OFS, PCP_MAP_NONCE_SIZE), nonce)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Mapping nonce mismatch\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + uint8_t protocol = response[PCP_HDR_SIZE + 12]; + uint16_t internal_port = ReadBE16(response.data() + PCP_HDR_SIZE + 16); + if (protocol != PCP_PROTOCOL_TCP || internal_port != port) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Response protocol or port doesn't match request\n"); + return false; // Wasn't response to what we expected, try receiving next packet. + } + return true; + }); + + if (!recv_res) { + return MappingError::NETWORK_ERROR; + } + if (is_natpmp) { + return MappingError::UNSUPP_VERSION; + } + + const std::span response = *recv_res; + // If we get here, we got a valid MAP response to our request. + // Check to see if we got the result we expected. + Assume(response.size() >= (PCP_HDR_SIZE + PCP_MAP_SIZE)); + uint8_t result_code = response[PCP_RESPONSE_HDR_RESULT_OFS]; + uint32_t lifetime_ret = ReadBE32(response.data() + PCP_HDR_LIFETIME_OFS); + uint16_t external_port = ReadBE16(response.data() + PCP_HDR_SIZE + PCP_MAP_EXTERNAL_PORT_OFS); + CNetAddr external_addr{PCPUnwrapAddress(response.subspan(PCP_HDR_SIZE + PCP_MAP_EXTERNAL_IP_OFS, ADDR_IPV6_SIZE))}; + if (result_code != PCP_RESULT_SUCCESS) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "pcp: Mapping failed with result %s\n", PCPResultString(result_code)); + if (result_code == PCP_RESULT_NO_RESOURCES) { + return MappingError::NO_RESOURCES; + } + return MappingError::PROTOCOL_ERROR; + } + + return MappingResult(PCP_VERSION, CService(internal, port), CService(external_addr, external_port), lifetime_ret); +} + +std::string MappingResult::ToString() +{ + Assume(version == NATPMP_VERSION || version == PCP_VERSION); + return strprintf("%s:%s -> %s (for %ds)", + version == NATPMP_VERSION ? "natpmp" : "pcp", + external.ToStringAddrPort(), + internal.ToStringAddrPort(), + lifetime + ); +} diff --git a/src/util/pcp.h b/src/util/pcp.h new file mode 100644 index 0000000000..0a7955fa57 --- /dev/null +++ b/src/util/pcp.h @@ -0,0 +1,68 @@ +// Copyright (c) 2024 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_PCP_H +#define BITCOIN_UTIL_PCP_H + +#include + +#include + +// RFC6886 NAT-PMP and RFC6887 Port Control Protocol (PCP) implementation. +// NAT-PMP and PCP use network byte order (big-endian). + +//! Mapping nonce size in bytes (see RFC6887 section 11.1). +constexpr size_t PCP_MAP_NONCE_SIZE = 12; + +//! PCP mapping nonce. Arbitrary data chosen by the client to identify a mapping. +typedef std::array PCPMappingNonce; + +//! Unsuccessful response to a port mapping. +enum class MappingError { + NETWORK_ERROR, ///< Any kind of network-level error. + PROTOCOL_ERROR, ///< Any kind of protocol-level error, except unsupported version or no resources. + UNSUPP_VERSION, ///< Unsupported protocol version. + NO_RESOURCES, ///< No resources available (port probably already mapped). +}; + +//! Successful response to a port mapping. +struct MappingResult { + MappingResult(uint8_t version, const CService &internal_in, const CService &external_in, uint32_t lifetime_in): + version(version), internal(internal_in), external(external_in), lifetime(lifetime_in) {} + //! Protocol version, one of NATPMP_VERSION or PCP_VERSION. + uint8_t version; + //! Internal host:port. + CService internal; + //! External host:port. + CService external; + //! Granted lifetime of binding (seconds). + uint32_t lifetime; + + //! Format mapping as string for logging. + std::string ToString(); +}; + +//! Try to open a port using RFC 6886 NAT-PMP. IPv4 only. +//! +//! * gateway: Destination address for PCP requests (usually the default gateway). +//! * port: Internal port, and desired external port. +//! * lifetime: Requested lifetime in seconds for mapping. The server may assign as shorter or longer lifetime. A lifetime of 0 deletes the mapping. +//! * num_tries: Number of tries in case of no response. +//! +//! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError. +std::variant NATPMPRequestPortMap(const CNetAddr &gateway, uint16_t port, uint32_t lifetime, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000)); + +//! Try to open a port using RFC 6887 Port Control Protocol (PCP). Handles IPv4 and IPv6. +//! +//! * nonce: Mapping cookie. Keep this the same over renewals. +//! * gateway: Destination address for PCP requests (usually the default gateway). +//! * bind: Specific local bind address for IPv6 pinholing. Set this as INADDR_ANY for IPv4. +//! * port: Internal port, and desired external port. +//! * lifetime: Requested lifetime in seconds for mapping. The server may assign as shorter or longer lifetime. A lifetime of 0 deletes the mapping. +//! * num_tries: Number of tries in case of no response. +//! +//! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError. +std::variant PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000)); + +#endif // BITCOIN_UTIL_PCP_H From 52f8ef66c61b82457a161f3b90cc87f57d1dda80 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 16:20:45 +0200 Subject: [PATCH 16/36] net: Replace libnatpmp with built-in NATPMP+PCP implementation in mapport --- src/init.cpp | 8 +- src/interfaces/node.h | 2 +- src/mapport.cpp | 208 +++++++++++++++++----------------------- src/mapport.h | 4 +- src/net.h | 2 +- src/node/interfaces.cpp | 2 +- 6 files changed, 97 insertions(+), 129 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 8fbdb880bc..a82f79ade4 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -571,11 +571,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) #else hidden_args.emplace_back("-upnp"); #endif -#ifdef USE_NATPMP - argsman.AddArg("-natpmp", strprintf("Use NAT-PMP to map the listening port (default: %u)", DEFAULT_NATPMP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); -#else - hidden_args.emplace_back("-natpmp"); -#endif // USE_NATPMP + argsman.AddArg("-natpmp", strprintf("Use PCP or NAT-PMP to map the listening port (default: %u)", DEFAULT_NATPMP), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-whitebind=<[permissions@]addr>", "Bind to the given address and add permission flags to the peers connecting to it. " "Use [host]:port notation for IPv6. Allowed permissions: " + Join(NET_PERMISSIONS_DOC, ", ") + ". " "Specify multiple permissions separated by commas (default: download,noban,mempool,relay). Can be specified multiple times.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -1858,7 +1854,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("nBestHeight = %d\n", chain_active_height); if (node.peerman) node.peerman->SetBestBlock(chain_active_height, std::chrono::seconds{best_block_time}); - // Map ports with UPnP or NAT-PMP. + // Map ports with UPnP or NAT-PMP StartMapPort(args.GetBoolArg("-upnp", DEFAULT_UPNP), args.GetBoolArg("-natpmp", DEFAULT_NATPMP)); CConnman::Options connOptions; diff --git a/src/interfaces/node.h b/src/interfaces/node.h index b87c78db52..91a623a65d 100644 --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -121,7 +121,7 @@ class Node virtual void resetSettings() = 0; //! Map port. - virtual void mapPort(bool use_upnp, bool use_natpmp) = 0; + virtual void mapPort(bool use_upnp, bool use_pcp) = 0; //! Get proxy. virtual bool getProxy(Network net, Proxy& proxy_info) = 0; diff --git a/src/mapport.cpp b/src/mapport.cpp index 1920297be6..fd15708edc 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -12,14 +12,12 @@ #include #include #include +#include +#include +#include #include #include -#ifdef USE_NATPMP -#include -#include -#endif // USE_NATPMP - #ifdef USE_UPNP #include #include @@ -36,7 +34,6 @@ static_assert(MINIUPNPC_API_VERSION >= 17, "miniUPnPc API version >= 17 assumed" #include #include -#if defined(USE_NATPMP) || defined(USE_UPNP) static CThreadInterrupt g_mapport_interrupt; static std::thread g_mapport_thread; static std::atomic_uint g_mapport_enabled_protos{MapPortProtoFlag::NONE}; @@ -46,104 +43,96 @@ using namespace std::chrono_literals; static constexpr auto PORT_MAPPING_REANNOUNCE_PERIOD{20min}; static constexpr auto PORT_MAPPING_RETRY_PERIOD{5min}; -#ifdef USE_NATPMP -static uint16_t g_mapport_external_port = 0; -static bool NatpmpInit(natpmp_t* natpmp) +static bool ProcessPCP() { - const int r_init = initnatpmp(natpmp, /* detect gateway automatically */ 0, /* forced gateway - NOT APPLIED*/ 0); - if (r_init == 0) return true; - LogPrintf("natpmp: initnatpmp() failed with %d error.\n", r_init); - return false; -} + // The same nonce is used for all mappings, this is allowed by the spec, and simplifies keeping track of them. + PCPMappingNonce pcp_nonce; + GetRandBytes(pcp_nonce); -static bool NatpmpDiscover(natpmp_t* natpmp, struct in_addr& external_ipv4_addr) -{ - const int r_send = sendpublicaddressrequest(natpmp); - if (r_send == 2 /* OK */) { - int r_read; - natpmpresp_t response; - do { - r_read = readnatpmpresponseorretry(natpmp, &response); - } while (r_read == NATPMP_TRYAGAIN); - - if (r_read == 0) { - external_ipv4_addr = response.pnu.publicaddress.addr; - return true; - } else if (r_read == NATPMP_ERR_NOGATEWAYSUPPORT) { - LogPrintf("natpmp: The gateway does not support NAT-PMP.\n"); + bool ret = false; + bool no_resources = false; + const uint16_t private_port = GetListenPort(); + // Multiply the reannounce period by two, as we'll try to renew approximately halfway. + const uint32_t requested_lifetime = std::chrono::seconds(PORT_MAPPING_REANNOUNCE_PERIOD * 2).count(); + uint32_t actual_lifetime = 0; + std::chrono::milliseconds sleep_time; + + // Local functor to handle result from PCP/NATPMP mapping. + auto handle_mapping = [&](std::variant &res) -> void { + if (MappingResult* mapping = std::get_if(&res)) { + LogPrintLevel(BCLog::NET, BCLog::Level::Info, "portmap: Added mapping %s\n", mapping->ToString()); + AddLocal(mapping->external, LOCAL_MAPPED); + ret = true; + actual_lifetime = std::min(actual_lifetime, mapping->lifetime); + } else if (MappingError *err = std::get_if(&res)) { + // Detailed error will already have been logged internally in respective Portmap function. + if (*err == MappingError::NO_RESOURCES) { + no_resources = true; + } + } + }; + + do { + actual_lifetime = requested_lifetime; + no_resources = false; // Set to true if there was any "no resources" error. + ret = false; // Set to true if any mapping succeeds. + + // IPv4 + std::optional gateway4 = QueryDefaultGateway(NET_IPV4); + if (!gateway4) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Could not determine IPv4 default gateway\n"); } else { - LogPrintf("natpmp: readnatpmpresponseorretry() for public address failed with %d error.\n", r_read); + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: gateway [IPv4]: %s\n", gateway4->ToStringAddr()); + + // Open a port mapping on whatever local address we have toward the gateway. + struct in_addr inaddr_any; + inaddr_any.s_addr = htonl(INADDR_ANY); + auto res = PCPRequestPortMap(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime); + MappingError* pcp_err = std::get_if(&res); + if (pcp_err && *pcp_err == MappingError::UNSUPP_VERSION) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Got unsupported PCP version response, falling back to NAT-PMP\n"); + res = NATPMPRequestPortMap(*gateway4, private_port, requested_lifetime); + } + handle_mapping(res); } - } else { - LogPrintf("natpmp: sendpublicaddressrequest() failed with %d error.\n", r_send); - } - return false; -} + // IPv6 + std::optional gateway6 = QueryDefaultGateway(NET_IPV6); + if (!gateway6) { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Could not determine IPv6 default gateway\n"); + } else { + LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: gateway [IPv6]: %s\n", gateway6->ToStringAddr()); -static bool NatpmpMapping(natpmp_t* natpmp, const struct in_addr& external_ipv4_addr, uint16_t private_port, bool& external_ip_discovered) -{ - const uint16_t suggested_external_port = g_mapport_external_port ? g_mapport_external_port : private_port; - const int r_send = sendnewportmappingrequest(natpmp, NATPMP_PROTOCOL_TCP, private_port, suggested_external_port, 3600 /*seconds*/); - if (r_send == 12 /* OK */) { - int r_read; - natpmpresp_t response; - do { - r_read = readnatpmpresponseorretry(natpmp, &response); - } while (r_read == NATPMP_TRYAGAIN); - - if (r_read == 0) { - auto pm = response.pnu.newportmapping; - if (private_port == pm.privateport && pm.lifetime > 0) { - g_mapport_external_port = pm.mappedpublicport; - const CService external{external_ipv4_addr, pm.mappedpublicport}; - if (!external_ip_discovered && fDiscover) { - AddLocal(external, LOCAL_MAPPED); - external_ip_discovered = true; - } - LogPrintf("natpmp: Port mapping successful. External address = %s\n", external.ToStringAddrPort()); - return true; - } else { - LogPrintf("natpmp: Port mapping failed.\n"); + // Try to open pinholes for all routable local IPv6 addresses. + for (const auto &addr: GetLocalAddresses()) { + if (!addr.IsRoutable() || !addr.IsIPv6()) continue; + auto res = PCPRequestPortMap(pcp_nonce, *gateway6, addr, private_port, requested_lifetime); + handle_mapping(res); } - } else if (r_read == NATPMP_ERR_NOGATEWAYSUPPORT) { - LogPrintf("natpmp: The gateway does not support NAT-PMP.\n"); - } else { - LogPrintf("natpmp: readnatpmpresponseorretry() for port mapping failed with %d error.\n", r_read); } - } else { - LogPrintf("natpmp: sendnewportmappingrequest() failed with %d error.\n", r_send); - } - - return false; -} -static bool ProcessNatpmp() -{ - bool ret = false; - natpmp_t natpmp; - struct in_addr external_ipv4_addr; - if (NatpmpInit(&natpmp) && NatpmpDiscover(&natpmp, external_ipv4_addr)) { - bool external_ip_discovered = false; - const uint16_t private_port = GetListenPort(); - do { - ret = NatpmpMapping(&natpmp, external_ipv4_addr, private_port, external_ip_discovered); - } while (ret && g_mapport_interrupt.sleep_for(PORT_MAPPING_REANNOUNCE_PERIOD)); - g_mapport_interrupt.reset(); + // Log message if we got NO_RESOURCES. + if (no_resources) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "portmap: At least one mapping failed because of a NO_RESOURCES error. This usually indicates that the port is already used on the router. If this is the only instance of bitcoin running on the network, this will resolve itself automatically. Otherwise, you might want to choose a different P2P port to prevent this conflict.\n"); + } - const int r_send = sendnewportmappingrequest(&natpmp, NATPMP_PROTOCOL_TCP, private_port, g_mapport_external_port, /* remove a port mapping */ 0); - g_mapport_external_port = 0; - if (r_send == 12 /* OK */) { - LogPrintf("natpmp: Port mapping removed successfully.\n"); - } else { - LogPrintf("natpmp: sendnewportmappingrequest(0) failed with %d error.\n", r_send); + // Sanity-check returned lifetime. + if (actual_lifetime < 30) { + LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "portmap: Got impossibly short mapping lifetime of %d seconds\n", actual_lifetime); + return false; } - } + // RFC6887 11.2.1 recommends that clients send their first renewal packet at a time chosen with uniform random + // distribution in the range 1/2 to 5/8 of expiration time. + std::chrono::seconds sleep_time_min(actual_lifetime / 2); + std::chrono::seconds sleep_time_max(actual_lifetime * 5 / 8); + sleep_time = sleep_time_min + FastRandomContext().randrange(sleep_time_max - sleep_time_min); + } while (ret && g_mapport_interrupt.sleep_for(sleep_time)); + + // We don't delete the mappings when the thread is interrupted because this would add additional complexity, so + // we rather just choose a fairly short expiry time. - closenatpmp(&natpmp); return ret; } -#endif // USE_NATPMP #ifdef USE_UPNP static bool ProcessUpnp() @@ -223,23 +212,21 @@ static void ThreadMapPort() do { ok = false; -#ifdef USE_UPNP // High priority protocol. - if (g_mapport_enabled_protos & MapPortProtoFlag::UPNP) { - g_mapport_current_proto = MapPortProtoFlag::UPNP; - ok = ProcessUpnp(); + if (g_mapport_enabled_protos & MapPortProtoFlag::PCP) { + g_mapport_current_proto = MapPortProtoFlag::PCP; + ok = ProcessPCP(); if (ok) continue; } -#endif // USE_UPNP -#ifdef USE_NATPMP +#ifdef USE_UPNP // Low priority protocol. - if (g_mapport_enabled_protos & MapPortProtoFlag::NAT_PMP) { - g_mapport_current_proto = MapPortProtoFlag::NAT_PMP; - ok = ProcessNatpmp(); + if (g_mapport_enabled_protos & MapPortProtoFlag::UPNP) { + g_mapport_current_proto = MapPortProtoFlag::UPNP; + ok = ProcessUpnp(); if (ok) continue; } -#endif // USE_NATPMP +#endif // USE_UPNP g_mapport_current_proto = MapPortProtoFlag::NONE; if (g_mapport_enabled_protos == MapPortProtoFlag::NONE) { @@ -281,7 +268,7 @@ static void DispatchMapPort() assert(g_mapport_thread.joinable()); assert(!g_mapport_interrupt); - // Interrupt a protocol-specific loop in the ThreadUpnp() or in the ThreadNatpmp() + // Interrupt a protocol-specific loop in the ThreadUpnp() or in the ThreadPCP() // to force trying the next protocol in the ThreadMapPort() loop. g_mapport_interrupt(); } @@ -295,10 +282,10 @@ static void MapPortProtoSetEnabled(MapPortProtoFlag proto, bool enabled) } } -void StartMapPort(bool use_upnp, bool use_natpmp) +void StartMapPort(bool use_upnp, bool use_pcp) { MapPortProtoSetEnabled(MapPortProtoFlag::UPNP, use_upnp); - MapPortProtoSetEnabled(MapPortProtoFlag::NAT_PMP, use_natpmp); + MapPortProtoSetEnabled(MapPortProtoFlag::PCP, use_pcp); DispatchMapPort(); } @@ -317,18 +304,3 @@ void StopMapPort() g_mapport_interrupt.reset(); } } - -#else // #if defined(USE_NATPMP) || defined(USE_UPNP) -void StartMapPort(bool use_upnp, bool use_natpmp) -{ - // Intentionally left blank. -} -void InterruptMapPort() -{ - // Intentionally left blank. -} -void StopMapPort() -{ - // Intentionally left blank. -} -#endif // #if defined(USE_NATPMP) || defined(USE_UPNP) diff --git a/src/mapport.h b/src/mapport.h index 6f55c46f6c..51202687f2 100644 --- a/src/mapport.h +++ b/src/mapport.h @@ -12,10 +12,10 @@ static constexpr bool DEFAULT_NATPMP = false; enum MapPortProtoFlag : unsigned int { NONE = 0x00, UPNP = 0x01, - NAT_PMP = 0x02, + PCP = 0x02, // PCP with NAT-PMP fallback. }; -void StartMapPort(bool use_upnp, bool use_natpmp); +void StartMapPort(bool use_upnp, bool use_pcp); void InterruptMapPort(); void StopMapPort(); diff --git a/src/net.h b/src/net.h index 2f65a01ce1..86618bed91 100644 --- a/src/net.h +++ b/src/net.h @@ -148,7 +148,7 @@ enum LOCAL_NONE, // unknown LOCAL_IF, // address a local interface listens on LOCAL_BIND, // address explicit bound to - LOCAL_MAPPED, // address reported by UPnP or NAT-PMP + LOCAL_MAPPED, // address reported by UPnP or PCP LOCAL_MANUAL, // address explicitly specified (-externalip=) LOCAL_MAX diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 0e418c58bb..babd6fdd83 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -181,7 +181,7 @@ class NodeImpl : public Node }); args().WriteSettingsFile(); } - void mapPort(bool use_upnp, bool use_natpmp) override { StartMapPort(use_upnp, use_natpmp); } + void mapPort(bool use_upnp, bool use_pcp) override { StartMapPort(use_upnp, use_pcp); } bool getProxy(Network net, Proxy& proxy_info) override { return GetProxy(net, proxy_info); } size_t getNodeCount(ConnectionDirection flags) override { From 7b04709862f48e9020c7bef79cb31dd794cf91d0 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 19:17:53 +0200 Subject: [PATCH 17/36] qt: Changes for built-in PCP+NAT-PMP Change option help, and remove conditionals. --- src/qt/forms/optionsdialog.ui | 4 ++-- src/qt/optionsdialog.cpp | 3 --- src/qt/optionsmodel.cpp | 4 ---- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/qt/forms/optionsdialog.ui b/src/qt/forms/optionsdialog.ui index 1ce23c72bb..2056b2cccd 100644 --- a/src/qt/forms/optionsdialog.ui +++ b/src/qt/forms/optionsdialog.ui @@ -328,10 +328,10 @@ - Automatically open the Bitcoin client port on the router. This only works when your router supports NAT-PMP and it is enabled. The external port could be random. + Automatically open the Bitcoin client port on the router. This only works when your router supports PCP or NAT-PMP and it is enabled. The external port could be random. - Map port using NA&T-PMP + Map port using PCP or NA&T-PMP diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp index 4d590c3eed..46669326e3 100644 --- a/src/qt/optionsdialog.cpp +++ b/src/qt/optionsdialog.cpp @@ -108,9 +108,6 @@ OptionsDialog::OptionsDialog(QWidget* parent, bool enableWallet) #ifndef USE_UPNP ui->mapPortUpnp->setEnabled(false); #endif -#ifndef USE_NATPMP - ui->mapPortNatpmp->setEnabled(false); -#endif ui->proxyIp->setEnabled(false); ui->proxyPort->setEnabled(false); diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index 0c21c6748d..6a997ce556 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -414,11 +414,7 @@ QVariant OptionsModel::getOption(OptionID option, const std::string& suffix) con return false; #endif // USE_UPNP case MapPortNatpmp: -#ifdef USE_NATPMP return SettingToBool(setting(), DEFAULT_NATPMP); -#else - return false; -#endif // USE_NATPMP case MinimizeOnClose: return fMinimizeOnClose; From 20a18bf6aa38e87f72e2645482d00d0c77a344f5 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 16:40:11 +0200 Subject: [PATCH 18/36] build: Drop libnatpmp from build system --- CMakeLists.txt | 10 +--------- CMakePresets.json | 1 - cmake/module/FindNATPMP.cmake | 32 -------------------------------- src/CMakeLists.txt | 1 - src/qt/CMakeLists.txt | 1 - 5 files changed, 1 insertion(+), 44 deletions(-) delete mode 100644 cmake/module/FindNATPMP.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ae988ac90..56ac7f4df5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -121,11 +121,6 @@ option(REDUCE_EXPORTS "Attempt to reduce exported symbols in the resulting execu option(WERROR "Treat compiler warnings as errors." OFF) option(WITH_CCACHE "Attempt to use ccache for compiling." ON) -option(WITH_NATPMP "Enable NAT-PMP." OFF) -if(WITH_NATPMP) - find_package(NATPMP MODULE REQUIRED) -endif() - option(WITH_MINIUPNPC "Enable UPnP." OFF) if(WITH_MINIUPNPC) find_package(MiniUPnPc MODULE REQUIRED) @@ -239,7 +234,6 @@ if(BUILD_FOR_FUZZING) set(BUILD_WALLET_TOOL OFF) set(BUILD_GUI OFF) set(ENABLE_EXTERNAL_SIGNER OFF) - set(WITH_NATPMP OFF) set(WITH_MINIUPNPC OFF) set(WITH_ZMQ OFF) set(BUILD_TESTS OFF) @@ -621,9 +615,7 @@ if(ENABLE_WALLET) message(" - legacy wallets (Berkeley DB) ..... ${WITH_BDB}") endif() message(" external signer ..................... ${ENABLE_EXTERNAL_SIGNER}") -message(" port mapping:") -message(" - using NAT-PMP .................... ${WITH_NATPMP}") -message(" - using UPnP ....................... ${WITH_MINIUPNPC}") +message(" port mapping using UPnP ............. ${WITH_MINIUPNPC}") message(" ZeroMQ .............................. ${WITH_ZMQ}") message(" USDT tracing ........................ ${WITH_USDT}") message(" QR code (GUI) ....................... ${WITH_QRENCODE}") diff --git a/CMakePresets.json b/CMakePresets.json index 041e3c1cbf..3bbb61afce 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -86,7 +86,6 @@ "WITH_BDB": "ON", "WITH_MINIUPNPC": "ON", "WITH_MULTIPROCESS": "ON", - "WITH_NATPMP": "ON", "WITH_QRENCODE": "ON", "WITH_SQLITE": "ON", "WITH_USDT": "ON", diff --git a/cmake/module/FindNATPMP.cmake b/cmake/module/FindNATPMP.cmake deleted file mode 100644 index 930555232b..0000000000 --- a/cmake/module/FindNATPMP.cmake +++ /dev/null @@ -1,32 +0,0 @@ -# Copyright (c) 2023-present The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or https://opensource.org/license/mit/. - -find_path(NATPMP_INCLUDE_DIR - NAMES natpmp.h -) - -find_library(NATPMP_LIBRARY - NAMES natpmp -) - -include(FindPackageHandleStandardArgs) -find_package_handle_standard_args(NATPMP - REQUIRED_VARS NATPMP_LIBRARY NATPMP_INCLUDE_DIR -) - -if(NATPMP_FOUND AND NOT TARGET NATPMP::NATPMP) - add_library(NATPMP::NATPMP UNKNOWN IMPORTED) - set_target_properties(NATPMP::NATPMP PROPERTIES - IMPORTED_LOCATION "${NATPMP_LIBRARY}" - INTERFACE_INCLUDE_DIRECTORIES "${NATPMP_INCLUDE_DIR}" - ) - set_property(TARGET NATPMP::NATPMP PROPERTY - INTERFACE_COMPILE_DEFINITIONS USE_NATPMP=1 $<$:NATPMP_STATICLIB> - ) -endif() - -mark_as_advanced( - NATPMP_INCLUDE_DIR - NATPMP_LIBRARY -) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e652d43d58..bffba65247 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -292,7 +292,6 @@ target_link_libraries(bitcoin_node Boost::headers $ $ - $ $ $ $ diff --git a/src/qt/CMakeLists.txt b/src/qt/CMakeLists.txt index 297408a926..7ec2b74cc8 100644 --- a/src/qt/CMakeLists.txt +++ b/src/qt/CMakeLists.txt @@ -133,7 +133,6 @@ target_link_libraries(bitcoinqt bitcoin_cli leveldb Boost::headers - $ $ $ $<$:-framework\ AppKit> From 061c3e32a26c6c04bf734d62627403758d7e51d9 Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 16:46:49 +0200 Subject: [PATCH 19/36] depends: Drop natpmp and associated option from depends --- depends/Makefile | 5 +---- depends/README.md | 1 - depends/packages/libnatpmp.mk | 20 -------------------- depends/packages/packages.mk | 1 - depends/toolchain.cmake.in | 7 ------- 5 files changed, 1 insertion(+), 33 deletions(-) delete mode 100644 depends/packages/libnatpmp.mk diff --git a/depends/Makefile b/depends/Makefile index ad1fb2b049..f492559307 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -42,7 +42,6 @@ NO_WALLET ?= NO_ZMQ ?= NO_UPNP ?= NO_USDT ?= -NO_NATPMP ?= MULTIPROCESS ?= LTO ?= NO_HARDEN ?= @@ -159,13 +158,12 @@ sqlite_packages_$(NO_SQLITE) = $(sqlite_packages) wallet_packages_$(NO_WALLET) = $(bdb_packages_) $(sqlite_packages_) upnp_packages_$(NO_UPNP) = $(upnp_packages) -natpmp_packages_$(NO_NATPMP) = $(natpmp_packages) zmq_packages_$(NO_ZMQ) = $(zmq_packages) multiprocess_packages_$(MULTIPROCESS) = $(multiprocess_packages) usdt_packages_$(NO_USDT) = $(usdt_$(host_os)_packages) -packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(boost_packages_) $(libevent_packages_) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(natpmp_packages_) $(usdt_packages_) +packages += $($(host_arch)_$(host_os)_packages) $($(host_os)_packages) $(boost_packages_) $(libevent_packages_) $(qt_packages_) $(wallet_packages_) $(upnp_packages_) $(usdt_packages_) native_packages += $($(host_arch)_$(host_os)_native_packages) $($(host_os)_native_packages) ifneq ($(zmq_packages_),) @@ -233,7 +231,6 @@ $(host_prefix)/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_$(fina -e 's|@bdb_packages@|$(bdb_packages_)|' \ -e 's|@sqlite_packages@|$(sqlite_packages_)|' \ -e 's|@upnp_packages@|$(upnp_packages_)|' \ - -e 's|@natpmp_packages@|$(natpmp_packages_)|' \ -e 's|@usdt_packages@|$(usdt_packages_)|' \ -e 's|@no_harden@|$(NO_HARDEN)|' \ -e 's|@multiprocess@|$(MULTIPROCESS)|' \ diff --git a/depends/README.md b/depends/README.md index b95c4b051d..f64d6207d6 100644 --- a/depends/README.md +++ b/depends/README.md @@ -113,7 +113,6 @@ The following can be set when running make: `make FOO=bar` - `NO_BDB`: Don't download/build/cache BerkeleyDB - `NO_SQLITE`: Don't download/build/cache SQLite - `NO_UPNP`: Don't download/build/cache packages needed for enabling UPnP -- `NO_NATPMP`: Don't download/build/cache packages needed for enabling NAT-PMP - `NO_USDT`: Don't download/build/cache packages needed for enabling USDT tracepoints - `MULTIPROCESS`: Build libmultiprocess (experimental, requires CMake) - `DEBUG`: Disable some optimizations and enable more runtime checking diff --git a/depends/packages/libnatpmp.mk b/depends/packages/libnatpmp.mk deleted file mode 100644 index 5a573a18e7..0000000000 --- a/depends/packages/libnatpmp.mk +++ /dev/null @@ -1,20 +0,0 @@ -package=libnatpmp -$(package)_version=f2433bec24ca3d3f22a8a7840728a3ac177f94ba -$(package)_download_path=https://github.com/miniupnp/libnatpmp/archive -$(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=ef84979950dfb3556705b63c9cd6c95501b75e887fba466234b187f3c9029669 -$(package)_build_subdir=build - -define $(package)_config_cmds - $($(package)_cmake) -S .. -B . -endef - -define $(package)_build_cmds - $(MAKE) natpmp -endef - -define $(package)_stage_cmds - mkdir -p $($(package)_staging_prefix_dir)/include $($(package)_staging_prefix_dir)/lib && \ - install ../natpmp.h ../natpmp_declspec.h $($(package)_staging_prefix_dir)/include && \ - install libnatpmp.a $($(package)_staging_prefix_dir)/lib -endef diff --git a/depends/packages/packages.mk b/depends/packages/packages.mk index 01ed0d7a92..08a91cbcbd 100644 --- a/depends/packages/packages.mk +++ b/depends/packages/packages.mk @@ -18,7 +18,6 @@ sqlite_packages=sqlite zmq_packages=zeromq upnp_packages=miniupnpc -natpmp_packages=libnatpmp multiprocess_packages = libmultiprocess capnp multiprocess_native_packages = native_libmultiprocess native_capnp diff --git a/depends/toolchain.cmake.in b/depends/toolchain.cmake.in index c733c81edf..c1c1904718 100644 --- a/depends/toolchain.cmake.in +++ b/depends/toolchain.cmake.in @@ -146,13 +146,6 @@ else() set(WITH_MINIUPNPC ON CACHE BOOL "") endif() -set(natpmp_packages @natpmp_packages@) -if("${natpmp_packages}" STREQUAL "") - set(WITH_NATPMP OFF CACHE BOOL "") -else() - set(WITH_NATPMP ON CACHE BOOL "") -endif() - set(usdt_packages @usdt_packages@) if("${usdt_packages}" STREQUAL "") set(WITH_USDT OFF CACHE BOOL "") From 7e7ec984da50f45491b994aaab180e7735ad1d8f Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 16:49:16 +0200 Subject: [PATCH 20/36] doc: Remove mention of natpmp build options --- doc/build-freebsd.md | 2 +- doc/build-openbsd.md | 2 +- doc/build-osx.md | 11 ----------- doc/build-unix.md | 10 +++++----- doc/dependencies.md | 1 - 5 files changed, 7 insertions(+), 19 deletions(-) diff --git a/doc/build-freebsd.md b/doc/build-freebsd.md index 6a25e9a834..8b3b10ab85 100644 --- a/doc/build-freebsd.md +++ b/doc/build-freebsd.md @@ -42,7 +42,7 @@ from ports. However, you can build DB 4.8 yourself [using depends](/depends). ```bash pkg install gmake -gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1 +gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1 ``` When the build is complete, the Berkeley DB installation location will be displayed: diff --git a/doc/build-openbsd.md b/doc/build-openbsd.md index fafc91fc5f..908b750a8f 100644 --- a/doc/build-openbsd.md +++ b/doc/build-openbsd.md @@ -44,7 +44,7 @@ from ports. However you can build it yourself, [using depends](/depends). Refer to [depends/README.md](/depends/README.md) for detailed instructions. ```bash -gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1 +gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1 ... to: /path/to/bitcoin/depends/*-unknown-openbsd* ``` diff --git a/doc/build-osx.md b/doc/build-osx.md index 600eebb6ff..c7cc84d42d 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -135,17 +135,6 @@ Skip if you do not need this functionality. brew install miniupnpc ``` -###### libnatpmp - -libnatpmp may be used for NAT-PMP port mapping. -Skip if you do not need this functionality. - -``` bash -brew install libnatpmp -``` - -Check out the [further configuration](#further-configuration) section for more information. - --- #### ZMQ Dependencies diff --git a/doc/build-unix.md b/doc/build-unix.md index 4c3c659bff..086731be5a 100644 --- a/doc/build-unix.md +++ b/doc/build-unix.md @@ -60,9 +60,9 @@ executables, which are based on BerkeleyDB 4.8. Otherwise, you can build Berkele To build Bitcoin Core without wallet, see [*Disable-wallet mode*](#disable-wallet-mode) -Optional port mapping libraries (see: `-DWITH_MINIUPNPC=ON` and `-DWITH_NATPMP=ON`): +Optional port mapping library (see: `-DWITH_MINIUPNPC=ON`): - sudo apt install libminiupnpc-dev libnatpmp-dev + sudo apt install libminiupnpc-dev ZMQ dependencies (provides ZMQ API): @@ -112,9 +112,9 @@ are based on Berkeley DB 4.8. Otherwise, you can build Berkeley DB [yourself](#b To build Bitcoin Core without wallet, see [*Disable-wallet mode*](#disable-wallet-mode) -Optional port mapping libraries (see: `-DWITH_MINIUPNPC=ON` and `-DWITH_NATPMP=ON`): +Optional port mapping library (see: `-DWITH_MINIUPNPC=ON`): - sudo dnf install miniupnpc-devel libnatpmp-devel + sudo dnf install miniupnpc-devel ZMQ dependencies (provides ZMQ API): @@ -153,7 +153,7 @@ The legacy wallet uses Berkeley DB. To ensure backwards compatibility it is recommended to use Berkeley DB 4.8. If you have to build it yourself, and don't want to use any other libraries built in depends, you can do: ```bash -make -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1 +make -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1 ... to: /path/to/bitcoin/depends/x86_64-pc-linux-gnu ``` diff --git a/doc/dependencies.md b/doc/dependencies.md index 4c620cf876..5789ea52f9 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -34,7 +34,6 @@ You can find installation instructions in the `build-*.md` file for your platfor ### Networking | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | -| [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [f2433be...](https://github.com/bitcoin/bitcoin/pull/29708) | | No | | [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.7](https://github.com/bitcoin/bitcoin/pull/29707) | 2.1 | No | ### Notifications From 5c7cacf649a6b474b876a7d219c7dc683a25e33d Mon Sep 17 00:00:00 2001 From: laanwj <126646+laanwj@users.noreply.github.com> Date: Sun, 5 May 2024 16:54:12 +0200 Subject: [PATCH 21/36] ci: Remove natpmp build option and libnatpmp dependency --- .github/workflows/ci.yml | 6 +++--- ci/test/00_setup_env_mac_native.sh | 2 +- ci/test/00_setup_env_native_asan.sh | 2 +- ci/test/00_setup_env_native_previous_releases.sh | 2 +- ci/test/00_setup_env_native_tidy.sh | 4 ++-- ci/test/00_setup_env_native_valgrind.sh | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99be137a24..645268ecf7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,12 +67,12 @@ jobs: echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD $EXCLUDE_MERGE_BASE_ANCESTORS | head -1)" >> "$GITHUB_ENV" - run: | sudo apt-get update - sudo apt-get install clang ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y + sudo apt-get install clang ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libzmq3-dev qtbase5-dev qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y - name: Compile and run tests run: | # Run tests on commits after the last merge commit and before the PR head commit # Use clang++, because it is a bit faster and uses less memory than g++ - git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} + git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} macos-native-arm64: name: 'macOS 14 native, arm64, no depends, sqlite only, gui' @@ -105,7 +105,7 @@ jobs: run: | # A workaround for "The `brew link` step did not complete successfully" error. brew install --quiet python@3 || brew link --overwrite python@3 - brew install --quiet ninja pkg-config gnu-getopt ccache boost libevent miniupnpc libnatpmp zeromq qt@5 qrencode + brew install --quiet ninja pkg-config gnu-getopt ccache boost libevent miniupnpc zeromq qt@5 qrencode - name: Set Ccache directory run: echo "CCACHE_DIR=${RUNNER_TEMP}/ccache_dir" >> "$GITHUB_ENV" diff --git a/ci/test/00_setup_env_mac_native.sh b/ci/test/00_setup_env_mac_native.sh index 1e197d8c71..45d644d9ca 100755 --- a/ci/test/00_setup_env_mac_native.sh +++ b/ci/test/00_setup_env_mac_native.sh @@ -11,7 +11,7 @@ export LC_ALL=C.UTF-8 export PIP_PACKAGES="--break-system-packages zmq" export GOAL="install" export CMAKE_GENERATOR="Ninja" -export BITCOIN_CONFIG="-DBUILD_GUI=ON -DWITH_ZMQ=ON -DWITH_MINIUPNPC=ON -DWITH_NATPMP=ON -DREDUCE_EXPORTS=ON" +export BITCOIN_CONFIG="-DBUILD_GUI=ON -DWITH_ZMQ=ON -DWITH_MINIUPNPC=ON -DREDUCE_EXPORTS=ON" export CI_OS_NAME="macos" export NO_DEPENDS=1 export OSX_SDK="" diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index e27563f323..dc84ef49a4 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -19,7 +19,7 @@ else fi export CONTAINER_NAME=ci_native_asan -export PACKAGES="systemtap-sdt-dev clang-18 llvm-18 libclang-rt-18-dev python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}" +export PACKAGES="systemtap-sdt-dev clang-18 llvm-18 libclang-rt-18-dev python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}" export NO_DEPENDS=1 export GOAL="install" export BITCOIN_CONFIG="\ diff --git a/ci/test/00_setup_env_native_previous_releases.sh b/ci/test/00_setup_env_native_previous_releases.sh index 2482e545e1..f978ac5b97 100755 --- a/ci/test/00_setup_env_native_previous_releases.sh +++ b/ci/test/00_setup_env_native_previous_releases.sh @@ -10,7 +10,7 @@ export CONTAINER_NAME=ci_native_previous_releases export CI_IMAGE_NAME_TAG="docker.io/ubuntu:22.04" # Use minimum supported python3.9 (or best effort 3.10) and gcc-11, see doc/dependencies.md export PACKAGES="gcc-11 g++-11 python3-zmq" -export DEP_OPTS="NO_UPNP=1 NO_NATPMP=1 DEBUG=1 CC=gcc-11 CXX=g++-11" +export DEP_OPTS="NO_UPNP=1 DEBUG=1 CC=gcc-11 CXX=g++-11" export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash export RUN_UNIT_TESTS_SEQUENTIAL="true" export RUN_UNIT_TESTS="false" diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 4a6f5bb3e2..cc1dea09cb 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -9,7 +9,7 @@ export LC_ALL=C.UTF-8 export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export CONTAINER_NAME=ci_native_tidy export TIDY_LLVM_V="18" -export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev qtbase5-dev qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev" +export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq libevent-dev libboost-dev libminiupnpc-dev libzmq3-dev systemtap-sdt-dev qtbase5-dev qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev" export NO_DEPENDS=1 export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false @@ -18,7 +18,7 @@ export RUN_CHECK_DEPS=true export RUN_TIDY=true export GOAL="install" export BITCOIN_CONFIG="\ - -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON -DWITH_BDB=ON -DWARN_INCOMPATIBLE_BDB=OFF \ + -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON -DWITH_BDB=ON -DWARN_INCOMPATIBLE_BDB=OFF \ -DENABLE_HARDENING=OFF \ -DCMAKE_C_COMPILER=clang-${TIDY_LLVM_V} \ -DCMAKE_CXX_COMPILER=clang++-${TIDY_LLVM_V} \ diff --git a/ci/test/00_setup_env_native_valgrind.sh b/ci/test/00_setup_env_native_valgrind.sh index 60bbe83119..3c5622cd02 100755 --- a/ci/test/00_setup_env_native_valgrind.sh +++ b/ci/test/00_setup_env_native_valgrind.sh @@ -8,14 +8,14 @@ export LC_ALL=C.UTF-8 export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export CONTAINER_NAME=ci_native_valgrind -export PACKAGES="valgrind clang-16 llvm-16 libclang-rt-16-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libsqlite3-dev" +export PACKAGES="valgrind clang-16 llvm-16 libclang-rt-16-dev python3-zmq libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libsqlite3-dev" export USE_VALGRIND=1 export NO_DEPENDS=1 export TEST_RUNNER_EXTRA="--exclude feature_init,rpc_bind,feature_bind_extra" # feature_init excluded for now, see https://github.com/bitcoin/bitcoin/issues/30011 ; bind tests excluded for now, see https://github.com/bitcoin/bitcoin/issues/17765#issuecomment-602068547 export GOAL="install" # TODO enable GUI export BITCOIN_CONFIG="\ - -DWITH_ZMQ=ON -DWITH_BDB=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWARN_INCOMPATIBLE_BDB=OFF -DBUILD_GUI=OFF \ + -DWITH_ZMQ=ON -DWITH_BDB=ON -DWITH_MINIUPNPC=ON -DWARN_INCOMPATIBLE_BDB=OFF -DBUILD_GUI=OFF \ -DCMAKE_C_COMPILER=clang-16 \ -DCMAKE_CXX_COMPILER=clang++-16 \ " From bbbb2e43ee95c9a8866aa1f65e3f001f752dfed2 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 19 Sep 2024 17:09:10 +0200 Subject: [PATCH 22/36] log: Enforce trailing newline, Remove redundant m_started_new_line All log lines already have a trailing newline, but enforcing it allows to delete unused code. --- src/logging.cpp | 21 ++++----------------- src/logging.h | 8 -------- src/test/logging_tests.cpp | 22 +++++++++++----------- 3 files changed, 15 insertions(+), 36 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index d04db767e6..5f055566ef 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -368,6 +369,8 @@ static size_t MemUsage(const BCLog::Logger::BufferedLog& buflog) void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags category, BCLog::Level level, std::string_view source_file, int source_line, std::string_view logging_function, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const { + if (!str.ends_with('\n')) str.push_back('\n'); + str.insert(0, GetLogPrefix(category, level)); if (m_log_sourcelocations) { @@ -391,21 +394,7 @@ void BCLog::Logger::LogPrintStr_(std::string_view str, std::string_view logging_ { std::string str_prefixed = LogEscapeMessage(str); - const bool starts_new_line = m_started_new_line; - m_started_new_line = !str.empty() && str[str.size()-1] == '\n'; - if (m_buffering) { - if (!starts_new_line) { - if (!m_msgs_before_open.empty()) { - m_msgs_before_open.back().str += str_prefixed; - m_cur_buffer_memusage += str_prefixed.size(); - return; - } else { - // unlikely edge case; add a marker that something was trimmed - str_prefixed.insert(0, "[...] "); - } - } - { BufferedLog buf{ .now=SystemClock::now(), @@ -435,9 +424,7 @@ void BCLog::Logger::LogPrintStr_(std::string_view str, std::string_view logging_ return; } - if (starts_new_line) { - FormatLogStrInPlace(str_prefixed, category, level, source_file, source_line, logging_function, util::ThreadGetInternalName(), SystemClock::now(), GetMockTime()); - } + FormatLogStrInPlace(str_prefixed, category, level, source_file, source_line, logging_function, util::ThreadGetInternalName(), SystemClock::now(), GetMockTime()); if (m_print_to_console) { // print to console diff --git a/src/logging.h b/src/logging.h index 8605c8cd64..fdc12c79b3 100644 --- a/src/logging.h +++ b/src/logging.h @@ -105,13 +105,6 @@ namespace BCLog { size_t m_cur_buffer_memusage GUARDED_BY(m_cs){0}; size_t m_buffer_lines_discarded GUARDED_BY(m_cs){0}; - /** - * m_started_new_line is a state variable that will suppress printing of - * the timestamp when multiple calls are made that don't end in a - * newline. - */ - std::atomic_bool m_started_new_line{true}; - //! Category-specific log level. Overrides `m_log_level`. std::unordered_map m_category_log_levels GUARDED_BY(m_cs); @@ -253,7 +246,6 @@ inline void LogPrintFormatInternal(std::string_view logging_function, std::strin try { log_msg = tfm::format(fmt, args...); } catch (tinyformat::format_error& fmterr) { - /* Original format string will have newline so don't add one here */ log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt; } LogInstance().LogPrintStr(log_msg, logging_function, source_file, source_line, flag, level); diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 8217a2385c..77ec81e597 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -86,12 +86,12 @@ BOOST_AUTO_TEST_CASE(logging_timer) BOOST_FIXTURE_TEST_CASE(logging_LogPrintStr, LogSetup) { LogInstance().m_log_sourcelocations = true; - LogInstance().LogPrintStr("foo1: bar1\n", "fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug); - LogInstance().LogPrintStr("foo2: bar2\n", "fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::Info); - LogInstance().LogPrintStr("foo3: bar3\n", "fn3", "src3", 3, BCLog::LogFlags::ALL, BCLog::Level::Debug); - LogInstance().LogPrintStr("foo4: bar4\n", "fn4", "src4", 4, BCLog::LogFlags::ALL, BCLog::Level::Info); - LogInstance().LogPrintStr("foo5: bar5\n", "fn5", "src5", 5, BCLog::LogFlags::NONE, BCLog::Level::Debug); - LogInstance().LogPrintStr("foo6: bar6\n", "fn6", "src6", 6, BCLog::LogFlags::NONE, BCLog::Level::Info); + LogInstance().LogPrintStr("foo1: bar1", "fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug); + LogInstance().LogPrintStr("foo2: bar2", "fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::Info); + LogInstance().LogPrintStr("foo3: bar3", "fn3", "src3", 3, BCLog::LogFlags::ALL, BCLog::Level::Debug); + LogInstance().LogPrintStr("foo4: bar4", "fn4", "src4", 4, BCLog::LogFlags::ALL, BCLog::Level::Info); + LogInstance().LogPrintStr("foo5: bar5", "fn5", "src5", 5, BCLog::LogFlags::NONE, BCLog::Level::Debug); + LogInstance().LogPrintStr("foo6: bar6", "fn6", "src6", 6, BCLog::LogFlags::NONE, BCLog::Level::Info); std::ifstream file{tmp_log_path}; std::vector log_lines; for (std::string log; std::getline(file, log);) { @@ -133,11 +133,11 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacrosDeprecated, LogSetup) BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup) { - LogTrace(BCLog::NET, "foo6: %s\n", "bar6"); // not logged - LogDebug(BCLog::NET, "foo7: %s\n", "bar7"); - LogInfo("foo8: %s\n", "bar8"); - LogWarning("foo9: %s\n", "bar9"); - LogError("foo10: %s\n", "bar10"); + LogTrace(BCLog::NET, "foo6: %s", "bar6"); // not logged + LogDebug(BCLog::NET, "foo7: %s", "bar7"); + LogInfo("foo8: %s", "bar8"); + LogWarning("foo9: %s", "bar9"); + LogError("foo10: %s", "bar10"); std::ifstream file{tmp_log_path}; std::vector log_lines; for (std::string log; std::getline(file, log);) { From fa2b7d8d6b3f8d53199921e1e542072441b26fab Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 19 Sep 2024 16:46:09 +0200 Subject: [PATCH 23/36] Remove redundant unterminated-logprintf tidy check --- contrib/devtools/bitcoin-tidy/CMakeLists.txt | 4 +- .../devtools/bitcoin-tidy/bitcoin-tidy.cpp | 2 - .../bitcoin-tidy/example_logprintf.cpp | 108 ------------------ contrib/devtools/bitcoin-tidy/logprintf.cpp | 60 ---------- contrib/devtools/bitcoin-tidy/logprintf.h | 29 ----- test/lint/lint-format-strings.py | 2 +- 6 files changed, 3 insertions(+), 202 deletions(-) delete mode 100644 contrib/devtools/bitcoin-tidy/example_logprintf.cpp delete mode 100644 contrib/devtools/bitcoin-tidy/logprintf.cpp delete mode 100644 contrib/devtools/bitcoin-tidy/logprintf.h diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt index 95345b4782..c6f683f7ab 100644 --- a/contrib/devtools/bitcoin-tidy/CMakeLists.txt +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt @@ -25,7 +25,7 @@ find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}") -add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp nontrivial-threadlocal.cpp) +add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp nontrivial-threadlocal.cpp) target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS}) # Disable RTTI and exceptions as necessary @@ -58,7 +58,7 @@ else() endif() # Create a dummy library that runs clang-tidy tests as a side-effect of building -add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp example_nontrivial-threadlocal.cpp) +add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_nontrivial-threadlocal.cpp) add_dependencies(bitcoin-tidy-tests bitcoin-tidy) set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") diff --git a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp index 1ef4494973..f2658b5a58 100644 --- a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp +++ b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "logprintf.h" #include "nontrivial-threadlocal.h" #include @@ -13,7 +12,6 @@ class BitcoinModule final : public clang::tidy::ClangTidyModule public: void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override { - CheckFactories.registerCheck("bitcoin-unterminated-logprintf"); CheckFactories.registerCheck("bitcoin-nontrivial-threadlocal"); } }; diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp deleted file mode 100644 index dc77f668e3..0000000000 --- a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright (c) 2023 Bitcoin Developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include - -// Test for bitcoin-unterminated-logprintf - -enum LogFlags { - NONE -}; - -enum Level { - None -}; - -template -static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const LogFlags flag, const Level level, const char* fmt, const Args&... args) -{ -} - -#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) -#define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__) - -#define LogDebug(category, ...) \ - do { \ - LogPrintf(__VA_ARGS__); \ - } while (0) - - -class CWallet -{ - std::string GetDisplayName() const - { - return "default wallet"; - } - -public: - template - void WalletLogPrintf(const char* fmt, Params... parameters) const - { - LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); - }; -}; - -struct ScriptPubKeyMan -{ - std::string GetDisplayName() const - { - return "default wallet"; - } - - template - void WalletLogPrintf(const char* fmt, Params... parameters) const - { - LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); - }; -}; - -void good_func() -{ - LogPrintf("hello world!\n"); -} -void good_func2() -{ - CWallet wallet; - wallet.WalletLogPrintf("hi\n"); - ScriptPubKeyMan spkm; - spkm.WalletLogPrintf("hi\n"); - - const CWallet& walletref = wallet; - walletref.WalletLogPrintf("hi\n"); - - auto* walletptr = new CWallet(); - walletptr->WalletLogPrintf("hi\n"); - delete walletptr; -} -void bad_func() -{ - LogPrintf("hello world!"); -} -void bad_func2() -{ - LogPrintf(""); -} -void bad_func3() -{ - // Ending in "..." has no special meaning. - LogPrintf("hello world!..."); -} -void bad_func4_ignored() -{ - LogPrintf("hello world!"); // NOLINT(bitcoin-unterminated-logprintf) -} -void bad_func5() -{ - CWallet wallet; - wallet.WalletLogPrintf("hi"); - ScriptPubKeyMan spkm; - spkm.WalletLogPrintf("hi"); - - const CWallet& walletref = wallet; - walletref.WalletLogPrintf("hi"); - - auto* walletptr = new CWallet(); - walletptr->WalletLogPrintf("hi"); - delete walletptr; -} diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp deleted file mode 100644 index 36beac28c8..0000000000 --- a/contrib/devtools/bitcoin-tidy/logprintf.cpp +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) 2023 Bitcoin Developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include "logprintf.h" - -#include -#include - - -namespace { -AST_MATCHER(clang::StringLiteral, unterminated) -{ - size_t len = Node.getLength(); - if (len > 0 && Node.getCodeUnit(len - 1) == '\n') { - return false; - } - return true; -} -} // namespace - -namespace bitcoin { - -void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder) -{ - using namespace clang::ast_matchers; - - /* - Logprintf(..., ..., ..., ..., ..., "foo", ...) - */ - - finder->addMatcher( - callExpr( - callee(functionDecl(hasName("LogPrintf_"))), - hasArgument(5, stringLiteral(unterminated()).bind("logstring"))), - this); - - /* - auto walletptr = &wallet; - wallet.WalletLogPrintf("foo"); - wallet->WalletLogPrintf("foo"); - */ - finder->addMatcher( - cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("WalletLogPrintf"))), - hasArgument(0, stringLiteral(unterminated()).bind("logstring"))), - this); -} - -void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result) -{ - if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs("logstring")) { - const clang::ASTContext& ctx = *Result.Context; - const auto user_diag = diag(lit->getEndLoc(), "Unterminated format string used with LogPrintf"); - const auto& loc = lit->getLocationOfByte(lit->getByteLength(), *Result.SourceManager, ctx.getLangOpts(), ctx.getTargetInfo()); - user_diag << clang::FixItHint::CreateInsertion(loc, "\\n"); - } -} - -} // namespace bitcoin diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h deleted file mode 100644 index db95dfe143..0000000000 --- a/contrib/devtools/bitcoin-tidy/logprintf.h +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) 2023 Bitcoin Developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef LOGPRINTF_CHECK_H -#define LOGPRINTF_CHECK_H - -#include - -namespace bitcoin { - -// Warn about any use of LogPrintf that does not end with a newline. -class LogPrintfCheck final : public clang::tidy::ClangTidyCheck -{ -public: - LogPrintfCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context) - : clang::tidy::ClangTidyCheck(Name, Context) {} - - bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override - { - return LangOpts.CPlusPlus; - } - void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; - void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; -}; - -} // namespace bitcoin - -#endif // LOGPRINTF_CHECK_H diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index a809851ec6..86a17fb0f8 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -62,7 +62,7 @@ def main(): matching_files_filtered = [] for matching_file in matching_files: - if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)|contrib/devtools/bitcoin-tidy/example_logprintf.cpp', matching_file): + if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)', matching_file): matching_files_filtered.append(matching_file) matching_files_filtered.sort() From 61cdb1c9d83778b95f4f9596f34617b7a191d0a5 Mon Sep 17 00:00:00 2001 From: Marnix <93143998+MarnixCroes@users.noreply.github.com> Date: Tue, 1 Oct 2024 13:56:16 +0200 Subject: [PATCH 24/36] doc: add testnet4 section header for config file --- contrib/devtools/gen-bitcoin-conf.sh | 5 ++++- doc/bitcoin-conf.md | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/contrib/devtools/gen-bitcoin-conf.sh b/contrib/devtools/gen-bitcoin-conf.sh index 2ebbd42022..d830852c9e 100755 --- a/contrib/devtools/gen-bitcoin-conf.sh +++ b/contrib/devtools/gen-bitcoin-conf.sh @@ -72,9 +72,12 @@ cat >> "${EXAMPLE_CONF_FILE}" << 'EOF' # Options for mainnet [main] -# Options for testnet +# Options for testnet3 [test] +# Options for testnet4 +[testnet4] + # Options for signet [signet] diff --git a/doc/bitcoin-conf.md b/doc/bitcoin-conf.md index 76711d0e7d..9b31879790 100644 --- a/doc/bitcoin-conf.md +++ b/doc/bitcoin-conf.md @@ -31,7 +31,7 @@ Comments may appear in two ways: ### Network specific options Network specific options can be: -- placed into sections with headers `[main]` (not `[mainnet]`), `[test]` (not `[testnet]`), `[signet]` or `[regtest]`; +- placed into sections with headers `[main]` (not `[mainnet]`), `[test]` (not `[testnet]`, for testnet3), `[testnet4]`, `[signet]` or `[regtest]`; - prefixed with a chain name; e.g., `regtest.maxmempool=100`. Network specific options take precedence over non-network specific options. From d51edecddcb7fa52349c8aa5ef88b01f72be44c7 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 1 Oct 2024 09:12:22 -0400 Subject: [PATCH 25/36] common: move pcp.cpp and netif.cpp files from util to common library since they depend on netaddress.cpp Prevents check-deps.sh errors reported by fanquake https://github.com/bitcoin/bitcoin/pull/30415#issuecomment-2385475097 --- src/CMakeLists.txt | 2 ++ src/{util => common}/netif.cpp | 2 +- src/{util => common}/netif.h | 6 +++--- src/{util => common}/pcp.cpp | 4 ++-- src/{util => common}/pcp.h | 6 +++--- src/mapport.cpp | 4 ++-- src/net.cpp | 2 +- src/util/CMakeLists.txt | 2 -- 8 files changed, 14 insertions(+), 14 deletions(-) rename src/{util => common}/netif.cpp (99%) rename src/{util => common}/netif.h (86%) rename src/{util => common}/pcp.cpp (99%) rename src/{util => common}/pcp.h (97%) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8c81e21f76..d10638e887 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -114,6 +114,8 @@ add_library(bitcoin_common STATIC EXCLUDE_FROM_ALL common/init.cpp common/interfaces.cpp common/messages.cpp + common/netif.cpp + common/pcp.cpp common/run_command.cpp common/settings.cpp common/signmessage.cpp diff --git a/src/util/netif.cpp b/src/common/netif.cpp similarity index 99% rename from src/util/netif.cpp rename to src/common/netif.cpp index 75d0cf00c5..d75123b265 100644 --- a/src/util/netif.cpp +++ b/src/common/netif.cpp @@ -4,7 +4,7 @@ #include // IWYU pragma: keep -#include +#include #include #include diff --git a/src/util/netif.h b/src/common/netif.h similarity index 86% rename from src/util/netif.h rename to src/common/netif.h index 5ff473fd4f..55bc023be6 100644 --- a/src/util/netif.h +++ b/src/common/netif.h @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_UTIL_NETIF_H -#define BITCOIN_UTIL_NETIF_H +#ifndef BITCOIN_COMMON_NETIF_H +#define BITCOIN_COMMON_NETIF_H #include @@ -16,4 +16,4 @@ std::optional QueryDefaultGateway(Network network); //! Return all local non-loopback IPv4 and IPv6 network addresses. std::vector GetLocalAddresses(); -#endif // BITCOIN_UTIL_NETIF_H +#endif // BITCOIN_COMMON_NETIF_H diff --git a/src/util/pcp.cpp b/src/common/pcp.cpp similarity index 99% rename from src/util/pcp.cpp rename to src/common/pcp.cpp index b02568db35..3cc1cba924 100644 --- a/src/util/pcp.cpp +++ b/src/common/pcp.cpp @@ -2,8 +2,9 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#include +#include +#include #include #include #include @@ -11,7 +12,6 @@ #include #include #include -#include #include #include #include diff --git a/src/util/pcp.h b/src/common/pcp.h similarity index 97% rename from src/util/pcp.h rename to src/common/pcp.h index 0a7955fa57..ce2273e140 100644 --- a/src/util/pcp.h +++ b/src/common/pcp.h @@ -2,8 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or https://www.opensource.org/licenses/mit-license.php. -#ifndef BITCOIN_UTIL_PCP_H -#define BITCOIN_UTIL_PCP_H +#ifndef BITCOIN_COMMON_PCP_H +#define BITCOIN_COMMON_PCP_H #include @@ -65,4 +65,4 @@ std::variant NATPMPRequestPortMap(const CNetAddr &g //! Returns the external_ip:external_port of the mapping if successful, otherwise a MappingError. std::variant PCPRequestPortMap(const PCPMappingNonce &nonce, const CNetAddr &gateway, const CNetAddr &bind, uint16_t port, uint32_t lifetime, int num_tries = 3, std::chrono::milliseconds timeout_per_try = std::chrono::milliseconds(1000)); -#endif // BITCOIN_UTIL_PCP_H +#endif // BITCOIN_COMMON_PCP_H diff --git a/src/mapport.cpp b/src/mapport.cpp index fd15708edc..993dade143 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -7,14 +7,14 @@ #include #include +#include +#include #include #include #include #include #include #include -#include -#include #include #include diff --git a/src/net.cpp b/src/net.cpp index e120aee2c4..3f9918d2b1 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -28,7 +29,6 @@ #include #include #include -#include #include #include #include diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index f4b9b47d10..4999dbf13f 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -15,8 +15,6 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL fs_helpers.cpp hasher.cpp moneystr.cpp - netif.cpp - pcp.cpp rbf.cpp readwritefile.cpp serfloat.cpp From fd38711217cafbd62e8abd22d2b43f85fede8cde Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 1 Oct 2024 09:13:20 -0400 Subject: [PATCH 26/36] ci: make CI job fail when check-deps.sh script fails Previously the check-deps.sh would write information about unexpected dependencies to stderr, but return exit code 0, so the error would be ignored by CI. Now it will return code 1 and cause CI to fail if unexpected dependencies are detected. --- contrib/devtools/check-deps.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/devtools/check-deps.sh b/contrib/devtools/check-deps.sh index 18d5fe0eb5..2fce249945 100755 --- a/contrib/devtools/check-deps.sh +++ b/contrib/devtools/check-deps.sh @@ -194,7 +194,10 @@ cd "$BUILD_DIR/src" extract_symbols "$TEMP_DIR" if check_libraries "$TEMP_DIR"; then echo "Success! No unexpected dependencies were detected." + RET=0 else echo >&2 "Error: Unexpected dependencies were detected. Check previous output." + RET=1 fi rm -r "$TEMP_DIR" +exit $RET From a1576edab356053c4c736691e4950b58e9a14f76 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 1 Oct 2024 17:56:29 -0400 Subject: [PATCH 27/36] test: add missing sync to feature_fee_estimation.py Fixes a race between node 1 catching up with the chain and mining a new block in the sanity_check_rbf_estimates subtest. --- test/functional/feature_fee_estimation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 83627ff5c2..974d8268a2 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -398,6 +398,7 @@ def clear_estimates(self): self.start_node(0) self.connect_nodes(0, 1) self.connect_nodes(0, 2) + self.sync_blocks() assert_equal(self.nodes[0].estimatesmartfee(1)["errors"], ["Insufficient data or no feerate found"]) def broadcast_and_mine(self, broadcaster, miner, feerate, count): From 91b65adff2aaf16f42c5ccca6e16b951e0e84f9a Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Thu, 5 Sep 2024 21:40:52 -0400 Subject: [PATCH 28/36] refactor: add OrphanTxBase for external use Enables external entities to obtain orphan information --- src/txorphanage.cpp | 2 +- src/txorphanage.h | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 35a215c88a..35cf26a7be 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -33,7 +33,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) return false; } - auto ret = m_orphans.emplace(wtxid, OrphanTx{tx, peer, Now() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()}); + auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, peer, Now() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()}); assert(ret.second); m_orphan_list.push_back(ret.first); for (const CTxIn& txin : tx->vin) { diff --git a/src/txorphanage.h b/src/txorphanage.h index 2c53d1d40f..5123bfe867 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -72,11 +72,15 @@ class TxOrphanage { return m_orphans.size(); } -protected: - struct OrphanTx { + /** Allows providing orphan information externally */ + struct OrphanTxBase { CTransactionRef tx; NodeId fromPeer; NodeSeconds nTimeExpire; + }; + +protected: + struct OrphanTx : public OrphanTxBase { size_t list_pos; }; From 532491faf1aa90053af52cbedce403b9eccf0bc3 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:28:04 -0400 Subject: [PATCH 29/36] net: add GetOrphanTransactions() to PeerManager Updates PeerManager (and Impl) to provide orphans with metadata --- src/net_processing.cpp | 7 +++++++ src/net_processing.h | 3 +++ src/txorphanage.cpp | 10 ++++++++++ src/txorphanage.h | 2 ++ 4 files changed, 22 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b7d0f5360d..be16884011 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -515,6 +515,7 @@ class PeerManagerImpl final : public PeerManager std::optional FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + std::vector GetOrphanTransactions() override EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex); PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -1917,6 +1918,12 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c return true; } +std::vector PeerManagerImpl::GetOrphanTransactions() +{ + LOCK(m_tx_download_mutex); + return m_orphanage.GetOrphanTransactions(); +} + PeerManagerInfo PeerManagerImpl::GetInfo() const { return PeerManagerInfo{ diff --git a/src/net_processing.h b/src/net_processing.h index ccacd15e42..0d2dc59c5a 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -7,6 +7,7 @@ #define BITCOIN_NET_PROCESSING_H #include +#include #include #include @@ -99,6 +100,8 @@ class PeerManager : public CValidationInterface, public NetEventsInterface /** Get statistics from node state */ virtual bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const = 0; + virtual std::vector GetOrphanTransactions() = 0; + /** Get peer manager info. */ virtual PeerManagerInfo GetInfo() const = 0; diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 35cf26a7be..ba4ba6c3b6 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -277,3 +277,13 @@ std::vector> TxOrphanage::GetChildrenFromDiff } return children_found; } + +std::vector TxOrphanage::GetOrphanTransactions() const +{ + std::vector ret; + ret.reserve(m_orphans.size()); + for (auto const& o : m_orphans) { + ret.push_back({o.second.tx, o.second.fromPeer, o.second.nTimeExpire}); + } + return ret; +} diff --git a/src/txorphanage.h b/src/txorphanage.h index 5123bfe867..5501d10922 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -79,6 +79,8 @@ class TxOrphanage { NodeSeconds nTimeExpire; }; + std::vector GetOrphanTransactions() const; + protected: struct OrphanTx : public OrphanTxBase { size_t list_pos; From f511ff3654d999951a64098c8a9f2f8e29771dad Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:28:43 -0400 Subject: [PATCH 30/36] refactor: move verbosity parsing to rpc/util Provides a common way for rpcs to obtain verbosity from an rpc parameter --- src/rpc/blockchain.cpp | 9 +-------- src/rpc/rawtransaction.cpp | 10 +--------- src/rpc/util.cpp | 12 ++++++++++++ src/rpc/util.h | 9 +++++++++ 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index c4fc06b34e..360f24ec55 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -766,14 +766,7 @@ static RPCHelpMan getblock() { uint256 hash(ParseHashV(request.params[0], "blockhash")); - int verbosity = 1; - if (!request.params[1].isNull()) { - if (request.params[1].isBool()) { - verbosity = request.params[1].get_bool() ? 1 : 0; - } else { - verbosity = request.params[1].getInt(); - } - } + int verbosity{ParseVerbosity(request.params[1], /*default_verbosity=*/1)}; const CBlockIndex* pblockindex; const CBlockIndex* tip; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index bb072370ce..65e6e40b0d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -338,15 +338,7 @@ static RPCHelpMan getrawtransaction() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The genesis block coinbase is not considered an ordinary transaction and cannot be retrieved"); } - // Accept either a bool (true) or a num (>=0) to indicate verbosity. - int verbosity{0}; - if (!request.params[1].isNull()) { - if (request.params[1].isBool()) { - verbosity = request.params[1].get_bool(); - } else { - verbosity = request.params[1].getInt(); - } - } + int verbosity{ParseVerbosity(request.params[1], /*default_verbosity=*/0)}; if (!request.params[2].isNull()) { LOCK(cs_main); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 678bac7a18..7757c258fd 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -81,6 +81,18 @@ void RPCTypeCheckObj(const UniValue& o, } } +int ParseVerbosity(const UniValue& arg, int default_verbosity) +{ + if (!arg.isNull()) { + if (arg.isBool()) { + return arg.get_bool(); // true = 1 + } else { + return arg.getInt(); + } + } + return default_verbosity; +} + CAmount AmountFromValue(const UniValue& value, int decimals) { if (!value.isNum() && !value.isStr()) diff --git a/src/rpc/util.h b/src/rpc/util.h index 23024376e0..b8e6759768 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -100,6 +100,15 @@ uint256 ParseHashO(const UniValue& o, std::string_view strKey); std::vector ParseHexV(const UniValue& v, std::string_view name); std::vector ParseHexO(const UniValue& o, std::string_view strKey); +/** + * Parses verbosity from provided UniValue. + * + * @param[in] arg The verbosity argument as a bool (true) or int (0, 1, 2,...) + * @param[in] default_verbosity The value to return if verbosity argument is null + * @returns An integer describing the verbosity level (e.g. 0, 1, 2, etc.) + */ +int ParseVerbosity(const UniValue& arg, int default_verbosity); + /** * Validate and return a CAmount from a UniValue number or string. * From 34a9c10e8cdb3e9cd40fc3a420df8f73e0208a48 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:29:32 -0400 Subject: [PATCH 31/36] rpc: add getorphantxs Adds an rpc to obtain data about the transactions currently in the orphanage. Hidden and marked as experimental --- src/rpc/client.cpp | 2 + src/rpc/mempool.cpp | 102 ++++++++++++++++++++++++++++++++++++++++++ src/test/fuzz/rpc.cpp | 1 + 3 files changed, 105 insertions(+) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 0112a261ce..601e4fa7bf 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -254,6 +254,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "keypoolrefill", 0, "newsize" }, { "getrawmempool", 0, "verbose" }, { "getrawmempool", 1, "mempool_sequence" }, + { "getorphantxs", 0, "verbosity" }, + { "getorphantxs", 0, "verbose" }, { "estimatesmartfee", 0, "conf_target" }, { "estimaterawfee", 0, "conf_target" }, { "estimaterawfee", 1, "threshold" }, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index d61898260b..27a00c5d91 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -8,8 +8,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -24,6 +26,7 @@ #include #include #include +#include #include @@ -812,6 +815,104 @@ static RPCHelpMan savemempool() }; } +static std::vector OrphanDescription() +{ + return { + RPCResult{RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, + RPCResult{RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"}, + RPCResult{RPCResult::Type::NUM, "bytes", "The serialized transaction size in bytes"}, + RPCResult{RPCResult::Type::NUM, "vsize", "The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, + RPCResult{RPCResult::Type::NUM, "weight", "The transaction weight as defined in BIP 141."}, + RPCResult{RPCResult::Type::NUM_TIME, "expiration", "The orphan expiration time expressed in " + UNIX_EPOCH_TIME}, + RPCResult{RPCResult::Type::ARR, "from", "", + { + RPCResult{RPCResult::Type::NUM, "peer_id", "Peer ID"}, + }}, + }; +} + +static UniValue OrphanToJSON(const TxOrphanage::OrphanTxBase& orphan) +{ + UniValue o(UniValue::VOBJ); + o.pushKV("txid", orphan.tx->GetHash().ToString()); + o.pushKV("wtxid", orphan.tx->GetWitnessHash().ToString()); + o.pushKV("bytes", orphan.tx->GetTotalSize()); + o.pushKV("vsize", GetVirtualTransactionSize(*orphan.tx)); + o.pushKV("weight", GetTransactionWeight(*orphan.tx)); + o.pushKV("expiration", int64_t{TicksSinceEpoch(orphan.nTimeExpire)}); + UniValue from(UniValue::VARR); + from.push_back(orphan.fromPeer); // only one fromPeer for now + o.pushKV("from", from); + return o; +} + +static RPCHelpMan getorphantxs() +{ + return RPCHelpMan{"getorphantxs", + "\nShows transactions in the tx orphanage.\n" + "\nEXPERIMENTAL warning: this call may be changed in future releases.\n", + { + {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{0}, "0 for an array of txids (may contain duplicates), 1 for an array of objects with tx details, and 2 for details from (1) and tx hex", + RPCArgOptions{.skip_type_check = true}}, + }, + { + RPCResult{"for verbose = 0", + RPCResult::Type::ARR, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, + }}, + RPCResult{"for verbose = 1", + RPCResult::Type::ARR, "", "", + { + {RPCResult::Type::OBJ, "", "", OrphanDescription()}, + }}, + RPCResult{"for verbose = 2", + RPCResult::Type::ARR, "", "", + { + {RPCResult::Type::OBJ, "", "", + Cat>( + OrphanDescription(), + {{RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded transaction data"}} + ) + }, + }}, + }, + RPCExamples{ + HelpExampleCli("getorphantxs", "2") + + HelpExampleRpc("getorphantxs", "2") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + const NodeContext& node = EnsureAnyNodeContext(request.context); + PeerManager& peerman = EnsurePeerman(node); + std::vector orphanage = peerman.GetOrphanTransactions(); + + int verbosity{ParseVerbosity(request.params[0], /*default_verbosity=*/0)}; + + UniValue ret(UniValue::VARR); + + if (verbosity <= 0) { + for (auto const& orphan : orphanage) { + ret.push_back(orphan.tx->GetHash().ToString()); + } + } else if (verbosity == 1) { + for (auto const& orphan : orphanage) { + ret.push_back(OrphanToJSON(orphan)); + } + } else { + // >= 2 + for (auto const& orphan : orphanage) { + UniValue o{OrphanToJSON(orphan)}; + o.pushKV("hex", EncodeHexTx(*orphan.tx)); + ret.push_back(o); + } + } + + return ret; + }, + }; +} + static RPCHelpMan submitpackage() { return RPCHelpMan{"submitpackage", @@ -1027,6 +1128,7 @@ void RegisterMempoolRPCCommands(CRPCTable& t) {"blockchain", &getrawmempool}, {"blockchain", &importmempool}, {"blockchain", &savemempool}, + {"hidden", &getorphantxs}, {"rawtransactions", &submitpackage}, }; for (const auto& c : commands) { diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 9122617e46..4db37ab7b7 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -143,6 +143,7 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ "getnetworkhashps", "getnetworkinfo", "getnodeaddresses", + "getorphantxs", "getpeerinfo", "getprioritisedtransactions", "getrawaddrman", From 93f48fceb7dd332ef980ce890ff7750b995d6077 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:29:55 -0400 Subject: [PATCH 32/36] test: add tx_in_orphanage() Allows tests to check if a transaction is contained within the orphanage --- test/functional/test_framework/mempool_util.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index fe47123e13..a6a7940c60 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -8,6 +8,7 @@ from .blocktools import ( COINBASE_MATURITY, ) +from .messages import CTransaction from .util import ( assert_equal, assert_greater_than, @@ -83,3 +84,8 @@ def send_batch(fee): test_framework.log.debug("Check that mempoolminfee is larger than minrelaytxfee") assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) assert_greater_than(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) + +def tx_in_orphanage(node, tx: CTransaction) -> bool: + """Returns true if the transaction is in the orphanage.""" + found = [o for o in node.getorphantxs(verbosity=1) if o["txid"] == tx.rehash() and o["wtxid"] == tx.getwtxid()] + return len(found) == 1 From 98c1536852d1de9a978b11046e7414e79ed40b46 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:30:15 -0400 Subject: [PATCH 33/36] test: add getorphantxs tests Adds functional tests for getorphantxs --- test/functional/rpc_getorphantxs.py | 130 ++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 131 insertions(+) create mode 100755 test/functional/rpc_getorphantxs.py diff --git a/test/functional/rpc_getorphantxs.py b/test/functional/rpc_getorphantxs.py new file mode 100755 index 0000000000..8d32ce1638 --- /dev/null +++ b/test/functional/rpc_getorphantxs.py @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +# Copyright (c) 2014-2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the getorphantxs RPC.""" + +from test_framework.mempool_util import tx_in_orphanage +from test_framework.messages import msg_tx +from test_framework.p2p import P2PInterface +from test_framework.util import assert_equal +from test_framework.test_framework import BitcoinTestFramework +from test_framework.wallet import MiniWallet + + +class GetOrphanTxsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + self.wallet = MiniWallet(self.nodes[0]) + self.test_orphan_activity() + self.test_orphan_details() + + def test_orphan_activity(self): + self.log.info("Check that orphaned transactions are returned with getorphantxs") + node = self.nodes[0] + + self.log.info("Create two 1P1C packages, but only broadcast the children") + tx_parent_1 = self.wallet.create_self_transfer() + tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) + tx_parent_2 = self.wallet.create_self_transfer() + tx_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_2["new_utxo"]) + peer = node.add_p2p_connection(P2PInterface()) + peer.send_and_ping(msg_tx(tx_child_1["tx"])) + peer.send_and_ping(msg_tx(tx_child_2["tx"])) + + self.log.info("Check that neither parent is in the mempool") + assert_equal(node.getmempoolinfo()["size"], 0) + + self.log.info("Check that both children are in the orphanage") + + orphanage = node.getorphantxs(verbosity=0) + self.log.info("Check the size of the orphanage") + assert_equal(len(orphanage), 2) + self.log.info("Check that negative verbosity is treated as 0") + assert_equal(orphanage, node.getorphantxs(verbosity=-1)) + assert tx_in_orphanage(node, tx_child_1["tx"]) + assert tx_in_orphanage(node, tx_child_2["tx"]) + + self.log.info("Broadcast parent 1") + peer.send_and_ping(msg_tx(tx_parent_1["tx"])) + self.log.info("Check that parent 1 and child 1 are in the mempool") + raw_mempool = node.getrawmempool() + assert_equal(len(raw_mempool), 2) + assert tx_parent_1["txid"] in raw_mempool + assert tx_child_1["txid"] in raw_mempool + + self.log.info("Check that orphanage only contains child 2") + orphanage = node.getorphantxs() + assert_equal(len(orphanage), 1) + assert tx_in_orphanage(node, tx_child_2["tx"]) + + peer.send_and_ping(msg_tx(tx_parent_2["tx"])) + self.log.info("Check that all parents and children are now in the mempool") + raw_mempool = node.getrawmempool() + assert_equal(len(raw_mempool), 4) + assert tx_parent_1["txid"] in raw_mempool + assert tx_child_1["txid"] in raw_mempool + assert tx_parent_2["txid"] in raw_mempool + assert tx_child_2["txid"] in raw_mempool + self.log.info("Check that the orphanage is empty") + assert_equal(len(node.getorphantxs()), 0) + + self.log.info("Confirm the transactions (clears mempool)") + self.generate(node, 1) + assert_equal(node.getmempoolinfo()["size"], 0) + + def test_orphan_details(self): + self.log.info("Check the transaction details returned from getorphantxs") + node = self.nodes[0] + + self.log.info("Create two orphans, from different peers") + tx_parent_1 = self.wallet.create_self_transfer() + tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) + tx_parent_2 = self.wallet.create_self_transfer() + tx_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_2["new_utxo"]) + peer_1 = node.add_p2p_connection(P2PInterface()) + peer_2 = node.add_p2p_connection(P2PInterface()) + peer_1.send_and_ping(msg_tx(tx_child_1["tx"])) + peer_2.send_and_ping(msg_tx(tx_child_2["tx"])) + + orphanage = node.getorphantxs(verbosity=2) + assert tx_in_orphanage(node, tx_child_1["tx"]) + assert tx_in_orphanage(node, tx_child_2["tx"]) + + self.log.info("Check that orphan 1 and 2 were from different peers") + assert orphanage[0]["from"][0] != orphanage[1]["from"][0] + + self.log.info("Unorphan child 2") + peer_2.send_and_ping(msg_tx(tx_parent_2["tx"])) + assert not tx_in_orphanage(node, tx_child_2["tx"]) + + self.log.info("Checking orphan details") + orphanage = node.getorphantxs(verbosity=1) + assert_equal(len(node.getorphantxs()), 1) + orphan_1 = orphanage[0] + self.orphan_details_match(orphan_1, tx_child_1, verbosity=1) + + self.log.info("Checking orphan details (verbosity 2)") + orphanage = node.getorphantxs(verbosity=2) + orphan_1 = orphanage[0] + self.orphan_details_match(orphan_1, tx_child_1, verbosity=2) + + def orphan_details_match(self, orphan, tx, verbosity): + self.log.info("Check txid/wtxid of orphan") + assert_equal(orphan["txid"], tx["txid"]) + assert_equal(orphan["wtxid"], tx["wtxid"]) + + self.log.info("Check the sizes of orphan") + assert_equal(orphan["bytes"], len(tx["tx"].serialize())) + assert_equal(orphan["vsize"], tx["tx"].get_vsize()) + assert_equal(orphan["weight"], tx["tx"].get_weight()) + + if verbosity == 2: + self.log.info("Check the transaction hex of orphan") + assert_equal(orphan["hex"], tx["hex"]) + + +if __name__ == '__main__': + GetOrphanTxsTest(__file__).main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 301f62d7b0..14a1e42924 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -160,6 +160,7 @@ 'wallet_importmulti.py --legacy-wallet', 'mempool_limit.py', 'rpc_txoutproof.py', + 'rpc_getorphantxs.py', 'wallet_listreceivedby.py --legacy-wallet', 'wallet_listreceivedby.py --descriptors', 'wallet_abandonconflict.py --legacy-wallet', From 27709f51ee02ed4f8c9c7e1c25c6f16faa0ef08b Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Thu, 26 Sep 2024 10:24:47 -0500 Subject: [PATCH 34/36] docs: Add instructions on how to self-sign bitcoin-core binaries for macOS Remove link and clear up language Move instructions to release-notes-empty-template.md Capitalize 'Bitcoin Core' project name --- doc/release-notes-empty-template.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/release-notes-empty-template.md b/doc/release-notes-empty-template.md index 96e28c3763..ac1c1382e0 100644 --- a/doc/release-notes-empty-template.md +++ b/doc/release-notes-empty-template.md @@ -32,6 +32,13 @@ Upgrading directly from a version of Bitcoin Core that has reached its EOL is possible, but it might take some time if the data directory needs to be migrated. Old wallet versions of Bitcoin Core are generally supported. +Running Bitcoin Core binaries on macOS requires self signing. +``` +cd /path/to/bitcoin-core/bin +xattr -d com.apple.quarantine bitcoin-cli bitcoin-qt bitcoin-tx bitcoin-util bitcoin-wallet bitcoind test_bitcoin +codesign -s - bitcoin-cli bitcoin-qt bitcoin-tx bitcoin-util bitcoin-wallet bitcoind test_bitcoin +``` + Compatibility ============== From 5625840c11db2065a1c8d8de3babb6466eda04d4 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 29 Sep 2024 14:20:17 +0100 Subject: [PATCH 35/36] qt6, test: Handle deprecated `QVERIFY_EXCEPTION_THROWN` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change ensures compatibility across all supported Qt versions. Co-Authored-By: João Barbosa --- src/qt/test/rpcnestedtests.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 72e8055425..0797d31a71 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -127,6 +127,11 @@ void RPCNestedTests::rpcNestedTests() RPCConsole::RPCExecuteCommandLine(m_node, result, "rpcNestedTest( abc , cba )"); QVERIFY(result == "[\"abc\",\"cba\"]"); +// Handle deprecated macro, can be removed once minimum Qt is at least 6.3.0. +#if (QT_VERSION >= QT_VERSION_CHECK(6, 3, 0)) +#undef QVERIFY_EXCEPTION_THROWN +#define QVERIFY_EXCEPTION_THROWN(expression, exceptiontype) QVERIFY_THROWS_EXCEPTION(exceptiontype, expression) +#endif QVERIFY_EXCEPTION_THROWN(RPCConsole::RPCExecuteCommandLine(m_node, result, "getblockchaininfo() .\n"), std::runtime_error); //invalid syntax QVERIFY_EXCEPTION_THROWN(RPCConsole::RPCExecuteCommandLine(m_node, result, "getblockchaininfo() getblockchaininfo()"), std::runtime_error); //invalid syntax RPCConsole::RPCExecuteCommandLine(m_node, result, "getblockchaininfo("); //tolerate non closing brackets if we have no arguments From f019fcec4126aa2618734016711063d3b44260fc Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 4 Oct 2024 19:25:11 -0400 Subject: [PATCH 36/36] doc: Archive 28.0 release notes --- doc/release-notes/release-notes-28.0.md | 371 ++++++++++++++++++++++++ 1 file changed, 371 insertions(+) create mode 100644 doc/release-notes/release-notes-28.0.md diff --git a/doc/release-notes/release-notes-28.0.md b/doc/release-notes/release-notes-28.0.md new file mode 100644 index 0000000000..d9e6a34d0f --- /dev/null +++ b/doc/release-notes/release-notes-28.0.md @@ -0,0 +1,371 @@ +Bitcoin Core version 28.0 is now available from: + + + +This release includes new features, various bug fixes and performance +improvements, as well as updated translations. + +Please report bugs using the issue tracker at GitHub: + + + +To receive security and update notifications, please subscribe to: + + + +How to Upgrade +============== + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes in some cases), then run the +installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on macOS) +or `bitcoind`/`bitcoin-qt` (on Linux). + +Upgrading directly from a version of Bitcoin Core that has reached its EOL is +possible, but it might take some time if the data directory needs to be migrated. Old +wallet versions of Bitcoin Core are generally supported. + +Running Bitcoin Core binaries on macOS requires self signing. +``` +cd /path/to/bitcoin-28.0/bin +xattr -d com.apple.quarantine bitcoin-cli bitcoin-qt bitcoin-tx bitcoin-util bitcoin-wallet bitcoind test_bitcoin +codesign -s - bitcoin-cli bitcoin-qt bitcoin-tx bitcoin-util bitcoin-wallet bitcoind test_bitcoin +``` + +Compatibility +============== + +Bitcoin Core is supported and extensively tested on operating systems +using the Linux Kernel 3.17+, macOS 11.0+, and Windows 7 and newer. Bitcoin +Core should also work on most other UNIX-like systems but is not as +frequently tested on them. It is not recommended to use Bitcoin Core on +unsupported systems. + +Notable changes +=============== + +Testnet4/BIP94 support +----- + +Support for Testnet4 as specified in [BIP94](https://github.com/bitcoin/bips/blob/master/bip-0094.mediawiki) +has been added. The network can be selected with the `-testnet4` option and +the section header is also named `[testnet4]`. + +While the intention is to phase out support for Testnet3 in an upcoming +version, support for it is still available via the known options in this +release. (#29775) + +Windows Data Directory +---------------------- + +The default data directory on Windows has been moved from `C:\Users\Username\AppData\Roaming\Bitcoin` +to `C:\Users\Username\AppData\Local\Bitcoin`. Bitcoin Core will check the existence +of the old directory first and continue to use that directory for backwards +compatibility if it is present. (#27064) + +JSON-RPC 2.0 Support +-------------------- + +The JSON-RPC server now recognizes JSON-RPC 2.0 requests and responds with +strict adherence to the [specification](https://www.jsonrpc.org/specification). +See [JSON-RPC-interface.md](https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#json-rpc-11-vs-20) for details. (#27101) + +JSON-RPC clients may need to be updated to be compatible with the JSON-RPC server. +Please open an issue on GitHub if any compatibility issues are found. + +libbitcoinconsensus Removal +--------------------------- + +The libbitcoin-consensus library was deprecated in 27.0 and is now completely removed. (#29648) + +P2P and Network Changes +----------------------- + +- Previously if Bitcoin Core was listening for P2P connections, either using + default settings or via `bind=addr:port` it would always also bind to + `127.0.0.1:8334` to listen for Tor connections. It was not possible to switch + this off, even if the node didn't use Tor. This has been changed and now + `bind=addr:port` results in binding on `addr:port` only. The default behavior + of binding to `0.0.0.0:8333` and `127.0.0.1:8334` has not been changed. + + If you are using a `bind=...` configuration without `bind=...=onion` and rely + on the previous implied behavior to accept incoming Tor connections at + `127.0.0.1:8334`, you need to now make this explicit by using + `bind=... bind=127.0.0.1:8334=onion`. (#22729) + +- Bitcoin Core will now fail to start up if any of its P2P binds fail, rather + than the previous behaviour where it would only abort startup if all P2P + binds had failed. (#22729) + +- UNIX domain sockets can now be used for proxy connections. Set `-onion` or `-proxy` + to the local socket path with the prefix `unix:` (e.g. `-onion=unix:/home/me/torsocket`). + (#27375) + +- UNIX socket paths are now accepted for `-zmqpubrawblock` and `-zmqpubrawtx` with + the format `-zmqpubrawtx=unix:/path/to/file` (#27679) + +- Additional "in" and "out" flags have been added to `-whitelist` to control whether + permissions apply to inbound connections and/or manual ones (default: inbound only). (#27114) + +- Transactions having a feerate that is too low will be opportunistically paired with + their child transactions and submitted as a package, thus enabling the node to download + 1-parent-1-child packages using the existing transaction relay protocol. Combined with + other mempool policies, this change allows limited "package relay" when a parent transaction + is below the mempool minimum feerate. Topologically Restricted Until Confirmation (TRUC) + parents are additionally allowed to be below the minimum relay feerate (i.e., pay 0 fees). + Use the `submitpackage` RPC to submit packages directly to the node. Warning: this P2P + feature is limited (unlike the `submitpackage` interface, a child with multiple unconfirmed + parents is not supported) and not yet reliable under adversarial conditions. (#28970) + +Mempool Policy Changes +---------------------- + +- Transactions with version number set to 3 are now treated as standard on all networks (#29496), + subject to opt-in Topologically Restricted Until Confirmation (TRUC) transaction policy as + described in [BIP 431](https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki). The + policy includes limits on spending unconfirmed outputs (#28948), eviction of a previous descendant + if a more incentive-compatible one is submitted (#29306), and a maximum transaction size of 10,000vB + (#29873). These restrictions simplify the assessment of incentive compatibility of accepting or + replacing TRUC transactions, thus ensuring any replacements are more profitable for the node and + making fee-bumping more reliable. + +- Pay To Anchor (P2A) is a new standard witness output type for spending, + a newly recognised output template. This allows for key-less anchor + outputs, with compact spending conditions for additional efficiencies on + top of an equivalent `sh(OP_TRUE)` output, in addition to the txid stability + of the spending transaction. + N.B. propagation of this output spending on the network will be limited + until a sufficient number of nodes on the network adopt this upgrade. (#30352) + +- Limited package RBF is now enabled, where the proposed conflicting package would result in + a connected component, aka cluster, of size 2 in the mempool. All clusters being conflicted + against must be of size 2 or lower. (#28984) + +- The default value of the `-mempoolfullrbf` configuration option has been changed from 0 to 1, + i.e. `mempoolfullrbf=1`. (#30493) + +Updated RPCs +------------ + +- The `dumptxoutset` RPC now returns the UTXO set dump in a new and + improved format. Correspondingly, the `loadtxoutset` RPC now expects + this new format in the dumps it tries to load. Dumps with the old + format are no longer supported and need to be recreated using the + new format to be usable. (#29612) + +- AssumeUTXO mainnet parameters have been added for height 840,000. + This means the `loadtxoutset` RPC can now be used on mainnet with + the matching UTXO set from that height. (#28553) + +- The `warnings` field in `getblockchaininfo`, `getmininginfo` and + `getnetworkinfo` now returns all the active node warnings as an array + of strings, instead of a single warning. The current behaviour + can be temporarily restored by running Bitcoin Core with the configuration + option `-deprecatedrpc=warnings`. (#29845) + +- Previously when using the `sendrawtransaction` RPC and specifying outputs + that are already in the UTXO set, an RPC error code of `-27` with the + message "Transaction already in block chain" was returned in response. + The error message has been changed to "Transaction outputs already in utxo set" + to more accurately describe the source of the issue. (#30212) + +- The default mode for the `estimatesmartfee` RPC has been updated from `conservative` to `economical`, + which is expected to reduce over-estimation for many users, particularly if Replace-by-Fee is an option. + For users that require high confidence in their fee estimates at the cost of potentially over-estimating, + the `conservative` mode remains available. (#30275) + +- RPC `scantxoutset` now returns 2 new fields in the "unspents" JSON array: `blockhash` and `confirmations`. + See the scantxoutset help for details. (#30515) + +- RPC `submitpackage` now allows 2 new arguments to be passed: `maxfeerate` and `maxburnamount`. See the + subtmitpackage help for details. (#28950) + +Changes to wallet-related RPCs can be found in the Wallet section below. + +Updated REST APIs +----------------- +- Parameter validation for `/rest/getutxos` has been improved by rejecting + truncated or overly large txids and malformed outpoint indices via raising + an HTTP_BAD_REQUEST "Parse error". These requests were previously handled + silently. (#30482, #30444) + +Build System +------------ + +- GCC 11.1 or later, or Clang 16.0 or later, +are now required to compile Bitcoin Core. (#29091, #30263) + +- The minimum required glibc to run Bitcoin Core is now +2.31. This means that RHEL 8 and Ubuntu 18.04 (Bionic) +are no-longer supported. (#29987) + +- `--enable-lcov-branch-coverage` has been removed, given +incompatibilities between lcov version 1 & 2. `LCOV_OPTS` +should be used to set any options instead. (#30192) + +Updated Settings +---------------- + +- When running with `-alertnotify`, an alert can now be raised multiple +times instead of just once. Previously, it was only raised when unknown +new consensus rules were activated. Its scope has now been increased to +include all kernel warnings. Specifically, alerts will now also be raised +when an invalid chain with a large amount of work has been detected. +Additional warnings may be added in the future. (#30058) + +Changes to GUI or wallet related settings can be found in the GUI or Wallet section below. + +Wallet +------ + +- The wallet now detects when wallet transactions conflict with the mempool. Mempool-conflicting + transactions can be seen in the `"mempoolconflicts"` field of `gettransaction`. The inputs + of mempool-conflicted transactions can now be respent without manually abandoning the + transactions when the parent transaction is dropped from the mempool, which can cause wallet + balances to appear higher. (#27307) + +- A new `max_tx_weight` option has been added to the RPCs `fundrawtransaction`, `walletcreatefundedpsbt`, and `send`. +It specifies the maximum transaction weight. If the limit is exceeded during funding, the transaction will not be built. +The default value is 4,000,000 WU. (#29523) + +- A new `createwalletdescriptor` RPC allows users to add new automatically generated + descriptors to their wallet. This can be used to upgrade wallets created prior to the + introduction of a new standard descriptor, such as taproot. (#29130) + +- A new RPC `gethdkeys` lists all of the BIP32 HD keys in use by all of the descriptors in the wallet. + These keys can be used in conjunction with `createwalletdescriptor` to create and add single key + descriptors to the wallet for a particular key that the wallet already knows. (#29130) + +- The `sendall` RPC can now spend unconfirmed change and will include additional fees as necessary + for the resulting transaction to bump the unconfirmed transactions' feerates to the specified feerate. (#28979) + +- In RPC `bumpfee`, if a `fee_rate` is specified, the feerate is no longer restricted + to following the wallet's incremental feerate of 5 sat/vb. The feerate must still be + at least the sum of the original fee and the mempool's incremental feerate. (#27969) + +GUI Changes +----------- + +- The "Migrate Wallet" menu allows users to migrate any legacy wallet in their wallet +directory, regardless of the wallets loaded. (gui#824) + +- The "Information" window now displays the maximum mempool size along with the +mempool usage. (gui#825) + +Low-level Changes +================= + +Tests +----- + +- The BIP94 timewarp attack mitigation is now active on the `regtest` network. (#30681) + +- A new `-testdatadir` option has been added to `test_bitcoin` to allow specifying the + location of unit test data directories. (#26564) + +Blockstorage +------------ + +- Block files are now XOR'd by default with a key stored in the blocksdir. +Previous releases of Bitcoin Core or previous external software will not be able to read the blocksdir with a non-zero XOR-key. +Refer to the `-blocksxor` help for more details. (#28052) + +Chainstate +---------- + +- The chainstate database flushes that occur when blocks are pruned will no longer +empty the database cache. The cache will remain populated longer, which significantly +reduces the time for initial block download to complete. (#28280) + +Dependencies +------------ + +- The dependency on Boost.Process has been replaced with cpp-subprocess, which is contained in source. +Builders will no longer need Boost.Process to build with external signer support. (#28981) + +Credits +======= + +Thanks to everyone who directly contributed to this release: +- 0xb10c +- Alfonso Roman Zubeldia +- Andrew Toth +- AngusP +- Anthony Towns +- Antoine Poinsot +- Anton A +- Ava Chow +- Ayush Singh +- Ben Westgate +- Brandon Odiwuor +- brunoerg +- bstin +- Charlie +- Christopher Bergqvist +- Cory Fields +- crazeteam +- Daniela Brozzoni +- David Gumberg +- dergoegge +- Edil Medeiros +- Epic Curious +- Fabian Jahr +- fanquake +- furszy +- glozow +- Greg Sanders +- hanmz +- Hennadii Stepanov +- Hernan Marino +- Hodlinator +- ishaanam +- ismaelsadeeq +- Jadi +- Jon Atack +- josibake +- jrakibi +- kevkevin +- kevkevinpal +- Konstantin Akimov +- laanwj +- Larry Ruane +- Lőrinc +- Luis Schwab +- Luke Dashjr +- MarcoFalke +- marcofleon +- Marnix +- Martin Saposnic +- Martin Zumsande +- Matt Corallo +- Matthew Zipkin +- Matt Whitlock +- Max Edwards +- Michael Dietz +- Murch +- nanlour +- pablomartin4btc +- Peter Todd +- Pieter Wuille +- @RandyMcMillan +- RoboSchmied +- Roman Zeyde +- Ryan Ofsky +- Sebastian Falbesoner +- Sergi Delgado Segura +- Sjors Provoost +- spicyzboss +- StevenMia +- stickies-v +- stratospher +- Suhas Daftuar +- sunerok +- tdb3 +- TheCharlatan +- umiumi +- Vasil Dimov +- virtu +- willcl-ark + +As well as to everyone that helped with translations on +[Transifex](https://www.transifex.com/bitcoin/bitcoin/).