From review of #454 (comment r3614978486). Confirmed still valid at HEAD (tip keeper sweep still records after submit). Target: L2 videre-sdk keeper sweep, possibly a small L1 nexum-sdk Journal primitive.
The keeper sweep dedups with contains -> submit -> record (videre-sdk/src/keeper.rs). The review raised two concerns; dispositions differ:
Not a bug: concurrent double-submit
Two overlapping sweeps both passing contains before either records cannot happen within an engine process. Dispatch is structurally serialized, not lock-based: the runtime moves one owned Supervisor into a single tokio task (nexum-runtime/src/builder.rs), whose event loop (runtime/event_loop.rs) selects one event and awaits dispatch_* to completion before the next; every dispatch and restart path takes &mut self; a timed-out guest call is dropped (cancelled) before dispatch_to returns; and restart replaces the wasmtime Store in-place (supervisor.rs reinstantiate_one), so at most one instance and one on_event is ever live. contains and record cannot be interleaved by a concurrent sweep of the same keeper. (Caveat: this is single-process. Two engine processes on the same redb state_dir could race, but redb is single-process anyway.)
Real bug: post-submit record is not durable
Concurrency-independent. If venues.submit returns Accepted but the following journal.record(&key)? faults (a local-store host write error), sweep propagates Err with the body submitted but unrecorded. The next redispatch (the intended at-least-once path across a crash/restart) then sees contains == false and resubmits, sending a duplicate order to the venue. This breaks the sweep's own doc claim that "an accepted body never reaches the venue twice", with no concurrency involved.
Scope
- A naive reorder (record before submit) is wrong: a retryable submit fault would leave the key recorded, so the next sweep sees
contains == true, skips it, and never retries the genuinely-failed submission.
- A correct fix is a reserve/commit/release protocol (reserve the key, commit on
Accepted, release on a retryable fault) or an atomic record_if_absent on the accepted path that is retried until durable. nexum-sdk's Journal currently exposes only contains + record (no CAS, no remove), so a small L1 primitive is likely needed.
- Tighten the
keeper.rs doc comment: "an accepted body never reaches the venue twice" holds against in-process concurrency but not against the post-submit record-fault path.
Timing
Land before the M4 cars re-point the keepers onto the live pool submit path (#469 / #470): that is when a duplicate submission becomes a real duplicate order against a venue.
Acceptance criteria
- A post-submit journal-write fault cannot cause the same accepted body to be resubmitted on the next sweep or redispatch.
- The retry path for a genuinely failed submit is preserved (no permanent skip of a retryable watch).
- The keeper dedup doc claim states exactly what it guarantees.
Second occurrence: the L3 CoW sweep
Added from review of #466 (comment r3616645873). Verified at the end-of-train tip (24c4d9e).
The same contains -> submit -> record shape exists on the CoW path, independently of the L2 keeper sweep above. At the tip it lives in composable-cow/src/sweep.rs: journal.contains(&intent_id)? at :133, the submit at :138, journal.record(&intent_id) at :157. The pre-carve path is shepherd-sdk/src/cow/run.rs, which #471 deletes, so quote the composable-cow location when working this.
Severity is lower here than at the L2 site, and deliberately so. This sweep does not propagate a record fault: it logs and carries on (:153-159) because the submit already succeeded. The next tick re-posts, the orderbook answers with a duplicate error type, classification::is_already_submitted maps it to Refusal::AlreadyHeld (cow-venue/src/adapter.rs:391), post_order returns Posted::AlreadyHeld (:325), and the adapter answers SubmitOutcome::Accepted with assembly::order_uid(chain, order, owner) (:172). The journal write then lands on that pass, so the gap self-heals at the cost of one redundant POST.
That backstop is venue-specific, not structural, so it does not remove the need for the fix. It does mean the L2 keeper sweep is the site that actually loses correctness, since it propagates Err after a submitted-but-unrecorded body with no venue-side absorption. Fix the primitive once and both sites inherit it.
Related: #558 covers what the CoW dedup key scopes to, which is a separate question from when it is written.
Ripple checkpoint: #473 re-asserts the claim
The doc-tightening item above has a moving target. #473 (feat/m4-eip1271-one-block-retry) rewrites this exact paragraph in videre-sdk/src/keeper.rs (its hunk spans lines 65-74) and carries the overstated sentence through unchanged, still reading "so an accepted body never reaches the venue twice".
So the doc correction belongs in #473, not in an earlier car: an edit at that line in any car below it would simply conflict when #473 is rippled. When #473 comes up for ripple, tighten the claim there in the same pass.
The keeper sweep dedups with
contains->submit->record(videre-sdk/src/keeper.rs). The review raised two concerns; dispositions differ:Not a bug: concurrent double-submit
Two overlapping sweeps both passing
containsbefore eitherrecords cannot happen within an engine process. Dispatch is structurally serialized, not lock-based: the runtime moves one ownedSupervisorinto a single tokio task (nexum-runtime/src/builder.rs), whose event loop (runtime/event_loop.rs) selects one event and awaitsdispatch_*to completion before the next; every dispatch and restart path takes&mut self; a timed-out guest call is dropped (cancelled) beforedispatch_toreturns; and restart replaces the wasmtimeStorein-place (supervisor.rsreinstantiate_one), so at most one instance and oneon_eventis ever live.containsandrecordcannot be interleaved by a concurrent sweep of the same keeper. (Caveat: this is single-process. Two engine processes on the same redbstate_dircould race, but redb is single-process anyway.)Real bug: post-submit record is not durable
Concurrency-independent. If
venues.submitreturnsAcceptedbut the followingjournal.record(&key)?faults (a local-store host write error),sweeppropagatesErrwith the body submitted but unrecorded. The next redispatch (the intended at-least-once path across a crash/restart) then seescontains == falseand resubmits, sending a duplicate order to the venue. This breaks the sweep's own doc claim that "an accepted body never reaches the venue twice", with no concurrency involved.Scope
contains == true, skips it, and never retries the genuinely-failed submission.Accepted, release on a retryable fault) or an atomicrecord_if_absenton the accepted path that is retried until durable.nexum-sdk'sJournalcurrently exposes onlycontains+record(no CAS, no remove), so a small L1 primitive is likely needed.keeper.rsdoc comment: "an accepted body never reaches the venue twice" holds against in-process concurrency but not against the post-submit record-fault path.Timing
Land before the M4 cars re-point the keepers onto the live pool submit path (#469 / #470): that is when a duplicate submission becomes a real duplicate order against a venue.
Acceptance criteria
Second occurrence: the L3 CoW sweep
The same
contains->submit->recordshape exists on the CoW path, independently of the L2 keeper sweep above. At the tip it lives incomposable-cow/src/sweep.rs:journal.contains(&intent_id)?at:133, the submit at:138,journal.record(&intent_id)at:157. The pre-carve path isshepherd-sdk/src/cow/run.rs, which #471 deletes, so quote thecomposable-cowlocation when working this.Severity is lower here than at the L2 site, and deliberately so. This sweep does not propagate a record fault: it logs and carries on (
:153-159) because the submit already succeeded. The next tick re-posts, the orderbook answers with a duplicate error type,classification::is_already_submittedmaps it toRefusal::AlreadyHeld(cow-venue/src/adapter.rs:391),post_orderreturnsPosted::AlreadyHeld(:325), and the adapter answersSubmitOutcome::Acceptedwithassembly::order_uid(chain, order, owner)(:172). The journal write then lands on that pass, so the gap self-heals at the cost of one redundant POST.That backstop is venue-specific, not structural, so it does not remove the need for the fix. It does mean the L2 keeper sweep is the site that actually loses correctness, since it propagates
Errafter a submitted-but-unrecorded body with no venue-side absorption. Fix the primitive once and both sites inherit it.Related: #558 covers what the CoW dedup key scopes to, which is a separate question from when it is written.
Ripple checkpoint: #473 re-asserts the claim
The doc-tightening item above has a moving target. #473 (
feat/m4-eip1271-one-block-retry) rewrites this exact paragraph invidere-sdk/src/keeper.rs(its hunk spans lines 65-74) and carries the overstated sentence through unchanged, still reading "so an accepted body never reaches the venue twice".So the doc correction belongs in #473, not in an earlier car: an edit at that line in any car below it would simply conflict when #473 is rippled. When #473 comes up for ripple, tighten the claim there in the same pass.