Skip to content

cow: run the cow keeper on the generic venue client#468

Merged
mfw78 merged 1 commit into
dev/m1from
feat/m4-keeper-on-pool
Jul 23, 2026
Merged

cow: run the cow keeper on the generic venue client#468
mfw78 merged 1 commit into
dev/m1from
feat/m4-keeper-on-pool

Conversation

@mfw78

@mfw78 mfw78 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

Flips the CoW keeper's submit path from CowApiHost::submit_order onto the typed videre_sdk::VenueClient (CowClient<T>). run/submit_ready now take a &CowClient<T> and dispatch through VenueTransport::submit, keying the idempotency journal on the generic submission_key(venue, body). A new CowApiTransport bridges the typed client onto the still-live legacy shepherd:cow/cow-api host until module worlds flip to videre:venue/client; it is the only caller left of CowApiHost::submit_order. videre_sdk::keeper::retry_action is made public and takes &VenueFault so run can fold venue refusals into RetryAction outside Keeper::sweep.

Why

M4's go/no-go gate: the flagship CoW keeper must submit through the generic venue seam, not bypass it. This is the Rust-side seam port only; the legacy host extension stays wired until the module worlds flip, and the fork-gated orderbook poll wire-swap is untouched.

Testing

  • crates/shepherd-sdk/tests/run.rs: existing suite ported onto run(&host, &venue(&host), ...); new ready_submits_the_encoded_intent_body_through_the_venue_seam spies the raw VenueTransport to prove the wire carries the encoded CowIntentBody under CowVenue::ID and the legacy cow-api path is never touched.
  • crates/shepherd-sdk-test/tests/mock_venue.rs: ported onto the typed client.
  • crates/shepherd-sdk/src/cow/transport.rs: unit tests on CowApiTransport's fault projection and receipt decoding.

AI Assistance

Implemented with Claude Code assistance; reviewed and directed by mfw78.

Closes #400

