Persist per-session vardiff difficulty across miner reconnects - #138
Merged
Conversation
A coordinator restart or miner reconnect previously reset every session to its lane's start difficulty; at 4x per 300s retarget a large miner needed 15-25 minutes to re-climb while flooding the share path, feeding the 2026-07-16 loop: coordinator slows -> watchdog exits -> every miner reconnects -> every session resets to lane start -> share rate spikes -> coordinator slows further. Remove the amplifier by retaining each worker's last converged difficulty in a bounded, TTL'd in-process store keyed by (listener name, exact Stratum username). The value is recorded at every committed retarget and on disconnect, and adopted on the first mining.authorize of a new connection, before password parsing, so an explicit d= still outranks it and an md=-only password clamps the resumed value. Adoption is capped at min(lane max difficulty, lane start * resume factor): a stale or absurd retained entry is clamped instead of adopted, while a value below the lane start resumes as-is (vardiff targets a share rate, so a converged session is on target by definition). New tunables, documented in .env.example: - PRISM_STRATUM_VARDIFF_RESUME (default 1): master switch; 0 restores lane-start reconnect behavior exactly. - PRISM_STRATUM_VARDIFF_RESUME_TTL_SECONDS (default 900): how long a converged difficulty stays adoptable; keep short where the idle step-down sweep is disabled. - PRISM_STRATUM_VARDIFF_RESUME_MAX_ENTRIES (default 8192): LRU bound on retained state (2x the deployed connection cap). - PRISM_STRATUM_VARDIFF_RESUME_MAX_START_FACTOR (default 1024, must be >= 1): plausibility ceiling as a multiple of the lane start. Also adds convergence observability -- sessions sitting at their vardiff ceiling, per-lane accepted share counts and rates, resume outcomes by bounded label, and a retained-sessions gauge -- mirrored into the frozen metrics reference implementation.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
SessionDifficultyStore.record re-stamped recorded_monotonic on every write, and the disconnect seam records on every disconnect with no share evidence. A session that resumed a retained difficulty, submitted nothing and disconnected therefore refreshed that value's clock, so reconnect cycles shorter than the TTL kept it adoptable indefinitely (reproduced: ttl=900 survives past 4000s on 800s cycles). That defeats the short TTL exactly where it matters: with the idle step-down sweep disabled as deployed, a session resumed above its true hashrate produces no accepted shares and cannot step itself down. record now takes an explicit share_backed flag and refreshes the timestamp only on a first record, on a value that actually moved, or on an unchanged value backed by accepted shares; otherwise the entry keeps ageing from its original convergence. The stored difficulty is always updated and LRU recency still moves on every record, so recency and TTL stay independent. ClientState gains vardiff_accepted_any, set on the first accepted share of a connection and never reset within it; the retarget commit point passes its own window's evidence (accepted_shares > 0), and the disconnect seam passes the connection's accumulated flag. An idle step-down carries no shares but lowers the value, so it refreshes on the changed-value branch as before. Also from the same review: - Disconnect retains client.share_difficulty only. The pending value is advertised with a future job, so a disconnect racing an in-flight retarget was retaining a difficulty the miner never received; a retarget whose paired send completed still records itself. - qbit_prism_vardiff_resume_total gains outcome="overridden", counted when an adopted resume is superseded by an explicit difficulty request in the same authorize, so resumed + clamped - overridden is the number that stuck. Routed through a coordinator seam like the adjacent ones. - Document shared-username retention as last-writer-wins next to PRISM_STRATUM_VARDIFF_RESUME, sharpen the per-lane rate gauge HELP to say it is a since-start average and point at rate() over the counter, and record the at-ceiling census cost at convergence_snapshot. Tests cover the reconnect-cycle repro and its share-backed counterpart, the undelivered-pending disconnect, the retarget commit-point recording, the env -> config -> coordinator -> store sizing chain, and an authorize whose username reservation fails. The frozen metrics reference mirrors both rendered-byte changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #132 (item 1). Targets
2.x.x.The feedback loop this breaks
Vardiff targets one accepted share per 15 s per connection. Both deployed Stratum lanes share the same
2^32ceiling and differ only in where a session starts:Choosing a lane therefore does not change what a miner can reach — it changes how long that miner floods before it converges.
Until now, a reconnect reset every session to its lane's start difficulty. At 4x per 300 s a large miner needs three to five retarget cycles — 15 to 25 minutes — to climb back to a difficulty it had already reached, submitting far above target the entire time, alongside everyone else who reconnected at the same moment. That is the loop the 2026-07-16 operator note names as "miner reconnect storms that reset vardiff and re-flood the share path":
Reconnect is the amplifier, not the ceiling. This PR removes it.
What changed
A bounded, TTL'd retention store (
SessionDifficultyStoreinlab/prism/vardiff_service.py) holds each worker's last converged share difficulty, keyed by(listener name, exact Stratum username). It is anOrderedDictbehind a leaf lock with LRU eviction at the entry cap. A hit refreshes recency but deliberately notrecorded_monotonic: the TTL measures the age of the value, not of the last access.Lane scoping keeps the two lanes' policies independent. A reconnect to the same port — the reconnect-storm case — always hits; a lane switch is simply a miss that behaves exactly as today.
Recorded at two points: at the commit point of every successful retarget (so a session that dies without a clean disconnect still retains its value), and on
disconnect_clientonce retirement is claimed, outside the coordinator lock, behind a guard that can never break disconnect cleanup.Adopted on the first
mining.authorizeof a connection only — a live session re-authorizing already holds a converged value and is left alone. Adoption lands before password parsing, which preserves the existing precedence exactly: an explicitd=computes its target fromrequested_difficultyand still outranks the resume, while anmd=-only password clamps the just-resumed value through theshare_difficultyfallback.Clamped so a stale or absurd entry can never be adopted as-is. The ceiling is
min(lane max difficulty, lane start × resume factor)— at the default 1024x that is 16,777,216 on the standard lane and 512,000,000 on the high-diff lane, i.e. five 4x retarget steps, exactly the climb reconnects used to repeat.A retained value below the lane start is adopted unchanged, on purpose. Vardiff targets a share rate, so a session sitting at its converged difficulty is on target by definition; cold-starting it higher would give the pool fewer shares, not more.
New tunables
PRISM_STRATUM_VARDIFF_RESUME10restores lane-start reconnect behaviour exactly.PRISM_STRATUM_VARDIFF_RESUME_TTL_SECONDS900PRISM_STRATUM_VARDIFF_RESUME_MAX_ENTRIES8192PRISM_STRATUM_VARDIFF_RESUME_MAX_START_FACTOR1024>= 1at startup.All four are documented in
.env.example.On the TTL default. 900 s is deliberately short. With
PRISM_STRATUM_VARDIFF_IDLE_SWEEP_SECONDS=0as deployed there is no idle step-down sweep, so retargeting only samples on an accepted share — a session resumed above its true hashrate produces few shares and cannot readily step itself down. A short TTL keeps retained values reflective of recent hashrate and is the shipped mitigation; re-enabling the idle sweep would be the structural one. That coupling is called out in.env.examplenext to the knob.For that mitigation to mean anything the TTL must age from real convergence, not from the last time the value was touched. It does: re-recording an unchanged difficulty refreshes the entry's timestamp only when the connection produced at least one accepted share. A session that resumes a retained value, submits nothing and disconnects leaves the clock running, so a reconnect loop shorter than the TTL can no longer keep a stale value alive indefinitely. See the review-fix commit below.
Observability
This area had none: no metric for a session pinned at
max_difficulty, and no per-lane share rate. Added to the renderer:qbit_prism_vardiff_sessions_at_max_difficulty(gauge) — sessions sitting at their effective vardiff ceiling.qbit_prism_vardiff_lane_accepted_shares_total{lane}(counter) andqbit_prism_vardiff_lane_accepted_shares_per_second{lane}(gauge). The per-second gauge uses the same elapsed formula asqbit_prism_shares_per_second, so the two are directly comparable. The lane counter increments before vardiff's enabled check, so lane load stays observable where vardiff is off.qbit_prism_vardiff_resume_total{outcome}over a bounded label set (resumed,clamped,overridden,expired,miss,rejected,disabled) andqbit_prism_vardiff_resume_retained_sessions(gauge).resumedandclampedcount attempts;overriddencounts an adopted value that an explicitd=/md=superseded in the same authorize, soresumed + clamped - overriddenis the number that actually stuck.The lane label set is deterministic: configured listener profiles in order, then any additional observed lane names, sorted.
The at-ceiling census respects the codebase's lock order (client vardiff lock → coordinator lock, never the reverse): membership is snapshotted under the coordinator lock alone, released, and only then is each client's vardiff lock taken. The two locks are never held together.
Metrics output is pinned byte-for-byte by the frozen-reference parity test, so
reference_render_metrics_payloadintests/test_prism_metrics.pygains a mirroredreference_vardiff_convergence_metrics_linesat the matching position, plus needles so an empty fixture cannot pass silently.Operational effect on the next coordinator restart
Worth stating precisely, because the boundary matters: the store is in-process. It survives miner-side reconnects, network flaps, load-balancer churn and slow-coordinator timeouts — every reconnect storm that does not kill the coordinator. It does not survive a coordinator process restart.
So on the next restart the store starts empty and that first reconnect wave still climbs from lane start, exactly as today. What changes is everything after: the store refills as sessions converge, and each subsequent reconnect — including the repeat reconnects that follow a watchdog exit while the coordinator is still settling — resumes at the converged value instead of re-climbing. The loop loses its amplifier from the second wave onward. Durable cross-restart retention would need a store this PR deliberately does not add; see follow-ups.
Rollback is an env flip, not a revert:
PRISM_STRATUM_VARDIFF_RESUME=0.Review fixes (second commit)
An independent adversarial review of the first commit found one major defect, four minor findings and two nits. The second commit applies the accepted subset.
Major, confirmed by execution — the TTL could be laundered.
SessionDifficultyStore.recordunconditionally overwroterecorded_monotonic, and the disconnect seam recorded on every disconnect with no share evidence. A session that resumed a retained difficulty, submitted nothing and disconnected therefore re-stamped that value with a fresh clock: with a 900 s TTL and 800 s reconnect cycles the value survived past 4000 s and in principle forever — defeating the mitigation the short TTL exists to provide, in exactly the zero-share case that the disabled idle sweep makes dangerous. The store now refreshes the timestamp only on a first record, a value that actually moved, or an unchanged value backed by an accepted share on that connection (tracked by a new per-connectionClientState.vardiff_accepted_any). Regression tests cover both directions: a silent reconnect loop expires on schedule, a share-backed one does not.Nit — undelivered values are no longer retained. The disconnect path read
pending_share_difficulty or share_difficulty;pending_share_difficultyis advertised with a future job, so a disconnect racing an in-flight retarget retained a difficulty the miner was never given. Disconnect now retains the delivered value only. Nothing is lost — a retarget whose paired send completed already records its own value at that commit point.Minor — resume counters made unambiguous. Added the
overriddenoutcome described above.Three review findings are accepted as documented limitations rather than code changes, and the docs now say so plainly:
(listener, exact username), so rigs deliberately sharing one username share one retained value. Per-rig worker names are the real fix;PRISM_STRATUM_MAX_CONNECTIONS_PER_USERNAMEbounds concurrency but not sequential reuse, and the clamp remains the backstop. Documented in.env.example._per_secondgauge is a since-start average and structurally cannot show a transient storm. Its HELP text now says so and points atrate()over the_totalcounter.convergence_snapshot.The review separately cleared, with reasoning: lock discipline throughout (the convergence lock is a strict leaf; no client vardiff lock is ever held across a socket write, so a slow miner cannot stall the scrape), the
difficulty_generationbump ordering against job delivery, the high-diffminimum_advertised_difficultyfloor on resumed sessions, authorize failure paths,prune/lookupdouble-counting, and that the parity test is a genuine transcription rather than a tautological delegation.Verification
python -m compileall -q docker lab tests examples scripts— clean.tests.test_prism_vardiff_resume,tests.test_prism_vardiff_service,tests.test_prism_vardiff— 66 tests, OK.tests.test_prism_stratum_session,tests.test_prism_stratum_restart_bind,tests.test_prism_metrics,tests.test_prism_coordinator_config,tests.test_auxpow_vardiff_coordinator— 164 tests, OK.unittest discover— 1996 tests; the only failures are the five knownos.splicecases intests/test_prism_job_builder.py, which fail on macOS, pass on Linux, and live in a file this PR does not touch. The review independently ran the full suite on Linux: 1996 green, zero failures.tests/test_prism_vardiff_resume.pycovers the three acceptance cases — resume after reconnect (converged 262,144 resumed rather than the 16,384 lane start), TTL expiry falling back to lane start, and an implausible retained1e15clamped to the 16,777,216 lane ceiling and asserted not adopted — plus LRU eviction, per-lane scoping,d=precedence,md=-only clamping, no re-resume on re-authorize, the kill switch, and the new metric series. The second commit adds the TTL-laundering regression in both directions, the delivered-value retention case, theretarget_lockedcommit-point recording path, the end-to-end env →StratumConfig→ coordinator → store sizing chain (whichgetattrdefaults would otherwise mask on a rename), and an authorize path wherereserve_client_usernamefails.Follow-ups (deliberately not in this PR)
Kept out to stay reviewable; all are #132 items.
calculate_next_difficultyalready receives an EWMAdifficulty_estimate, so a larger first-window step could gate onclient.vardiff_difficulty_estimate is Nonewithout touching the steady-state bound.