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.

1 files changed, 160 insertions, 34 deletions

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<Miner*> 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