@lgahdl lgahdl left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CowApiTransport bridge itself is solid — submit fully implemented (including the already-submitted case), quote/status/cancel are explicit tested Unsupported stubs (not silent no-ops), fault projection preserves retry-hint granularity (RateLimited with its ms hint, Timeout, HTTP 429 vs. other, routed through the already-ratified #464 classifier) rather than flattening, retry_action's publicization is signature-only with no behavior change, and the "legacy path never touched" test genuinely asserts a zero call-count on a spy mock rather than just omitting a negative assertion.

Two things carry over unresolved from #466 (confirmed still live at this point in the stack, not reintroduced by this PR): the journal's contains/record check-then-act pair still sandwiches the network submit with no atomicity (same race), and the journal key still hashes the full signed body including signature bytes (a re-signed identical order still defeats dedup rather than triggering it) — this PR only swaps what runs between the guard, not the guard itself. Worth tracking those against #466 rather than as new findings here.

Two new things worth a look, flagged here in the body since one line isn't near a diff hunk:

  • crates/shepherd-sdk/src/cow/mod.rs:32classify_submit_error stays in this public re-export list, but its only in-crate caller (run.rs's old submit_ready) was deleted by this PR — the live path now calls a different, newly-introduced classify_api_error in transport.rs instead. So this is now an orphaned public export: nothing in this crate exercises it anymore, but an external consumer could still import it, and since it's a completely separate function from the one that actually runs, its logic can silently drift from the live classification path with no compiler signal. Worth removing the stale re-export in this same PR, or folding it into classify_api_error if it's meant to still be equivalent.

  • crates/shepherd-sdk/src/cow/transport.rs:527 (CowApiError::Fault(fault) => VenueFault::Unavailable(fault.to_string())) — any host-level Fault, including Fault::Denied (e.g. a misconfigured capability allowlist, a genuinely permanent condition), projects to VenueFault::Unavailable, which retry_action treats as retryable. The PR's own test (host_faults_stay_retryable_and_never_drop_the_watch) confirms this is intentional, but it means a misconfigured allowlist retries every tick forever with only log noise, rather than surfacing distinctly from a transient infrastructure hiccup. Not a new regression (carried-over design choice), but worth a bounded retry count or distinct alerting for Fault::Denied specifically.

@mfw78
mfw78 force-pushed the feat/m4-cow-adapter-cdylib branch 2 times, most recently from a12853a to 6bc2bce Compare July 23, 2026 06:07
Base automatically changed from feat/m4-cow-adapter-cdylib to dev/m1 July 23, 2026 06:21
@mfw78
mfw78 force-pushed the feat/m4-keeper-on-pool branch from fbe2eca to 97d60bf Compare July 23, 2026 06:29
@mfw78

mfw78 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Checked all four against the end-of-train tip (feat/m5-git-tag-pins-umbrella-patch, 24c4d9e) to decide fold-now versus file. Outcome: nothing to fold and nothing new to file, because both new findings are superseded by #471, and the two carried-over ones are already tracked.

The two carried over from #466

Correctly diagnosed as not reintroduced here. The journal contains/record race is #538, which also now records this exact site: at the tip it lives in composable-cow/src/sweep.rs (contains at :133, submit at :138, record at :157), since shepherd-sdk/src/cow/run.rs is deleted by #471. The decided fix is reserve/commit/release over the existing verbs, no WIT change.

The signature-in-the-key point was #558, which I resolved by folding the documentation option into #466 rather than changing the key, and closed on merge. The reason it is documented rather than fixed: a re-signed identical order does not actually defeat dedup end to end, because the orderbook's identity for an order is signature-independent (assembly::order_uid is order.uid(&chain.settlement_domain(), owner)). A differently-keyed second attempt therefore lands on the duplicate path, is_already_submitted maps it to Refusal::AlreadyHeld, and the adapter answers SubmitOutcome::Accepted with the canonical uid. The cost is one redundant POST, not a duplicate order. intent_id's rustdoc now states that the key scopes to the exact signed payload rather than the economic order.

Orphaned classify_submit_error re-export

Confirmed exactly as described: defined at cow/error.rs:145, exercised only by its own unit tests in that file, re-exported at cow/mod.rs:32, and zero production callers left once this PR deleted the old submit_ready. I also checked #469 and #470, and neither adds a caller.

Deliberately not folding the removal in, because #471 deletes cow/mod.rs and cow/transport.rs outright (both show as D in its diff) and shepherd-sdk has zero files by the end of the train. Editing cow/mod.rs now would guarantee a modify/delete conflict when #471 is rippled, in exchange for retiring an export that lives for three cars. The drift risk you are guarding against also closes there: the tip keeps exactly one classifier, cow-venue/src/classification.rs, so the two-copy window cannot outlive #471.

Fault::Denied projecting to retryable

Confirmed, at cow/transport.rs:111 in the merged form (the diff view numbers it 527), and it is deliberate rather than accidental, with the comment stating the intent: any non-throttle, non-timeout host fault "stays retryable so an unprovisioned capability or unknown chain never drops a still-valid order".

The end state already takes your preferred behaviour, so no tracker is warranted. Once #471 removes this bridge, a host fault reaches the keeper through From<host::Fault> for VenueError, which maps Denied(s) structurally to VenueError::Denied(s) (videre-sdk/src/faults.rs:122), then to VenueFault::Denied(s) (:64), which retry_action turns into RetryAction::Drop (videre-sdk/src/keeper.rs:197). A misconfigured allowlist therefore drops the watch at the end of the train instead of retrying every tick. The infinite-retry window is exactly the three cars this legacy bridge lives for, and only for the legacy shepherd:cow/cow-api path.

@mfw78
mfw78 merged commit a926f79 into dev/m1 Jul 23, 2026
7 checks passed
@mfw78
mfw78 deleted the feat/m4-keeper-on-pool branch July 23, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants