Skip to content

keeper: make journal dedup durable across a post-submit record fault #538

Description

@mfw78

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working.component/local-storeredb per-module state + byte quotascomponent/sdknexum-sdk / shepherd-sdk, proc macros, cargo-nexumeffort/daysFew days. Needs design.p1-brokenActually broken. Fix this week.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions