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