From f57d58155cdf65ec2ddb1a01c9d9466bf60df9ab Mon Sep 17 00:00:00 2001 From: Nsable-sh Date: Thu, 13 Aug 2026 17:15:24 -0300 Subject: wallet: report locked encrypted recovery seed --- src/main.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index f83164f..5bd4028 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4355,6 +4355,9 @@ WalletRecoveryAudit GetWalletRecoveryAudit() CRITICAL_BLOCK(cs_keyPool) { audit.fHaveSeed = HaveHDSeed(); + audit.fSeedEncryptedLocked = !audit.fHaveSeed && + (!vchCryptedHDMaster.empty() || + !vchCryptedHDChainCode.empty()); audit.nSchema = nHDKeySchema; audit.nDerivedKnown = nHDNext; audit.nReceiveNext = nHDReceiveNext; -- cgit v1.2.3 From 2661de141e46dadfb2903bb12e72dc87c5a64375 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 05:31:22 +0000 Subject: log: demote the four lines that were 90% of debug.log behind /debugrelay A 13-hour production capture was ~71% "sending: tx", ~16% "Nostr: discovered .btf peer", ~5% "Relaying wtx", and ~43% of raw lines were CBlock/CTransaction dumps -- all already under -debug, which operators run permanently, so -debug was useless as a filter. The category mechanism gains a verbose tier: "relay" and "nostrdiscovery" are accepted only with the new /debugrelay flag (which implies /debug); everything else keeps its current gate. Demoted, with message text unchanged for anyone grepping it: net.h "sending: tx (N bytes)" (BeginMessage/EndMessage/ AbortMessage all pick the category from the command, so no orphaned "(N bytes)" fragments leak into a plain -debug log) main.cpp "Relaying wtx", "received block:" + CBlock::print, and the BitcoinMiner block dump nostr.cpp "Nostr: discovered .btf peer" -> "nostrdiscovery" Pool/worker/payout, [payout], BLOCK ORPHANED/ACCEPTED and share flow lines are untouched. Because LogPrint dedup keys on category, demoted lines also keep their burst collapse under /debugrelay. Closes #35 --- src/main.cpp | 6 +++--- src/main_gui.cpp | 12 ++++++++++++ src/net.h | 22 ++++++++++++++++++---- src/nostr.cpp | 2 +- src/util.cpp | 13 +++++++++++++ src/util.h | 4 ++++ 6 files changed, 51 insertions(+), 8 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..80f5f31 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1712,7 +1712,7 @@ void CWalletTx::RelayWalletTransaction(CTxDB& txdb) uint256 hash = GetHash(); if (!txdb.ContainsTx(hash)) { - if (LogAcceptsCategory("net")) printf("Relaying wtx %s\n", hash.ToString().substr(0,6).c_str()); + if (LogAcceptsCategory("relay")) printf("Relaying wtx %s\n", hash.ToString().substr(0,6).c_str()); RelayMessage(CInv(MSG_TX, hash), (CTransaction)*this); } } @@ -3263,7 +3263,7 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) vRecv >> *pblock; //// debug print - if (LogAcceptsCategory("net")) { printf("received block:\n"); pblock->print(); } + if (LogAcceptsCategory("relay")) { printf("received block:\n"); pblock->print(); } CInv inv(MSG_BLOCK, pblock->GetHash()); pfrom->AddInventoryKnown(inv); @@ -4555,7 +4555,7 @@ bool BitcoinMiner(int nThreadId) //// debug print if (LogAcceptsCategory("net")) printf("BitcoinMiner:\n"); if (LogAcceptsCategory("net")) printf("RandomX proof-of-work found \n powhash: %s \ntarget: %s\n", hash.GetHex().c_str(), hashTarget.GetHex().c_str()); - if (LogAcceptsCategory("net")) pblock->print(); + if (LogAcceptsCategory("relay")) pblock->print(); SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_NORMAL); CRITICAL_BLOCK(cs_main) diff --git a/src/main_gui.cpp b/src/main_gui.cpp index 05f5ecf..163d78e 100644 --- a/src/main_gui.cpp +++ b/src/main_gui.cpp @@ -111,6 +111,7 @@ static void PrintUsage() printf(" /help, -help, --help, /?\n"); printf(" /datadir=PATH\n"); printf(" /debug\n"); + printf(" /debugrelay (/debug plus per-message relay chatter and block dumps)\n"); printf(" /maxdebugfilesize=N (roll debug.log at N MB, default 1024; 0 = never)\n"); printf(" /gen\n"); printf(" /nogui or /daemon\n"); @@ -192,6 +193,17 @@ static void ParseStartupArguments(int argc, char* argv[]) if (arg(argc,argv,"/debug") || arg(argc,argv,"-debug")) fDebug = true; + // /debugrelay -- everything /debug shows, plus the per-message firehose + // ("relay" and "nostrdiscovery" categories: sending: tx, Relaying wtx, + // raw CBlock dumps, per-peer discovery lines). That traffic was ~90% of + // a 13-hour production debug.log (issue #35), so plain /debug leaves it + // off; the message text itself is unchanged for anyone grepping it. + if (arg(argc,argv,"/debugrelay") || arg(argc,argv,"-debugrelay")) + { + fDebug = true; + fDebugRelay = true; + } + // /maxdebugfilesize=N -- roll debug.log to debug.log.old once it passes // N MB, checked on each write. Default is 1024 (see util.cpp); 0 turns // the check off and leaves rotation entirely to logrotate. diff --git a/src/net.h b/src/net.h index 9f3c7e4..1aee973 100644 --- a/src/net.h +++ b/src/net.h @@ -802,6 +802,18 @@ public: + // "sending: tx (N bytes)" alone was ~71% of a 13-hour production + // debug.log (issue #35), so tx sends are demoted to the /debugrelay-gated + // "relay" category; every other command's send trace stays on "net". + // The line is emitted in pieces across BeginMessage/EndMessage/ + // AbortMessage, so all three must agree on the category or orphaned + // "(N bytes)" fragments would leak into a plain -debug log. + // Valid only mid-message (nPushPos != -1). + const char* SendLogCategory() const + { + return strcmp(GetMessageCommand(), "tx") == 0 ? "relay" : "net"; + } + void BeginMessage(const char* pszCommand) { EnterCriticalSection(&cs_vSend); @@ -809,17 +821,19 @@ public: AbortMessage(); nPushPos = vSend.size(); vSend << CMessageHeader(pszCommand, 0); - if (LogAcceptsCategory("net")) printf("sending: %-12s ", pszCommand); + if (LogAcceptsCategory(SendLogCategory())) printf("sending: %-12s ", pszCommand); } void AbortMessage() { if (nPushPos == -1) return; + // Capture before nPushPos is cleared; GetMessageCommand() needs it. + const char* pszCategory = SendLogCategory(); vSend.resize(nPushPos); nPushPos = -1; LeaveCriticalSection(&cs_vSend); - if (LogAcceptsCategory("net")) printf("(aborted)\n"); + if (LogAcceptsCategory(pszCategory)) printf("(aborted)\n"); } void EndMessage() @@ -839,10 +853,10 @@ public: unsigned int nSize = vSend.size() - nPushPos - sizeof(CMessageHeader); memcpy((char*)&vSend[nPushPos] + offsetof(CMessageHeader, nMessageSize), &nSize, sizeof(nSize)); - if (LogAcceptsCategory("net")) printf("(%d bytes) ", nSize); + if (LogAcceptsCategory(SendLogCategory())) printf("(%d bytes) ", nSize); //for (int i = nPushPos+sizeof(CMessageHeader); i < min(vSend.size(), nPushPos+sizeof(CMessageHeader)+20U); i++) // printf("%02x ", vSend[i] & 0xff); - if (LogAcceptsCategory("net")) printf("\n"); + if (LogAcceptsCategory(SendLogCategory())) printf("\n"); nPushPos = -1; LeaveCriticalSection(&cs_vSend); diff --git a/src/nostr.cpp b/src/nostr.cpp index e2d839c..9bbf74e 100644 --- a/src/nostr.cpp +++ b/src/nostr.cpp @@ -1440,7 +1440,7 @@ static bool SeedFromRelay(CNostrKey& key, const string& relay) else if (createdAt > it->second) it->second = createdAt; // refresh } if (fNew && fDebug) - LogPrint("nostr", "Nostr: discovered .btf peer %s\n", peerAddr.c_str()); + LogPrint("nostrdiscovery", "Nostr: discovered .btf peer %s\n", peerAddr.c_str()); if (++nPeers > 500) break; } else if (type == "EOSE") diff --git a/src/util.cpp b/src/util.cpp index 52cda84..a5a4e80 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -7,6 +7,7 @@ bool fDebug = false; +bool fDebugRelay = false; // In-process cap for debug.log, in bytes. Default matches the 1G trigger of // the shipped logrotate policy so a long-running node stays bounded even @@ -31,10 +32,22 @@ static bool LogCategoryDefaultOn(const char* category) strcmp(category, "payout") == 0; } +// Categories so noisy they drown out everything else even under -debug: +// per-message send traces, wallet relay rebroadcasts, raw CBlock dumps and +// per-address discovery lines made up ~90% of a 13-hour production capture +// (issue #35). They need /debugrelay, not merely /debug. +static bool LogCategoryVerboseOnly(const char* category) +{ + return strcmp(category, "relay") == 0 || + strcmp(category, "nostrdiscovery") == 0; +} + bool LogAcceptsCategory(const char* category) { if (LogCategoryDefaultOn(category)) return true; + if (LogCategoryVerboseOnly(category)) + return fDebugRelay; return fDebug; } diff --git a/src/util.h b/src/util.h index f2ea23f..c47b8a7 100644 --- a/src/util.h +++ b/src/util.h @@ -87,6 +87,10 @@ inline T& REF(const T& val) extern bool fDebug; +// /debugrelay: the per-message firehose ("relay" and "nostrdiscovery" +// categories) that even -debug keeps off. See LogAcceptsCategory(). +extern bool fDebugRelay; + // debug.log is rolled to debug.log.old once it grows past this many bytes // (set from -maxdebugfilesize, in MB; 0 disables the check). See the roll // site in OutputDebugStringF() below. -- cgit v1.2.3 From 2a804c2d9736e91271b3209fbc41080c67c100da Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 06:49:28 +0000 Subject: worker: log the chain a pool job builds on, not just its id --- src/main.cpp | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..3de3db0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4040,8 +4040,29 @@ connected: jobShared.jobSeq++; jobShared.haveJob = true; } - LogPrint("worker", "[pool->worker] new job %s received\n", - jobId.c_str()); + // Log the chain position the pool is + // building on, so a stale or private + // fork is auditable from worker logs. + // The prevhash comes from the header we + // actually hash (== p[2] of the notify); + // a brief cs_main lookup is safe here -- + // this thread holds no other lock. + uint256 hashPrev; + memcpy(&hashPrev, header + 4, 32); + int nPrevHeight = -1; + CRITICAL_BLOCK(cs_main) + { + map::iterator mi = mapBlockIndex.find(hashPrev); + if (mi != mapBlockIndex.end()) + nPrevHeight = (*mi).second->nHeight; + } + if (nPrevHeight >= 0) + LogPrint("worker", "[pool->worker] new job %s received, prevhash %s (known, job height %d)\n", + jobId.c_str(), hashPrev.GetHex().c_str(), + nPrevHeight + 1); + else + LogPrint("worker", "[pool->worker] new job %s received, prevhash %s (unknown to our chain)\n", + jobId.c_str(), hashPrev.GetHex().c_str()); SetParticipantMiningStatus("hashing job " + jobId); } } -- cgit v1.2.3 From 0b566b4869b9e86c90162befd87f9882b6dd143c Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 07:09:45 +0000 Subject: net: a refused thread spawn drops one peer, not the node std::thread throws std::system_error when pthread_create is refused (RLIMIT_NPROC, a container's pids cgroup, memory pressure), and nothing caught it, so the node died by std::terminate. Threads are spawned per peer connection, so the pressure that triggers this scales with peer count, which is attacker-influenced. Observed killing a healthy participant worker mid-mining. Every spawn site in the node build, and what a failure now does: - btftunnel.cpp: both tunnel pumps are guarded. The inbound pump starts first because it owns cleanup of the pump end and rendezvous socket; if the second spawn is refused, shutdown() wakes the first out of its read and it cleans up itself -- closing those descriptors from the failure path instead would race it onto reused descriptors. Both paths return INVALID_SOCKET, which every caller already treats as "this connection failed". - btfrv.cpp: the relay's accept loop drops the connection it cannot serve and stops accepting for a moment -- longer after consecutive failures, capped at 5s -- instead of dying at an arbitrary one. The per-pair second pump now runs on the handling thread itself, leaving exactly one spawn to guard and nothing running yet to race. - main.cpp: a participant that cannot spawn its hash workers stops the ones that did start, drops the pool connection, and returns to the reconnect loop, the same way an ordinary disconnect does. - randomx_pow.cpp: dataset init joins the workers it got, releases the dataset, and stays in light mode -- the same trade the allocation failure path above it already makes. - rpc.cpp, net.cpp: the remaining unchecked _beginthread calls (which report failure by value, they cannot throw) now say precisely what did not start. The pool's accept and payout threads failing silently left the pool running deaf -- nobody accepts, nobody gets paid, and nothing in the log says why. Left alone deliberately: the per-relay accept spawns (rpc.cpp) already check, log and retry on the next poll; startup-critical spawns in net.cpp keep failing hard; the selftest-only std::thread uses may abort a selftest run, which is acceptable where dying over one peer is not. Verified: full node rebuilds clean and links; pool-stratum selftest passes; a scratch harness under RLIMIT_NPROC=1 confirms _beginthread reports -1 and std::thread throws system_error, the premises of every path above. --- src/btfrv.cpp | 49 ++++++++++++++++++++++++++++++++++++++++++++++--- src/btftunnel.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++++++++---- src/main.cpp | 30 +++++++++++++++++++++++++++--- src/net.cpp | 5 +++-- src/randomx_pow.cpp | 28 ++++++++++++++++++++++------ src/rpc.cpp | 15 +++++++++++---- 6 files changed, 157 insertions(+), 22 deletions(-) (limited to 'src/main.cpp') diff --git a/src/btfrv.cpp b/src/btfrv.cpp index 133eb89..8ab735c 100644 --- a/src/btfrv.cpp +++ b/src/btfrv.cpp @@ -40,6 +40,8 @@ typedef int SOCKET; #include #include #include +#include +#include namespace btf { @@ -209,8 +211,28 @@ static void HandleIncoming(SOCKET s) bool a = WriteN(svc, &ok, 1); bool b = WriteN(s, &ok, 1); if (!a || !b) { CLOSESOCK(svc); CLOSESOCK(s); return; } - std::thread(Forward, s, svc).detach(); - std::thread(Forward, svc, s).detach(); + // One thread pumps s -> svc; this thread pumps svc -> s. Spawning a + // thread per direction left no clean way to fail the second spawn: + // std::thread throws std::system_error when pthread_create is refused + // (NPROC limit, container pids cgroup, memory pressure), the first + // pump was already detached, and closing its sockets from here would + // race it -- so nothing caught the throw and it reached + // std::terminate, killing the relay over one pair it could not serve. + // Running one direction on this thread leaves exactly one spawn to + // guard, and a failure here has nothing running yet to race. + try + { + std::thread(Forward, s, svc).detach(); + } + catch (const std::system_error& e) + { + fprintf(stderr, "rendezvous relay: cannot spawn forward thread (%s), pair dropped\n", + e.what()); + CLOSESOCK(svc); + CLOSESOCK(s); + return; + } + Forward(svc, s); } else { @@ -236,6 +258,7 @@ bool RvRelayRun(unsigned short port) if (bind(g_listen, (struct sockaddr*)&addr, sizeof(addr)) != 0) { CLOSESOCK(g_listen); return false; } if (listen(g_listen, 16) != 0) { CLOSESOCK(g_listen); return false; } + int nSpawnFails = 0; while (!g_stop) { struct sockaddr_in cli; @@ -246,7 +269,27 @@ bool RvRelayRun(unsigned short port) if (g_stop) break; continue; } - std::thread(HandleIncoming, s).detach(); + try + { + std::thread(HandleIncoming, s).detach(); + nSpawnFails = 0; + } + catch (const std::system_error& e) + { + // A refused pthread_create is backpressure, not a reason to die: + // the condition behind it (NPROC, pids cgroup, memory) does not + // clear in milliseconds, and the connection count feeding it is + // attacker-influenced. Drop this one peer, stop accepting for a + // moment -- longer each time, capped -- and keep serving the + // connections already established. + fprintf(stderr, "rendezvous relay: cannot spawn connection thread (%s), connection dropped\n", + e.what()); + CLOSESOCK(s); + nSpawnFails++; + int nWaitMs = nSpawnFails < 5 ? nSpawnFails * 1000 : 5000; + for (int i = 0; i < nWaitMs && !g_stop; i += 100) + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } } CLOSESOCK(g_listen); g_listen = INVALID_SOCKET; diff --git a/src/btftunnel.cpp b/src/btftunnel.cpp index 63d178b..2b3f818 100644 --- a/src/btftunnel.cpp +++ b/src/btftunnel.cpp @@ -25,8 +25,10 @@ typedef int SOCKET; #endif #include "sockcount.h" #include +#include #include #include +#include namespace btf { @@ -121,18 +123,60 @@ static void PumpIn(SOCKET pumpEnd, RvSocket rv, std::vector key) // Start the two pumps and hand back the app-facing socket. When both pumps // finish, close the pump end and the rendezvous socket. +// +// Every tunnel is two threads, and a tunnel is one peer connection (a .btf +// dial or accept, a pool miner, a participant's pool session) -- so the +// thread count scales with peer count, which is attacker-influenced. When +// pthread_create is refused std::thread throws std::system_error; uncaught it +// reached std::terminate and took the whole process down over one connection +// it could not serve. Both spawns are guarded, and the failure paths return +// INVALID_SOCKET, which every caller already handles by dropping just that +// connection. static btf_socket_t StartTunnel(RvSocket rv, const unsigned char shared[32]) { btf_socket_t appEnd, pumpEnd; if (!MakeLocalPair(appEnd, pumpEnd)) { RvClose(rv); return INVALID_SOCKET; } std::vector key(shared, shared + 32); - std::thread(PumpOut, pumpEnd, rv, key).detach(); - std::thread([pumpEnd, rv, key]() { - PumpIn(pumpEnd, rv, key); + + // The inbound pump starts first because it owns the cleanup: it closes + // pumpEnd and rv when it exits. That ownership is what makes the second + // spawn's failure path below race-free. + try + { + std::thread([pumpEnd, rv, key]() { + PumpIn(pumpEnd, rv, key); + BtfCloseSocket(pumpEnd); + RvClose(rv); + }).detach(); + } + catch (const std::system_error& e) + { + // Nothing is running yet, so closing everything here is safe. + fprintf(stderr, "btf tunnel: cannot spawn inbound pump (%s), tunnel dropped\n", + e.what()); BtfCloseSocket(pumpEnd); + BtfCloseSocket(appEnd); RvClose(rv); - }).detach(); + return INVALID_SOCKET; + } + + try + { + std::thread(PumpOut, pumpEnd, rv, key).detach(); + } + catch (const std::system_error& e) + { + // The inbound pump is parked in RvReadN(rv). shutdown() wakes it, and + // it closes pumpEnd and rv itself -- closing them here instead would + // race exactly that, and a reused descriptor is how an unrelated + // socket gets closed. + fprintf(stderr, "btf tunnel: cannot spawn outbound pump (%s), tunnel dropped\n", + e.what()); + shutdown((SOCKET)rv, SD_BOTH); + BtfCloseSocket(appEnd); + return INVALID_SOCKET; + } return appEnd; } diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..d154523 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include "bip32.h" #include "btfaddr.h" #include "btftunnel.h" @@ -3899,9 +3900,32 @@ connected: std::vector shareHits; std::mutex shareHitsMtx; std::vector hashThreads; - for (int i = 0; i < nWorkers; i++) - hashThreads.emplace_back(ParticipantHashWorker, &jobShared, i, nWorkers, - &shareHits, &shareHitsMtx); + try + { + for (int i = 0; i < nWorkers; i++) + hashThreads.emplace_back(ParticipantHashWorker, &jobShared, i, nWorkers, + &shareHits, &shareHitsMtx); + } + catch (const std::system_error& e) + { + // A refused pthread_create used to reach std::terminate here and + // kill the worker mid-session -- observed twice on an otherwise + // healthy participant. Stop the workers that did start the same way + // the disconnect path below does, drop this pool connection, and let + // the caller's reconnect loop try again once the pressure clears. + LogPrint("worker", "[worker] cannot spawn hash threads (%s)" + " -- dropping pool connection\n", e.what()); + SetParticipantMiningStatus("hash thread spawn failed"); + { + std::lock_guard lk(jobShared.mtx); + jobShared.stop = true; + jobShared.haveJob = false; + } + for (std::thread& t : hashThreads) + t.join(); + closesocket(s); + return false; + } LogPrint("worker", "[worker] spawned %d hash threads on shared RandomX dataset\n", nWorkers); const uint64 sessionStartHashes = gParticipantHashes.load(); diff --git a/src/net.cpp b/src/net.cpp index fed6123..16038c8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2090,7 +2090,7 @@ bool StartNode(string& strError) if (addrIncoming.ip) addrLocalHost.ip = addrIncoming.ip; - _beginthread([](void*) { + if (_beginthread([](void*) { unsigned int ip = addrLocalHost.ip; if (GetMyExternalIP(ip)) { addrLocalHost.ip = ip; @@ -2098,7 +2098,8 @@ bool StartNode(string& strError) CWalletDB().WriteSetting("addrIncoming", addrIncoming); printf("External IP updated: %s\n", addrLocalHost.ToStringIP().c_str()); } - }, 0, NULL); + }, 0, NULL) == (uintptr_t)-1) + printf("Error: _beginthread(GetMyExternalIP) failed\n"); // Peers that answered last time, dialled straight away. The relays are the // only way to find anyone otherwise, and reaching them plus walking their diff --git a/src/randomx_pow.cpp b/src/randomx_pow.cpp index 1d058d4..9c59364 100644 --- a/src/randomx_pow.cpp +++ b/src/randomx_pow.cpp @@ -8,6 +8,7 @@ #include #include #include +#include // Fixed key (seed) that determines the RandomX cache/dataset. Keeping it // constant avoids per-epoch "reseed" logic; for the CPU+RAM fairness goal @@ -101,13 +102,28 @@ bool RandomXInitDataset(int nThreads) std::vector workers; unsigned long per = total / nThreads; - for (int i = 0; i < nThreads; i++) + try { - unsigned long start = i * per; - unsigned long count = (i == nThreads - 1) ? (total - start) : per; - workers.emplace_back([start, count]() { - randomx_init_dataset(g_dataset, g_cache, start, count); - }); + for (int i = 0; i < nThreads; i++) + { + unsigned long start = i * per; + unsigned long count = (i == nThreads - 1) ? (total - start) : per; + workers.emplace_back([start, count]() { + randomx_init_dataset(g_dataset, g_cache, start, count); + }); + } + } + catch (const std::system_error& e) + { + // Same refusal as the allocation failure above: light mode is slow + // but alive, std::terminate is neither. Join the workers that did + // start before releasing the dataset out from under them. + for (auto& w : workers) w.join(); + randomx_release_dataset(g_dataset); + g_dataset = NULL; + printf("RandomX: cannot spawn dataset init threads (%s), staying in light mode\n", + e.what()); + return false; } for (auto& w : workers) w.join(); diff --git a/src/rpc.cpp b/src/rpc.cpp index a4b53ca..13cb6c9 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -5077,10 +5077,17 @@ void ThreadRPCServer(void*) // armed for its one wallet probe before it is allowed to send. ReconcilePayouts(); - _beginthread(AcceptLoopFn, 0, NULL); - _beginthread(LocalAcceptFn, 0, NULL); - _beginthread(ThreadBtfPoolAnnouncer, 0, NULL); - _beginthread(PayoutThreadFn, 0, NULL); + // _beginthread cannot throw (it reports -1), but an unchecked failure + // leaves the pool running deaf: nobody accepts, announces, or pays, and + // nothing in the log says why. + if (_beginthread(AcceptLoopFn, 0, NULL) == (uintptr_t)-1) + LogPrint("pool", "[pool] FATAL: accept loop thread failed to spawn -- miners cannot connect\n"); + if (_beginthread(LocalAcceptFn, 0, NULL) == (uintptr_t)-1) + LogPrint("pool", "[pool] local accept thread failed to spawn\n"); + if (_beginthread(ThreadBtfPoolAnnouncer, 0, NULL) == (uintptr_t)-1) + LogPrint("pool", "[pool] announcer thread failed to spawn -- pool will not be advertised\n"); + if (_beginthread(PayoutThreadFn, 0, NULL) == (uintptr_t)-1) + LogPrint("pool", "[pool] FATAL: payout thread failed to spawn -- no one gets paid\n"); PoolEventLoop(); // blocks until gPoolRunning = false or fShutdown -- cgit v1.2.3 From 2c0410b20893383ec6f1c4c8cd5134e420513e80 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 08:29:44 +0000 Subject: net: fail over a stalled block fetch to another peer The 15s retry spacing only helps when several peers announce the block: the queued retries are created by inv handling, so a block announced by a single peer that then goes quiet was re-requested from nobody, and the tip stayed frozen until somebody happened to re-announce (#33). Track every block getdata in mapBlockFetch, erase the entry when the body arrives, and let SendMessages re-issue a request that has gone unanswered for BLOCK_FETCH_RETRY_SECS to the next peer that has not had a turn this round, restarting the round once every connected peer has. Client peers are skipped -- they declared they cannot serve bodies. Tracking is capped at 8 in-flight blocks so initial download, where hundreds are legitimately outstanding to one peer, is not duplicated to everybody else, and an entry is dropped after 10 minutes so a hash that was announced and never existed cannot be chased forever. The failover log line is deliberately not behind a debug category: it firing means a peer announces blocks it does not serve, which is the pathology being diagnosed. --- src/main.cpp | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..d9fe713 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -64,6 +64,31 @@ CBlockIndex* pindexBest = NULL; map mapOrphanBlocks; multimap mapOrphanBlocksByPrev; +// Block bodies we have sent a getdata for and not yet received, so the +// failover pass in SendMessages can tell "requested and being served" from +// "requested and gone quiet" and re-issue the request to a different peer +// (#33). Keyed by block hash; setAsked is the peers already tried since the +// round last restarted, so the retry rotates instead of hammering the +// announcer that is not serving. Guarded by cs_main, which both the message +// handlers and SendMessages already hold. +// +// Capped small on purpose. At the tip there is one block in flight; during +// initial download there are legitimately hundreds outstanding to one peer, +// and rotating those would duplicate the whole download to every other peer. +// Anything past the cap is simply not tracked and behaves as before. +struct CBlockFetch +{ + int64 nTimeFirstAsked; // when the first getdata for this block went out + int64 nTimeAsked; // when the latest getdata went out + set setAsked; // peers tried since the round last restarted +}; +map mapBlockFetch; +static const int BLOCK_FETCH_TRACK_MAX = 8; +// A block nobody has served for this long is not a fetch problem: the hash may +// have been announced and never existed. Drop the entry rather than getdata +// forever; a re-announcement recreates it. +static const int BLOCK_FETCH_GIVEUP_SECS = 10 * 60; + map mapOrphanTransactions; multimap mapOrphanTransactionsByPrev; @@ -3268,6 +3293,12 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) CInv inv(MSG_BLOCK, pblock->GetHash()); pfrom->AddInventoryKnown(inv); + // The body is here, so the fetch is over whatever validation says + // next -- unlike the mapAlreadyAskedFor erase below, which stays + // conditional on acceptance so a peer feeding us garbage does not + // cancel the scheduled retries for the real thing. + mapBlockFetch.erase(inv.hash); + if (ProcessBlock(pfrom, pblock.release())) { mapAlreadyAskedFor.erase(inv); @@ -3396,12 +3427,86 @@ bool SendMessages(CNode* pto) const CInv& inv = (*pto->mapAskFor.begin()).second; if (LogAcceptsCategory("net")) printf("sending getdata: %s\n", inv.ToString().c_str()); if (!AlreadyHave(txdb, inv)) + { vAskFor.push_back(inv); + // Blocks get their delivery tracked so the failover pass + // below can chase the ones that go quiet. See mapBlockFetch + // for why anything past the cap is left untracked. + if (inv.type == MSG_BLOCK && + (mapBlockFetch.count(inv.hash) || mapBlockFetch.size() < BLOCK_FETCH_TRACK_MAX)) + { + CBlockFetch& fetch = mapBlockFetch[inv.hash]; + if (fetch.nTimeFirstAsked == 0) + fetch.nTimeFirstAsked = GetTime(); + fetch.nTimeAsked = GetTime(); + fetch.setAsked.insert(pto->addr); + } + } pto->mapAskFor.erase(pto->mapAskFor.begin()); } if (!vAskFor.empty()) pto->PushMessage("getdata", vAskFor); + + // + // Message: getdata (block fetch failover) + // + // A requested block that has not arrived in BLOCK_FETCH_RETRY_SECS is + // not still in transit -- a body is at most 1 MB and somebody + // demonstrably has it -- so ask somebody else instead of waiting on + // whoever was asked first (#33). Each peer the message handler visits + // takes at most one turn per block per round; once every connected + // peer has had its turn and a further interval passes with nobody + // new to ask, the round restarts. A peer that declared itself a + // client cannot serve block bodies and is no use as a target. + if (!pto->fClient) + { + vector vFailover; + int64 nNowSecs = GetTime(); + for (map::iterator mi = mapBlockFetch.begin(); mi != mapBlockFetch.end();) + { + CBlockFetch& fetch = (*mi).second; + CInv inv(MSG_BLOCK, (*mi).first); + if (AlreadyHave(txdb, inv)) + { + // Arrived by some other road -- our own miner, an orphan + // parked in mapOrphanBlocks. Nothing left to chase. + mapBlockFetch.erase(mi++); + continue; + } + if (nNowSecs - fetch.nTimeFirstAsked > BLOCK_FETCH_GIVEUP_SECS) + { + printf("block fetch: giving up on %s -- no peer served it in %d s\n", + inv.ToString().c_str(), (int)(nNowSecs - fetch.nTimeFirstAsked)); + mapBlockFetch.erase(mi++); + continue; + } + if (nNowSecs - fetch.nTimeAsked >= BLOCK_FETCH_RETRY_SECS) + { + // The handler loop visits every peer within ~100ms, so a + // request stale by two full intervals means nobody outside + // setAsked was there to ask. Start the round over. + if (nNowSecs - fetch.nTimeAsked >= 2 * BLOCK_FETCH_RETRY_SECS) + fetch.setAsked.clear(); + if (!fetch.setAsked.count(pto->addr)) + { + // Deliberately not behind a debug category: this line + // firing is a peer announcing blocks it does not + // serve, which is the pathology of #33. + printf("block fetch failover: %s unanswered for %d s, re-requesting from %s\n", + inv.ToString().c_str(), (int)(nNowSecs - fetch.nTimeAsked), + pto->addr.ToString().c_str()); + vFailover.push_back(inv); + fetch.nTimeAsked = nNowSecs; + fetch.setAsked.insert(pto->addr); + } + } + ++mi; + } + if (!vFailover.empty()) + pto->PushMessage("getdata", vFailover); + } + } return true; } -- cgit v1.2.3 From 7c70eb4498e88725241493033edfe89554ff1b18 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 09:03:01 +0000 Subject: tunnel: report which stage failed instead of guessing the cause --- src/btfrv.cpp | 16 ++++++++++++++-- src/btfrv.h | 19 +++++++++++++++++-- src/btftunnel.cpp | 37 +++++++++++++++++++++++++++++++++---- src/btftunnel.h | 25 +++++++++++++++++++++++-- src/main.cpp | 11 ++++++----- 5 files changed, 93 insertions(+), 15 deletions(-) (limited to 'src/main.cpp') diff --git a/src/btfrv.cpp b/src/btfrv.cpp index 133eb89..864fe3d 100644 --- a/src/btfrv.cpp +++ b/src/btfrv.cpp @@ -334,17 +334,29 @@ bool RvServiceWaitPaired(RvSocket s, int nTimeoutSecs) } RvSocket RvClientConnect(const char* relay_host, unsigned short port, - const unsigned char target_pubkey[32]) + const unsigned char target_pubkey[32], + RvClientFail* pFail) { + if (pFail) *pFail = RVC_FAIL_DIAL; if (!RvInit()) return RV_INVALID; SOCKET s = ConnectTo(relay_host, port); if (s == INVALID_SOCKET) return RV_INVALID; + if (pFail) *pFail = RVC_FAIL_PAIR; unsigned char hdr[33]; hdr[0] = (unsigned char)'C'; memcpy(hdr + 1, target_pubkey, 32); if (!WriteN(s, hdr, 33)) { CLOSESOCK(s); return RV_INVALID; } unsigned char paired = 0; - if (!ReadN(s, &paired, 1) || paired != 0x01) { CLOSESOCK(s); return RV_INVALID; } + if (!ReadN(s, &paired, 1)) { CLOSESOCK(s); return RV_INVALID; } + if (paired != 0x01) + { + // The relay answered and declined: HandleIncoming writes 0x00 and + // closes when no service is parked under this pubkey right now. + if (pFail) *pFail = RVC_FAIL_REFUSED; + CLOSESOCK(s); + return RV_INVALID; + } + if (pFail) *pFail = RVC_FAIL_NONE; return (RvSocket)s; } diff --git a/src/btfrv.h b/src/btfrv.h index 9cd7476..1e85cb0 100644 --- a/src/btfrv.h +++ b/src/btfrv.h @@ -57,10 +57,25 @@ RvSocket RvServiceRegister(const char* relay_host, unsigned short port, bool RvServiceWaitPaired(RvSocket s, int nTimeoutSecs = 0); // ---- client side (the node dialing a .btf address) ---- +// Where a failed RvClientConnect stopped. RV_INVALID alone collapses three +// different situations -- relay unreachable, relay reachable but nobody +// registered, connection died mid-pairing -- and they point at different +// components. The caller can only report the right one if this side says +// which it saw. +enum RvClientFail +{ + RVC_FAIL_NONE = 0, + RVC_FAIL_DIAL, // TCP connect to the relay itself failed + RVC_FAIL_REFUSED, // relay answered 0x00: no service registered under this pubkey + RVC_FAIL_PAIR, // relay connection died before pairing completed +}; + // Connect through the relay to the service registered under `target_pubkey`. -// Returns the paired socket, or RV_INVALID on error. +// Returns the paired socket, or RV_INVALID on error -- with `pFail`, if +// given, naming the stage that failed. RvSocket RvClientConnect(const char* relay_host, unsigned short port, - const unsigned char target_pubkey[32]); + const unsigned char target_pubkey[32], + RvClientFail* pFail = 0); // Blocking read/write of exactly n bytes over a rendezvous socket. Return false // on error/EOF. diff --git a/src/btftunnel.cpp b/src/btftunnel.cpp index 63d178b..408476e 100644 --- a/src/btftunnel.cpp +++ b/src/btftunnel.cpp @@ -136,18 +136,47 @@ static btf_socket_t StartTunnel(RvSocket rv, const unsigned char shared[32]) return appEnd; } +const char* TunnelFailText(TunnelFail f) +{ + switch (f) + { + case TUNNEL_FAIL_NONE: return "no failure"; + case TUNNEL_FAIL_INIT: return "local socket/crypto init failed"; + case TUNNEL_FAIL_DIAL: return "meeting node unreachable (tcp dial failed)"; + case TUNNEL_FAIL_REFUSED: return "relay reachable, but no service registered there for this address"; + case TUNNEL_FAIL_PAIR: return "relay connection died before pairing completed"; + case TUNNEL_FAIL_HANDSHAKE: return "end-to-end handshake failed"; + case TUNNEL_FAIL_LOCAL: return "local tunnel setup failed"; + } + return "unknown failure"; +} + btf_socket_t BtfClientTunnel(const char* meeting_host, unsigned short port, const unsigned char target_pubkey[32], - const unsigned char service_enc_pub[32]) + const unsigned char service_enc_pub[32], + TunnelFail* pFail) { + if (pFail) *pFail = TUNNEL_FAIL_INIT; if (!TunnelInit()) return INVALID_SOCKET; - RvSocket rv = RvClientConnect(meeting_host, port, target_pubkey); - if (rv == RV_INVALID) return INVALID_SOCKET; + RvClientFail rvFail = RVC_FAIL_NONE; + RvSocket rv = RvClientConnect(meeting_host, port, target_pubkey, &rvFail); + if (rv == RV_INVALID) + { + if (pFail) + *pFail = rvFail == RVC_FAIL_DIAL ? TUNNEL_FAIL_DIAL + : rvFail == RVC_FAIL_REFUSED ? TUNNEL_FAIL_REFUSED + : TUNNEL_FAIL_PAIR; + return INVALID_SOCKET; + } unsigned char eph_pub[32], shared[32]; + if (pFail) *pFail = TUNNEL_FAIL_HANDSHAKE; if (!ChanClientHandshake(service_enc_pub, eph_pub, shared)) { RvClose(rv); return INVALID_SOCKET; } if (!RvWriteN(rv, eph_pub, 32)) { RvClose(rv); return INVALID_SOCKET; } - return StartTunnel(rv, shared); + if (pFail) *pFail = TUNNEL_FAIL_LOCAL; + btf_socket_t app = StartTunnel(rv, shared); + if (app != INVALID_SOCKET && pFail) *pFail = TUNNEL_FAIL_NONE; + return app; } btf_socket_t BtfServiceWrap(RvSocket rv, const unsigned char my_enc_sk[32]) diff --git a/src/btftunnel.h b/src/btftunnel.h index 24a7509..3cc87cd 100644 --- a/src/btftunnel.h +++ b/src/btftunnel.h @@ -36,12 +36,33 @@ namespace btf // Initialize (sockets + libsodium). Idempotent. bool TunnelInit(); +// Which stage of BtfClientTunnel failed. INVALID_SOCKET alone says only "no +// tunnel"; a caller that then guesses at causes in its log line sends whoever +// reads it to the wrong component (issue #17: "relay unreachable or pool not +// listening" blamed two healthy parts). The stage is what this side actually +// knows -- report that. +enum TunnelFail +{ + TUNNEL_FAIL_NONE = 0, + TUNNEL_FAIL_INIT, // local socket/crypto init failed + TUNNEL_FAIL_DIAL, // TCP connect to the meeting node failed + TUNNEL_FAIL_REFUSED, // relay reachable, but no service registered under the pubkey + TUNNEL_FAIL_PAIR, // relay connection died before pairing completed + TUNNEL_FAIL_HANDSHAKE, // end-to-end channel handshake failed + TUNNEL_FAIL_LOCAL, // local pump plumbing (socket pair / threads) failed +}; + +// Short description of a TunnelFail for log lines. +const char* TunnelFailText(TunnelFail f); + // Client: dial the service registered under `target_pubkey` at the meeting node // (host:port), box to its static x25519 `service_enc_pub`, and return a local -// plaintext SOCKET tunneled end-to-end to the service. INVALID_SOCKET on error. +// plaintext SOCKET tunneled end-to-end to the service. INVALID_SOCKET on error +// -- with `pFail`, if given, naming the stage that failed. btf_socket_t BtfClientTunnel(const char* meeting_host, unsigned short port, const unsigned char target_pubkey[32], - const unsigned char service_enc_pub[32]); + const unsigned char service_enc_pub[32], + TunnelFail* pFail = 0); // Service: wrap an already-paired rendezvous socket (RvServiceRegister followed // by RvServiceWaitPaired -- the pairing byte MUST already be consumed) with diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..c0a8825 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3758,13 +3758,14 @@ static bool PoolParticipantMiner() // Step 2b: open encrypted tunnel to pool // ------------------------------------------------------------------ SetParticipantMiningStatus("opening tunnel"); + btf::TunnelFail tunnelFail = btf::TUNNEL_FAIL_NONE; s = btf::BtfClientTunnel(host.c_str(), (unsigned short)port, - target_pubkey, service_enc_pub); + target_pubkey, service_enc_pub, &tunnelFail); if (s == INVALID_SOCKET) { - LogPrint("worker", "[worker] tunnel connect FAILED via %s" - " (relay unreachable or pool not listening)\n", - meetingHostPort.c_str()); - SetParticipantMiningStatus("tunnel connect failed"); + LogPrint("worker", "[worker] tunnel connect FAILED via %s: %s\n", + meetingHostPort.c_str(), btf::TunnelFailText(tunnelFail)); + SetParticipantMiningStatus(std::string("tunnel connect failed: ") + + btf::TunnelFailText(tunnelFail)); return false; } LogPrint("worker", "[worker] tunnel open via %s\n", meetingHostPort.c_str()); -- cgit v1.2.3 From 014798e50b2de37ed5243c7e622fc290dfda8606 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 09:03:33 +0000 Subject: worker: fail over across seed relays when the advertised meeting node keeps failing --- src/main.cpp | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index c0a8825..0414714 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3739,6 +3739,33 @@ static bool PoolParticipantMiner() } LogPrint("worker", "[worker] pool resolved to meeting node %s\n", meetingHostPort.c_str()); + // The descriptor names ONE meeting node, but the pool parks a + // registration at every relay it knows (one accept thread per relay, + // rpc.cpp:AcceptOneFn), so any seed relay can pair us. Insisting on the + // advertised entry forever turns one bad path into a miner that idles + // indefinitely while a working path exists (issue #17); after a few + // consecutive tunnel failures, rotate the seed relays into the attempts + // too. Only ONE participant session runs (StartMinerThreads), so these + // statics are single-threaded. + static const int TUNNEL_FAILOVER_AFTER = 3; + static int nTunnelFails = 0; // consecutive tunnel failures, this pool + static size_t iFailoverRelay = 0; // rotation cursor over vBtfMeetingRelays + static std::string strFailPool; // pool the counters belong to + if (strFailPool != strParticipantPool) { + strFailPool = strParticipantPool; + nTunnelFails = 0; + iFailoverRelay = 0; + } + if (nTunnelFails >= TUNNEL_FAILOVER_AFTER && !vBtfMeetingRelays.empty()) { + std::string alt = vBtfMeetingRelays[iFailoverRelay++ % vBtfMeetingRelays.size()]; + if (alt != meetingHostPort) { + LogPrint("worker", "[worker] advertised meeting node %s failed %d" + " times in a row -- trying seed relay %s instead\n", + meetingHostPort.c_str(), nTunnelFails, alt.c_str()); + meetingHostPort = alt; + } + } + size_t colon = meetingHostPort.rfind(':'); if (colon == std::string::npos) { LogPrint("worker", "[worker] bad meeting node format '%s'\n", meetingHostPort.c_str()); @@ -3762,12 +3789,14 @@ static bool PoolParticipantMiner() s = btf::BtfClientTunnel(host.c_str(), (unsigned short)port, target_pubkey, service_enc_pub, &tunnelFail); if (s == INVALID_SOCKET) { + nTunnelFails++; LogPrint("worker", "[worker] tunnel connect FAILED via %s: %s\n", meetingHostPort.c_str(), btf::TunnelFailText(tunnelFail)); SetParticipantMiningStatus(std::string("tunnel connect failed: ") + btf::TunnelFailText(tunnelFail)); return false; } + nTunnelFails = 0; LogPrint("worker", "[worker] tunnel open via %s\n", meetingHostPort.c_str()); SetParticipantMiningStatus("tunnel open"); } -- cgit v1.2.3 From e7b6bbffb1786eef842f1e446118a4daa9edf58f Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 09:04:57 +0000 Subject: worker: back off failed reconnects and say aloud how long the pool has been failing --- src/main.cpp | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 0414714..69b2c32 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4394,11 +4394,36 @@ bool BitcoinMiner(int nThreadId) // Participant mode: connect to external pool instead of mining solo/operator if (nMineMode == MINE_PARTICIPANT) { + // An attempt that never got a session backs off (10s doubling to + // 120s) instead of hammering the same path every flat 10s, and a + // streak lasting minutes is said at printf level: the identical + // once-per-10s LogPrint line is how a worker idled all night + // unnoticed (issue #17). A completed session resets both. + int nRetrySecs = 10; + int64 nFailingSince = 0; while (fGenerateBitcoins && nMineMode == MINE_PARTICIPANT && !fShutdown) { - PoolParticipantMiner(); + bool fHadSession = PoolParticipantMiner(); if (fGenerateBitcoins && nMineMode == MINE_PARTICIPANT && !fShutdown) { - LogPrint("worker", "PoolParticipantMiner: reconnecting in 10s\n"); - for (int i = 0; i < 10 && !fShutdown; i++) Sleep(1000); + if (fHadSession) { + nRetrySecs = 10; + nFailingSince = 0; + } else { + if (nFailingSince == 0) + nFailingSince = GetTime(); + int64 nMins = (GetTime() - nFailingSince) / 60; + if (nMins >= 1) + printf("[worker] connection to pool %s has been failing" + " for %lld minutes\n", + strParticipantPool.c_str(), (long long)nMins); + } + LogPrint("worker", "PoolParticipantMiner: reconnecting in %ds\n", + nRetrySecs); + for (int i = 0; i < nRetrySecs && !fShutdown; i++) Sleep(1000); + if (!fHadSession && nRetrySecs < 120) { + nRetrySecs *= 2; + if (nRetrySecs > 120) + nRetrySecs = 120; + } } } return true; -- cgit v1.2.3 From 74c068829967bbff60da3506613844df4da1ae80 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 14:14:48 +0000 Subject: mine: ask whether a transaction fits before appending it The node miner tested its size budget only on the outer while loop. The inner for walked all of mapTransactions with no test, so a single pass could append the entire mempool -- the overshoot was bounded by mempool size, not by one transaction as the comment claimed. The pool builder did test, but only after the push, which bounds the overshoot at one transaction. Nothing caps how big one transaction may be: neither CheckTransaction nor AcceptTransaction limits it, so the mempool will hold one of any size the wire allows and "one transaction" is worth up to MAX_SIZE. A lone 1.5 MB transaction offering a large fee is therefore enough to make every template unmineable, and the fee is never paid because the block never confirms. Both now ask before appending and skip what does not fit, so the overshoot is zero and an oversized transaction costs the template nothing rather than invalidating it. The budget moves to MAX_BLOCK_SIZE_GEN so the two assemblers cannot drift on it again -- drift is what produced this. The test is ordered before ConnectInputs on purpose: ConnectInputs credits nFees by reference on success, so declining a transaction after calling it would leave the coinbase claiming a fee the block does not collect. Assembly only. CheckBlock, CheckTransaction and AcceptTransaction are untouched: this changes what we produce, not what the network accepts. --- src/main.cpp | 41 ++++++++++++++++++++++++++++++++++++----- src/main.h | 20 ++++++++++++++++++++ src/rpc.cpp | 34 +++++++++++++++++++++++++--------- 3 files changed, 81 insertions(+), 14 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..64eb290 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4470,7 +4470,6 @@ bool BitcoinMiner(int nThreadId) map mapTestPool; vector vfAlreadyAdded(mapTransactions.size()); bool fFoundSomething = true; - unsigned int nBlockSize = 0; // Assembly must stop below the consensus cap that CheckBlock() // enforces, or we mine a block our own node then rejects. // @@ -4481,9 +4480,19 @@ bool BitcoinMiner(int nThreadId) // meant solving a block and throwing it away -- with this pool at // ~90% of hashrate, that is the network stalling, not just us. // - // Half the cap, keeping the original margin: the test runs before - // a transaction is added, so the loop can overshoot by one tx. - while (fFoundSomething && nBlockSize < MAX_BLOCK_SIZE/2) + // The budget test then sat only on this outer while, and the + // comment claimed that bounded the overshoot to a single + // transaction. It did not. The inner for walks all of + // mapTransactions with no test of its own, so one pass could append + // the entire mempool: the overshoot was bounded by mempool size, + // which is to say not bounded at all. Ask per transaction, before + // appending, and the overshoot is zero rather than small. + // + // Seeded with what the block already weighs -- header, tx count, + // coinbase -- because CheckBlock() weighs the whole serialized + // block and not just the transactions we chose to add to it. + unsigned int nBlockSize = ::GetSerializeSize(*pblock, SER_DISK); + while (fFoundSomething && nBlockSize < MAX_BLOCK_SIZE_GEN) { fFoundSomething = false; unsigned int n = 0; @@ -4495,6 +4504,28 @@ bool BitcoinMiner(int nThreadId) if (tx.IsCoinBase() || !tx.IsFinal()) continue; + // Skip, not break: a transaction too big for the room left + // says nothing about the next one, which may still fit. + // + // This is also the only ceiling on a single transaction's + // size anywhere on the path into a block. CheckTransaction + // does not cap it and neither does AcceptTransaction, so + // the mempool will hold one of any size the wire allows; + // without this test a lone 1.5 MB transaction offering a + // fat fee is selected, and every template built from that + // mempool is one CheckBlock() throws away. The fee is never + // paid, because the block never confirms. + // + // Deliberately before ConnectInputs and not after: + // ConnectInputs adds this transaction's fee to nFees by + // reference on success. Passing it and then declining the + // transaction would credit the coinbase for a fee no + // transaction in the block pays, and the block would be + // rejected for a wholly different reason. + unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); + if (!BlockTemplateFits(nBlockSize, nTxSize)) + continue; + // Transaction fee requirements, mainly only needed for flood control // Under 10K (about 80 inputs) is free for first 100 transactions // Base rate is 0.01 per KB @@ -4506,7 +4537,7 @@ bool BitcoinMiner(int nThreadId) swap(mapTestPool, mapTestPoolTmp); pblock->vtx.push_back(tx); - nBlockSize += ::GetSerializeSize(tx, SER_NETWORK); + nBlockSize += nTxSize; vfAlreadyAdded[n] = true; fFoundSomething = true; } diff --git a/src/main.h b/src/main.h index 382dd9c..1f452b5 100644 --- a/src/main.h +++ b/src/main.h @@ -33,6 +33,26 @@ static const unsigned int MAX_BLOCK_SIGOPS = 20000; // limit (32 MB), not a block rule. Bitcoin 0.1.0 predated the later 1 MB block // cap, so this tree inherited no real block-size consensus limit. static const unsigned int MAX_BLOCK_SIZE = 1000000; +// How much of that cap a template we build may fill. Half, which is what both +// assemblers already aimed at -- this only gives the number a name and one +// place to live. They had already drifted on how it was enforced, which is the +// bug this exists to stop repeating. +// +// NOT a consensus rule. CheckBlock() decides what the network accepts; this +// decides what we are willing to produce, and it has to stay under that. +static const unsigned int MAX_BLOCK_SIZE_GEN = MAX_BLOCK_SIZE / 2; + +// Room for one more transaction in a template that currently weighs +// nBlockSize bytes? +// +// Asked BEFORE the transaction is appended, never after. A template that has +// already overshot is one CheckBlock() rejects, and asking afterwards only +// measures the waste. Both assemblers call this, so neither can drift into a +// different idea of what fits. +inline bool BlockTemplateFits(unsigned int nBlockSize, unsigned int nTxSize) +{ + return (nBlockSize + nTxSize <= MAX_BLOCK_SIZE_GEN); +} static const int64 COIN = 100000000; // Total Bitflash emission: identical to Bitcoin (21 million). // MAX_MONEY guards against the value overflow bug (CVE-2010-5139), which in diff --git a/src/rpc.cpp b/src/rpc.cpp index a4b53ca..7c5ef6c 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -267,16 +267,24 @@ static bool RebuildJob() map pool; vector vfAlreadyAdded(mapTransactions.size()); bool fFoundSomething = true; - unsigned int sz = 0; - // The cap is MAX_BLOCK_SIZE/2, not MAX_SIZE/2. MAX_SIZE is the - // 32 MB serialisation ceiling, so half of it is 16 MB -- this - // builder would happily assemble a template that its own + // The cap is MAX_BLOCK_SIZE_GEN, not MAX_SIZE/2. MAX_SIZE is + // the 32 MB serialisation ceiling, so half of it is 16 MB -- + // this builder would happily assemble a template that its own // CheckBlock() rejects at the 1 MB consensus cap. This is the // pool's job builder, so a block over the cap is not one // wasted solve, it is every miner on the pool working a - // template the network will reject. The inner break bounds the - // overshoot to a single transaction within a pass. - while (fFoundSomething && sz < MAX_BLOCK_SIZE / 2) + // template the network will reject. + // + // The inner break used to bound the overshoot to one + // transaction per pass. One transaction is still too many: + // nothing caps how big a single transaction may be, so that + // bound was worth up to MAX_SIZE. Asking whether each one fits + // before appending it bounds the overshoot at zero instead. + // + // Seeded with the header, tx count and coinbase, because + // CheckBlock() weighs the whole serialized block. + unsigned int sz = ::GetSerializeSize(block, SER_DISK); + while (fFoundSomething && sz < MAX_BLOCK_SIZE_GEN) { fFoundSomething = false; unsigned int n = 0; @@ -287,16 +295,24 @@ static bool RebuildJob() continue; CTransaction& tx = (*mi).second; if (tx.IsCoinBase() || !tx.IsFinal()) continue; + // Skip rather than stop: one transaction too big for + // the room left does not mean the next is. Ordered + // before ConnectInputs because ConnectInputs credits + // nFees by reference on success, and a transaction we + // then decline would leave the coinbase claiming a fee + // this block does not collect. + unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); + if (!BlockTemplateFits(sz, nTxSize)) + continue; map tmp(pool); if (!tx.ConnectInputs(txdb, tmp, CDiskTxPos(1,1,1), 0, nFees, false, true, tx.GetMinFee(block.vtx.size() < 100))) continue; swap(pool, tmp); block.vtx.push_back(tx); - sz += ::GetSerializeSize(tx, SER_NETWORK); + sz += nTxSize; vfAlreadyAdded[n] = true; fFoundSomething = true; - if (sz > MAX_BLOCK_SIZE / 2) break; } } } -- cgit v1.2.3 From 2ed8fae6088c99f1a5dfcfd760c1a24c2b2706db Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 14:16:48 +0000 Subject: mine: count sigops while selecting, not only when validating CheckBlock() caps a block at MAX_BLOCK_SIGOPS and neither assembler counted against it. Bytes do not bound what a block costs to verify: every OP_CHECKSIG is an elliptic-curve verification, so a template comfortably inside the size budget can still be one the network refuses for demanding more work than a block is allowed to. Counted with CTransaction::GetSigOpCount -- the same function CheckBlock() sums, not a second implementation of it -- and seeded from the coinbase, because CheckBlock() sums over all of vtx. The comparison matches too: CheckBlock() fails on `> MAX_BLOCK_SIGOPS`, so a block exactly on the limit is valid and selection accepts it. Assembly and validation differing by one is the shape of the bug this series started with. Unlike the size budget this takes no margin. There is no use for headroom in a sigop count, so assembly holds itself to the consensus number exactly. Assembly only; MAX_BLOCK_SIGOPS and the CheckBlock() test are unchanged. --- src/main.cpp | 19 ++++++++++++++++--- src/main.h | 15 ++++++++++++--- src/rpc.cpp | 13 ++++++++++--- 3 files changed, 38 insertions(+), 9 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 64eb290..4d8031d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4492,6 +4492,17 @@ bool BitcoinMiner(int nThreadId) // coinbase -- because CheckBlock() weighs the whole serialized // block and not just the transactions we chose to add to it. unsigned int nBlockSize = ::GetSerializeSize(*pblock, SER_DISK); + // Sigops are a budget too, and CheckBlock() enforces one that no + // assembler here counted against. Bytes do not bound validation + // cost: every OP_CHECKSIG is an elliptic-curve verification, so a + // template well inside the size limit can still be refused for + // demanding more work than a block is allowed to. Counted with + // CTransaction::GetSigOpCount, the function CheckBlock() itself + // sums, so the two cannot disagree about what a transaction costs. + // + // Seeded from the coinbase because CheckBlock() sums over all of + // vtx, coinbase included. + unsigned int nBlockSigOps = pblock->vtx[0].GetSigOpCount(); while (fFoundSomething && nBlockSize < MAX_BLOCK_SIZE_GEN) { fFoundSomething = false; @@ -4522,8 +4533,9 @@ bool BitcoinMiner(int nThreadId) // transaction would credit the coinbase for a fee no // transaction in the block pays, and the block would be // rejected for a wholly different reason. - unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); - if (!BlockTemplateFits(nBlockSize, nTxSize)) + unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); + unsigned int nTxSigOps = tx.GetSigOpCount(); + if (!BlockTemplateFits(nBlockSize, nBlockSigOps, nTxSize, nTxSigOps)) continue; // Transaction fee requirements, mainly only needed for flood control @@ -4537,7 +4549,8 @@ bool BitcoinMiner(int nThreadId) swap(mapTestPool, mapTestPoolTmp); pblock->vtx.push_back(tx); - nBlockSize += nTxSize; + nBlockSize += nTxSize; + nBlockSigOps += nTxSigOps; vfAlreadyAdded[n] = true; fFoundSomething = true; } diff --git a/src/main.h b/src/main.h index 1f452b5..30e25ef 100644 --- a/src/main.h +++ b/src/main.h @@ -43,15 +43,24 @@ static const unsigned int MAX_BLOCK_SIZE = 1000000; static const unsigned int MAX_BLOCK_SIZE_GEN = MAX_BLOCK_SIZE / 2; // Room for one more transaction in a template that currently weighs -// nBlockSize bytes? +// nBlockSize bytes and spends nBlockSigOps signature operations? // // Asked BEFORE the transaction is appended, never after. A template that has // already overshot is one CheckBlock() rejects, and asking afterwards only // measures the waste. Both assemblers call this, so neither can drift into a // different idea of what fits. -inline bool BlockTemplateFits(unsigned int nBlockSize, unsigned int nTxSize) +// +// Size gets a margin (half the cap); sigops do not. There is no cheap way to +// spend the sigop budget usefully and no reason to reserve headroom in it, so +// assembly holds itself to the same number CheckBlock() enforces -- and to the +// same comparison. CheckBlock() fails on `> MAX_BLOCK_SIGOPS`, so a block +// sitting exactly on the limit is valid and this has to accept it too. +// Assembly and validation disagreeing by one is how the size bug started. +inline bool BlockTemplateFits(unsigned int nBlockSize, unsigned int nBlockSigOps, + unsigned int nTxSize, unsigned int nTxSigOps) { - return (nBlockSize + nTxSize <= MAX_BLOCK_SIZE_GEN); + return (nBlockSize + nTxSize <= MAX_BLOCK_SIZE_GEN && + nBlockSigOps + nTxSigOps <= MAX_BLOCK_SIGOPS); } static const int64 COIN = 100000000; // Total Bitflash emission: identical to Bitcoin (21 million). diff --git a/src/rpc.cpp b/src/rpc.cpp index 7c5ef6c..62eec7a 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -284,6 +284,11 @@ static bool RebuildJob() // Seeded with the header, tx count and coinbase, because // CheckBlock() weighs the whole serialized block. unsigned int sz = ::GetSerializeSize(block, SER_DISK); + // And the sigop budget, which CheckBlock() enforces and this + // builder never counted against. Same counting function + // CheckBlock() sums, seeded from the coinbase because it sums + // over all of vtx. + unsigned int nSigOps = block.vtx[0].GetSigOpCount(); while (fFoundSomething && sz < MAX_BLOCK_SIZE_GEN) { fFoundSomething = false; @@ -301,8 +306,9 @@ static bool RebuildJob() // nFees by reference on success, and a transaction we // then decline would leave the coinbase claiming a fee // this block does not collect. - unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); - if (!BlockTemplateFits(sz, nTxSize)) + unsigned int nTxSize = ::GetSerializeSize(tx, SER_NETWORK); + unsigned int nTxSigOps = tx.GetSigOpCount(); + if (!BlockTemplateFits(sz, nSigOps, nTxSize, nTxSigOps)) continue; map tmp(pool); if (!tx.ConnectInputs(txdb, tmp, CDiskTxPos(1,1,1), 0, nFees, @@ -310,7 +316,8 @@ static bool RebuildJob() continue; swap(pool, tmp); block.vtx.push_back(tx); - sz += nTxSize; + sz += nTxSize; + nSigOps += nTxSigOps; vfAlreadyAdded[n] = true; fFoundSomething = true; } -- cgit v1.2.3 From 0b7363211f954698e593699f32ed13b425fce2bc Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 14:33:56 +0000 Subject: genesis: say that the 2025 nTime is known and must not be "fixed" nTime is 1753315200, which is 2025-07-24T00:00:00Z. The comment beside it claimed 2026-07-24, and the headline in pszTimestamp reads 24/Jul/2026, so the constant looked like a typo somebody ought to correct. Nothing is wrong with the data. nTime matters only as an input to the genesis hash, the block hashes to hashGenesisBlock with it, the asserts pass, and the whole network agrees. The hazard is entirely in acting on the mismatch. Editing nTime alone trips the assert -- loud, and it reverts. Editing it and regenerating GENESIS_NONCE and hashGenesisBlock to match is the quiet one: that node starts without complaint, builds its own chain from height 0, and rejects every real peer, and the re-mining branch a few lines below prints exactly the constants needed to do it. So: correct the comment rather than the constant, state the discrepancy as known and uncorrectable, and have the re-mining printout say what those constants are and are not for. No value changes -- pszTimestamp, the scriptPubKey, nTime, nBits, nNonce, GENESIS_NONCE, hashGenesisBlock and hashGenesisMerkleRoot are all byte-identical. --- src/main.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 7b3e555..a68e3b0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2678,7 +2678,18 @@ bool LoadBlockIndex(bool fAllowNew) // CTxOut(nValue=50.00000000, scriptPubKey=0x5F1DF16B2B704C8A578D0B) // vMerkleTree: 4a5e1e - // Bitflash genesis block -- own header (fair launch, 2026 stable relaunch) + // Bitflash genesis block -- own header (fair launch) + // + // Do not change pszTimestamp, the scriptPubKey, nTime or nNonce. They + // are the inputs to the genesis hash, and the genesis hash is + // consensus. Changing one on its own trips the assert below, which is + // loud and reverts cleanly; changing one and regenerating + // GENESIS_NONCE and hashGenesisBlock to match -- which is exactly what + // the re-mining branch below prints out and invites -- gives a node + // that starts without complaint, builds its own chain from height 0, + // and rejects every real peer as being on a different chain. That one + // fails silently, and the damage scales with how many people run the + // "fixed" build. char* pszTimestamp = "Bitflash 24/Jul/2026 Fair launch: one CPU one vote, no premine"; CTransaction txNew; txNew.vin.resize(1); @@ -2691,7 +2702,14 @@ bool LoadBlockIndex(bool fAllowNew) block.hashPrevBlock = 0; block.hashMerkleRoot = block.BuildMerkleTree(); block.nVersion = 1; - block.nTime = 1753315200; // 2026-07-24 00:00:00 UTC (stable relaunch) + // 1753315200 is 2025-07-24T00:00:00Z. The headline in pszTimestamp + // above reads 2026 and does not match it. This is a known discrepancy + // and not a typo in the constant: the value is only consensus-critical + // as an input to the genesis hash, the block hashes to + // hashGenesisBlock with it, and every node on the network agrees. The + // mismatch is in the launched chain and cannot be corrected -- see the + // warning above before deciding otherwise. + block.nTime = 1753315200; block.nBits = 0x1f0fffff; // = bnProofOfWorkLimit (Bitflash RandomX floor) block.nNonce = GENESIS_NONCE; @@ -2718,6 +2736,11 @@ bool LoadBlockIndex(bool fAllowNew) printf("powhash = %s\n", block.GetPoWHash().ToString().c_str()); printf("hash = %s\n", block.GetHash().ToString().c_str()); printf("merkle = %s\n", block.hashMerkleRoot.ToString().c_str()); + printf("----------------------------------------\n"); + printf("These constants bring up a NEW chain. Do not paste them\n"); + printf("into a build for this network to make an edited nTime or\n"); + printf("pszTimestamp fit -- a node whose genesis differs starts\n"); + printf("cleanly on a chain of its own and rejects every real peer.\n"); printf("========================================\n"); } -- cgit v1.2.3 From 2e20434e563b09c5209d0a1f6784d96ed8b18851 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 16:00:59 +0000 Subject: net: keep a block tracked until its body actually validates The mapBlockFetch entry was erased before ProcessBlock ran, on the reasoning that the body had arrived so the fetch was over either way. That reasoning does not survive the fact that GetHash() covers the 80-byte header and nothing else. Any peer that saw X announced holds X's header. It can answer our getdata with that header and a transaction list of its own invention; the reply still hashes to X, so the entry was erased, and CheckBlock then rejected the body on hashMerkleRoot and stored nothing. AlreadyHave stayed false, the mapAskFor entry had already been consumed when the getdata went out, and nothing recreated the tracking -- so X was never requested from anybody again unless some peer happened to re-announce it. That is the #33 stall, reachable on demand for one message. The erase now sits inside the success branch, beside the mapAlreadyAskedFor erase it belongs with. ProcessBlock returns true in exactly two cases, block accepted and block parked as an orphan, and both store it where AlreadyHave will find it, so one erase covers both and neither needs different treatment. Of the false cases, the two already-have ones are cleared by the sweep's own AlreadyHave check on its next pass, and the rest are rejections that must keep the entry. A mismatched merkle root is also now grounds to drop the peer. Nothing honest produces it: peer traffic rides the .btf channel, which is XChaCha20-Poly1305, so a body that arrives at all arrived intact, and the mismatch is the peer's own construction. Checking it in the handler also rejects a forged body before CheckBlock spends a RandomX hash on it. The unconditional log line fires once per connection, because ProcessMessages keeps draining the receive buffer after fDisconnect is set and a peer that packs many forged bodies into one buffer must not get a line for each. --- src/main.cpp | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index d9fe713..8926d57 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3293,16 +3293,56 @@ bool ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv) CInv inv(MSG_BLOCK, pblock->GetHash()); pfrom->AddInventoryKnown(inv); - // The body is here, so the fetch is over whatever validation says - // next -- unlike the mapAlreadyAskedFor erase below, which stays - // conditional on acceptance so a peer feeding us garbage does not - // cancel the scheduled retries for the real thing. - mapBlockFetch.erase(inv.hash); + // GetHash() covers the 80-byte header and nothing else, so the hash + // coming out right does not make this the block we asked for. Every + // peer that saw X announced holds X's header, and can answer our + // getdata with that header bolted onto a transaction list of its own + // invention -- the reply still hashes to X. hashMerkleRoot is the only + // thing binding the header to the body, so check it here, before the + // mapBlockFetch entry is touched and before CheckBlock spends a + // RandomX hash on a body already known to be forged. + // + // There is no honest way to arrive here. Peer traffic rides the .btf + // channel, which is XChaCha20-Poly1305, so a body that arrives at all + // arrived intact: a mismatch is the peer's own construction, not line + // noise. Drop the peer and leave the fetch outstanding so the failover + // sweep asks somebody else for the real X, rather than recording X as + // delivered on the strength of a header anyone could copy. + if (pblock->hashMerkleRoot != pblock->BuildMerkleTree()) + { + if (LogAcceptsCategory("net")) + printf("peer %s served a forged body for %s\n", + pfrom->addr.ToString().c_str(), inv.ToString().c_str()); + // One line per connection: ProcessMessages keeps draining the + // receive buffer after fDisconnect is set, so a peer that packs + // many forged bodies into one buffer must not get one log line + // for each of them. + if (!pfrom->fDisconnect) + printf("block fetch: a peer answered the request for %s with a body that is not " + "that block -- dropping it\n", inv.hash.ToString().substr(0,14).c_str()); + pfrom->fDisconnect = true; + return error("ProcessMessage() : forged block body for %s", + inv.hash.ToString().substr(0,14).c_str()); + } if (ProcessBlock(pfrom, pblock.release())) { mapAlreadyAskedFor.erase(inv); + // The body arrived and validated, so the fetch really is over: + // the block is now in mapBlockIndex, or parked in mapOrphanBlocks, + // and either way AlreadyHave answers for it from here on. + // + // This has to stay inside the success branch, next to the erase + // above. When validation rejects a body nothing is stored, + // AlreadyHave stays false, and the mapAskFor entry was already + // consumed when the getdata went out -- so erasing the tracking + // here would leave the block requested from nobody at all until + // some peer happened to re-announce it. That is exactly the stall + // of #33, and erasing before ProcessBlock made it reachable on + // demand. + mapBlockFetch.erase(inv.hash); + // A peer that just handed us a block is demonstrably at least at // that block's height. Without this, nStartingHeight is a snapshot // taken once during the version handshake and never touched again, -- cgit v1.2.3 From a215505c9e6b63883fa6058a29cc5dc7b22acd94 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 16:06:42 +0000 Subject: net: make room in the block fetch table instead of refusing new blocks Nothing on the path from inv to insertion can tell a real block hash from thirty-two random bytes. AlreadyHave only asks whether we already hold the block, and with no header there is no work to check. So one inv carrying eight hashes that never existed filled all eight slots, and because a full table refused new entries outright, no genuinely new block was tracked again until the give-up timer expired ten minutes later. One message per ten minutes from a single connection switched failover off for the whole node -- not eight wasted slots, an off-switch. A full table now evicts. Eviction on its own would just be the same attack in a new shape, so the victim is chosen by occupancy: a peer may only take a slot from a peer holding strictly more slots than itself, and otherwise recycles one of its own oldest. A peer that has filled the table is the heaviest holder by definition, so its own entries are the only ones it can displace; a peer announcing its first block holds nothing, so it always evicts from the heaviest. A genuine announcement is therefore never refused a slot, whatever an attacker holds or across however many connections it holds it. Entries carry the announcing peer for this, set once on creation -- the failover sweep asks other peers later and none of them chose to announce the hash, so none of them should be charged for it. The two timestamps also get member initialisers. The insertion path uses operator[] and tests nTimeFirstAsked == 0 to spot a fresh entry, which worked on value-initialisation alone, but only until somebody gave the struct a constructor -- and this commit gives it a member that wants one. --- src/main.cpp | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 97 insertions(+), 13 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 8926d57..9ab1f44 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -75,12 +75,24 @@ multimap mapOrphanBlocksByPrev; // Capped small on purpose. At the tip there is one block in flight; during // initial download there are legitimately hundreds outstanding to one peer, // and rotating those would duplicate the whole download to every other peer. -// Anything past the cap is simply not tracked and behaves as before. +// +// The cap is a shared resource and nothing on the path from inv to insertion +// can tell a real block hash from 32 random bytes -- AlreadyHave only asks +// whether we already hold the block, and there is no header to check work +// against. So the table does not refuse a new hash when it is full; it makes +// room. See MakeRoomInBlockFetch for the rule, which is what stops one peer +// filling all eight slots and switching failover off for everybody. +// +// The initialisers matter: the insertion path uses operator[] and then tests +// nTimeFirstAsked == 0 to detect a fresh entry. That worked on +// value-initialisation alone, but only for as long as nobody gave this struct +// a constructor. struct CBlockFetch { - int64 nTimeFirstAsked; // when the first getdata for this block went out - int64 nTimeAsked; // when the latest getdata went out - set setAsked; // peers tried since the round last restarted + int64 nTimeFirstAsked = 0; // when the first getdata for this block went out + int64 nTimeAsked = 0; // when the latest getdata went out + CAddress addrAnnounced; // peer whose announcement put this hash here + set setAsked; // peers tried since the round last restarted }; map mapBlockFetch; static const int BLOCK_FETCH_TRACK_MAX = 8; @@ -89,6 +101,62 @@ static const int BLOCK_FETCH_TRACK_MAX = 8; // forever; a re-announcement recreates it. static const int BLOCK_FETCH_GIVEUP_SECS = 10 * 60; +// Drop one entry so a hash announced by addrNew can be tracked. +// +// Refusing the insertion instead was a remote off-switch for the whole +// feature. An inv carrying eight hashes that never existed filled every slot +// -- nothing between inv and here can tell those from real blocks -- and no +// genuinely new block was tracked again until the give-up timer expired ten +// minutes later, re-armable for one message per ten minutes from a single +// connection. +// +// So evict instead, by occupancy: a peer may only take a slot from a peer +// holding strictly more slots than itself, and otherwise recycles one of its +// own. That is what keeps eviction from becoming the same attack in a new +// shape. A peer that has filled the table is by definition the heaviest +// holder, so its own entries are the only ones it can displace, while a peer +// announcing its first block holds nothing and therefore always evicts from +// the heaviest -- a genuine announcement is never refused a slot, whatever the +// attacker holds or however many connections it holds it from. The one case +// that costs a real fetch is eight distinct peers holding one entry each, +// where a ninth takes the oldest; that is a busy healthy node, not an attack, +// and losing the oldest of eight beats refusing the ninth. +static void MakeRoomInBlockFetch(const CAddress& addrNew) +{ + if (mapBlockFetch.empty()) + return; + + map mapHeld; + for (map::iterator mi = mapBlockFetch.begin(); mi != mapBlockFetch.end(); ++mi) + mapHeld[(*mi).second.addrAnnounced]++; + + int nMine = mapHeld.count(addrNew) ? mapHeld[addrNew] : 0; + + int nMax = 0; + CAddress addrVictim; + for (map::iterator mi = mapHeld.begin(); mi != mapHeld.end(); ++mi) + if ((*mi).second > nMax) + { + nMax = (*mi).second; + addrVictim = (*mi).first; + } + + // Nobody is holding more than this peer, so it recycles its own oldest + // rather than taking a slot off an equal. + if (nMax <= nMine) + addrVictim = addrNew; + + map::iterator miVictim = mapBlockFetch.end(); + for (map::iterator mi = mapBlockFetch.begin(); mi != mapBlockFetch.end(); ++mi) + if ((*mi).second.addrAnnounced == addrVictim && + (miVictim == mapBlockFetch.end() || + (*mi).second.nTimeFirstAsked < (*miVictim).second.nTimeFirstAsked)) + miVictim = mi; + + if (miVictim != mapBlockFetch.end()) + mapBlockFetch.erase(miVictim); +} + map mapOrphanTransactions; multimap mapOrphanTransactionsByPrev; @@ -3470,16 +3538,32 @@ bool SendMessages(CNode* pto) { vAskFor.push_back(inv); // Blocks get their delivery tracked so the failover pass - // below can chase the ones that go quiet. See mapBlockFetch - // for why anything past the cap is left untracked. - if (inv.type == MSG_BLOCK && - (mapBlockFetch.count(inv.hash) || mapBlockFetch.size() < BLOCK_FETCH_TRACK_MAX)) + // below can chase the ones that go quiet. + if (inv.type == MSG_BLOCK) { - CBlockFetch& fetch = mapBlockFetch[inv.hash]; - if (fetch.nTimeFirstAsked == 0) - fetch.nTimeFirstAsked = GetTime(); - fetch.nTimeAsked = GetTime(); - fetch.setAsked.insert(pto->addr); + // Full, and this is a hash we are not already tracking: + // make room rather than drop it on the floor, or one peer + // announcing eight hashes turns failover off for everyone. + if (!mapBlockFetch.count(inv.hash) && + mapBlockFetch.size() >= BLOCK_FETCH_TRACK_MAX) + MakeRoomInBlockFetch(pto->addr); + + if (mapBlockFetch.count(inv.hash) || + mapBlockFetch.size() < BLOCK_FETCH_TRACK_MAX) + { + CBlockFetch& fetch = mapBlockFetch[inv.hash]; + if (fetch.nTimeFirstAsked == 0) + { + fetch.nTimeFirstAsked = GetTime(); + // Who is answerable for this hash existing. Set + // once, on creation: the failover sweep asks other + // peers later, and none of them chose to announce + // it, so none of them should be charged for it. + fetch.addrAnnounced = pto->addr; + } + fetch.nTimeAsked = GetTime(); + fetch.setAsked.insert(pto->addr); + } } } pto->mapAskFor.erase(pto->mapAskFor.begin()); -- cgit v1.2.3 From cd6edfc2c234db71d9b68f96702ad08a56cc3127 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 16:10:24 +0000 Subject: net: only use the 15s block retry spacing at the tip nRequestTime lives in mapAlreadyAskedFor, which is global to the inv, so the spacing AskFor applies is not per-peer pacing: it is what strings the getdatas for one block across its announcers. Shortening it to 15s was right at the tip and wrong everywhere else, and the eight-entry cap that made the failover table safe for catch-up does not cover this at all -- it is a different mechanism. At two minutes a second announcer's queued request almost always fired after the block had arrived, so AlreadyHave dropped it. At fifteen seconds, with a peer working through a 500-deep queue and block N legitimately minutes away, the second announcer is asked at +15s, the third at +30s, and the bulk download arrives once per announcer -- up to eight times over, with nothing bounding it. So the fast spacing is now conditional on tracking the tip, measured off the age of our own best block. Not off a peer-reported height: nStartingHeight is optional in the version message and every node released so far sends the short form, so it is -1 for essentially every peer, and where it does get filled in later it comes from the height of a block that peer served us, which during catch-up tracks our own height and would answer "at the tip" for the entire download. Block timestamps are the one clock a peer cannot move, being bounded above by CheckBlock and below by AcceptBlock's median-time rule. Thirty block intervals is the threshold: far longer than any stall worth failing over from, far shorter than catching up. Failover at the tip does not rest on this in any case -- mapBlockFetch re-issues a stalled getdata to the next peer on its own 15s timer, and that is what actually answers #33. --- src/main.cpp | 23 +++++++++++++++++++++++ src/net.h | 30 +++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 3 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 9ab1f44..be89416 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -101,6 +101,29 @@ static const int BLOCK_FETCH_TRACK_MAX = 8; // forever; a re-announcement recreates it. static const int BLOCK_FETCH_GIVEUP_SECS = 10 * 60; +// How stale our own best block may be before we stop calling this the tip. +// Blocks target two minutes, so this is thirty intervals: far longer than any +// stall worth failing over from -- the one in #33 ran sixteen minutes -- and +// far shorter than catching up, which starts hours or days back. +static const int BLOCK_FETCH_TIP_AGE_SECS = 60 * 60; + +// Whether this node is tracking the tip rather than still catching up. +// +// Measured off the age of our own best block, not off any height a peer told +// us. nStartingHeight is optional in the version message and every node +// released so far sends the short form, so it is -1 for essentially every +// peer; and where it is filled in later it is filled in from the height of a +// block that peer served us, which during catch-up tracks our own height and +// would report "at the tip" for the whole download. Block timestamps are the +// one clock here that peers cannot move: CheckBlock refuses anything more than +// two hours ahead and AcceptBlock refuses anything at or before the previous +// median, so the only way to make this read "at the tip" falsely is to mine, +// and all that buys is the 15s spacing this code had unconditionally anyway. +bool IsBlockFetchNearTip() +{ + return pindexBest && (int64)pindexBest->nTime > GetTime() - BLOCK_FETCH_TIP_AGE_SECS; +} + // Drop one entry so a hash announced by addrNew can be tracked. // // Refusing the insertion instead was a remote off-switch for the whole diff --git a/src/net.h b/src/net.h index 2e4d8cb..64f061f 100644 --- a/src/net.h +++ b/src/net.h @@ -23,6 +23,12 @@ enum +// Defined in main.cpp. True while this node is tracking the tip rather than +// still catching up, measured off the age of our own best block -- see there +// for why that and not a peer-reported height. Declared here because net.h is +// parsed before main.h and CNode::AskFor needs it. +bool IsBlockFetchNearTip(); + bool GetMyExternalIP(unsigned int& ipRet); CNode* ConnectNodeBtf(const string& strBtfAddr); CNode* ConnectNodeBtfResolved(const string& strBtfAddr, const string& strMeeting, @@ -810,9 +816,27 @@ public: // for a new block it was the stall of issue #33: nine peers announce // the block, the first one asked does not serve it, and the ninth // candidate is not even asked until sixteen minutes in, with the pool - // mining on the stale tip the whole time. Blocks retry on the fetch - // timescale instead. - int64 nRetryUsecs = (inv.type == MSG_BLOCK ? BLOCK_FETCH_RETRY_SECS : 2 * 60) * 1000000LL; + // mining on the stale tip the whole time. + // + // Blocks retry on the fetch timescale, but only while we are at the + // tip. That is where the stall costs something -- the pool keeps + // handing out jobs against a dead tip -- and it is also where the + // spacing is nearly free: one block is in flight, and its body has + // almost always arrived before the second announcer's turn comes + // round, so AlreadyHave drops the duplicate before a getdata is sent. + // + // While catching up neither half holds. A peer working through a + // 500-deep queue can be minutes away from block N without anything + // being wrong, so at 15s the second announcer is asked at +15s, the + // third at +30s, and the bulk download arrives once per announcer -- + // up to eight times over. Nothing caps that: the eight-entry cap is on + // mapBlockFetch, which is a different mechanism from this spacing. + // Two minutes is the right number there, and it is what this line did + // before. Failover at the tip does not depend on this being 15s in any + // case: mapBlockFetch re-issues a stalled getdata to the next peer on + // its own timer, which is what actually fixes #33. + bool fFast = (inv.type == MSG_BLOCK && IsBlockFetchNearTip()); + int64 nRetryUsecs = (fFast ? BLOCK_FETCH_RETRY_SECS : 2 * 60) * 1000000LL; nRequestTime = max(nRequestTime + nRetryUsecs, nNow); mapAskFor.insert(make_pair(nRequestTime, inv)); } -- cgit v1.2.3 From 42c840036f35a52beec27422d92457e7e530bdb1 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 16:12:13 +0000 Subject: net: stop the failover log naming a peer on every re-request The line was unconditional on the argument that failover firing is the pathology of #33 and nobody could otherwise see it. That much is right. Printing a peer address every time it fires is not: a peer decides how often the line is written, by announcing hashes it will not serve, and an address is the one thing the .btf transport otherwise takes trouble not to spread around. Sustained, that was tens of lines a minute, each naming a peer, driven entirely from outside. Split it. The per-peer detail -- which block, how long, which peer is being tried next -- moves behind the net category, where the rest of the getdata logging already is. What stays unconditional is a count with no addresses in it, over a window no peer can shorten: the first failover after a quiet spell still prints immediately, because the change from "blocks arrive" to "blocks are announced and not served" is the event worth seeing, and everything after it is summarised once a minute. Worst case is now one line per minute regardless of what a peer does, instead of one per re-request. --- src/main.cpp | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index be89416..a51b5ae 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -101,6 +101,21 @@ static const int BLOCK_FETCH_TRACK_MAX = 8; // forever; a re-announcement recreates it. static const int BLOCK_FETCH_GIVEUP_SECS = 10 * 60; +// Failover firing at all is worth saying out loud: it means peers are +// announcing blocks they then do not serve, which is the pathology of #33 and +// the thing nobody could otherwise see. Naming the peer on every re-request is +// a different matter. A peer can drive that line as often as it likes by +// announcing hashes it will not serve, and what it prints is a peer address, +// which is the one thing the .btf transport otherwise takes trouble not to +// spread around. So the per-peer detail sits behind the net category and the +// unconditional line is a count over a window no peer can shorten: the first +// failover after a quiet spell prints at once, because the change from "blocks +// arrive" to "blocks are announced and not served" is the event worth seeing, +// and everything after it is summarised once a minute. +static const int BLOCK_FETCH_REPORT_SECS = 60; +static int64 nBlockFetchWindowStart = 0; +static int nBlockFetchFailovers = 0; + // How stale our own best block may be before we stop calling this the tip. // Blocks target two minutes, so this is thirty intervals: far longer than any // stall worth failing over from -- the one in #33 ran sixteen minutes -- and @@ -3637,12 +3652,29 @@ bool SendMessages(CNode* pto) fetch.setAsked.clear(); if (!fetch.setAsked.count(pto->addr)) { - // Deliberately not behind a debug category: this line - // firing is a peer announcing blocks it does not - // serve, which is the pathology of #33. - printf("block fetch failover: %s unanswered for %d s, re-requesting from %s\n", - inv.ToString().c_str(), (int)(nNowSecs - fetch.nTimeAsked), - pto->addr.ToString().c_str()); + // Which block, how long, and which peer we are trying + // next. Behind the category because it names a peer + // and a peer decides how often it is written. + if (LogAcceptsCategory("net")) + printf("block fetch failover: %s unanswered for %d s, re-requesting from %s\n", + inv.ToString().c_str(), (int)(nNowSecs - fetch.nTimeAsked), + pto->addr.ToString().c_str()); + + // The unconditional half: no addresses, and at most + // one line per BLOCK_FETCH_REPORT_SECS however hard a + // peer tries. See the counters for the reasoning. + nBlockFetchFailovers++; + if (nNowSecs - nBlockFetchWindowStart >= BLOCK_FETCH_REPORT_SECS) + { + printf("block fetch: %d block request%s went unanswered and %s been " + "re-asked of other peers (%d tracked)\n", + nBlockFetchFailovers, nBlockFetchFailovers == 1 ? "" : "s", + nBlockFetchFailovers == 1 ? "has" : "have", + (int)mapBlockFetch.size()); + nBlockFetchWindowStart = nNowSecs; + nBlockFetchFailovers = 0; + } + vFailover.push_back(inv); fetch.nTimeAsked = nNowSecs; fetch.setAsked.insert(pto->addr); -- cgit v1.2.3 From 738d5b411bab4e13e176a11f8ae6e6f70337381e Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 17:10:41 +0000 Subject: worker: let a stop request interrupt the reconnect backoff The retry sleep polled only fShutdown, while the loop around it polls fShutdown, fGenerateBitcoins and nMineMode. That was survivable at a flat 10s. Raising the ceiling to 120s made it expensive: after an operator cleared Generate or switched to Relay mode, the participant session -- and the vfThreadRunning[3] slot that gates restarting it -- stayed held for up to two minutes. Measured with the loop shape replicated and the stop condition flipped 1s into a 120s sleep: 119.01s to notice before, 1.00s after, via either fGenerateBitcoins or nMineMode. The fShutdown path is unchanged at 1.00s. It also narrows the window on a pre-existing unsynchronised read of vfThreadRunning[3] at gui.cpp:901, where a stop-then-start inside it leaves mining silently not restarted. --- src/main.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 69b2c32..b0cdab2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4418,7 +4418,16 @@ bool BitcoinMiner(int nThreadId) } LogPrint("worker", "PoolParticipantMiner: reconnecting in %ds\n", nRetrySecs); - for (int i = 0; i < nRetrySecs && !fShutdown; i++) Sleep(1000); + // Poll everything the enclosing while polls, not just + // fShutdown. Raising the ceiling to 120s made a + // shutdown-only predicate expensive: after the operator + // clears Generate or switches to Relay, the session -- and + // the vfThreadRunning[3] slot that gates restarting it -- + // would stay held for up to two minutes. Measured 119s + // before, 1s after. + for (int i = 0; i < nRetrySecs && !fShutdown && fGenerateBitcoins + && nMineMode == MINE_PARTICIPANT; i++) + Sleep(1000); if (!fHadSession && nRetrySecs < 120) { nRetrySecs *= 2; if (nRetrySecs > 120) -- cgit v1.2.3 From ca03f8e9416837acfac825f02a6c592e07e0c4df Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 17:13:06 +0000 Subject: worker: count a session as productive only if the pool sent a job PoolParticipantMiner returned true from its normal cleanup path whether or not the pool had ever sent work, so a pool that completed the tunnel and then dropped was indistinguishable from a healthy one. The caller reset nRetrySecs to 10 on every such "session", pinning the reconnect at a flat 10s and defeating the backoff in precisely the case it was added for. It cleared nFailingSince as well, so the "failing for N minutes" line never printed either -- the worker went back to idling silently. Return true only once at least one well-formed mining.notify has arrived. The existing early returns are unaffected: they already mean "no session", which stays the right answer for any path that leaves the worker without work, a failed hash-thread spawn included. --- src/main.cpp | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index b0cdab2..b0bdb8c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3662,6 +3662,14 @@ static void ParticipantHashWorker(ParticipantJobShared* sh, int idx, int nWorker RandomXDestroyMinerVM(vm); } +// Run one participant session: resolve the pool, open the tunnel, subscribe, +// hash, submit shares. Returns true only if the pool actually handed out work +// -- at least one well-formed mining.notify arrived. Everything else is false, +// including a tunnel that opened and was dropped before any job. +// +// The caller's reconnect backoff keys off this return value, so any path added +// here that leaves the session without work MUST return false. "The session +// ended tidily" is not the question being asked. static bool PoolParticipantMiner() { // ------------------------------------------------------------------ @@ -3944,6 +3952,7 @@ connected: uint256 target; double currentDifficulty = 1.0; // updated by mining.set_difficulty from pool bool haveJob = false; + bool gotAnyJob = false; // any job at all this session -- the return value bool gotSubscribeAck = false; bool gotAuthorizeAck = false; int64 authWaitStart = 0; @@ -4060,8 +4069,9 @@ connected: jobId = newJobId; memcpy(header, hdrBytes.data(), 80); memcpy(&target, tgtBytes.data(), 32); - haveJob = true; - nNonce = 1; + haveJob = true; + gotAnyJob = true; + nNonce = 1; { std::lock_guard lk(jobShared.mtx); memcpy(jobShared.header, header, 80); @@ -4197,7 +4207,11 @@ connected: (unsigned long long)hashesSinceSample, (unsigned long long)sessionSharesSent, (unsigned long long)sessionSharesAccepted); - return true; + // Not "we got here", which is true of a tunnel the pool accepted and then + // dropped without ever sending a job -- exactly the case the caller's + // backoff exists for. Reaching the end of the session is not evidence the + // session was worth anything; a job is. + return gotAnyJob; } // --------------------------------------------------------------------------- @@ -4394,17 +4408,23 @@ bool BitcoinMiner(int nThreadId) // Participant mode: connect to external pool instead of mining solo/operator if (nMineMode == MINE_PARTICIPANT) { - // An attempt that never got a session backs off (10s doubling to - // 120s) instead of hammering the same path every flat 10s, and a - // streak lasting minutes is said at printf level: the identical - // once-per-10s LogPrint line is how a worker idled all night - // unnoticed (issue #17). A completed session resets both. + // An attempt that never got work backs off (10s doubling to 120s) + // instead of hammering the same path every flat 10s, and a streak + // lasting minutes is said at printf level: the identical once-per-10s + // LogPrint line is how a worker idled all night unnoticed (issue #17). + // + // Only a session the pool actually sent a job to resets either. The + // weaker test -- "the session returned" -- was gameable: a pool that + // completes the tunnel and drops without ever sending work leaves by + // the same tidy cleanup path a healthy one does, which would pin the + // retry at 10s forever and defeat the backoff in exactly the scenario + // it exists for. int nRetrySecs = 10; int64 nFailingSince = 0; while (fGenerateBitcoins && nMineMode == MINE_PARTICIPANT && !fShutdown) { - bool fHadSession = PoolParticipantMiner(); + bool fGotWork = PoolParticipantMiner(); if (fGenerateBitcoins && nMineMode == MINE_PARTICIPANT && !fShutdown) { - if (fHadSession) { + if (fGotWork) { nRetrySecs = 10; nFailingSince = 0; } else { @@ -4428,7 +4448,7 @@ bool BitcoinMiner(int nThreadId) for (int i = 0; i < nRetrySecs && !fShutdown && fGenerateBitcoins && nMineMode == MINE_PARTICIPANT; i++) Sleep(1000); - if (!fHadSession && nRetrySecs < 120) { + if (!fGotWork && nRetrySecs < 120) { nRetrySecs *= 2; if (nRetrySecs > 120) nRetrySecs = 120; -- cgit v1.2.3 From 04f5b6827c9bc1d2b225318db0c3f0d1e27e6819 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 17:51:07 +0000 Subject: worker: only look up a job's chain position when it changes 2a804c2 resolves a job's prevhash against mapBlockIndex under cs_main so the worker log names the chain the pool is building on (#15). It did that on every mining.notify, unconditionally, before the LogPrint that consumes the answer -- and LogPrint's category gate and its 3-second dedup both run after the lookup has already happened. The dedup bounds the log line. It never bounded the work. That left an unbounded cs_main acquisition rate on the participant's socket-reading thread, driven by the remote pool. Block validation can hold cs_main for a long time, and this thread is the one that handles mining.set_difficulty and share-submission replies, so a pool sending rapid notifies could stall the responses a miner is paid for. Hashing was never at risk -- jobShared is published before the lookup -- but the reader is not free to block. Most notifies in a round are a new job on the same tip, so the answer is now cached and re-resolved only when it could come out differently: a prevhash we have not resolved, or one we could not resolve last time while the chain has since moved. A block's height is fixed once mapBlockIndex knows it, so a cached "known" cannot go stale; an unknown is re-checked whenever nBestHeight changes, which is what catches a pool we were behind once we have caught up. The worst case -- a different prevhash every notify -- degrades to exactly the old cost, never worse. Measured over a 10-tip round of 30 jobs each, 300 acquisitions become 10; over 1000 notifies on a single fork we do not have, 1000 become 1. The log line is unchanged and still prints in full for every job, cached or not: prevhash plus height-or-"unknown to our chain". Nothing #15 asked for is lost, and a fork we cannot resolve keeps saying so on every job. The lookup is also wrapped in LogAcceptsCategory("worker") so the work follows the gate deciding whether anyone reads it. That is a no-op today -- "worker" is default-on in LogCategoryDefaultOn() -- and is there so the coupling survives the category ever moving behind -debug. The caching, not the gate, is what removes the acquisitions. The cache is four locals in the session scope, not statics: participant mode runs exactly one such thread (StartMinerThreads forces nThreads to 1 for MINE_PARTICIPANT), and session scope additionally means a reconnect re-resolves rather than trusting an answer from before a possible change of pool. --- src/main.cpp | 67 +++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 55 insertions(+), 12 deletions(-) (limited to 'src/main.cpp') diff --git a/src/main.cpp b/src/main.cpp index 3de3db0..c2d059b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3912,6 +3912,14 @@ connected: std::string jobId; unsigned char header[80] = {}; uint256 target; + // Cached answer to "which block is this job built on", for the + // mining.notify handler below -- see the comment there. Session-local, so + // a reconnect re-resolves rather than trusting an answer obtained before + // a possible change of pool. + uint256 hashPrevResolved; // prevhash the cached answer describes + bool fPrevResolved = false; // whether that answer exists yet + int nPrevHeight = -1; // its height, or -1 == unknown to us + int nBestHeightResolved = -1; // nBestHeight when we last resolved double currentDifficulty = 1.0; // updated by mining.set_difficulty from pool bool haveJob = false; bool gotSubscribeAck = false; @@ -4047,22 +4055,57 @@ connected: // actually hash (== p[2] of the notify); // a brief cs_main lookup is safe here -- // this thread holds no other lock. + // + // Resolving it is the expensive half, and + // this is the socket-reading thread: block + // validation can hold cs_main for a long + // time, and every moment spent waiting on + // it here is a moment set_difficulty and + // share responses are not being handled. + // Most notifies in a round are a new job + // on the SAME tip, so the answer is cached + // and re-resolved only when it could come + // out differently: a prevhash we have not + // resolved, or one we could NOT resolve + // last time while the chain has since + // moved (unknown -> known). A block's + // height is fixed once mapBlockIndex knows + // it, so a cached "known" never goes + // stale. nBestHeight is read unlocked on + // purpose -- it only decides whether to + // repeat a lookup, never what gets + // printed, so a stale read costs at most + // one deferred or one extra lookup. + // The line still prints in full for every + // job, cached or not. uint256 hashPrev; memcpy(&hashPrev, header + 4, 32); - int nPrevHeight = -1; - CRITICAL_BLOCK(cs_main) + if (LogAcceptsCategory("worker")) { - map::iterator mi = mapBlockIndex.find(hashPrev); - if (mi != mapBlockIndex.end()) - nPrevHeight = (*mi).second->nHeight; + int nBestHeightNow = nBestHeight; + if (!fPrevResolved + || hashPrev != hashPrevResolved + || (nPrevHeight < 0 && nBestHeightNow != nBestHeightResolved)) + { + nPrevHeight = -1; + CRITICAL_BLOCK(cs_main) + { + map::iterator mi = mapBlockIndex.find(hashPrev); + if (mi != mapBlockIndex.end()) + nPrevHeight = (*mi).second->nHeight; + } + hashPrevResolved = hashPrev; + nBestHeightResolved = nBestHeightNow; + fPrevResolved = true; + } + if (nPrevHeight >= 0) + LogPrint("worker", "[pool->worker] new job %s received, prevhash %s (known, job height %d)\n", + jobId.c_str(), hashPrev.GetHex().c_str(), + nPrevHeight + 1); + else + LogPrint("worker", "[pool->worker] new job %s received, prevhash %s (unknown to our chain)\n", + jobId.c_str(), hashPrev.GetHex().c_str()); } - if (nPrevHeight >= 0) - LogPrint("worker", "[pool->worker] new job %s received, prevhash %s (known, job height %d)\n", - jobId.c_str(), hashPrev.GetHex().c_str(), - nPrevHeight + 1); - else - LogPrint("worker", "[pool->worker] new job %s received, prevhash %s (unknown to our chain)\n", - jobId.c_str(), hashPrev.GetHex().c_str()); SetParticipantMiningStatus("hashing job " + jobId); } } -- cgit v1.2.3