Preserve the audit envelope when two found blocks land in one confirm-publish window - #136
Merged
kiwidream merged 2 commits intoAug 18, 2026
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Two distinct found blocks can land inside one confirm->publish window. The synchronous miner-connection-thread tail confirms block A under the payout-balance lock and the publication order guard, then releases both before it publishes; the accounting-actor tail lands block B in that gap, allocates the next ordinal and publishes first. The per-hash disposition lease does not serialise two distinct hashes, so nothing prevents the interleaving. Block A then reached publish_success already superseded and returned without writing anything: its live audit envelope -- the block's only published evidence pointer -- was lost permanently and silently. The Postgres audit row, canonical bundle, body, accounting, share credit and settlement all survived; only the public evidence trail was dropped. The old code conflated two invariants behind one floor check. The evidence path is a single mutable pointer that must advance monotonically in sequence, and floor gating is correct for it. The live envelope is a per-block immutable artifact on its own height+hash path, and a superseded ordinal's envelope competes with nothing. Being behind the floor means you may not become current; it does not mean your own envelope is not yours to write. Sequence-versus-floor alone cannot authorise that write, though. A process whose writer lease has been taken over is also behind the floor, and it must stay fenced out of the shared audit root entirely: its in-memory report may be stale, and publishing a stale report as a block's public evidence is worse than leaving the envelope missing. Only the caller knows which it is. The write is therefore opt-in through a new restore_superseded_envelope keyword that defaults to false, and only _publish_finalization_evidence -- the live writer finalizing a block it confirmed itself in this landing -- passes true. A deposed writer replaying stale state never reaches that call site, so the A1 process gate's C3 fencing invariants are preserved exactly. When the write is authorised it routes through _write_superseded_live_envelope_locked, which writes the envelope only when it is absent and logs the event. The publication order guard and the process flock are held throughout, and the write is rolled back if any post-write identity assertion fails. A present envelope is returned untouched, so a same-hash stale replay keeps its existing behaviour exactly. The current evidence reference, the cached identity and retention pruning are not touched, and the publication still reports published=False. The diagnostic goes to stderr: this store is hosted inside worker processes that reserve stdout for a strict JSON protocol stream, and the A1 process gate drives this exact superseded path. Add two regression tests, both failing before the fix at the envelope-existence assertion: the distinct-hash interleaving across the synchronous and actor tails, and the convergence path where the peer is confirmed but not yet published so the earlier block is retried behind the floor. Because the write is opt-in, no existing assertion changes -- the test diff is purely additive. Fixes #133 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kiwidream
force-pushed
the
preserve-audit-envelope-on-interleaved-landings
branch
from
August 17, 2026 23:46
3e3c729 to
015f85c
Compare
An adversarial review of the interleaved-landing fix for issue #133 returned four findings; this lands all four. The restore's opt-in rested on "a deposed writer never reaches the call site", which is false for a writer deposed inside its own confirm->publish gap: it confirms with a valid lease, loses it during the gap, and everything from there to the publish call is lease-unfenced. Content divergence was independently impossible -- the report and persistence digests bind to the durably confirmed ordinal -- so this was a coherence defect against the A1 process gate's C3 immobility invariant rather than a corruption path, but that invariant is cheap to keep literally true. _publish_finalization_evidence now asks the coordinator's bounded, non-blocking exact-session fence (require_fresh_lease_for_external_side_effect) for restore authority before entering the publication order guard, holding no store lock across the verification. The asymmetry is deliberate: a clean proof grants restore_superseded_envelope=True, while a lease failure (WriterLeaseRenewalDeferred, ShutdownInProgress) withholds the restore, logs the withholding with the component and exception type, and lets the publication proceed unchanged. The restore is a repair of an evidence pointer, not the publication itself; degrading to False is exactly the previously shipped behaviour, whereas failing the found-block path on a lease hiccup would be a live regression. The fence stays a no-op for ledgers without the writer lease guard, never waits on the lease row, and its internal hard-exit arming for a genuinely dead lease is left intact. The production opt-in was also untested: flipping the call site back to False passed all 254 tests. A new coordinator-level regression test drives the real two-tail topology -- handle_submit's synchronous landing paused inside its confirm->publish gap while a second distinct-hash candidate rides the submitter->accounting-actor handoff, lands, and publishes -- and asserts both blocks' live envelopes exist afterwards. It fails at the envelope-existence assertion with the call site reverted to False. A sibling test pins the degrade path: a fence that raises withholds the restore without failing either landing. The present-file early return in _write_superseded_live_envelope_locked accepted any bytes and fsynced nothing, so a truncated or corrupt envelope was silently accepted and could never be repaired. It now compares the existing envelope against the one this identity would build, ignoring created_at exactly as the main publish path does -- the comparison is extracted into a helper shared with that path -- raises the same invalid/conflicts errors, and fsyncs the parent directory on a match before returning. The superseded-write diagnostic used to print while holding both the store lock and the publication-order flock; an undrained stderr would have blocked the found-block path with locks held. The store now records the write on the returned publication (AuditPublication.superseded_envelope_written, defaulted so every existing construction site is unchanged) and _publish_finalization_evidence emits the message -- still on stderr, because worker stdout is a strict JSON-lines protocol -- after the balance lock and the order guard have been released. Verified: the three unittest suites (256 tests), the Postgres ledger gate and the four A1 gates including C3-B-wins-late-A, and ruff E4/E7/E9/F on the changed files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #133.
The interleaving
Two distinct found blocks can land inside one confirm→publish window, because
production landings run on two tails that never exclude each other across hashes:
_submit_synchronous_block_candidateon a miner connection thread, and the accountingactor. The disposition lease is per-hash, so two different block hashes hold two
different leases and serialise against nothing.
ordinal N under the payout-balance lock and the publication order guard — and
releases both when the confirm scope returns, before the publish scope is entered.
with ordinal N+1, and publishes. The durable floor is now N+1.
publish_successwith sequence N while the current identity is N+1. Thesuperseded branch returned
published=Falsewithout writing anything — no error,no log.
The durable Postgres audit record, canonical bundle, body, block accounting, share credit
and settlement all survived. What was lost is the published evidence pointer for block
A: a permanent, silent gap in the public audit trail for a block that was mined and paid
correctly. Because the repair path only ever repairs the current identity and the
finalized outbox row leaves no replay source, the loss was irrecoverable.
The second shape converged to the same place: if B was confirmed but not yet published, A
raised "audit publication is behind the durable ledger floor", was retained as retryable,
and then hit the silent skip once B published.
Neither ingredient is wrong alone. The publication-ordinal floor gating is correct for
same-hash replay, which is what it was designed for. The dual-tail topology predates it.
Which fix shape, and why
Shape 1 — write the envelope for a ledger-proven superseded ordinal.
The alternative was to serialise synchronous-route landings through the accounting actor.
That was rejected. It would put the miner connection thread behind another block's full
landing — which contains a synchronous full-ledger
accepted_share_stats()aggregate on acold cache — reintroducing exactly the latency and lock-convoy problem on the found-block
path that this repository has a documented history of. It also changes the topology of the
most consequential code here to fix a store-level bug, and would leave the store silently
lossy for any other concurrent publisher, so the evidence trail would stay one topology
change away from the same permanent gap.
The fix separates two invariants the old code conflated behind a single floor check:
sequence — floor gating is correct and stays exactly as it was;
and a superseded ordinal's envelope competes with nothing.
Being behind the floor means you may not become current. It does not mean your own
envelope is not yours to write.
Why the write is opt-in
Sequence-versus-floor alone is not sufficient authority to write. A process whose
writer lease has been taken over is also behind the floor, and it must stay fenced out of
the shared audit root entirely — its in-memory report may be stale, and publishing a stale
report as a block's public evidence is worse than leaving the envelope missing.
Only the caller knows which situation it is in. So the write is opt-in through a new
restore_superseded_envelopekeyword defaulting toFalse, and only_publish_finalization_evidence— the live writer finalizing a block it confirmed itselfin this landing — passes
True, and only after proving its lease is still live. See thereview round below: an earlier revision granted that authority unconditionally at this call
site, which was not sufficient.
This was not the first design. The first version keyed the write on sequence-versus-floor
alone, and the A1 process gate's C3 "B wins and late A" scenario caught it: C3 models
this same interleaving across two real processes where A's writer lease has been revoked,
and asserts
stale A never repairs Nplus full immobility of the shared audit root. Thatgate is right.
It also has a useful consequence: because the default behaviour is unchanged, no
existing assertion had to be flipped. The test diff is purely additive.
What is now guaranteed
how landings interleave — provided the writer can still prove its lease is live.
writer deposed inside its own confirm→publish gap. The A1 process gate's C3 immobility
invariant stays literally true.
behaviour of skipping the restore, and says so in the log.
moves backwards. The cached identity, evidence state and retention pruning are untouched,
and the call still reports
published=False.path and the parent directory is fsynced, so a corrupt or truncated envelope is surfaced
rather than silently retained forever.
JSON-lines protocol, and emitted by the caller after every lock scope has been released
so an undrained stderr cannot block the found-block path.
The landing topology is deliberately unchanged;
block_candidates.pyis untouched andblock_finalization.pyonly opts in at the publish call.Review round: lease-gating the restore
An adversarial review returned merge with changes with four findings. All four are
addressed in the second commit.
F1 — the opt-in claim was falsified. "A deposed writer never reaches the opt-in call
site" is false for a writer deposed inside its own confirm→publish gap: it confirms with
a valid lease, loses the lease during the gap, and everything from there to the publish
call is lease-unfenced. That is exactly the A1 process gate's C3-"late" topology. It went
undetected because the gate harness publishes with the default
False, so it neverexercised the production opt-in.
Content divergence was independently impossible — report and persistence digests bind to
the durably confirmed ordinal — so this was a coherence defect against C3's immobility
invariant, not a corruption path. That invariant is cheap to keep literally true, so it
is kept.
_publish_finalization_evidencenow asks the coordinator's bounded, non-blockingexact-session fence (
require_fresh_lease_for_external_side_effect) for restore authoritybefore entering the publication order guard, holding no store lock across the
verification. The asymmetry is deliberate:
restore_superseded_envelope=True;WriterLeaseRenewalDeferred,ShutdownInProgress— verified to be thecomplete set the fence can raise) withholds the restore, logs it, and lets the
publication proceed unchanged.
The restore is a repair of an evidence pointer, not the publication itself. Degrading to
Falseis exactly the previously shipped behaviour, whereas failing the found-block pathon a lease hiccup would be a live regression — the same latency concern that ruled out the
alternative fix shape. The fence stays a no-op for ledgers without the writer lease guard,
never waits on the lease row, and its internal hard-exit arming for a genuinely dead lease
is left intact.
F2 — the production opt-in was untested. Flipping the call site to
Falsepassed all254 tests. A new coordinator-level regression test now drives the real two-tail topology:
the synchronous landing is paused inside its confirm→publish gap while a second
distinct-hash candidate rides the submitter→accounting-actor handoff, lands, and publishes.
It asserts both live envelopes exist. Verified to fail with the call site reverted:
A sibling test pins the degrade path — a fence that raises withholds the restore without
failing either landing.
F3 — the present-file early return was too permissive. It accepted any bytes and
fsynced nothing, so a truncated or corrupt envelope was silently retained and could never
be repaired. It now holds present bytes to the main publish path's immutable-content
standard (the comparison is extracted into a helper shared with that path), raises the same
invalid/conflictserrors, and fsyncs the parent directory on a match.F4 — the diagnostic printed under locks. It ran while holding both the store lock and
the publication-order flock, so an undrained stderr could block the found-block path with
locks held. The store now records the write on the returned publication
(
AuditPublication.superseded_envelope_written, defaulted so every existing constructionsite is unchanged) and the caller emits the message after both lock scopes have exited.
Tests
The regression test is the point of this change — no existing test interleaved two
distinct-hash landings, which is why the defect survived two review passes.
Added, both at the store level against a real
SingleWriterShareLedgerwith deterministicthreading.Eventhandoffs (no sleeps):test_interleaved_distinct_landings_preserve_the_superseded_envelope— the synchronoustail confirms A and releases the balance lock and publication guard before publishing;
the actor tail lands and publishes B inside that gap; A then publishes superseded.
Holding either lock across the gap would make the interleaving unreachable, so the test
deliberately does not.
test_superseded_landing_converges_after_the_peer_publishes— the second shape: Araises behind the floor, B publishes, A's retry lands its envelope.
Confirmed failing before the fix. Verified with the new keyword present but the restore
path neutered, so the failure is behavioural rather than a missing-argument
TypeError:Suites (
python -m unittest, matching CI):tests.test_prism_audit_artifactstests.test_prism_block_candidatestests.test_prism_block_finalization256 total.
Postgres gates: all pass on the final tree, run locally against a Postgres 16 container
(
test/test-prism-postgres-ledger.sh, verified real exit 0, noTracebackorGateFailurein the output):Full discovery: 1967 tests. On Linux this is clean. On the macOS reviewing host, 2 failures
and 3 errors appear in
test_prism_job_builder(thesplicespool paths); these wereverified to reproduce identically on the pristine base commit with an empty diff, so they
are pre-existing and platform-specific, not from this change.
Lint in Docker:
ruff 0.12.5 check --select E4,E7,E9,Fon all changed files — all checkspassed;
compileallclean.