Skip to content

cow: build the cow venue adapter component with bounded request timeouts - #467

Merged
mfw78 merged 1 commit into
dev/m1from
feat/m4-cow-adapter-cdylib
Jul 23, 2026
Merged

cow: build the cow venue adapter component with bounded request timeouts#467
mfw78 merged 1 commit into
dev/m1from
feat/m4-cow-adapter-cdylib

Conversation

@mfw78

@mfw78 mfw78 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

Adds the adapter feature slice to cow-venue: CowAdapter under #[videre_sdk::venue] decodes CowIntentBody, derives the value-flow header, and speaks the orderbook REST API over scoped wasi:http. A signed order posts EIP-1271 (receipt is the canonical 56-byte UID); an unsigned order posts pre-sign and returns requires-signing carrying the setPreSignature call. An already-held rejection succeeds with the client-derived UID; errorType rejections project onto venue-error through the classification table so the retry hint survives the collapse.

Chain-edge order assembly (gpv2_to_order_data, order_data_to_body, order_uid_hex, build_order_creation) moves into a new assembly slice the adapter owns; shepherd-sdk re-exports it for the legacy keeper run(), otherwise unchanged. videre-sdk gains BoundedFetch, which caps the wasi:http connect, first-byte and between-bytes timeouts of every adapter request; the adapter's default bound is nexum_sdk::http::DEFAULT_TIMEOUT, overridable per instance via the http-timeout-ms config key. Codec vectors and header goldens ship under tests/vectors with a conformance suite. The built component bundles into the shepherd distribution: engine adapters stanza, Docker/CI wasm build, justfile target.

Closes #324.

Why

Routes outbound orderbook calls through typed transport wrappers carrying a per-request timeout, resolving #41 in the adapter rather than patching cow_orderbook.rs. The bound is applied by clamping wasi:http's own request-options phase timeouts, not by any guest-side timing. The adapter is the videre:adapter/venue-adapter component the CoW-on-videre split plan calls for; the legacy keeper path stays on CowApiHost but now shares the assembly slice. Pool-router wiring is a later port.

Testing

  • cargo nextest run -p cow-venue -p videre-sdk -p shepherd-sdk
  • cargo test --doc
  • cargo build --release --target wasm32-wasip2 -p cow-venue --features adapter
  • cargo fmt --all --check, cargo clippy --workspace --all-targets --all-features -- -D warnings

AI Assistance

Implemented with Claude Code.

@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.

Thorough review given the stakes here (real funds/orders flowing through this eventually) — the vast majority of this checks out clean: EIP-1271 signature attachment is correctly wired to the same decoded body throughout, the pre-sign flow's requires-signing carries the real contract address/UID/calldata with no path that marks an order locally 'live' before the host actually sends the tx, TimedFetch genuinely clamps every adapter request with no bypass found, errorType classification correctly preserves retry hints through the collapse and reuses the already-ratified table from #464 unchanged, the assembly slice move is a pure relocation with the legacy keeper and new adapter now genuinely sharing one function rather than two copies, wasi:http is scoped to exactly api.cow.fi, and no unsafe unwraps exist outside test code. A few things worth a look:

.map_err(|e| VenueError::InvalidBody(e.to_string()))?;
let uid = match post_order(fetch, config, &creation)? {
Posted::Accepted(uid) => uid,
Posted::AlreadyHeld => assembly::order_uid(config.chain, &order, owner),

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.

On Posted::AlreadyHeld, the returned UID is computed purely client-side (assembly::order_uid) with nothing from the server to cross-check it against — the CoW API's 409/already-held response carries no UID in its body by design, so this is the only lever available. This is inherent to the "already-held is success" design, not something this diff introduced a bug into, but it's worth having status_with (called right after, in the same flow) hard-fail loudly if the returned order's sell/buy token or validTo don't match the just-submitted body, rather than trusting the locally-derived UID alone all the way through. Same consideration applies to the Accepted branch just above (line 171) — nothing asserts the orderbook's actual stored order matches what was locally derived, which would catch an orderbook/library UID-derivation drift immediately instead of silently trusting the server.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed still valid at the end-of-train tip, and agreed it is a trust gap rather than a bug this diff introduced. Filed as #559.

One correction to the suggested fix, which changed the shape of the tracker. status_with cannot do this comparison as written: it receives only the 56-byte receipt, and it deliberately deserializes a single field (struct OrderStatusView { status: OrderStatus }, commented "the one server field the lifecycle projection reads"). So there is nothing to compare the tokens or validTo against without threading the expected order into the status path, which is an interface change rather than a guard added to the existing read.

#559 therefore carries three options: verify at submit time with one extra GET, verify at status time by widening both the view and the signature, or the cheap one that I think should happen regardless, which is asserting the server's returned UID against assembly::order_uid(...) on the Accepted branch where both values are already in hand. That last one catches exactly the derivation-drift case you are worried about at zero request cost and with no signature change, though it cannot cover AlreadyHeld, where no server UID exists at all.

impl Refusal {
/// Collapse for call sites where already-held is not a success
/// shape (reads); the orderbook only emits it on submission.
fn into_error(self) -> VenueError {

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.

already_held is correct as success for submit, but this into_error() exists specifically because status/quote reads must NOT treat it as success — nothing at the type level forces a future call site to pick the right interpretation if someone adds a new read-path call to refusal() and forgets .into_error(). Consider splitting into refusal_for_submit/refusal_for_read (or renaming) to make the caller's obligation explicit rather than relying on every future caller remembering which context they're in.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed still valid at the end-of-train tip. Filed as #560.

Worth recording that all three production call sites are currently correct: quote_with and status_with both take refusal(&response).into_error(), and submit_with is the single site that matches on the variant and treats AlreadyHeld as success. So it is a foot-gun for the next read-path caller rather than a live defect.

#560 takes your first suggestion as the recommendation, splitting into refusal_for_submit returning Refusal and refusal_for_read returning VenueError directly, so a read path can never hold an unresolved already-held meaning. I have not folded it into this car: #473 also edits cow-venue/src/adapter.rs, so doing it here would just conflict when that car is rippled. It should land after #473, or inside #473's own pass.

@mfw78
mfw78 force-pushed the feat/m4-idempotency-seam branch 2 times, most recently from dbadb56 to f82f4a0 Compare July 23, 2026 04:47
@mfw78
mfw78 force-pushed the feat/m4-idempotency-seam branch from f82f4a0 to fb5d1a8 Compare July 23, 2026 05:11
Base automatically changed from feat/m4-idempotency-seam to dev/m1 July 23, 2026 05:20
@mfw78
mfw78 force-pushed the feat/m4-cow-adapter-cdylib branch from 39f178a to a12853a Compare July 23, 2026 05:32
@mfw78
mfw78 force-pushed the feat/m4-cow-adapter-cdylib branch from a12853a to 6bc2bce Compare July 23, 2026 06:07
@mfw78 mfw78 changed the title cow: build the cow venue adapter component with timeout transport middleware cow: build the cow venue adapter component with bounded request timeouts Jul 23, 2026
@mfw78
mfw78 merged commit 4d358ca into dev/m1 Jul 23, 2026
7 checks passed
@mfw78
mfw78 deleted the feat/m4-cow-adapter-cdylib branch July 23, 2026 06:21
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