cow: settle the idempotency seam on the venue-and-body intent-id#466
cow: settle the idempotency seam on the venue-and-body intent-id#466mfw78 wants to merge 1 commit into
Conversation
The submitted: journal now keys on the deterministic intent-id (the generic sweep's venue-and-body submission key over the encoded CowIntentBody), derived pre-submit without assembling OrderCreation. SubmitOutcome's accepted receipt is fixed as the canonical 56-byte order UID (OrderUid), and the CowIntent schema gains the Signed kind a conditional-order keeper emits. A regression test covers resubmit after restart with a single orderbook POST.
lgahdl
left a comment
lgahdl
left a comment
There was a problem hiding this comment.
The intent_id derivation itself is sound (deterministic, body-scoped, verified different Signed payloads produce different ids), and the restart-regression test genuinely simulates a fresh process (independent MockHost::default(), only the local-store snapshot carried across) rather than just single-process dedup. Three things worth addressing:
| } | ||
| }; | ||
| let journal = Journal::submitted(host); | ||
| if journal.contains(&intent_id)? { |
There was a problem hiding this comment.
This is the real production journal site (submit_ready), and it has the same non-atomic check-then-act shape as the gap I flagged on #454 (videre-sdk::keeper.rs's journal.contains/journal.record): contains(&intent_id) here, then record(&intent_id) later after the actual network submit — two separate host calls with no atomicity between them. Two overlapping sweeps on the same host could both pass contains before either records. This PR's framing ("closes the double-post window") is about the pre-submit-derivability problem (the keeper can't derive a UID once order assembly moves into the adapter) — it doesn't touch this concurrency/atomicity race, which is a different problem and predates this PR (it was already split on the old UID key). Worth flagging as still open rather than implicitly closed by this PR's title.
| /// An owner-signed order ready for the orderbook: what a | ||
| /// conditional-order keeper emits after a poll. | ||
| #[derive(BorshSerialize, BorshDeserialize, Clone, Debug, PartialEq, Eq)] | ||
| pub struct SignedOrder { |
There was a problem hiding this comment.
SignedOrder derives BorshSerialize over the whole struct including signature: Vec<u8>, and intent_id hashes the full encoded body — so the signature bytes are part of the dedup key. That's fine for collision-safety, but it means a legitimate re-sign of the same underlying order (same order+owner, fresh EIP-1271 signature after e.g. a keeper restart mid-flight, before the first attempt's journal write landed) produces a different intent_id and defeats dedup rather than falsely triggering it — the opposite failure mode from what this PR is guarding against. Concrete scenario: keeper signs, crashes before journaling, restarts, re-signs the same order, and if the first submit actually reached the orderbook, the second (differently-keyed) attempt isn't recognized as a duplicate. Worth either hashing (venue, owner, order) and excluding signature, or documenting explicitly that this key scopes to "this exact signed payload," not "this economic order."
| /// The CoW venue marker: every [`CowClient`] call routes to | ||
| /// [`Venue::ID`] and encodes a [`CowIntentBody`]. | ||
| /// [`Venue::ID`] and encodes a [`CowIntentBody`]. An accepted submit's | ||
| /// receipt is the canonical [`OrderUid`](crate::OrderUid) in wire form. |
There was a problem hiding this comment.
This comment (and the PR body) states "SubmitOutcome's accepted receipt is now the canonical 56-byte OrderUid," but nothing in this diff actually wires OrderUid into submit_order's return type or SubmitOutcome — neither symbol is touched here. OrderUid is added with solid conversions and round-trip tests, but it's constructed only in this PR's own unit tests; no production call site builds one from a real submit response yet. Worth either wiring it into the real return path in this PR, or scoping this comment/the PR description to "the type is added, integration follows" so it doesn't read as already-shipped.
2 participants
What
Keys the CoW
submitted:journal on the deterministic venue-and-bodyintent_id(cow-venue::intent_id, wrapping the generic sweep'ssubmission_keyover the encodedCowIntentBody) instead of the client-computed order UID. The id is derived pre-submit from the same body bytes a venue submit carries, beforeOrderCreationis assembled.SubmitOutcome's accepted receipt is now the canonical 56-byteOrderUid.CowIntentgains aSignedvariant for a conditional-order keeper's owner-signed order.videre-sdk::keeper::submission_keyis now public so a keeper journalling outsideKeeper::sweepwrites the same key.twap-monitorandshepherd-sdktests and docs follow the rename.Why
Once order assembly moves into the venue adapter, the keeper can no longer derive a UID pre-submit, opening a double-post window across the keeper-to-adapter boundary. The venue-and-body intent-id is derivable before any network work and survives that move.
Testing
intent_iddeterminism and body-scoping,SignedOrderborsh round-trip,OrderUidconversions andDisplay.cargo fmt --all --check,cargo check --workspace --all-features,cargo clippy --workspace --all-targets --all-features -- -D warnings,cargo nextest run --workspace --all-features,cargo test --doc.AI Assistance
Implemented with Claude Code.
Closes #396