diff options
35 files changed, 296 insertions, 289 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index 70fa76705e..b37fce7002 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -8,27 +8,44 @@ env: # Global defaults CCACHE_DIR: "/tmp/ccache_dir" CCACHE_NOHASHDIR: "1" # Debug info might contain a stale path if the build dir changes, but this is fine +# A self-hosted machine(s) can be used via Cirrus CI. It can be configured with +# multiple users to run tasks in parallel. No sudo permission is required. +# # https://cirrus-ci.org/guide/persistent-workers/ # -# It is possible to select a specific persistent worker by label. Refer to the +# Generally, a persistent worker must run Ubuntu 23.04+ or Debian 12+. +# +# The following specific types should exist, with the following requirements: +# - small: For an x86_64 machine, recommended to have 2 CPUs and 8 GB of memory. +# - medium: For an x86_64 machine, recommended to have 4 CPUs and 16 GB of memory. +# - arm64: For an aarch64 machine, recommended to have 2 CPUs and 8 GB of memory. +# +# CI jobs for the latter configuration can be run on x86_64 hardware +# by installing qemu-user-static, which works out of the box with +# podman or docker. Background: https://stackoverflow.com/a/72890225/313633 +# +# The above machine types are matched to each task by their label. Refer to the # Cirrus CI docs for more details. # -# Generally, a persistent worker must run Ubuntu 23.04+ or Debian 12+. -# Specifically, +# On machines that are persisted between CI jobs, RESTART_CI_DOCKER_BEFORE_RUN=1 +# ensures that previous containers and artifacts are cleared before each run. +# This requires installing Podman instead of Docker. +# +# Futhermore: # - apt-get is required due to PACKAGE_MANAGER_INSTALL -# - podman-docker-4.1+ is required due to the use of `podman` when -# RESTART_CI_DOCKER_BEFORE_RUN is set and 4.1+ due to the bugfix in 4.1 +# - podman-docker-4.1+ is required due to the bugfix in 4.1 # (https://github.com/bitcoin/bitcoin/pull/21652#issuecomment-1657098200) -# - The ./ci/ depedencies (with cirrus-cli) should be installed: +# - The ./ci/ dependencies (with cirrus-cli) should be installed. One-liner example +# for a single user setup with sudo permission: # # ``` # apt update && apt install git screen python3 bash podman-docker curl -y && curl -L -o cirrus "https://github.com/cirruslabs/cirrus-cli/releases/latest/download/cirrus-linux-$(dpkg --print-architecture)" && mv cirrus /usr/local/bin/cirrus && chmod +x /usr/local/bin/cirrus # ``` # -# - There are no strict requirements on the hardware, because having less CPUs -# runs the same CI script (maybe slower). To avoid rare and intermittent OOM -# due to short memory usage spikes, it is recommended to add (and persist) -# swap: +# - There are no strict requirements on the hardware. Having fewer CPU threads +# than recommended merely causes the CI script to run slower. +# To avoid rare and intermittent OOM due to short memory usage spikes, +# it is recommended to add (and persist) swap: # # ``` # fallocate -l 16G /swapfile_ci && chmod 600 /swapfile_ci && mkswap /swapfile_ci && swapon /swapfile_ci && ( echo '/swapfile_ci none swap sw 0 0' | tee -a /etc/fstab ) @@ -39,11 +56,6 @@ env: # Global defaults # ``` # RESTART_CI_DOCKER_BEFORE_RUN=1 screen cirrus worker run --labels type=todo_fill_in_type --token todo_fill_in_token # ``` -# -# The following specific types should exist, with the following requirements: -# - small: For an x86_64 machine, recommended to have 2 CPUs and 8 GB of memory. -# - medium: For an x86_64 machine, recommended to have 4 CPUs and 16 GB of memory. -# - arm64: For an aarch64 machine, recommended to have 2 CPUs and 8 GB of memory. # https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks filter_template: &FILTER_TEMPLATE diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab314c47b6..54795332e8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -313,6 +313,7 @@ jobs: timeout-minutes: 120 env: FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" + DANGER_CI_ON_HOST_CACHE_FOLDERS: 1 steps: - name: Checkout uses: actions/checkout@v4 @@ -320,6 +321,9 @@ jobs: - name: Set Ccache directory run: echo "CCACHE_DIR=${RUNNER_TEMP}/ccache_dir" >> "$GITHUB_ENV" + - name: Set base root directory + run: echo "BASE_ROOT_DIR=${RUNNER_TEMP}" >> "$GITHUB_ENV" + - name: Restore Ccache cache id: ccache-cache uses: actions/cache/restore@v4 diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 0fd169f523..23d9180f96 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -26,3 +26,4 @@ export BITCOIN_CONFIG="--enable-usdt --enable-zmq --with-incompatible-bdb --with CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' \ --with-sanitizers=address,float-divide-by-zero,integer,undefined \ CC='clang-18 -ftrivial-auto-var-init=pattern' CXX='clang++-18 -ftrivial-auto-var-init=pattern'" +export CCACHE_MAXSIZE=300M diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh index 17a940de07..afd447c347 100755 --- a/ci/test/02_run_container.sh +++ b/ci/test/02_run_container.sh @@ -30,6 +30,24 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then docker volume create "${CONTAINER_NAME}_depends_sources" || true docker volume create "${CONTAINER_NAME}_previous_releases" || true + CI_CCACHE_MOUNT="type=volume,src=${CONTAINER_NAME}_ccache,dst=$CCACHE_DIR" + CI_DEPENDS_MOUNT="type=volume,src=${CONTAINER_NAME}_depends,dst=$DEPENDS_DIR/built" + CI_DEPENDS_SOURCES_MOUNT="type=volume,src=${CONTAINER_NAME}_depends_sources,dst=$DEPENDS_DIR/sources" + CI_PREVIOUS_RELEASES_MOUNT="type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" + + if [ "$DANGER_CI_ON_HOST_CACHE_FOLDERS" ]; then + # ensure the directories exist + mkdir -p "${CCACHE_DIR}" + mkdir -p "${DEPENDS_DIR}/built" + mkdir -p "${DEPENDS_DIR}/sources" + mkdir -p "${PREVIOUS_RELEASES_DIR}" + + CI_CCACHE_MOUNT="type=bind,src=${CCACHE_DIR},dst=$CCACHE_DIR" + CI_DEPENDS_MOUNT="type=bind,src=${DEPENDS_DIR}/built,dst=$DEPENDS_DIR/built" + CI_DEPENDS_SOURCES_MOUNT="type=bind,src=${DEPENDS_DIR}/sources,dst=$DEPENDS_DIR/sources" + CI_PREVIOUS_RELEASES_MOUNT="type=bind,src=${PREVIOUS_RELEASES_DIR},dst=$PREVIOUS_RELEASES_DIR" + fi + docker network create --ipv6 --subnet 1111:1111::/112 ci-ip6net || true if [ -n "${RESTART_CI_DOCKER_BEFORE_RUN}" ] ; then @@ -52,10 +70,10 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then # shellcheck disable=SC2086 CI_CONTAINER_ID=$(docker run --cap-add LINUX_IMMUTABLE $CI_CONTAINER_CAP --rm --interactive --detach --tty \ --mount "type=bind,src=$BASE_READ_ONLY_DIR,dst=$BASE_READ_ONLY_DIR,readonly" \ - --mount "type=volume,src=${CONTAINER_NAME}_ccache,dst=$CCACHE_DIR" \ - --mount "type=volume,src=${CONTAINER_NAME}_depends,dst=$DEPENDS_DIR/built" \ - --mount "type=volume,src=${CONTAINER_NAME}_depends_sources,dst=$DEPENDS_DIR/sources" \ - --mount "type=volume,src=${CONTAINER_NAME}_previous_releases,dst=$PREVIOUS_RELEASES_DIR" \ + --mount "${CI_CCACHE_MOUNT}" \ + --mount "${CI_DEPENDS_MOUNT}" \ + --mount "${CI_DEPENDS_SOURCES_MOUNT}" \ + --mount "${CI_PREVIOUS_RELEASES_MOUNT}" \ --env-file /tmp/env-$USER-$CONTAINER_NAME \ --name "$CONTAINER_NAME" \ --network ci-ip6net \ diff --git a/configure.ac b/configure.ac index ab369cc98a..23b8870d43 100644 --- a/configure.ac +++ b/configure.ac @@ -405,6 +405,7 @@ AX_CHECK_COMPILE_FLAG([-Wimplicit-fallthrough], [WARN_CXXFLAGS="$WARN_CXXFLAGS - AX_CHECK_COMPILE_FLAG([-Wunreachable-code], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunreachable-code"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wdocumentation], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wdocumentation"], [], [$CXXFLAG_WERROR]) AX_CHECK_COMPILE_FLAG([-Wself-assign], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wself-assign"], [], [$CXXFLAG_WERROR]) +AX_CHECK_COMPILE_FLAG([-Wundef], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wundef"], [], [$CXXFLAG_WERROR]) dnl Some compilers (gcc) ignore unknown -Wno-* options, but warn about all dnl unknown options if any other warning is produced. Test the -Wfoo case, and @@ -1392,6 +1393,9 @@ if test "$with_libmultiprocess" = "yes" || test "$with_libmultiprocess" = "auto" PKG_CHECK_MODULES([LIBMULTIPROCESS], [libmultiprocess], [ libmultiprocess_found=yes; libmultiprocess_prefix=`$PKG_CONFIG --variable=prefix libmultiprocess`; + if test "$suppress_external_warnings" != "no" ; then + LIBMULTIPROCESS_CFLAGS=SUPPRESS_WARNINGS($LIBMULTIPROCESS_CFLAGS) + fi ], [true]) elif test "$with_libmultiprocess" != "no"; then AC_MSG_ERROR([--with-libmultiprocess=$with_libmultiprocess value is not yes, auto, or no]) diff --git a/src/Makefile.am b/src/Makefile.am index f95a6fa123..0c47a737d0 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1089,7 +1089,7 @@ libbitcoin_ipc_a_SOURCES = \ ipc/process.cpp \ ipc/process.h \ ipc/protocol.h -libbitcoin_ipc_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) +libbitcoin_ipc_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libbitcoin_ipc_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) $(LIBMULTIPROCESS_CFLAGS) include $(MPGEN_PREFIX)/include/mpgen.mk diff --git a/src/banman.h b/src/banman.h index c6df7ec3c3..57ba2ac23c 100644 --- a/src/banman.h +++ b/src/banman.h @@ -34,7 +34,7 @@ class CSubNet; // disk on shutdown and reloaded on startup. Banning can be used to // prevent connections with spy nodes or other griefers. // -// 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in +// 2. Discouragement. If a peer misbehaves (see Misbehaving() in // net_processing.cpp), we'll mark that address as discouraged. We still allow // incoming connections from them, but they're preferred for eviction when // we receive new incoming connections. We never make outgoing connections to diff --git a/src/init.cpp b/src/init.cpp index 0c57dfea7f..c6ef62372e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -110,7 +110,7 @@ #include <boost/signals2/signal.hpp> -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ #include <zmq/zmqabstractnotifier.h> #include <zmq/zmqnotificationinterface.h> #include <zmq/zmqrpc.h> @@ -365,7 +365,7 @@ void Shutdown(NodeContext& node) client->stop(); } -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ if (g_zmq_notification_interface) { if (node.validation_signals) node.validation_signals->UnregisterValidationInterface(g_zmq_notification_interface.get()); g_zmq_notification_interface.reset(); @@ -532,7 +532,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-maxreceivebuffer=<n>", strprintf("Maximum per-connection receive buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxsendbuffer=<n>", strprintf("Maximum per-connection memory usage for the send buffer, <n>*1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=<n>", strprintf("Tries to keep outbound traffic under the given target per 24h. Limit does not apply to peers with 'download' permission or blocks created within past week. 0 = no limit (default: %s). Optional suffix units [k|K|m|M|g|G|t|T] (default: M). Lowercase is 1000 base while uppercase is 1024 base", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN argsman.AddArg("-onion=<ip:port|path>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy). May be a local file path prefixed with 'unix:'.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); #else argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); @@ -545,7 +545,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-txreconciliation", strprintf("Enable transaction reconciliations per BIP 330 (default: %d)", DEFAULT_TXRECONCILIATION_ENABLE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::CONNECTION); argsman.AddArg("-port=<port>", strprintf("Listen for connections on <port> (default: %u, testnet: %u, signet: %u, regtest: %u). Not relevant for I2P (see doc/i2p.md).", defaultChainParams->GetDefaultPort(), testnetChainParams->GetDefaultPort(), signetChainParams->GetDefaultPort(), regtestChainParams->GetDefaultPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION); -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN argsman.AddArg("-proxy=<ip:port|path>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled). May be a local file path prefixed with 'unix:' if the proxy supports it.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION); #else argsman.AddArg("-proxy=<ip:port>", "Connect through SOCKS5 proxy, set -noproxy to disable (default: disabled)", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_ELISION, OptionsCategory::CONNECTION); @@ -579,7 +579,7 @@ void SetupServerArgs(ArgsManager& argsman) g_wallet_init_interface.AddWalletOptions(argsman); -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ argsman.AddArg("-zmqpubhashblock=<address>", "Enable publish hash block in <address>", ArgsManager::ALLOW_ANY, OptionsCategory::ZMQ); argsman.AddArg("-zmqpubhashtx=<address>", "Enable publish hash transaction in <address>", ArgsManager::ALLOW_ANY, OptionsCategory::ZMQ); argsman.AddArg("-zmqpubrawblock=<address>", "Enable publish raw block in <address>", ArgsManager::ALLOW_ANY, OptionsCategory::ZMQ); @@ -1202,7 +1202,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (const auto& client : node.chain_clients) { client->registerRpcs(); } -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ RegisterZMQRPCCommands(tableRPC); #endif @@ -1327,7 +1327,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) std::string host_out; uint16_t port_out{0}; if (!SplitHostPort(socket_addr, port_out, host_out)) { -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN // Allow unix domain sockets for some options e.g. unix:/some/file/path if (!unix || socket_addr.find(ADDR_PREFIX_UNIX) != 0) { return InitError(InvalidPortErrMsg(arg, socket_addr)); @@ -1474,7 +1474,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(ResolveErrMsg("externalip", strAddr)); } -#if ENABLE_ZMQ +#ifdef ENABLE_ZMQ g_zmq_notification_interface = CZMQNotificationInterface::Create( [&chainman = node.chainman](std::vector<uint8_t>& block, const CBlockIndex& index) { assert(chainman); diff --git a/src/mapport.cpp b/src/mapport.cpp index 80670230c7..1920297be6 100644 --- a/src/mapport.cpp +++ b/src/mapport.cpp @@ -161,8 +161,11 @@ static bool ProcessUpnp() struct UPNPUrls urls; struct IGDdatas data; int r; - +#if MINIUPNPC_API_VERSION <= 17 r = UPNP_GetValidIGD(devlist, &urls, &data, lanaddr, sizeof(lanaddr)); +#else + r = UPNP_GetValidIGD(devlist, &urls, &data, lanaddr, sizeof(lanaddr), nullptr, 0); +#endif if (r == 1) { if (fDiscover) { diff --git a/src/net.cpp b/src/net.cpp index de974f39cb..990c58ee3d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3029,7 +3029,7 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError, return false; } - std::unique_ptr<Sock> sock = CreateSock(addrBind.GetSAFamily()); + std::unique_ptr<Sock> sock = CreateSock(addrBind.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP); if (!sock) { strError = strprintf(Untranslated("Couldn't open socket for incoming connections (socket returned error %s)"), NetworkErrorString(WSAGetLastError())); LogPrintLevel(BCLog::NET, BCLog::Level::Error, "%s\n", strError.original); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 795b798f9c..89b9488584 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -131,8 +131,6 @@ static constexpr double BLOCK_DOWNLOAD_TIMEOUT_BASE = 1; static constexpr double BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 0.5; /** Maximum number of headers to announce when relaying blocks with headers message.*/ static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8; -/** Maximum number of unconnecting headers announcements before DoS score */ -static const int MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10; /** Minimum blocks required to signal NODE_NETWORK_LIMITED */ static const unsigned int NODE_NETWORK_LIMITED_MIN_BLOCKS = 288; /** Window, in blocks, for connecting to NODE_NETWORK_LIMITED peers */ @@ -226,8 +224,6 @@ struct Peer { /** Protects misbehavior data members */ Mutex m_misbehavior_mutex; - /** Accumulated misbehavior score for this peer */ - int m_misbehavior_score GUARDED_BY(m_misbehavior_mutex){0}; /** Whether this peer should be disconnected and marked as discouraged (unless it has NetPermissionFlags::NoBan permission). */ bool m_should_discourage GUARDED_BY(m_misbehavior_mutex){false}; @@ -383,9 +379,6 @@ struct Peer { /** Whether we've sent our peer a sendheaders message. **/ std::atomic<bool> m_sent_sendheaders{false}; - /** Length of current-streak of unconnecting headers announcements */ - int m_num_unconnecting_headers_msgs GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; - /** When to potentially disconnect peer for stalling headers download */ std::chrono::microseconds m_headers_sync_timeout GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0us}; @@ -527,7 +520,7 @@ public: m_best_height = height; m_best_block_time = time; }; - void UnitTestMisbehaving(NodeId peer_id, int howmuch) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), howmuch, ""); }; + void UnitTestMisbehaving(NodeId peer_id) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex) { Misbehaving(*Assert(GetPeerRef(peer_id)), ""); }; void ProcessMessage(CNode& pfrom, const std::string& msg_type, DataStream& vRecv, const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); @@ -552,11 +545,9 @@ private: * May return an empty shared_ptr if the Peer object can't be found. */ PeerRef RemovePeer(NodeId id) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - /** - * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node - * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. - */ - void Misbehaving(Peer& peer, int howmuch, const std::string& message); + /** Mark a peer as misbehaving, which will cause it to be disconnected and its + * address discouraged. */ + void Misbehaving(Peer& peer, const std::string& message); /** * Potentially mark a node discouraged based on the contents of a BlockValidationState object @@ -565,19 +556,15 @@ private: * punish peers differently depending on whether the data was provided in a compact * block message or not. If the compact block had a valid header, but contained invalid * txs, the peer should not be punished. See BIP 152. - * - * @return Returns true if the peer was punished (probably disconnected) */ - bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, + void MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, bool via_compact_block, const std::string& message = "") EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** * Potentially disconnect and discourage a node based on the contents of a TxValidationState object - * - * @return Returns true if the peer was punished (probably disconnected) */ - bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state) + void MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); /** Maybe disconnect a peer and discourage future connections from its address. @@ -668,10 +655,10 @@ private: bool CheckHeadersPoW(const std::vector<CBlockHeader>& headers, const Consensus::Params& consensusParams, Peer& peer); /** Calculate an anti-DoS work threshold for headers chains */ arith_uint256 GetAntiDoSWorkThreshold(); - /** Deal with state tracking and headers sync for peers that send the - * occasional non-connecting header (this can happen due to BIP 130 headers + /** Deal with state tracking and headers sync for peers that send + * non-connecting headers (this can happen due to BIP 130 headers * announcements for blocks interacting with the 2hr (MAX_FUTURE_BLOCK_TIME) rule). */ - void HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); + void HandleUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); /** Return true if the headers connect to each other, false otherwise */ bool CheckHeadersAreContinuous(const std::vector<CBlockHeader>& headers) const; /** Try to continue a low-work headers sync that has already begun. @@ -1177,7 +1164,7 @@ private: void PushAddress(Peer& peer, const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex); }; -const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main) +const CNodeState* PeerManagerImpl::State(NodeId pnode) const { std::map<NodeId, CNodeState>::const_iterator it = m_node_states.find(pnode); if (it == m_node_states.end()) @@ -1185,7 +1172,7 @@ const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQ return &it->second; } -CNodeState* PeerManagerImpl::State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +CNodeState* PeerManagerImpl::State(NodeId pnode) { return const_cast<CNodeState*>(std::as_const(*this).State(pnode)); } @@ -1718,7 +1705,6 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler) void PeerManagerImpl::FinalizeNode(const CNode& node) { NodeId nodeid = node.GetId(); - int misbehavior{0}; { LOCK(cs_main); { @@ -1729,7 +1715,6 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) // destructed. PeerRef peer = RemovePeer(nodeid); assert(peer != nullptr); - misbehavior = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score); m_wtxid_relay_peers -= peer->m_wtxid_relay; assert(m_wtxid_relay_peers >= 0); } @@ -1772,7 +1757,7 @@ void PeerManagerImpl::FinalizeNode(const CNode& node) assert(m_orphanage.Size() == 0); } } // cs_main - if (node.fSuccessfullyConnected && misbehavior == 0 && + if (node.fSuccessfullyConnected && !node.IsBlockOnlyConn() && !node.IsInboundConn()) { // Only change visible addrman state for full outbound peers. We don't // call Connected() for feeler connections since they don't have @@ -1893,28 +1878,16 @@ void PeerManagerImpl::AddToCompactExtraTransactions(const CTransactionRef& tx) vExtraTxnForCompactIt = (vExtraTxnForCompactIt + 1) % m_opts.max_extra_txs; } -void PeerManagerImpl::Misbehaving(Peer& peer, int howmuch, const std::string& message) +void PeerManagerImpl::Misbehaving(Peer& peer, const std::string& message) { - assert(howmuch > 0); - LOCK(peer.m_misbehavior_mutex); - const int score_before{peer.m_misbehavior_score}; - peer.m_misbehavior_score += howmuch; - const int score_now{peer.m_misbehavior_score}; const std::string message_prefixed = message.empty() ? "" : (": " + message); - std::string warning; - - if (score_now >= DISCOURAGEMENT_THRESHOLD && score_before < DISCOURAGEMENT_THRESHOLD) { - warning = " DISCOURAGE THRESHOLD EXCEEDED"; - peer.m_should_discourage = true; - } - - LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s%s\n", - peer.m_id, score_before, score_now, warning, message_prefixed); + peer.m_should_discourage = true; + LogPrint(BCLog::NET, "Misbehaving: peer=%d%s\n", peer.m_id, message_prefixed); } -bool PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, +void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, bool via_compact_block, const std::string& message) { PeerRef peer{GetPeerRef(nodeid)}; @@ -1929,8 +1902,8 @@ bool PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati case BlockValidationResult::BLOCK_CONSENSUS: case BlockValidationResult::BLOCK_MUTATED: if (!via_compact_block) { - if (peer) Misbehaving(*peer, 100, message); - return true; + if (peer) Misbehaving(*peer, message); + return; } break; case BlockValidationResult::BLOCK_CACHED_INVALID: @@ -1944,21 +1917,20 @@ bool PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati // Discourage outbound (but not inbound) peers if on an invalid chain. // Exempt HB compact block peers. Manual connections are always protected from discouragement. if (!via_compact_block && !node_state->m_is_inbound) { - if (peer) Misbehaving(*peer, 100, message); - return true; + if (peer) Misbehaving(*peer, message); + return; } break; } case BlockValidationResult::BLOCK_INVALID_HEADER: case BlockValidationResult::BLOCK_CHECKPOINT: case BlockValidationResult::BLOCK_INVALID_PREV: - if (peer) Misbehaving(*peer, 100, message); - return true; + if (peer) Misbehaving(*peer, message); + return; // Conflicting (but not necessarily invalid) data or different policy: case BlockValidationResult::BLOCK_MISSING_PREV: - // TODO: Handle this much more gracefully (10 DoS points is super arbitrary) - if (peer) Misbehaving(*peer, 10, message); - return true; + if (peer) Misbehaving(*peer, message); + return; case BlockValidationResult::BLOCK_RECENT_CONSENSUS_CHANGE: case BlockValidationResult::BLOCK_TIME_FUTURE: break; @@ -1966,10 +1938,9 @@ bool PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati if (message != "") { LogPrint(BCLog::NET, "peer=%d: %s\n", nodeid, message); } - return false; } -bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state) +void PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state) { PeerRef peer{GetPeerRef(nodeid)}; switch (state.GetResult()) { @@ -1977,8 +1948,8 @@ bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationStat break; // The node is providing invalid data: case TxValidationResult::TX_CONSENSUS: - if (peer) Misbehaving(*peer, 100, ""); - return true; + if (peer) Misbehaving(*peer, ""); + return; // Conflicting (but not necessarily invalid) data or different policy: case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE: case TxValidationResult::TX_INPUTS_NOT_STANDARD: @@ -1994,7 +1965,6 @@ bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationStat case TxValidationResult::TX_UNKNOWN: break; } - return false; } bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) @@ -2679,7 +2649,7 @@ void PeerManagerImpl::SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlo BlockTransactions resp(req); for (size_t i = 0; i < req.indexes.size(); i++) { if (req.indexes[i] >= block.vtx.size()) { - Misbehaving(peer, 100, "getblocktxn with out-of-bounds tx indices"); + Misbehaving(peer, "getblocktxn with out-of-bounds tx indices"); return; } resp.txn[i] = block.vtx[req.indexes[i]]; @@ -2692,13 +2662,13 @@ bool PeerManagerImpl::CheckHeadersPoW(const std::vector<CBlockHeader>& headers, { // Do these headers have proof-of-work matching what's claimed? if (!HasValidProofOfWork(headers, consensusParams)) { - Misbehaving(peer, 100, "header with invalid proof of work"); + Misbehaving(peer, "header with invalid proof of work"); return false; } // Are these headers connected to each other? if (!CheckHeadersAreContinuous(headers)) { - Misbehaving(peer, 20, "non-continuous headers sequence"); + Misbehaving(peer, "non-continuous headers sequence"); return false; } return true; @@ -2722,37 +2692,24 @@ arith_uint256 PeerManagerImpl::GetAntiDoSWorkThreshold() * announcement. * * We'll send a getheaders message in response to try to connect the chain. - * - * The peer can send up to MAX_NUM_UNCONNECTING_HEADERS_MSGS in a row that - * don't connect before given DoS points. - * - * Once a headers message is received that is valid and does connect, - * m_num_unconnecting_headers_msgs gets reset back to 0. */ -void PeerManagerImpl::HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, +void PeerManagerImpl::HandleUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers) { - peer.m_num_unconnecting_headers_msgs++; // Try to fill in the missing headers. const CBlockIndex* best_header{WITH_LOCK(cs_main, return m_chainman.m_best_header)}; if (MaybeSendGetHeaders(pfrom, GetLocator(best_header), peer)) { - LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d, m_num_unconnecting_headers_msgs=%d)\n", + LogPrint(BCLog::NET, "received header %s: missing prev block %s, sending getheaders (%d) to end (peer=%d)\n", headers[0].GetHash().ToString(), headers[0].hashPrevBlock.ToString(), best_header->nHeight, - pfrom.GetId(), peer.m_num_unconnecting_headers_msgs); + pfrom.GetId()); } // Set hashLastUnknownBlock for this peer, so that if we // eventually get the headers - even from a different peer - // we can use this peer to download. WITH_LOCK(cs_main, UpdateBlockAvailability(pfrom.GetId(), headers.back().GetHash())); - - // The peer may just be broken, so periodically assign DoS points if this - // condition persists. - if (peer.m_num_unconnecting_headers_msgs % MAX_NUM_UNCONNECTING_HEADERS_MSGS == 0) { - Misbehaving(peer, 20, strprintf("%d non-connecting headers", peer.m_num_unconnecting_headers_msgs)); - } } bool PeerManagerImpl::CheckHeadersAreContinuous(const std::vector<CBlockHeader>& headers) const @@ -2771,25 +2728,21 @@ bool PeerManagerImpl::IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfro { if (peer.m_headers_sync) { auto result = peer.m_headers_sync->ProcessNextHeaders(headers, headers.size() == MAX_HEADERS_RESULTS); + // If it is a valid continuation, we should treat the existing getheaders request as responded to. + if (result.success) peer.m_last_getheaders_timestamp = {}; if (result.request_more) { auto locator = peer.m_headers_sync->NextHeadersRequestLocator(); // If we were instructed to ask for a locator, it should not be empty. Assume(!locator.vHave.empty()); + // We can only be instructed to request more if processing was successful. + Assume(result.success); if (!locator.vHave.empty()) { // It should be impossible for the getheaders request to fail, - // because we should have cleared the last getheaders timestamp - // when processing the headers that triggered this call. But - // it may be possible to bypass this via compactblock - // processing, so check the result before logging just to be - // safe. + // because we just cleared the last getheaders timestamp. bool sent_getheaders = MaybeSendGetHeaders(pfrom, locator, peer); - if (sent_getheaders) { - LogPrint(BCLog::NET, "more getheaders (from %s) to peer=%d\n", - locator.vHave.front().ToString(), pfrom.GetId()); - } else { - LogPrint(BCLog::NET, "error sending next getheaders (from %s) to continue sync with peer=%d\n", - locator.vHave.front().ToString(), pfrom.GetId()); - } + Assume(sent_getheaders); + LogPrint(BCLog::NET, "more getheaders (from %s) to peer=%d\n", + locator.vHave.front().ToString(), pfrom.GetId()); } } @@ -2998,11 +2951,6 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer, const CBlockIndex& last_header, bool received_new_header, bool may_have_more_headers) { - if (peer.m_num_unconnecting_headers_msgs > 0) { - LogPrint(BCLog::NET, "peer=%d: resetting m_num_unconnecting_headers_msgs (%d -> 0)\n", pfrom.GetId(), peer.m_num_unconnecting_headers_msgs); - } - peer.m_num_unconnecting_headers_msgs = 0; - LOCK(cs_main); CNodeState *nodestate = State(pfrom.GetId()); @@ -3068,6 +3016,9 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, LOCK(m_headers_presync_mutex); m_headers_presync_stats.erase(pfrom.GetId()); } + // A headers message with no headers cannot be an announcement, so assume + // it is a response to our last getheaders request, if there is one. + peer.m_last_getheaders_timestamp = {}; return; } @@ -3121,17 +3072,18 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, bool headers_connect_blockindex{chain_start_header != nullptr}; if (!headers_connect_blockindex) { - if (nCount <= MAX_BLOCKS_TO_ANNOUNCE) { - // If this looks like it could be a BIP 130 block announcement, use - // special logic for handling headers that don't connect, as this - // could be benign. - HandleFewUnconnectingHeaders(pfrom, peer, headers); - } else { - Misbehaving(peer, 10, "invalid header received"); - } + // This could be a BIP 130 block announcement, use + // special logic for handling headers that don't connect, as this + // could be benign. + HandleUnconnectingHeaders(pfrom, peer, headers); return; } + // If headers connect, assume that this is in response to any outstanding getheaders + // request we may have sent, and clear out the time of our last request. Non-connecting + // headers cannot be a response to a getheaders request. + peer.m_last_getheaders_timestamp = {}; + // If the headers we received are already in memory and an ancestor of // m_best_header or our tip, skip anti-DoS checks. These headers will not // use any more memory (and we are not leaking information that could be @@ -3649,7 +3601,7 @@ void PeerManagerImpl::ProcessCompactBlockTxns(CNode& pfrom, Peer& peer, const Bl ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(peer, 100, "invalid compact block/non-matching block transactions"); + Misbehaving(peer, "invalid compact block/non-matching block transactions"); return; } else if (status == READ_STATUS_FAILED) { if (first_in_flight) { @@ -4133,7 +4085,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (vAddr.size() > MAX_ADDR_TO_SEND) { - Misbehaving(*peer, 20, strprintf("%s message size = %u", msg_type, vAddr.size())); + Misbehaving(*peer, strprintf("%s message size = %u", msg_type, vAddr.size())); return; } @@ -4215,7 +4167,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) { - Misbehaving(*peer, 20, strprintf("inv message size = %u", vInv.size())); + Misbehaving(*peer, strprintf("inv message size = %u", vInv.size())); return; } @@ -4307,7 +4259,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, vRecv >> vInv; if (vInv.size() > MAX_INV_SZ) { - Misbehaving(*peer, 20, strprintf("getdata message size = %u", vInv.size())); + Misbehaving(*peer, strprintf("getdata message size = %u", vInv.size())); return; } @@ -4847,7 +4799,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { RemoveBlockRequest(pindex->GetBlockHash(), pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(*peer, 100, "invalid compact block"); + Misbehaving(*peer, "invalid compact block"); return; } else if (status == READ_STATUS_FAILED) { if (first_in_flight) { @@ -4987,16 +4939,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, return; } - // Assume that this is in response to any outstanding getheaders - // request we may have sent, and clear out the time of our last request - peer->m_last_getheaders_timestamp = {}; - std::vector<CBlockHeader> headers; // Bypass the normal CBlock deserialization, as we don't want to risk deserializing 2000 full blocks. unsigned int nCount = ReadCompactSize(vRecv); if (nCount > MAX_HEADERS_RESULTS) { - Misbehaving(*peer, 20, strprintf("headers message size = %u", nCount)); + Misbehaving(*peer, strprintf("headers message size = %u", nCount)); return; } headers.resize(nCount); @@ -5043,7 +4991,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (prev_block && IsBlockMutated(/*block=*/*pblock, /*check_witness_root=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT))) { LogDebug(BCLog::NET, "Received mutated block from peer=%d\n", peer->m_id); - Misbehaving(*peer, 100, "mutated block"); + Misbehaving(*peer, "mutated block"); WITH_LOCK(cs_main, RemoveBlockRequest(pblock->GetHash(), peer->m_id)); return; } @@ -5224,7 +5172,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (!filter.IsWithinSizeConstraints()) { // There is no excuse for sending a too-large filter - Misbehaving(*peer, 100, "too-large bloom filter"); + Misbehaving(*peer, "too-large bloom filter"); } else if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { { LOCK(tx_relay->m_bloom_filter_mutex); @@ -5260,7 +5208,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } } if (bad) { - Misbehaving(*peer, 100, "bad filteradd message"); + Misbehaving(*peer, "bad filteradd message"); } return; } diff --git a/src/net_processing.h b/src/net_processing.h index e6430d390e..bf9698ee02 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -29,8 +29,6 @@ static const uint32_t DEFAULT_MAX_ORPHAN_TRANSACTIONS{100}; static const uint32_t DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN{100}; static const bool DEFAULT_PEERBLOOMFILTERS = false; static const bool DEFAULT_PEERBLOCKFILTERS = false; -/** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */ -static const int DISCOURAGEMENT_THRESHOLD{100}; /** Maximum number of outstanding CMPCTBLOCK requests for the same block. */ static const unsigned int MAX_CMPCTBLOCKS_INFLIGHT_PER_BLOCK = 3; @@ -108,7 +106,7 @@ public: virtual void SetBestBlock(int height, std::chrono::seconds time) = 0; /* Public for unit testing. */ - virtual void UnitTestMisbehaving(NodeId peer_id, int howmuch) = 0; + virtual void UnitTestMisbehaving(NodeId peer_id) = 0; /** * Evict extra outbound peers. If we think our tip may be stale, connect to an extra outbound. diff --git a/src/netbase.cpp b/src/netbase.cpp index ff46061d3d..f5f0997ba6 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -23,7 +23,7 @@ #include <limits> #include <memory> -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN #include <sys/un.h> #endif @@ -218,7 +218,7 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupF bool IsUnixSocketPath(const std::string& name) { -#if HAVE_SOCKADDR_UN +#ifdef HAVE_SOCKADDR_UN if (name.find(ADDR_PREFIX_UNIX) != 0) return false; // Split off "unix:" prefix @@ -487,24 +487,23 @@ bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* a } } -std::unique_ptr<Sock> CreateSockOS(sa_family_t address_family) +std::unique_ptr<Sock> CreateSockOS(int domain, int type, int protocol) { // Not IPv4, IPv6 or UNIX - if (address_family == AF_UNSPEC) return nullptr; - - int protocol{IPPROTO_TCP}; -#if HAVE_SOCKADDR_UN - if (address_family == AF_UNIX) protocol = 0; -#endif + if (domain == AF_UNSPEC) return nullptr; // Create a socket in the specified address family. - SOCKET hSocket = socket(address_family, SOCK_STREAM, protocol); + SOCKET hSocket = socket(domain, type, protocol); if (hSocket == INVALID_SOCKET) { return nullptr; } auto sock = std::make_unique<Sock>(hSocket); + if (domain != AF_INET && domain != AF_INET6 && domain != AF_UNIX) { + return sock; + } + // Ensure that waiting for I/O on this socket won't result in undefined // behavior. if (!sock->IsSelectable()) { @@ -528,19 +527,22 @@ std::unique_ptr<Sock> CreateSockOS(sa_family_t address_family) return nullptr; } -#if HAVE_SOCKADDR_UN - if (address_family == AF_UNIX) return sock; +#ifdef HAVE_SOCKADDR_UN + if (domain == AF_UNIX) return sock; #endif - // Set the no-delay option (disable Nagle's algorithm) on the TCP socket. - const int on{1}; - if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) { - LogPrint(BCLog::NET, "Unable to set TCP_NODELAY on a newly created socket, continuing anyway\n"); + if (protocol == IPPROTO_TCP) { + // Set the no-delay option (disable Nagle's algorithm) on the TCP socket. + const int on{1}; + if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) { + LogPrint(BCLog::NET, "Unable to set TCP_NODELAY on a newly created socket, continuing anyway\n"); + } } + return sock; } -std::function<std::unique_ptr<Sock>(const sa_family_t&)> CreateSock = CreateSockOS; +std::function<std::unique_ptr<Sock>(int, int, int)> CreateSock = CreateSockOS; template<typename... Args> static void LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args) { @@ -609,7 +611,7 @@ static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connection) { - auto sock = CreateSock(dest.GetSAFamily()); + auto sock = CreateSock(dest.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP); if (!sock) { LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Cannot create a socket for connecting to %s\n", dest.ToStringAddrPort()); return {}; @@ -636,8 +638,8 @@ std::unique_ptr<Sock> Proxy::Connect() const if (!m_is_unix_socket) return ConnectDirectly(proxy, /*manual_connection=*/true); -#if HAVE_SOCKADDR_UN - auto sock = CreateSock(AF_UNIX); +#ifdef HAVE_SOCKADDR_UN + auto sock = CreateSock(AF_UNIX, SOCK_STREAM, 0); if (!sock) { LogPrintLevel(BCLog::NET, BCLog::Level::Error, "Cannot create a socket for connecting to %s\n", m_unix_socket_path); return {}; diff --git a/src/netbase.h b/src/netbase.h index 321c288f67..8ef6c28996 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -262,16 +262,18 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLoo CSubNet LookupSubNet(const std::string& subnet_str); /** - * Create a TCP or UNIX socket in the given address family. - * @param[in] address_family to use for the socket. + * Create a real socket from the operating system. + * @param[in] domain Communications domain, first argument to the socket(2) syscall. + * @param[in] type Type of the socket, second argument to the socket(2) syscall. + * @param[in] protocol The particular protocol to be used with the socket, third argument to the socket(2) syscall. * @return pointer to the created Sock object or unique_ptr that owns nothing in case of failure */ -std::unique_ptr<Sock> CreateSockOS(sa_family_t address_family); +std::unique_ptr<Sock> CreateSockOS(int domain, int type, int protocol); /** * Socket factory. Defaults to `CreateSockOS()`, but can be overridden by unit tests. */ -extern std::function<std::unique_ptr<Sock>(const sa_family_t&)> CreateSock; +extern std::function<std::unique_ptr<Sock>(int, int, int)> CreateSock; /** * Create a socket and try to connect to the specified service. diff --git a/src/randomenv.cpp b/src/randomenv.cpp index aeec959c28..49033deef2 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -42,15 +42,15 @@ #if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS #include <ifaddrs.h> #endif -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL #include <sys/sysctl.h> -#if HAVE_VM_VM_PARAM_H +#ifdef HAVE_VM_VM_PARAM_H #include <vm/vm_param.h> #endif -#if HAVE_SYS_RESOURCES_H +#ifdef HAVE_SYS_RESOURCES_H #include <sys/resources.h> #endif -#if HAVE_SYS_VMMETER_H +#ifdef HAVE_SYS_VMMETER_H #include <sys/vmmeter.h> #endif #endif @@ -162,7 +162,7 @@ void AddPath(CSHA512& hasher, const char *path) } #endif -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL template<int... S> void AddSysctl(CSHA512& hasher) { @@ -274,7 +274,7 @@ void RandAddDynamicEnv(CSHA512& hasher) AddFile(hasher, "/proc/self/status"); #endif -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL # ifdef CTL_KERN # if defined(KERN_PROC) && defined(KERN_PROC_ALL) AddSysctl<CTL_KERN, KERN_PROC, KERN_PROC_ALL>(hasher); @@ -419,7 +419,7 @@ void RandAddStaticEnv(CSHA512& hasher) // For MacOS/BSDs, gather data through sysctl instead of /proc. Not all of these // will exist on every system. -#if HAVE_SYSCTL +#ifdef HAVE_SYSCTL # ifdef CTL_HW # ifdef HW_MACHINE AddSysctl<CTL_HW, HW_MACHINE>(hasher); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index e493d532a5..42db28daf5 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -330,7 +330,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(*nodes[0], NODE_NETWORK); nodes[0]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[0]); - peerLogic->UnitTestMisbehaving(nodes[0]->GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged + peerLogic->UnitTestMisbehaving(nodes[0]->GetId()); // Should be discouraged BOOST_CHECK(peerLogic->SendMessages(nodes[0])); BOOST_CHECK(banman->IsDiscouraged(addr[0])); @@ -350,7 +350,6 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(*nodes[1], NODE_NETWORK); nodes[1]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[1]); - peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1); BOOST_CHECK(peerLogic->SendMessages(nodes[1])); // [0] is still discouraged/disconnected. BOOST_CHECK(banman->IsDiscouraged(addr[0])); @@ -358,7 +357,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) // [1] is not discouraged/disconnected yet. BOOST_CHECK(!banman->IsDiscouraged(addr[1])); BOOST_CHECK(!nodes[1]->fDisconnect); - peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), 1); // [1] reaches discouragement threshold + peerLogic->UnitTestMisbehaving(nodes[1]->GetId()); BOOST_CHECK(peerLogic->SendMessages(nodes[1])); // Expect both [0] and [1] to be discouraged/disconnected now. BOOST_CHECK(banman->IsDiscouraged(addr[0])); @@ -381,7 +380,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement) peerLogic->InitializeNode(*nodes[2], NODE_NETWORK); nodes[2]->fSuccessfullyConnected = true; connman->AddTestNode(*nodes[2]); - peerLogic->UnitTestMisbehaving(nodes[2]->GetId(), DISCOURAGEMENT_THRESHOLD); + peerLogic->UnitTestMisbehaving(nodes[2]->GetId()); BOOST_CHECK(peerLogic->SendMessages(nodes[2])); BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[1])); @@ -423,7 +422,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) peerLogic->InitializeNode(dummyNode, NODE_NETWORK); dummyNode.fSuccessfullyConnected = true; - peerLogic->UnitTestMisbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); + peerLogic->UnitTestMisbehaving(dummyNode.GetId()); BOOST_CHECK(peerLogic->SendMessages(&dummyNode)); BOOST_CHECK(banman->IsDiscouraged(addr)); diff --git a/src/test/fuzz/fuzz.cpp b/src/test/fuzz/fuzz.cpp index 9a54a44bd3..c1c9945a04 100644 --- a/src/test/fuzz/fuzz.cpp +++ b/src/test/fuzz/fuzz.cpp @@ -101,8 +101,9 @@ void ResetCoverageCounters() {} void initialize() { - // Terminate immediately if a fuzzing harness ever tries to create a TCP socket. - CreateSock = [](const sa_family_t&) -> std::unique_ptr<Sock> { std::terminate(); }; + // Terminate immediately if a fuzzing harness ever tries to create a socket. + // Individual tests can override this by pointing CreateSock to a mocked alternative. + CreateSock = [](int, int, int) -> std::unique_ptr<Sock> { std::terminate(); }; // Terminate immediately if a fuzzing harness ever tries to perform a DNS lookup. g_dns_lookup = [](const std::string& name, bool allow_lookup) { diff --git a/src/test/fuzz/i2p.cpp b/src/test/fuzz/i2p.cpp index 3af5bed30a..51517187a0 100644 --- a/src/test/fuzz/i2p.cpp +++ b/src/test/fuzz/i2p.cpp @@ -27,7 +27,7 @@ FUZZ_TARGET(i2p, .init = initialize_i2p) // Mock CreateSock() to create FuzzedSock. auto CreateSockOrig = CreateSock; - CreateSock = [&fuzzed_data_provider](const sa_family_t&) { + CreateSock = [&fuzzed_data_provider](int, int, int) { return std::make_unique<FuzzedSock>(fuzzed_data_provider); }; diff --git a/src/test/fuzz/rbf.cpp b/src/test/fuzz/rbf.cpp index 4c7e70e3b0..eb981352ec 100644 --- a/src/test/fuzz/rbf.cpp +++ b/src/test/fuzz/rbf.cpp @@ -91,8 +91,10 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); SetMockTime(ConsumeTime(fuzzed_data_provider)); - std::optional<CMutableTransaction> child = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS); - if (!child) return; + // "Real" virtual size is not important for this test since ConsumeTxMemPoolEntry generates its own virtual size values + // so we construct small transactions for performance reasons. Child simply needs an input for later to perhaps connect to parent. + CMutableTransaction child; + child.vin.resize(1); bilingual_str error; CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node), error}; @@ -113,15 +115,13 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), NUM_ITERS) { // Make sure txns only have one input, and that a unique input is given to avoid circular references - std::optional<CMutableTransaction> parent = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider, TX_WITH_WITNESS); - if (!parent) { - return; - } + CMutableTransaction parent; assert(iter <= g_outpoints.size()); - parent->vin.resize(1); - parent->vin[0].prevout = g_outpoints[iter++]; + parent.vin.resize(1); + parent.vin[0].prevout = g_outpoints[iter++]; + parent.vout.emplace_back(0, CScript()); - mempool_txs.emplace_back(*parent); + mempool_txs.emplace_back(parent); const auto parent_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); running_vsize_total += parent_entry.GetTxSize(); if (running_vsize_total > std::numeric_limits<int32_t>::max()) { @@ -130,10 +130,10 @@ FUZZ_TARGET(package_rbf, .init = initialize_package_rbf) break; } pool.addUnchecked(parent_entry); - if (fuzzed_data_provider.ConsumeBool() && !child->vin.empty()) { - child->vin[0].prevout = COutPoint{mempool_txs.back().GetHash(), 0}; + if (fuzzed_data_provider.ConsumeBool()) { + child.vin[0].prevout = COutPoint{mempool_txs.back().GetHash(), 0}; } - mempool_txs.emplace_back(*child); + mempool_txs.emplace_back(child); const auto child_entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, mempool_txs.back()); running_vsize_total += child_entry.GetTxSize(); if (running_vsize_total > std::numeric_limits<int32_t>::max()) { diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp index d7249d88f4..0512c6134f 100644 --- a/src/test/i2p_tests.cpp +++ b/src/test/i2p_tests.cpp @@ -39,15 +39,14 @@ public: private: const BCLog::Level m_prev_log_level; - const std::function<std::unique_ptr<Sock>(const sa_family_t&)> m_create_sock_orig; + const decltype(CreateSock) m_create_sock_orig; }; BOOST_FIXTURE_TEST_SUITE(i2p_tests, EnvTestingSetup) BOOST_AUTO_TEST_CASE(unlimited_recv) { - // Mock CreateSock() to create MockSock. - CreateSock = [](const sa_family_t&) { + CreateSock = [](int, int, int) { return std::make_unique<StaticContentsSock>(std::string(i2p::sam::MAX_MSG_SIZE + 1, 'a')); }; @@ -69,7 +68,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv) BOOST_AUTO_TEST_CASE(listen_ok_accept_fail) { size_t num_sockets{0}; - CreateSock = [&num_sockets](const sa_family_t&) { + CreateSock = [&num_sockets](int, int, int) { // clang-format off ++num_sockets; // First socket is the control socket for creating the session. @@ -133,9 +132,7 @@ BOOST_AUTO_TEST_CASE(listen_ok_accept_fail) BOOST_AUTO_TEST_CASE(damaged_private_key) { - const auto CreateSockOrig = CreateSock; - - CreateSock = [](const sa_family_t&) { + CreateSock = [](int, int, int) { return std::make_unique<StaticContentsSock>("HELLO REPLY RESULT=OK VERSION=3.1\n" "SESSION STATUS RESULT=OK DESTINATION=\n"); }; @@ -172,8 +169,6 @@ BOOST_AUTO_TEST_CASE(damaged_private_key) BOOST_CHECK(!session.Connect(CService{}, conn, proxy_error)); } } - - CreateSock = CreateSockOrig; } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 11d5a8ae55..34176626f0 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -72,17 +72,16 @@ static std::map<std::string, unsigned int> mapFlagNames = { unsigned int ParseScriptFlags(std::string strFlags) { - if (strFlags.empty() || strFlags == "NONE") return 0; - unsigned int flags = 0; - std::vector<std::string> words = SplitString(strFlags, ','); + unsigned int flags = SCRIPT_VERIFY_NONE; + if (strFlags.empty() || strFlags == "NONE") return flags; + std::vector<std::string> words = SplitString(strFlags, ','); for (const std::string& word : words) { if (!mapFlagNames.count(word)) BOOST_ERROR("Bad test: unknown verification flag '" << word << "'"); flags |= mapFlagNames[word]; } - return flags; } @@ -98,7 +97,7 @@ bool CheckMapFlagNames() std::string FormatScriptFlags(unsigned int flags) { - if (flags == 0) { + if (flags == SCRIPT_VERIFY_NONE) { return ""; } std::string ret; @@ -370,6 +369,41 @@ BOOST_AUTO_TEST_CASE(tx_invalid) } } +BOOST_AUTO_TEST_CASE(tx_no_inputs) +{ + CMutableTransaction empty; + + TxValidationState state; + BOOST_CHECK_MESSAGE(!CheckTransaction(CTransaction(empty), state), "Transaction with no inputs should be invalid."); + BOOST_CHECK(state.GetRejectReason() == "bad-txns-vin-empty"); +} + +BOOST_AUTO_TEST_CASE(tx_oversized) +{ + auto createTransaction =[](size_t payloadSize) { + CMutableTransaction tx; + tx.vin.resize(1); + tx.vout.emplace_back(1, CScript() << OP_RETURN << std::vector<unsigned char>(payloadSize)); + return CTransaction(tx); + }; + const auto maxTransactionSize = MAX_BLOCK_WEIGHT / WITNESS_SCALE_FACTOR; + const auto oversizedTransactionBaseSize = ::GetSerializeSize(TX_NO_WITNESS(createTransaction(maxTransactionSize))) - maxTransactionSize; + + auto maxPayloadSize = maxTransactionSize - oversizedTransactionBaseSize; + { + TxValidationState state; + CheckTransaction(createTransaction(maxPayloadSize), state); + BOOST_CHECK(state.GetRejectReason() != "bad-txns-oversize"); + } + + maxPayloadSize += 1; + { + TxValidationState state; + BOOST_CHECK_MESSAGE(!CheckTransaction(createTransaction(maxPayloadSize), state), "Oversized transaction should be invalid"); + BOOST_CHECK(state.GetRejectReason() == "bad-txns-oversize"); + } +} + BOOST_AUTO_TEST_CASE(basic_transaction_tests) { // Random real transaction (e2769b09e784f32f62ef849763d4f45b98e07ba658647343b915ff832b110436) @@ -615,11 +649,11 @@ BOOST_AUTO_TEST_CASE(test_witness) // Normal pay-to-compressed-pubkey. CreateCreditAndSpend(keystore, scriptPubkey1, output1, input1); CreateCreditAndSpend(keystore, scriptPubkey2, output2, input2); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); - CheckWithFlag(output1, input2, 0, false); + CheckWithFlag(output1, input2, SCRIPT_VERIFY_NONE, false); CheckWithFlag(output1, input2, SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); @@ -628,11 +662,11 @@ BOOST_AUTO_TEST_CASE(test_witness) CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(scriptPubkey1)), output1, input1); CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(scriptPubkey2)), output2, input2); ReplaceRedeemScript(input2.vin[0].scriptSig, scriptPubkey1); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); - CheckWithFlag(output1, input2, 0, true); + CheckWithFlag(output1, input2, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input2, SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); @@ -640,11 +674,11 @@ BOOST_AUTO_TEST_CASE(test_witness) // Witness pay-to-compressed-pubkey (v0). CreateCreditAndSpend(keystore, destination_script_1, output1, input1); CreateCreditAndSpend(keystore, destination_script_2, output2, input2); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); - CheckWithFlag(output1, input2, 0, true); + CheckWithFlag(output1, input2, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input2, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input2, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); @@ -653,11 +687,11 @@ BOOST_AUTO_TEST_CASE(test_witness) CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_1)), output1, input1); CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(destination_script_2)), output2, input2); ReplaceRedeemScript(input2.vin[0].scriptSig, destination_script_1); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); - CheckWithFlag(output1, input2, 0, true); + CheckWithFlag(output1, input2, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input2, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input2, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); @@ -665,11 +699,11 @@ BOOST_AUTO_TEST_CASE(test_witness) // Normal pay-to-uncompressed-pubkey. CreateCreditAndSpend(keystore, scriptPubkey1L, output1, input1); CreateCreditAndSpend(keystore, scriptPubkey2L, output2, input2); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); - CheckWithFlag(output1, input2, 0, false); + CheckWithFlag(output1, input2, SCRIPT_VERIFY_NONE, false); CheckWithFlag(output1, input2, SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); @@ -678,11 +712,11 @@ BOOST_AUTO_TEST_CASE(test_witness) CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(scriptPubkey1L)), output1, input1); CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(scriptPubkey2L)), output2, input2); ReplaceRedeemScript(input2.vin[0].scriptSig, scriptPubkey1L); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, true); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); - CheckWithFlag(output1, input2, 0, true); + CheckWithFlag(output1, input2, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input2, SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_P2SH, false); CheckWithFlag(output1, input2, STANDARD_SCRIPT_VERIFY_FLAGS, false); @@ -697,19 +731,19 @@ BOOST_AUTO_TEST_CASE(test_witness) // Normal 2-of-2 multisig CreateCreditAndSpend(keystore, scriptMulti, output1, input1, false); - CheckWithFlag(output1, input1, 0, false); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, false); CreateCreditAndSpend(keystore2, scriptMulti, output2, input2, false); - CheckWithFlag(output2, input2, 0, false); + CheckWithFlag(output2, input2, SCRIPT_VERIFY_NONE, false); BOOST_CHECK(*output1 == *output2); UpdateInput(input1.vin[0], CombineSignatures(input1, input2, output1)); CheckWithFlag(output1, input1, STANDARD_SCRIPT_VERIFY_FLAGS, true); // P2SH 2-of-2 multisig CreateCreditAndSpend(keystore, GetScriptForDestination(ScriptHash(scriptMulti)), output1, input1, false); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH, false); CreateCreditAndSpend(keystore2, GetScriptForDestination(ScriptHash(scriptMulti)), output2, input2, false); - CheckWithFlag(output2, input2, 0, true); + CheckWithFlag(output2, input2, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH, false); BOOST_CHECK(*output1 == *output2); UpdateInput(input1.vin[0], CombineSignatures(input1, input2, output1)); @@ -718,10 +752,10 @@ BOOST_AUTO_TEST_CASE(test_witness) // Witness 2-of-2 multisig CreateCreditAndSpend(keystore, destination_script_multi, output1, input1, false); - CheckWithFlag(output1, input1, 0, true); + CheckWithFlag(output1, input1, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output1, input1, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); CreateCreditAndSpend(keystore2, destination_script_multi, output2, input2, false); - CheckWithFlag(output2, input2, 0, true); + CheckWithFlag(output2, input2, SCRIPT_VERIFY_NONE, true); CheckWithFlag(output2, input2, SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS, false); BOOST_CHECK(*output1 == *output2); UpdateInput(input1.vin[0], CombineSignatures(input1, input2, output1)); diff --git a/src/validation.cpp b/src/validation.cpp index c34d60f137..3e9ba08bb1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1849,7 +1849,6 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTransactionRef& tx, int64_t accept_time, bool bypass_limits, bool test_accept) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); const CChainParams& chainparams{active_chainstate.m_chainman.GetParams()}; diff --git a/src/wallet/migrate.cpp b/src/wallet/migrate.cpp index 09254a76ad..d7d8577374 100644 --- a/src/wallet/migrate.cpp +++ b/src/wallet/migrate.cpp @@ -551,7 +551,7 @@ void BerkeleyRODatabase::Open() // } // Check the last page number - uint32_t expected_last_page = (size / page_size) - 1; + uint32_t expected_last_page{uint32_t((size / page_size) - 1)}; if (outer_meta.last_page != expected_last_page) { throw std::runtime_error("Last page number could not fit in file"); } diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index eb23c4555b..67b5ae0fe2 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -179,7 +179,7 @@ void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& st } } -void AppendLastProcessedBlock(UniValue& entry, const CWallet& wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +void AppendLastProcessedBlock(UniValue& entry, const CWallet& wallet) { AssertLockHeld(wallet.cs_wallet); UniValue lastprocessedblock{UniValue::VOBJ}; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 4cbcfdb60f..b9b4666208 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -260,7 +260,7 @@ static OutputType GetOutputType(TxoutType type, bool is_from_p2sh) // Fetch and validate the coin control selected inputs. // Coins could be internal (from the wallet) or external. util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const CCoinControl& coin_control, - const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) + const CoinSelectionParams& coin_selection_params) { PreSelectedInputs result; const bool can_grind_r = wallet.CanGrindR(); diff --git a/src/wallet/test/fuzz/wallet_bdb_parser.cpp b/src/wallet/test/fuzz/wallet_bdb_parser.cpp index 5216e09769..6fbd695fc5 100644 --- a/src/wallet/test/fuzz/wallet_bdb_parser.cpp +++ b/src/wallet/test/fuzz/wallet_bdb_parser.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2023 The Bitcoin Core developers +// Copyright (c) 2023-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -63,6 +63,7 @@ FUZZ_TARGET(wallet_bdb_parser, .init = initialize_wallet_bdb_parser) #endif if (error.original.starts_with("AutoFile::ignore: end of file") || error.original.starts_with("AutoFile::read: end of file") || + error.original.starts_with("AutoFile::seek: ") || error.original == "Not a BDB file" || error.original == "Unexpected page type, should be 9 (BTree Metadata)" || error.original == "Unexpected database flags, should only be 0x20 (subdatabases)" || diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 85cd67dab9..d569c64b43 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1366,13 +1366,13 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c } -void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { +void CWallet::RecursiveUpdateTxState(const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) { // Do not flush the wallet here for performance reasons WalletBatch batch(GetDatabase(), false); RecursiveUpdateTxState(&batch, tx_hash, try_updating_state); } -void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { +void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const uint256& tx_hash, const TryUpdatingStateFn& try_updating_state) { std::set<uint256> todo; std::set<uint256> done; diff --git a/test/functional/feature_settings.py b/test/functional/feature_settings.py index 0214e781de..1cd0aeabd3 100755 --- a/test/functional/feature_settings.py +++ b/test/functional/feature_settings.py @@ -25,7 +25,7 @@ class SettingsTest(BitcoinTestFramework): # Assert default settings file was created self.stop_node(0) - default_settings = {"_warning_": "This file is automatically generated and updated by Bitcoin Core. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."} + default_settings = {"_warning_": f"This file is automatically generated and updated by {self.config['environment']['PACKAGE_NAME']}. Please do not edit this file while the node is running, as any changes might be ignored or overwritten."} with settings.open() as fp: assert_equal(json.load(fp), default_settings) diff --git a/test/functional/mempool_accept.py b/test/functional/mempool_accept.py index 3d205ffa62..e1cee46839 100755 --- a/test/functional/mempool_accept.py +++ b/test/functional/mempool_accept.py @@ -18,6 +18,7 @@ from test_framework.messages import ( CTxInWitness, CTxOut, MAX_BLOCK_WEIGHT, + WITNESS_SCALE_FACTOR, MAX_MONEY, SEQUENCE_FINAL, tx_from_hex, @@ -228,7 +229,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework): self.log.info('A really large transaction') tx = tx_from_hex(raw_tx_reference) - tx.vin = [tx.vin[0]] * math.ceil(MAX_BLOCK_WEIGHT // 4 / len(tx.vin[0].serialize())) + tx.vin = [tx.vin[0]] * math.ceil((MAX_BLOCK_WEIGHT // WITNESS_SCALE_FACTOR) / len(tx.vin[0].serialize())) self.check_mempool_result( result_expected=[{'txid': tx.rehash(), 'allowed': False, 'reject-reason': 'bad-txns-oversize'}], rawtxs=[tx.serialize().hex()], diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index b23ec1028b..d10e47e036 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -142,7 +142,8 @@ class AddrTest(BitcoinTestFramework): msg = self.setup_addr_msg(1010) with self.nodes[0].assert_debug_log(['addr message size = 1010']): - addr_source.send_and_ping(msg) + addr_source.send_message(msg) + addr_source.wait_for_disconnect() self.nodes[0].disconnect_p2ps() diff --git a/test/functional/p2p_addrv2_relay.py b/test/functional/p2p_addrv2_relay.py index ea114e7d70..4ec8e0bc04 100755 --- a/test/functional/p2p_addrv2_relay.py +++ b/test/functional/p2p_addrv2_relay.py @@ -86,11 +86,6 @@ class AddrTest(BitcoinTestFramework): addr_source = self.nodes[0].add_p2p_connection(P2PInterface()) msg = msg_addrv2() - self.log.info('Send too-large addrv2 message') - msg.addrs = ADDRS * 101 - with self.nodes[0].assert_debug_log(['addrv2 message size = 1010']): - addr_source.send_and_ping(msg) - self.log.info('Check that addrv2 message content is relayed and added to addrman') addr_receiver = self.nodes[0].add_p2p_connection(AddrReceiver()) msg.addrs = ADDRS @@ -106,6 +101,13 @@ class AddrTest(BitcoinTestFramework): assert addr_receiver.addrv2_received_and_checked assert_equal(len(self.nodes[0].getnodeaddresses(count=0, network="i2p")), 0) + self.log.info('Send too-large addrv2 message') + msg.addrs = ADDRS * 101 + with self.nodes[0].assert_debug_log(['addrv2 message size = 1010']): + addr_source.send_message(msg) + addr_source.wait_for_disconnect() + + if __name__ == '__main__': AddrTest().main() diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index adcbb4fd05..8e459ba676 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -260,7 +260,9 @@ class InvalidMessagesTest(BitcoinTestFramework): msg_type = msg.msgtype.decode('ascii') self.log.info("Test {} message of size {} is logged as misbehaving".format(msg_type, size)) with self.nodes[0].assert_debug_log(['Misbehaving', '{} message size = {}'.format(msg_type, size)]): - self.nodes[0].add_p2p_connection(P2PInterface()).send_and_ping(msg) + conn = self.nodes[0].add_p2p_connection(P2PInterface()) + conn.send_message(msg) + conn.wait_for_disconnect() self.nodes[0].disconnect_p2ps() def test_oversized_inv_msg(self): @@ -321,7 +323,8 @@ class InvalidMessagesTest(BitcoinTestFramework): # delete arbitrary block header somewhere in the middle to break link del block_headers[random.randrange(1, len(block_headers)-1)] with self.nodes[0].assert_debug_log(expected_msgs=MISBEHAVING_NONCONTINUOUS_HEADERS_MSGS): - peer.send_and_ping(msg_headers(block_headers)) + peer.send_message(msg_headers(block_headers)) + peer.wait_for_disconnect() self.nodes[0].disconnect_p2ps() def test_resource_exhaustion(self): diff --git a/test/functional/p2p_mutated_blocks.py b/test/functional/p2p_mutated_blocks.py index 64d2fc96a8..708b19b1e5 100755 --- a/test/functional/p2p_mutated_blocks.py +++ b/test/functional/p2p_mutated_blocks.py @@ -104,11 +104,10 @@ class MutatedBlocksTest(BitcoinTestFramework): block_missing_prev.hashPrevBlock = 123 block_missing_prev.solve() - # Attacker gets a DoS score of 10, not immediately disconnected, so we do it 10 times to get to 100 - for _ in range(10): - assert_equal(len(self.nodes[0].getpeerinfo()), 2) - with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]): - attacker.send_message(msg_block(block_missing_prev)) + # Check that non-connecting block causes disconnect + assert_equal(len(self.nodes[0].getpeerinfo()), 2) + with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]): + attacker.send_message(msg_block(block_missing_prev)) attacker.wait_for_disconnect(timeout=5) diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 27a3aa8fb9..5c463267a1 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -71,19 +71,13 @@ f. Announce 1 more header that builds on that fork. Expect: no response. Part 5: Test handling of headers that don't connect. -a. Repeat 10 times: +a. Repeat 100 times: 1. Announce a header that doesn't connect. Expect: getheaders message 2. Send headers chain. Expect: getdata for the missing blocks, tip update. -b. Then send 9 more headers that don't connect. +b. Then send 99 more headers that don't connect. Expect: getheaders message each time. -c. Announce a header that does connect. - Expect: no response. -d. Announce 49 headers that don't connect. - Expect: getheaders message each time. -e. Announce one more that doesn't connect. - Expect: disconnect. """ from test_framework.blocktools import create_block, create_coinbase from test_framework.messages import CInv @@ -526,7 +520,8 @@ class SendHeadersTest(BitcoinTestFramework): # First we test that receipt of an unconnecting header doesn't prevent # chain sync. expected_hash = tip - for i in range(10): + NUM_HEADERS = 100 + for i in range(NUM_HEADERS): self.log.debug("Part 5.{}: starting...".format(i)) test_node.last_message.pop("getdata", None) blocks = [] @@ -550,41 +545,24 @@ class SendHeadersTest(BitcoinTestFramework): blocks = [] # Now we test that if we repeatedly don't send connecting headers, we # don't go into an infinite loop trying to get them to connect. - MAX_NUM_UNCONNECTING_HEADERS_MSGS = 10 - for _ in range(MAX_NUM_UNCONNECTING_HEADERS_MSGS + 1): + for _ in range(NUM_HEADERS + 1): blocks.append(create_block(tip, create_coinbase(height), block_time)) blocks[-1].solve() tip = blocks[-1].sha256 block_time += 1 height += 1 - for i in range(1, MAX_NUM_UNCONNECTING_HEADERS_MSGS): - # Send a header that doesn't connect, check that we get a getheaders. + for i in range(1, NUM_HEADERS): + with p2p_lock: + test_node.last_message.pop("getheaders", None) + # Send an empty header as a failed response to the received getheaders + # (from the previous iteration). Otherwise, the new headers will be + # treated as a response instead of as an announcement. + test_node.send_header_for_blocks([]) + # Send the actual unconnecting header, which should trigger a new getheaders. test_node.send_header_for_blocks([blocks[i]]) test_node.wait_for_getheaders(block_hash=expected_hash) - # Next header will connect, should re-set our count: - test_node.send_header_for_blocks([blocks[0]]) - expected_hash = blocks[0].sha256 - - # Remove the first two entries (blocks[1] would connect): - blocks = blocks[2:] - - # Now try to see how many unconnecting headers we can send - # before we get disconnected. Should be 5*MAX_NUM_UNCONNECTING_HEADERS_MSGS - for i in range(5 * MAX_NUM_UNCONNECTING_HEADERS_MSGS - 1): - # Send a header that doesn't connect, check that we get a getheaders. - test_node.send_header_for_blocks([blocks[i % len(blocks)]]) - test_node.wait_for_getheaders(block_hash=expected_hash) - - # Eventually this stops working. - test_node.send_header_for_blocks([blocks[-1]]) - - # Should get disconnected - test_node.wait_for_disconnect() - - self.log.info("Part 5: success!") - # Finally, check that the inv node never received a getdata request, # throughout the test assert "getdata" not in inv_node.last_message diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py index f368434895..776eaf5255 100755 --- a/test/functional/p2p_unrequested_blocks.py +++ b/test/functional/p2p_unrequested_blocks.py @@ -170,9 +170,11 @@ class AcceptBlockTest(BitcoinTestFramework): tip = next_block # Now send the block at height 5 and check that it wasn't accepted (missing header) - test_node.send_and_ping(msg_block(all_blocks[1])) + test_node.send_message(msg_block(all_blocks[1])) + test_node.wait_for_disconnect() assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblock, all_blocks[1].hash) assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getblockheader, all_blocks[1].hash) + test_node = self.nodes[0].add_p2p_connection(P2PInterface()) # The block at height 5 should be accepted if we provide the missing header, though headers_message = msg_headers() |