Apply stability-review fixups from PRs 134-139 (PRISM core + infra/vardiff) - #141
Merged
Conversation
F1: _acquire_operation_gate's slicing path now reads the injected self._monotonic clock at all three deadline sites, so a virtual test clock can drive admission-wait slicing deterministically; covered by a new virtual-clock test. F2: load_coordinator_config rejects PRISM_WATCHDOG_TIMEOUT_SECONDS below 1.0 at startup, keeping the derived landing-budget ceiling (half the tolerance) meaningfully below the watchdog tolerance. F3: the sliced-admission liveness test drops its per-gap 0.3s wall clock bound, which false-reds on loaded CI hosts; the determinism intent (stamp count, total elapsed, raised timeout) is unchanged and cadence is pinned by the new virtual-clock test instead. F4: the concurrency harness baton timeout is overridable via PRISM_HARNESS_BATON_TIMEOUT_SECONDS (default unchanged at 20.0); it remains a false-red failure detector and never influences ordering. F5: startup warns on stderr when the idle-in-transaction timeout exceeds the writer-lease acquire budget (attempts x per-attempt lock deadline), where an orphaned backend holding the lease row lock can no longer be reaped mid-budget and startup crash-loops instead of self-healing. F6: the superseded-envelope restore authority proof now runs after the payout balance lock and the publication order guard are acquired, so a writer deposed during the unbounded lock wait no longer restores on stale authority; the withheld-authority diagnostic is deferred until the locks are released. A new test deposes the writer inside the publication guard and fails against the old check-then-act shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ted probe plus SELECT 1 confirm The postgres image's initdb temporary server accepts unix-socket connections and then shuts down, so a socket-only pg_isready can release dependents against the throwaway server (the race PR #134 fixed in the test scripts). Gate on the real TCP listener first, then confirm with a real query. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and backfill scripts Both operator-run scripts construct PsqlShareLedger directly and got the class defaults for lease_acquire_lock_timeout_seconds, lease_acquire_attempts, and the four Postgres session-guard knobs, so operator tuning of PRISM_LEDGER_LEASE_ACQUIRE_* / PRISM_POSTGRES_* never reached them. Mirror the coordinator: read the same six env vars with the coordinator_config helpers and defaults and pass them through. The backfill script exposes them as CLI flags with env defaults, matching its existing --lease-ttl-seconds pattern, with construction extracted into ledger_from_args so the path is testable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ck-high resume The SessionDifficultyStore is in-memory: a coordinator restart, including the watchdog's terminal exit_process(1), empties it, so the post-restart reconnect wave is not smoothed by retention. And with the idle sweep off (the deployed default) a rig that converged high and reconnects within TTL on weaker hardware can resume too high with no share signal to bring it down, bounded by the resume TTL. Honest-disclosure comments and .env.example notes only; no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Summary
Lands the actionable fixups surfaced by the PRs-134-139 stability review (six
independent read-only reviews run on Fable 5 against the merged
2.x.xtree).The changes split into two disjoint groups — PRISM core Python and infra/vardiff —
implemented in parallel and integrated here. No behavior is changed beyond what
each finding specifies; the production fixes are tight guards and the rest is
honest-disclosure documentation plus a racy-healthcheck repair.
PRISM core (F1-F6)
_acquire_operation_gate(share_ledger.py).The admission-wait slicing path read raw
time.monotonic()at three sites;now reads
self._monotonic()so the deterministic harness can drive theslicing loop. Production binds
self._monotonic = time.monotonic, so behavioris unchanged. New virtual-clock test asserts the deadline fires exactly at the
injected-clock budget.
PRISM_WATCHDOG_TIMEOUT_SECONDS(coordinator_config.py).Rejects
< 1.0with a clearSystemExitso thex 0.5landing-budget ceilingstays meaningfully below the tolerance. Default unchanged (120s). Review of Cap the escalated landing budget below the watchdog tolerance #135,
LOW CONFIRMED.
test_prism_share_ledger.py).Drops the tight
0.3sper-gap upper bound on the sliced-admission test (a loadedCI host can stall a 0.05s slice wakeup past it); keeps the determinism intent
(>= 2 stamps, elapsed >= 0.4s, < 3.0s) with a maintainer comment. Review of Cap the escalated landing budget below the watchdog tolerance #135,
LOW PLAUSIBLE.
prism_concurrency_harness.py).BATON_TIMEOUT_SECONDSis now overridable viaPRISM_HARNESS_BATON_TIMEOUT_SECONDS(default unchanged at 20.0) for heavily oversubscribed CI hosts; remains a
false-red failure detector that never influences ordering. Review of Add a deterministic concurrency harness for PRISM, proven against the writer-lease lifecycle #139, LOW PLAUSIBLE.
(
coordinator_config.py). Warns on stderr at startup whenpostgres_idle_in_transaction_timeout_seconds > lease_acquire_attempts x lease_acquire_lock_timeout_seconds— the condition under which an orphanedidle-in-transaction backend holding the lease row lock cannot be reaped
mid-budget, so startup exhausts every acquire attempt at the same locked row
instead of self-healing. Warning only (not a crash); names both knobs, their
values, and the consequence. Review of Bound PRISM writer-lease acquisition and guard coordinator Postgres sessions #137, MEDIUM.
(
block_finalization.py). The superseded-envelope restore'srequire_fresh_leaseproof now runs after the payout-balance lock and publication order guard are
acquired, so a writer deposed during the (unbounded) lock wait no longer
restores on authority it proved before the wait. Same exception classes, same
stderr message (deferred until after the locks are released), same degrade
behavior. New test models a deposal inside the publication guard and fails
against the old check-then-act shape. Review of Preserve the audit envelope when two found blocks land in one confirm-publish window #136, LOW CONFIRMED.
Infra / vardiff (G1-G4)
compose.yaml).The
prism-postgreshealthcheck used socket-onlypg_isready, which is racy:the image's initdb temporary server accepts unix-socket connections and then
shuts down, so a dependent can be released against the throwaway server (the
race Gate PRISM Postgres test containers on the real server #134 fixed in the test scripts). Now gates on the real TCP listener
(
pg_isready -h 127.0.0.1 -p 5432) then confirms withpsql ... SELECT 1,using
$$escaping so the container shell resolves thePOSTGRES_*vars.Review of Gate PRISM Postgres test containers on the real server #134, out-of-diff.
(
run_ctv_broadcaster_daemon.py,backfill_ctv_fanouts.py). Both operator-runprocesses construct
PsqlShareLedgerdirectly and got the class defaults forall six
PRISM_LEDGER_LEASE_ACQUIRE_*/PRISM_POSTGRES_*knobs, so operatortuning never reached them. Now mirrors the coordinator: reads the same six env
vars with the same helpers and defaults. Backfill exposes them as CLI flags with
env defaults; construction extracted into
ledger_from_argsfor testability.Two passthrough tests pin that operator tuning reaches the constructed ledger.
Review of Bound PRISM writer-lease acquisition and guard coordinator Postgres sessions #137, LOW CONFIRMED.
.env.example,vardiff_service.py,background_services.py). TheSessionDifficultyStoreisin-memory: a coordinator restart — including the watchdog's terminal
exit_process(1)— empties it, so the post-restart reconnect wave is notsmoothed by retention. Honest-disclosure comments and
.env.examplenotes only;no behavior or default change. Review of Persist per-session vardiff difficulty across miner reconnects #138, MEDIUM CONFIRMED.
.env.example). With theidle sweep off (the deployed default), a rig that converged high and reconnects
within TTL on weaker hardware can resume too high, emit no shares, and give the
retargeter no signal to come down — stuck for that connection's life (bounded by
the 900s TTL; harms only that miner). Default unchanged (1024). Review of Persist per-session vardiff difficulty across miner reconnects #138,
PLAUSIBLE.
Verification
python -m compileall -q docker lab tests examples scripts— clean.python -m unittest tests.test_mainnet_compose_contract— 23 OK.python -m unittest tests.test_prism_coordinator_config_loading tests.test_prism_block_candidates tests.test_prism_share_ledger tests.test_ctv_broadcaster_daemon tests.test_prism_vardiff_resume— 453 OK.python -m unittest discover -s tests -p 'test_*.py'— 2106 tests,no regressions (5 pre-existing macOS
os.splicefailures intest_prism_job_builderand 9 git-fixture tests that need
GIT_CONFIG_GLOBAL=/dev/nullfor local GPGsigning config; both pass on Linux CI and are unrelated to these changes).
shellcheck -x(via docker) on all tracked*.sh— clean (no.shfileschanged in this PR).
docker compose config— valid for all four CI profiles:permissionless,real-miner-smoke,auxpow,prism.Review context
Per-PR raw reports and the consolidated write-up live in
.context/pr-stability-reviews/CONSOLIDATED.md. The findings acted on here arethe MEDIUM and LOW items the review flagged as worth fixing; the one HIGH (merge
hygiene for #137) was already resolved on the branch tip before it landed.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.