merge: !3 fix/pool-accounting into deploy/all-2026-08-24

1 files changed, 279 insertions, 6 deletions

diff --git a/src/rpc.cpp b/src/rpc.cpp
index bd5fd8b..49746ee 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,
@@ -2473,6 +2486,31 @@ 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. 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
+// 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;
     bool         authorised    = false;
@@ -2500,11 +2538,80 @@ 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<uint32_t> seenNonces;
+    std::string        seenNoncesJob;
+
     explicit Miner(btf_socket_t f)
         : fd(f), connectTime(GetTime()),
           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; };
@@ -2555,6 +2662,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);
     }
@@ -2598,6 +2717,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:
@@ -2682,6 +2811,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
@@ -2940,6 +3080,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;
@@ -2967,6 +3123,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]++;
@@ -4581,6 +4738,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;
 
@@ -4608,6 +4841,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;
@@ -4616,9 +4853,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) {
@@ -4851,6 +5090,35 @@ static void PoolEventLoop()
             }
         }
 
+        // 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
+        // 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<Miner*> 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);
@@ -4873,6 +5141,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);