diff --git a/.github/workflows/c-cpp.yml b/.github/workflows/c-cpp.yml index 878c2473..bb494836 100644 --- a/.github/workflows/c-cpp.yml +++ b/.github/workflows/c-cpp.yml @@ -2,7 +2,7 @@ name: C/C++ CI on: push: - branches: ["master", "kryptokrona"] + branches: ["master"] tags: ["v*"] paths-ignore: - 'docker-compose/**' @@ -20,17 +20,43 @@ jobs: runs-on: ubuntu-latest strategy: + fail-fast: false matrix: config: - {arch: x86_64, branch: latest-stable, flags: "-ffunction-sections"} - {arch: aarch64, branch: latest-stable, flags: "-ffunction-sections -mfix-cortex-a53-835769 -mfix-cortex-a53-843419"} steps: + # setup-alpine downloads apk.static from gitlab.alpinelinux.org by default, + # which is frequently unreachable from GitHub's Azure runners (curl connect + # timeout) and fails the whole job. Pre-fetch apk.static from the Alpine CDN + # (always reachable) into $RUNNER_TEMP/apk. setup-alpine's download_file() + # sees an existing file whose sha256 matches apk-tools-url and skips its own + # flaky download entirely. + # + # apk.static ALWAYS has to be the x86_64 (host) binary, even for the aarch64 + # matrix entry: the action runs it ON THE RUNNER to build the rootfs and to + # fetch the qemu- emulator (apk itself cross-fetches other arches via + # --arch). Fetching the aarch64 apk.static here made the amd64 host choke with + # "cannot execute binary file: Exec format error". + - name: Prefetch apk.static from Alpine CDN (avoid flaky gitlab.alpinelinux.org) + shell: bash + run: | + set -euo pipefail + ARCH='x86_64' + BASE="https://dl-cdn.alpinelinux.org/alpine/${{ matrix.config.branch }}/main/$ARCH" + FILE=$(curl -fsSL --retry 6 --retry-all-errors "$BASE/" | grep -oE 'apk-tools-static-[0-9][^"]*\.apk' | sort -V | tail -1) + curl -fsSL --retry 6 --retry-all-errors -o "$RUNNER_TEMP/pkg.apk" "$BASE/$FILE" + tar -xzf "$RUNNER_TEMP/pkg.apk" -C "$RUNNER_TEMP" sbin/apk.static + install -m755 "$RUNNER_TEMP/sbin/apk.static" "$RUNNER_TEMP/apk" + echo "APK_STATIC_SHA=$(sha256sum "$RUNNER_TEMP/apk" | cut -d' ' -f1)" >> "$GITHUB_ENV" + - name: Setup Alpine Linux uses: jirutka/setup-alpine@v1 with: arch: ${{ matrix.config.arch }} branch: ${{ matrix.config.branch }} + apk-tools-url: 'https://dl-cdn.alpinelinux.org/unused#!sha256!${{ env.APK_STATIC_SHA }}' - name: Install dependencies shell: alpine.sh --root {0} diff --git a/src/p2p_server.cpp b/src/p2p_server.cpp index 62e1e14a..99b13e2c 100644 --- a/src/p2p_server.cpp +++ b/src/p2p_server.cpp @@ -57,7 +57,19 @@ static const char* seed_nodes_mini[] = { "" }; static const char* seed_nodes_nano[] = { "" }; static constexpr int DEFAULT_BACKLOG = 16; -static constexpr uint64_t DEFAULT_BAN_TIME = 600; +// Keep bans short: peering matters more than abuse-protection for the XKR +// sidechain right now. A node that restarts and re-syncs from a low sidechain +// height temporarily can't verify honest peers' valid blocks (its own mainchain +// view is still catching up), so it must not lock them out for long. 60s lets +// the network re-mesh within a minute instead of 10. +static constexpr uint64_t DEFAULT_BAN_TIME = 60; +// Number of unacceptable blocks a peer may send before it is actually banned. +// Block-verification failures are usually transient during sync (a peer's valid +// block we can't verify yet), not abuse, so tolerate several before banning. +static constexpr uint32_t BAN_FAULT_THRESHOLD = 5; +// Force a hard RPC reconnect if no fresh miner data has arrived for this long. A +// healthy poll refreshes it every ~2s, so 60s unambiguously means the RPC wedged. +static constexpr uint64_t RPC_HARD_RECONNECT_TIMEOUT = 60; static constexpr uint64_t PEER_REQUEST_DELAY = 60; static constexpr uint64_t MAX_PENDING_BLOCK_REQUESTS = 25; static constexpr uint64_t MAX_PENDING_MONERO_BLOCK_BROADCASTS = 60; @@ -1699,10 +1711,19 @@ void P2PServer::check_host() LOGINFO(6, "polling daemon for new miner data"); m_pool->reconnect_to_host(); - // If no fresh data has arrived from the node in 5 minutes, it's probably - // stuck or unreachable. + // A healthy poll refreshes m_lastActive roughly every couple of seconds, so a + // long gap means the RPC is wedged (an orphaned request whose curl socket poll + // died and never times out). The in-flight overlap guard often can't recover + // from this on its own, so force a hard reconnect: reconnect_to_host() will + // clear the stuck pending flag and issue a fresh request on a new connection. const uint64_t cur_time = seconds_since_epoch(); const uint64_t last_active = m_pool->last_active(); + if (last_active && (cur_time >= last_active + RPC_HARD_RECONNECT_TIMEOUT)) { + m_pool->request_rpc_reconnect(); + } + + // If no fresh data has arrived from the node in 5 minutes, it's probably + // stuck or unreachable. if (last_active && (cur_time >= last_active + 300)) { const uint64_t dt = static_cast(cur_time - last_active); const Params::Host& host = m_pool->current_host(); @@ -2180,6 +2201,7 @@ void P2PServer::P2PClient::reset() m_connectedTime = 0; m_connectedDomain = false; m_broadcastMaxHeight = 0; + m_banScore = 0; m_expectedMessage = MessageId::HANDSHAKE_CHALLENGE; m_handshakeChallenge = 0; m_handshakeSolutionSent = false; @@ -2642,9 +2664,18 @@ void P2PServer::P2PClient::on_disconnected() m_pingTime = -1; if (!m_handshakeComplete) { - LOGWARN(5, "peer " << static_cast(m_addrString) << " disconnected before finishing handshake"); + // A connection that drops before the handshake finishes is almost always + // transient on a small mesh, not abuse: the remote reset us (ECONNRESET), + // a simultaneous inbound/outbound connect got de-duplicated ("already + // connected as ..."), the seed was momentarily full, a NAT mapping + // blinked, etc. The challenge/response handshake is itself the gate + // against junk peers, so there is nothing to protect against yet here. + // Banning on this was making nodes permanently blacklist their own seed + // nodes (every retry raced/reset the same way and re-armed the ban), + // which fractured the mesh into isolated single-node chains. Just drop + // it from the candidate list; it is retried on the next connect cycle. + LOGWARN(5, "peer " << static_cast(m_addrString) << " disconnected before finishing handshake (not banning)"); - ban(DEFAULT_BAN_TIME); if (server) { server->remove_peer_from_list(this); } @@ -3945,16 +3976,27 @@ void P2PServer::monero_block_broadcast_after_work_cb(uv_work_t* req, int /*statu else if (work->pow_check_failed && !server->is_banned(work->is_v6, work->addr)) { P2PClient* client = work->client; - if (!client->is_gone(work->reset_counter)) { - client->close(); - LOGWARN(3, "peer " << static_cast(client->m_addrString) << " banned for " << DEFAULT_BAN_TIME << " seconds"); + const bool client_here = !client->is_gone(work->reset_counter); + + // Same leniency as sidechain blocks: a failed PoW check on a broadcast + // mainchain block can be transient during sync, so tolerate a few per + // connection before banning (and only briefly). + if (client_here && (++client->m_banScore < BAN_FAULT_THRESHOLD)) { + LOGWARN(5, "peer " << static_cast(client->m_addrString) << " failed a PoW check (" + << client->m_banScore << '/' << BAN_FAULT_THRESHOLD << "), not banning yet"); } else { - LOGWARN(3, work->addr << " banned for " << DEFAULT_BAN_TIME << " seconds"); - } + if (client_here) { + client->close(); + LOGWARN(3, "peer " << static_cast(client->m_addrString) << " banned for " << DEFAULT_BAN_TIME << " seconds"); + } + else { + LOGWARN(3, work->addr << " banned for " << DEFAULT_BAN_TIME << " seconds"); + } - server->ban(work->is_v6, work->addr, DEFAULT_BAN_TIME); - server->remove_peer_from_list(work->addr); + server->ban(work->is_v6, work->addr, DEFAULT_BAN_TIME); + server->remove_peer_from_list(work->addr); + } } delete work; @@ -4128,7 +4170,19 @@ void P2PServer::P2PClient::post_handle_incoming_block(p2pool* pool, const PoolBl const bool gone = is_gone(reset_counter); if (!result) { - // Client sent bad data, disconnect and ban it + // The block wasn't accepted. This is often transient rather than abuse: + // while we're re-syncing the sidechain (e.g. right after a restart) our + // mainchain view can lag, so an honest peer's perfectly valid block fails + // verification here. Tolerate several such blocks per connection before + // banning, and even then only briefly (DEFAULT_BAN_TIME) — keeping the + // network meshed matters more than punishing a peer immediately. + if (!gone && (++m_banScore < BAN_FAULT_THRESHOLD)) { + LOGWARN(5, "peer " << static_cast(m_addrString) << " sent an unacceptable block (" + << m_banScore << '/' << BAN_FAULT_THRESHOLD << "), not banning yet"); + return; + } + + // Repeatedly bad — disconnect and (briefly) ban it if (!gone) { close(); LOGWARN(3, "peer " << static_cast(m_addrString) << " banned for " << DEFAULT_BAN_TIME << " seconds"); diff --git a/src/p2p_server.h b/src/p2p_server.h index 74c148d0..0c74d365 100644 --- a/src/p2p_server.h +++ b/src/p2p_server.h @@ -141,6 +141,11 @@ class P2PServer : public TCPServer uint64_t m_broadcastMaxHeight; + // Count of unacceptable blocks this peer has sent this connection. We only + // ban once it reaches BAN_FAULT_THRESHOLD, so transient sync failures don't + // insta-ban honest peers. Reset per connection in reset(). + uint32_t m_banScore; + MessageId m_expectedMessage; uint64_t m_handshakeChallenge; bool m_handshakeSolutionSent; diff --git a/src/p2pool.cpp b/src/p2pool.cpp index 994c174f..7ed58332 100644 --- a/src/p2pool.cpp +++ b/src/p2pool.cpp @@ -1747,6 +1747,15 @@ void p2pool::parse_get_miner_data_rpc(const char* data, size_t size) return; } + // We only reach here on a non-empty, error-free RPC response, so the node is + // responsive right now -- record liveness BEFORE the duplicate-response + // short-circuit below. Between main-chain blocks (XKR ~90s) get_miner_data + // keeps returning byte-identical data that gets deduplicated, but the RPC is + // perfectly alive; updating m_lastActive only in handle_miner_data (new data + // only) made check_host falsely flag the node "unresponsive" in the gap + // between blocks and force needless reconnects (and spam the log). + m_lastActive = seconds_since_epoch(); + hash h; keccak(reinterpret_cast(data), static_cast(size), h.h); if (h == m_getMinerDataHash) { @@ -2340,6 +2349,18 @@ void p2pool::reconnect_to_host() } #ifndef P2POOL_UNIT_TESTS + // If check_host() flagged the RPC as wedged (no fresh data for a long time), + // break the overlap guard here: clear the stuck pending state so the + // get_miner_data() below is actually issued on a brand-new curl connection + // instead of being suppressed by an orphaned request that never completes. + // (The orphaned request, if any, still cleans itself up when its own timeout + // finally fires; we don't wait for it.) + if (m_rpcForceReconnect.exchange(false)) { + LOGWARN(1, "RPC has been unresponsive for too long; forcing a hard reconnect to " << current_host().m_displayName); + m_getMinerDataPending = false; + m_getMinerDataPendingSince = 0; + } + // The node is driven by polling kryptokronad for fresh miner data. // get_miner_data() is deduplicated (by response hash) and self-guarded // against overlap, and its handler refreshes m_lastActive, which serves as diff --git a/src/p2pool.h b/src/p2pool.h index c57d5a24..a428fbda 100644 --- a/src/p2pool.h +++ b/src/p2pool.h @@ -135,6 +135,9 @@ class p2pool : public MinerCallbackHandler, public nocopy_nomove uint64_t last_active() const { return m_lastActive; } uint64_t start_time() const { return m_startTime; } void reconnect_to_host(); + // Ask for a hard RPC reconnect on the next poll (see m_rpcForceReconnect). + // Safe to call from any thread. + void request_rpc_reconnect() { m_rpcForceReconnect = true; } bool startup_finished() const { return m_startupFinished.load(); } @@ -304,6 +307,17 @@ class p2pool : public MinerCallbackHandler, public nocopy_nomove // polls forever -> the node serves stale templates and gets banned by peers. uint64_t m_getMinerDataPendingSince = 0; + // Detector-driven hard-reconnect flag. The overlap guard above only recovers + // if get_miner_data() is reached with the request pending >30s, which in + // practice sometimes never happens (the recovery is main-loop-bound and the + // orphaned curl request never times out), so the RPC can stay wedged for + // hours. check_host() runs reliably on the P2P loop and watches m_lastActive; + // when it sees no fresh data for RPC_HARD_RECONNECT_TIMEOUT it sets this, and + // reconnect_to_host() (main loop) consumes it to force a brand-new request on + // a fresh curl connection regardless of the stuck pending state. atomic: set + // on the P2P thread, read on the main thread. + std::atomic m_rpcForceReconnect{ false }; + std::atomic m_lastMinerDataReceived; uv_timer_t m_timer; diff --git a/src/side_chain.cpp b/src/side_chain.cpp index c3f5e768..32633c4a 100644 --- a/src/side_chain.cpp +++ b/src/side_chain.cpp @@ -568,9 +568,14 @@ bool SideChain::add_external_block(PoolBlock& block, std::vector& missing_ } if (!m_pool->get_seed(block.m_txinGenHeight, block.m_seed)) { - LOGWARN(3, "add_external_block mined by " << block.m_minerWallet << ": couldn't get seed hash for mainchain height " << block.m_txinGenHeight); + // We simply can't verify this block yet (our mainchain view for this + // height isn't available — common right after a restart while we're still + // catching up). This is not the peer's fault, so ignore the block instead + // of returning false, which would ban an honest peer. Mirrors the + // "couldn't get PoW hash" case just below. + LOGWARN(3, "add_external_block mined by " << block.m_minerWallet << ": couldn't get seed hash for mainchain height " << block.m_txinGenHeight << ". Ignoring it."); forget_incoming_block(block); - return false; + return true; } if (!block.get_pow_hash(m_pool->hasher(), block.m_txinGenHeight, block.m_seed, block.m_powHash, false, RandomX_Hasher_Base::VM_LANE_P2P)) {