From d7b1e8a1d54192df2fcf633fbcdffb43389ff8e2 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 05:42:17 +0000 Subject: pool: guard the share-target floor where the old cap branch was dead ShareTargetFromDifficulty ended with if (t < diff1 && difficulty < 1.0) t = ~uint256(0); which can never fire: t = diff1/difficulty, so difficulty < 1.0 implies t > diff1 and the two conditions exclude each other. The branch was reaching for a real hazard, though, so say the hazard directly instead of deleting the guard: diff1 is ~uint256(0) >> 12, a hair under 2^244, so the true share target stops fitting in a uint256 once difficulty reaches 2^-12 (1/4096), and the fixed-point divisor (uint64)(difficulty * 65536.0) truncates to zero -- a division by zero -- for any difficulty under 2^-16. Past the first threshold getuint256() would return the low 256 bits of an oversized quotient, a wrapped target that is arbitrarily HARDER than difficulty 1. Both thresholds sit far below VARDIFF_MIN (0.001), so nothing live comes near this; the guard exists for any caller that bypasses vardiff. At those difficulties every hash is meant to be a share, so return all-ones, the easiest representable target. Boundary verified numerically against the exact fixed-point algorithm: at difficulty exactly 1/4096 the quotient is 2^256 - 4096 (fits), one ulp below it overflows, and below 2^-16 the divisor is zero. Closes #11 --- src/rpc.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index a4b53ca..82f895b 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -115,15 +115,28 @@ static uint256 ShareTargetFromDifficulty(double difficulty) { if (difficulty <= 0.0) difficulty = 1.0; static const uint256 diff1 = CBigNum(bnProofOfWorkLimit).getuint256(); + // Representability floor. shareTarget = diff1 / difficulty, and diff1 is + // ~uint256(0) >> 12, i.e. a hair under 2^244 -- so the true target stops + // fitting in a uint256 once difficulty reaches 2^-12 (= 1/4096), and the + // fixed-point divide below breaks even earlier in a second way: the + // (uint64)(difficulty * 65536.0) divisor truncates to zero for any + // difficulty under 2^-16. Both thresholds sit far below VARDIFF_MIN + // (0.001), so live operation never comes near this, but a caller that + // bypasses vardiff (or a future VARDIFF_MIN) must not divide by zero or + // hand back getuint256()'s low-256-bit truncation of an oversized + // quotient -- a wrapped target is not merely wrong, it is arbitrarily + // HARDER than difficulty 1. At these difficulties every hash is meant to + // be a share, so the honest answer is the easiest representable target. + // (This replaces a dead branch that read `if (t < diff1 && difficulty < + // 1.0)`: t = diff1/difficulty, so difficulty < 1.0 implies t > diff1 and + // the two conditions could never both hold.) + if (difficulty < 1.0 / 4096.0) return ~uint256(0); // shareTarget = diff1 / difficulty CBigNum bn; bn.setuint256(diff1); // Multiply by 2^16 to keep precision, divide, shift back bn <<= 16; bn /= (uint64)(difficulty * 65536.0); - uint256 t = bn.getuint256(); - // Cap at all-ones (maximum, easiest possible) - if (t < diff1 && difficulty < 1.0) t = ~uint256(0); - return t; + return bn.getuint256(); } // Expected number of hashes to find one share at a given Stratum difficulty, -- cgit v1.2.3 From a3666bded1553b101b631d87e99961bf3ccd0a3e Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 05:44:41 +0000 Subject: pool: refuse a (job, nonce) pair this connection already submitted mining.submit validated the nonce, rebuilt the block, re-hashed it with RandomX -- and never asked whether it had seen this exact (job_id, nonce) pair before. Nothing in that pipeline distinguishes a replay from a first submission: a duplicated valid share passes every check and lands in roundShareCount[address]++ a second time, and roundShareCount is the divisor the payout split uses, so replaying one valid share N times multiplies that address's cut of the round by N. Keep a per-connection set of credited nonces on the Miner, reject a repeat with a "Duplicate share" stratum error before the rebuild and hash it would waste, and record the nonce at the moment the share is credited. The set is keyed to the job id the miner is currently working and cleared inside SendJob -- the single funnel every new-job path (initial authorize, block found, new block, 60s refresh, no-job watchdog, vardiff retargets) goes through -- so legitimate resubmissions across jobs are never refused. The clear is keyed on the id rather than unconditional because the vardiff retarget and stall watchdog re-send the CURRENT job, and an unconditional clear there would reopen the very replay window this closes. Job ids come from a monotonic atomic counter in RebuildJob, so an id is never recycled within a process. Closes #8 --- src/rpc.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index 82f895b..f1a7153 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -2511,6 +2511,16 @@ struct Miner { int64 lastShareTime = 0; // GetTime() of the previous accepted share double hashRate = 0.0; // smoothed H/s estimate + // Replay guard for mining.submit (#8): nonces already credited on this + // connection for the job it is currently working. Keyed to the job id in + // seenNoncesJob and cleared by SendJob whenever that id changes, so a + // resubmission under a NEW job is never turned away, while a replayed + // (job_id, nonce) pair under the same job is refused before it can be + // credited twice into roundShareCount -- the figure the payout split + // divides by. + std::set seenNonces; + std::string seenNoncesJob; + explicit Miner(btf_socket_t f) : fd(f), connectTime(GetTime()), extranonce2(gNextExtranonce2.fetch_add(1)) {} @@ -2693,6 +2703,17 @@ static json MakeNotifyParams(const StratumJob& job, const CBlock& b, static void SendJob(Miner* m, bool clean) { if (!gHaveJob) return; + // Every path that hands a miner a new job funnels through here, so this + // is the one place the replay guard needs to follow the job id: nonces + // credited under the previous job mean nothing under the new one. Keying + // the clear to the id (rather than clearing unconditionally) keeps a + // same-job re-notify -- the vardiff retarget and stall watchdog both + // re-send the CURRENT job -- from reopening the duplicate window it + // exists to close. + if (m->seenNoncesJob != gCurrentJob.jobId) { + m->seenNonces.clear(); + m->seenNoncesJob = gCurrentJob.jobId; + } CBlock mb = BuildMinerBlock(gCurrentJob, m->extranonce2); // Use this miner's own shareTarget (set by vardiff) in the notify params. // The participant miner reads p[3] as its share target, and HandleLine @@ -2951,6 +2972,22 @@ static bool HandleLine(Miner* m, const std::string& rawLine, } unsigned int nNonce; memcpy(&nNonce, nb.data(), 4); + // Reject a replayed (job, nonce) before it costs a rebuild and a + // RandomX hash: nothing about the work below distinguishes a second + // submission of a valid share from the first, so without this check + // it passes every test and is credited again -- and roundShareCount + // is what the payout split divides by, so each replay multiplies the + // address's cut of the round. The set is per-connection and cleared + // by SendJob on every job change, so only a same-job repeat is + // refused. + if (m->seenNonces.count(nNonce)) { + LogPrint("worker", "[worker->pool] duplicate share from %s" + " nonce=%u job=%s -- already credited, not counting again\n", + m->address.c_str(), nNonce, submitJobId.c_str()); + reply(false, "Duplicate share"); + return true; + } + // Rebuild this miner's exact block (unique merkle via extranonce2) CBlock b = BuildMinerBlock(gCurrentJob, m->extranonce2); b.nNonce = nNonce; @@ -2978,6 +3015,7 @@ static bool HandleLine(Miner* m, const std::string& rawLine, m->roundShares++; m->totalShares++; m->vardiffShares++; + m->seenNonces.insert(nNonce); // replay guard: credit each nonce once per job int64 shareTime = GetTime(); m->lastSeen = shareTime; roundShareCount[m->address]++; -- cgit v1.2.3 From 2bca62a6a9984297633af42d9e19d1f328d2f7f2 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 05:46:57 +0000 Subject: pool: decay a miner's hashrate estimate when its shares stop arriving m->hashRate is an EWMA re-sampled only on the valid-share path, with no time-based term: a miner that stops hashing keeps its last estimate for as long as the connection stays open, and RefreshStats sums every such stale estimate into gStats.totalHashRate, which is what the pool status and the nostr announcement publish. The announced pool hashrate ran several times over ground truth because miners that had gone quiet were still contributing their last sample. Decay on time in the event loop's miner sweep. Per miner, compute the expected share interval -- ExpectedHashesForDifficulty(m->difficulty) divided by its current rate estimate -- and give it a grace of four such intervals with no share (at the vardiff target of one share per 15s that is about a minute of silence). Past the grace, decay the estimate with a half-life of one more expected interval, computed from absolute time so the curve does not depend on the loop's tick rate. The interval is clamped to [1s, 1h] so a fast miner at the vardiff floor, whose expected interval is milliseconds, is not decayed for ordinary share-delivery jitter, and a very slow miner's grace cannot stretch past the point where its estimate means anything. Per-share crediting and ExpectedHashesForDifficulty are untouched: this changes only what is reported about a miner that has stopped reporting itself. Closes #7 --- src/rpc.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index f1a7153..23e25ef 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -2484,6 +2484,26 @@ static const double VARDIFF_MAX = 1e12; static const int64 VARDIFF_STALL_SECS = 45; // 3x share target -- reacts faster without false-triggering on normal variance static const double VARDIFF_STALL_CUT = 4.0; // divide difficulty by this much +// #7: m->hashRate is an EWMA sampled only when a share arrives, so a miner +// that stops hashing keeps its last estimate forever and RefreshStats sums +// it into the announced pool total for as long as the connection sits open +// -- the published pool hashrate ran several times over ground truth +// because of exactly this. The decay pass in PoolEventLoop applies this +// policy: a miner gets a grace of HASHRATE_IDLE_MISSES expected share +// intervals (expected interval = expected hashes per share at its current +// difficulty / its last rate estimate) with no share, then its estimate +// decays with a half-life of one more such interval, computed from +// absolute time so the result does not depend on how often the loop ticks. +// The interval is clamped to [HASHRATE_MIN_INTERVAL, HASHRATE_MAX_INTERVAL] +// so a fast miner at the vardiff floor (expected interval of milliseconds) +// is not punished for ordinary share-delivery jitter, and a very slow +// miner's grace does not stretch past the point where its estimate means +// anything. At the vardiff target of one share per 15s, a stopped miner's +// announced rate is gone within minutes. +static const double HASHRATE_IDLE_MISSES = 4.0; // grace, in expected share intervals +static const double HASHRATE_MIN_INTERVAL = 1.0; // seconds; floor for the expected interval +static const double HASHRATE_MAX_INTERVAL = 3600.0; // seconds; ceiling for the expected interval + struct Miner { btf_socket_t fd; bool authorised = false; @@ -4900,6 +4920,26 @@ static void PoolEventLoop() } } + // 3.6. Hashrate decay (#7) -- see the policy comment at + // HASHRATE_IDLE_MISSES. The EWMA is only re-sampled on share arrival, + // so without this pass a miner that stops hashing keeps its last + // estimate forever and the summed pool hashrate RefreshStats (and + // the nostr announcement built from it) publishes is fiction. + for (Miner* m : gMiners) { + if (m->hashRate <= 0.0 || m->lastShareTime <= 0) continue; + double expected = ExpectedHashesForDifficulty(m->difficulty) / + std::max(m->hashRate, 1e-9); + if (expected < HASHRATE_MIN_INTERVAL) expected = HASHRATE_MIN_INTERVAL; + if (expected > HASHRATE_MAX_INTERVAL) expected = HASHRATE_MAX_INTERVAL; + double silent = (double)(GetTime() - m->lastShareTime); + double grace = HASHRATE_IDLE_MISSES * expected; + if (silent <= grace) continue; + // Half-life of one expected interval past the grace period. + // Written off absolute time, not per-tick, so the decay curve is + // the same however often this loop runs. + m->hashRate *= pow(0.5, (silent - grace) / expected); + } + // 4. Select over miner sockets if (!gMiners.empty()) { fd_set fds; FD_ZERO(&fds); -- cgit v1.2.3 From f5def7625e6369d117cc8f02b7c0244b70d6f779 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 05:49:27 +0000 Subject: pool: reap miners that have said nothing for ten minutes A Miner left gMiners in exactly one place: RecvLine reporting a clean disconnect. Half-open TCP sessions send no FIN and raise no error, so they lived forever -- on the live pool authorize outpaced disconnect 2:1, over half the table never submitted a share, and the leak grew toward MAX_POOL_MINERS, past which the accept path refuses real miners. Two changes. First, an idle reap in the event loop's miner sweep: any miner whose last activity is older than POOL_IDLE_TIMEOUT_SECS (600s) is closed and erased through exactly the bookkeeping the clean-disconnect path uses -- log, sock_close, erase via std::find, delete -- collected into a side vector first so the sweep's iteration stays valid. Shares an idle miner already earned stay in roundShareCount, just as they do when a miner cleanly disconnects mid-round. To give the reap something honest to measure, lastSeen is now stamped on every complete line received (authorize and valid shares already stamped it; a miner that only subscribes, or sends unrecognised methods, previously looked dead from birth), with connectTime standing in for a connection that never completed a line. Ten minutes is far beyond the 15s vardiff share target and the 45s stall watchdog, so no miner that is actually hashing comes close. Second, SO_KEEPALIVE on sockets accepted by the loopback stratum listener, so a half-open session eventually errors out of recv and lands in the reap path on its own. This is a backstop only: the default cadence before the first probe is hours on most stacks, so the idle timeout fires long before keepalive does. Miners arriving via a relay ride a loopback socket pair whose real transport is the rendezvous socket owned by the tunnel pump threads, which keepalive cannot reach across -- for those the idle reap is also what finally tears the tunnel down, since closing the app end unblocks the pumps. Closes #6 --- src/rpc.cpp | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index 23e25ef..df7ca05 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -2596,6 +2596,18 @@ static void LocalAcceptFn(void*) break; continue; } + // #6: let the OS probe a peer that has gone silent, so a half-open + // session eventually errors out of recv and lands in the event + // loop's existing reap path instead of sitting in gMiners forever. + // This is a backstop, not the mechanism: the default cadence before + // the first probe is hours on most stacks, so the idle timeout in + // PoolEventLoop fires long before keepalive does. Miners arriving + // via a relay ride a loopback socket pair whose remote transport is + // the rendezvous socket owned by the tunnel pump threads; keepalive + // cannot reach across that, which is part of why the idle timeout + // exists. + int keepalive = 1; + setsockopt(c, SOL_SOCKET, SO_KEEPALIVE, (char*)&keepalive, sizeof(keepalive)); LogPrint("pool", "[pool] miner arrived via loopback -- pushing to event loop\n"); PushReady((btf_socket_t)c); } @@ -2639,6 +2651,16 @@ static const int64 POOL_ACTIVE_WINDOW = 300; // a miner is "active" if it static const int64 POOL_UNAUTH_GRACE_SECS = 60; // a connection still not // authorized after this long is // idle, not merely starting up +// #6: a Miner used to leave gMiners only when RecvLine reported a clean +// disconnect. A half-open session (miner host died, NAT state expired, +// cable pulled) sends no FIN and reports no error, so it sat in the table +// forever -- authorize outpaced disconnect 2:1 on the live pool, and the +// table grew toward MAX_POOL_MINERS, past which real miners are refused. +// lastSeen is stamped on every complete line received; a miner that has +// said nothing for this long is gone in every way that matters. Ten +// minutes is far beyond the vardiff target (a share every 15s) and the +// stall watchdog (45s), so no miner that is actually hashing comes close. +static const int64 POOL_IDLE_TIMEOUT_SECS = 600; // Non-blocking recv: pull whatever bytes are available now into buf, then // return one complete line if one is ready. Three outcomes: @@ -4940,6 +4962,35 @@ static void PoolEventLoop() m->hashRate *= pow(0.5, (silent - grace) / expected); } + // 3.7. Idle reap (#6). Mirror the clean-disconnect path exactly -- + // log, close, erase from gMiners via std::find, delete -- just with + // a different reason. Shares the miner already earned stay in + // roundShareCount, exactly as they do when a miner cleanly + // disconnects mid-round. connectTime stands in for lastSeen on a + // connection that never completed a line. Erasing inside the loop + // above would invalidate the iteration, so collect then reap. + { + int64 now = GetTime(); + std::vector idle; + for (Miner* m : gMiners) { + int64 last = (m->lastSeen > 0) ? m->lastSeen : m->connectTime; + if (now - last > POOL_IDLE_TIMEOUT_SECS) + idle.push_back(m); + } + for (Miner* m : idle) { + LogPrint("worker", "[pool] dropping idle miner %s -- nothing" + " received in over %llds (session shares: %llu," + " round shares: %llu)\n", + m->worker.empty() ? "(never authorized)" : m->worker.c_str(), + (long long)POOL_IDLE_TIMEOUT_SECS, + (unsigned long long)m->sessionShares, + (unsigned long long)m->roundShares); + sock_close(m->fd); + gMiners.erase(std::find(gMiners.begin(), gMiners.end(), m)); + delete m; + } + } + // 4. Select over miner sockets if (!gMiners.empty()) { fd_set fds; FD_ZERO(&fds); @@ -4962,6 +5013,11 @@ static void PoolEventLoop() // terminate the whole node -- not just the pool. Every // line arrives from an unauthenticated socket. while (RecvLine(m->fd, m->readBuf, line, disc)) { + // Any complete line is proof of life (#6): authorize + // and valid shares already stamp lastSeen themselves, + // but a miner that only ever subscribes (or sends + // unrecognised methods) must count as alive too. + m->lastSeen = GetTime(); try { HandleLine(m, line, roundShareCount, roundShareTotal, blocksFoundThisSession); -- cgit v1.2.3 From 07051b2378734d62fb94a4a62819d8150452b661 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 15:23:02 +0000 Subject: pool: compute a miner's faded hashrate instead of decaying it in place 2bca62a decayed m->hashRate from the event loop's miner sweep, which turned a figure that is only ever displayed into an accumulator, and got the curve wrong at both ends. Each pass recomputed `expected` -- and so `grace` -- from the estimate it had just decayed. Every decay raised `expected`, which raised the grace that gates the next decay, so `grace` chased `silent` upward and the reported rate settled into a 1/t crawl instead of the half-life the commit's own comment documents: 291x too high at ten half-lives past the grace, and still announcing 0.4% of baseline four hours into a silence. #7 is about the pool over-reporting its hashrate, and this under-corrected it for hours. The far end was worse. Once `expected` reached HASHRATE_MAX_INTERVAL it could not rise any further, the feedback that had been gating each pass was gone, and the decay did begin to compound -- once a second, against the total elapsed silence, because lastShareTime does not advance while a miner is quiet and nothing recorded when the decay had last run. A miner four hours silent fell from 0.4% of its baseline to 1e-11 of it over the next hundred seconds, with nothing in the code or the announced number to explain the cliff. Neither behaviour is worth tuning, so drop the sweep. hashRate is read in exactly one place -- RefreshStats, building the per-worker rows and the total that the pool status and the nostr announcement publish -- and vardiff retargets from share timing alone and never reads it. So leave the stored value as the sample it always was and have RefreshStats ask MinerReportedHashRate() what to publish: a pure function of that sample and the absolute silence since the last share. With no stored decay state there is nothing for a tick rate, a missed pass or a repeated pass to perturb, and the answer is bit-identical from 1/60 Hz to 1000 Hz because nothing accumulates, not because the arithmetic was written carefully. It also keeps the fade out of the estimator: the EWMA on the share path still blends against the value it last computed rather than a faded copy of it, so a miner that resumes recovers exactly as it did before any of this, and there is no decay timestamp for a newly accepted or newly authorised connection to be caught holding unset. RefreshStats takes one GetTime() for the whole sweep, so the rows and the total they are summed into describe a single instant rather than a smear across it. The clamp is written !(expected > MIN) so a NaN lands on the floor instead of sailing through; a negative `silent` from a clock stepping backwards reports the sample unchanged rather than scaling it up; and past HASHRATE_DECAY_CUTOFF half-lives the answer is a flat zero rather than pow() walking down into the denormals. This also drops the std::max(m->hashRate, 1e-9) the old pass carried, which the `hashRate <= 0.0` guard immediately above it had already made dead. pool-stratum gains ten checks over the policy: the full estimate inside the grace, a thousand samples at one instant returning one value and leaving the estimate untouched, a few percent lost one second past the grace rather than the whole estimate, exactly half at one expected interval past it, zero after an hour of silence, no inflation from a backwards clock, zero for a miner that has no rate sample yet, and nothing non-finite anywhere across a long silence. --- src/rpc.cpp | 194 +++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 160 insertions(+), 34 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index df7ca05..9d98a4d 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -2488,21 +2488,26 @@ static const double VARDIFF_STALL_CUT = 4.0; // divide difficulty by thi // that stops hashing keeps its last estimate forever and RefreshStats sums // it into the announced pool total for as long as the connection sits open // -- the published pool hashrate ran several times over ground truth -// because of exactly this. The decay pass in PoolEventLoop applies this +// because of exactly this. MinerReportedHashRate() below applies the // policy: a miner gets a grace of HASHRATE_IDLE_MISSES expected share -// intervals (expected interval = expected hashes per share at its current -// difficulty / its last rate estimate) with no share, then its estimate -// decays with a half-life of one more such interval, computed from -// absolute time so the result does not depend on how often the loop ticks. -// The interval is clamped to [HASHRATE_MIN_INTERVAL, HASHRATE_MAX_INTERVAL] -// so a fast miner at the vardiff floor (expected interval of milliseconds) -// is not punished for ordinary share-delivery jitter, and a very slow -// miner's grace does not stretch past the point where its estimate means -// anything. At the vardiff target of one share per 15s, a stopped miner's -// announced rate is gone within minutes. +// intervals (expected interval = expected hashes per share at its +// difficulty, divided by the estimate itself) with no share, after which +// the figure we report falls off with a half-life of one more such +// interval. The interval is clamped to [HASHRATE_MIN_INTERVAL, +// HASHRATE_MAX_INTERVAL] so a fast miner at the vardiff floor (expected +// interval of milliseconds) is not punished for ordinary share-delivery +// jitter, and a very slow miner's grace does not stretch past the point +// where its estimate means anything. At the vardiff target of one share +// per 15s that is a minute of grace, half the rate reported 75s in, and +// nothing reported after about seventeen minutes of silence. static const double HASHRATE_IDLE_MISSES = 4.0; // grace, in expected share intervals static const double HASHRATE_MIN_INTERVAL = 1.0; // seconds; floor for the expected interval static const double HASHRATE_MAX_INTERVAL = 3600.0; // seconds; ceiling for the expected interval +// Half-lives past the grace beyond which we report a flat zero. 2^-64 of the +// original estimate is not a number anyone needs, and stopping here keeps +// pow() from walking down into the denormals for a miner that has been quiet +// for hours. +static const double HASHRATE_DECAY_CUTOFF = 64.0; struct Miner { btf_socket_t fd; @@ -2546,6 +2551,65 @@ struct Miner { extranonce2(gNextExtranonce2.fetch_add(1)) {} }; +// What we publish as a miner's H/s: the last EWMA sample, faded out if shares +// have stopped arriving (#7, policy at HASHRATE_IDLE_MISSES). +// +// This reads m->hashRate and never writes it. That is the whole design. The +// first attempt decayed the stored estimate in place from the event loop's +// sweep, which put a running accumulator behind a figure that is only ever +// displayed, and the accumulator misbehaved in two ways: +// +// - `expected` was recomputed from the already-decayed estimate on every +// pass, so each decay raised `expected`, which raised `grace`, which is +// the very thing gating the next decay. `grace` simply chased `silent` +// upward and the reported rate settled into a 1/t crawl instead of the +// half-life curve the comment above promises -- 291x too high at ten +// half-lives past the grace. Issue #7 is about the pool over-reporting; +// that under-corrected it for hours. +// - Once `expected` reached HASHRATE_MAX_INTERVAL it could not rise any +// further, the feedback that had been gating each pass was gone, and the +// decay began compounding once a second against the total elapsed +// silence -- taking a miner from 0.4% of its baseline to 1e-11 of it in +// the next hundred seconds, four hours in, for no reason a reader of +// either the code or the announced number could see. +// +// Computing the value instead of accumulating it removes the failure mode +// rather than tuning it: with no stored decay state there is nothing for a +// tick rate, a missed pass or a duplicated pass to perturb, and the answer +// depends only on how long the miner has actually been quiet. It also keeps +// the fade out of the estimator -- the EWMA on the share path still blends +// against the value it last computed, not against a faded copy of it, so a +// miner that resumes recovers on exactly the curve it would have without any +// of this. +// +// `now` is a parameter rather than a GetTime() call so that one refresh fades +// every miner to the same instant, and so the policy is directly testable. +static double MinerReportedHashRate(const Miner* m, int64 now) +{ + // Nothing sampled yet: a fresh connection, or one that has found a single + // share and so has no interval to have been timed over. Nothing to fade, + // and nothing to divide by below. + if (m->hashRate <= 0.0 || m->lastShareTime <= 0) return m->hashRate; + + double expected = ExpectedHashesForDifficulty(m->difficulty) / m->hashRate; + // Written so that a NaN would land on the floor rather than sail through: + // the clamp is what guarantees `expected` is finite and strictly positive + // below, whatever the division produced. + if (!(expected > HASHRATE_MIN_INTERVAL)) expected = HASHRATE_MIN_INTERVAL; + if (expected > HASHRATE_MAX_INTERVAL) expected = HASHRATE_MAX_INTERVAL; + + double silent = (double)(now - m->lastShareTime); + double grace = HASHRATE_IDLE_MISSES * expected; + // grace is at least HASHRATE_IDLE_MISSES seconds, so a clock that has + // stepped backwards gives a negative `silent`, takes this branch, and + // reports the sample unchanged. Nothing here can scale an estimate up. + if (!(silent > grace)) return m->hashRate; + + double halflives = (silent - grace) / expected; + if (halflives >= HASHRATE_DECAY_CUTOFF) return 0.0; + return m->hashRate * pow(0.5, halflives); +} + static std::vector gMiners; struct FoundBlock { int height; int64 ts; std::string addr; }; @@ -4672,6 +4736,82 @@ int RunPoolStratumSelfTest() strPoolRoundsFile = savedRoundsFile; gPoolRounds = savedRounds; + // -- hashrate decay (#7) -------------------------------------------------- + // The first version of this decayed m->hashRate in place from the event + // loop's sweep, which made the reported figure a function of how the loop + // happened to run rather than of how long the miner had been quiet. These + // pin the properties that fixes: sampling changes nothing, and the answer + // depends only on elapsed silence. + { + // A miner exactly on the vardiff target: 4096 hashes per share at + // difficulty 1.0, one share per 15s, so 4096/15 H/s. Its expected + // interval is 15s and its grace is 60s. + const double base = 4096.0 / 15.0; + const int64 t0 = 1766000000; // any plausible non-zero wall clock + Miner decayed(INVALID_SOCKET); + decayed.difficulty = 1.0; + decayed.hashRate = base; + decayed.lastShareTime = t0; + + check(MinerReportedHashRate(&decayed, t0 + 60) == base, + "hashrate decay: a miner inside its grace period reports its full estimate"); + + // Sampling must not consume the estimate. Twenty passes is one second + // of the event loop's old sweep; a thousand is nearly a minute of it. + double once = MinerReportedHashRate(&decayed, t0 + 61); + bool idempotent = (decayed.hashRate == base); + for (int i = 0; i < 1000 && idempotent; i++) + idempotent = (MinerReportedHashRate(&decayed, t0 + 61) == once) && + (decayed.hashRate == base); + check(idempotent, + "hashrate decay: repeated sampling returns one value and leaves the estimate alone"); + + // One second past a 60s grace is 1/15th of a half-life: ~4.5% off, not + // the estimate gone. A miner at the vardiff target overshoots its grace + // roughly 1.8% of the time (e^-4) purely by share-timing variance, and + // must not be zeroed for it. + check(once > 0.94 * base && once < 0.96 * base, + "hashrate decay: one second past grace costs a few percent, not the estimate"); + double three = MinerReportedHashRate(&decayed, t0 + 63); + check(three > 0.86 * base && three < 0.88 * base, + "hashrate decay: three seconds past grace costs ~13%"); + + // One half-life past the grace is exactly half. + double half = MinerReportedHashRate(&decayed, t0 + 75); + check(half > 0.499 * base && half < 0.501 * base, + "hashrate decay: one expected interval past grace halves the estimate"); + + // A miner that has genuinely stopped ends up reported as nothing -- + // the whole point of #7. The old in-place version could not get here: + // its decay fed back into its own grace and settled into a 1/t crawl. + check(MinerReportedHashRate(&decayed, t0 + 3600) == 0.0, + "hashrate decay: a miner silent for an hour is reported as zero"); + + // A clock that steps backwards must never inflate what we announce. + check(MinerReportedHashRate(&decayed, t0 - 3600) == base, + "hashrate decay: a backwards clock reports the estimate unchanged, never higher"); + + // Fresh connection, and a connection with one share and so no interval + // to have timed: both must report nothing rather than divide by zero. + Miner fresh(INVALID_SOCKET); + check(MinerReportedHashRate(&fresh, t0) == 0.0, + "hashrate decay: a miner that never submitted a share reports zero"); + Miner single(INVALID_SOCKET); + single.lastShareTime = t0; + check(MinerReportedHashRate(&single, t0 + 86400) == 0.0, + "hashrate decay: a miner with no rate sample yet reports zero"); + + // Nothing in the swept range may produce a NaN, an infinity, a negative + // rate, or a rate above the estimate it started from. + bool finite = true; + for (int64 s = -100; s < 200000 && finite; s += 37) { + double v = MinerReportedHashRate(&decayed, t0 + s); + finite = !std::isnan(v) && !std::isinf(v) && v >= 0.0 && v <= base; + } + check(finite, + "hashrate decay: stays finite, non-negative and non-amplifying across a long silence"); + } + gCurrentJob = savedJob; gHaveJob = savedHaveJob; @@ -4699,6 +4839,10 @@ static void RefreshStats(int blocksFoundThisSession, uint64 roundShareTotal) if (m->authorised) gStats.authorizedMiners++; gWorkerStats.clear(); + // One timestamp for the whole sweep (#7): every miner is faded to the + // same instant, so the per-worker rows and the total they are summed + // into describe one moment rather than a smear across the loop. + int64 nowStats = GetTime(); for (Miner* m : gMiners) { if (!m->authorised || m->lastSeen <= 0) continue; PoolWorkerStatView v; @@ -4707,9 +4851,11 @@ static void RefreshStats(int blocksFoundThisSession, uint64 roundShareTotal) v.roundShares = m->roundShares; v.totalShares = m->totalShares; v.lastSeen = m->lastSeen; - v.hashRate = m->hashRate; + // Not m->hashRate: a miner that has stopped submitting shares must + // not keep contributing its last sample to the announced total. + v.hashRate = MinerReportedHashRate(m, nowStats); gWorkerStats.push_back(v); - gStats.totalHashRate += m->hashRate; + gStats.totalHashRate += v.hashRate; } std::sort(gWorkerStats.begin(), gWorkerStats.end(), [](const PoolWorkerStatView& a, const PoolWorkerStatView& b) { @@ -4942,27 +5088,7 @@ static void PoolEventLoop() } } - // 3.6. Hashrate decay (#7) -- see the policy comment at - // HASHRATE_IDLE_MISSES. The EWMA is only re-sampled on share arrival, - // so without this pass a miner that stops hashing keeps its last - // estimate forever and the summed pool hashrate RefreshStats (and - // the nostr announcement built from it) publishes is fiction. - for (Miner* m : gMiners) { - if (m->hashRate <= 0.0 || m->lastShareTime <= 0) continue; - double expected = ExpectedHashesForDifficulty(m->difficulty) / - std::max(m->hashRate, 1e-9); - if (expected < HASHRATE_MIN_INTERVAL) expected = HASHRATE_MIN_INTERVAL; - if (expected > HASHRATE_MAX_INTERVAL) expected = HASHRATE_MAX_INTERVAL; - double silent = (double)(GetTime() - m->lastShareTime); - double grace = HASHRATE_IDLE_MISSES * expected; - if (silent <= grace) continue; - // Half-life of one expected interval past the grace period. - // Written off absolute time, not per-tick, so the decay curve is - // the same however often this loop runs. - m->hashRate *= pow(0.5, (silent - grace) / expected); - } - - // 3.7. Idle reap (#6). Mirror the clean-disconnect path exactly -- + // 3.6. Idle reap (#6). Mirror the clean-disconnect path exactly -- // log, close, erase from gMiners via std::find, delete -- just with // a different reason. Shares the miner already earned stay in // roundShareCount, exactly as they do when a miner cleanly -- cgit v1.2.3