Lenient bans

.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-<arch> 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}

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<uint64_t>(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<char*>(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<char*>(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<const char*>(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<const char*>(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<const char*>(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<char*>(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<char*>(m_addrString) << " banned for " << DEFAULT_BAN_TIME << " seconds");

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;

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<const uint8_t*>(data), static_cast<int>(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

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<bool> m_rpcForceReconnect{ false };
+
 	std::atomic<uint64_t> m_lastMinerDataReceived;
 
 	uv_timer_t m_timer;

src/side_chain.cpp

@@ -568,9 +568,14 @@ bool SideChain::add_external_block(PoolBlock& block, std::vector<hash>& 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)) {