From 95ea54144ad1ac44e9a8927630efb4bd72d4acd1 Mon Sep 17 00:00:00 2001 From: mfw78 Date: Fri, 17 Jul 2026 11:10:21 +0000 Subject: [PATCH] cow: ratify the retry classification table against the upstream errorType enum Prune the phantom PriceExceedsMarketPrice row, record the table (not cowprotocol RetryHint) as the classification source of truth in the data header, and pin the reconciliation with parity tests: every row must name a real upstream errorType, and the divergence from retry_hint() must be exactly the ratified set. --- Cargo.lock | 1 + crates/cow-venue/Cargo.toml | 3 ++ crates/cow-venue/data/classification.toml | 35 +++++++------ crates/cow-venue/src/classification.rs | 60 +++++++++++++++++++++++ crates/shepherd-sdk/src/cow/error.rs | 13 ++--- 5 files changed, 89 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e25aca46..4edc18c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1570,6 +1570,7 @@ name = "cow-venue" version = "0.1.0" dependencies = [ "borsh", + "cowprotocol", "nexum-sdk", "serde", "thiserror 2.0.18", diff --git a/crates/cow-venue/Cargo.toml b/crates/cow-venue/Cargo.toml index 8e303765..3a780947 100644 --- a/crates/cow-venue/Cargo.toml +++ b/crates/cow-venue/Cargo.toml @@ -41,6 +41,9 @@ toml = { workspace = true } thiserror = { workspace = true } # The conformance kit: holds the body codec to its published vector set. videre-test = { path = "../videre-test" } +# Parity tests only: the upstream errorType enum and `retry_hint()` the +# shipped table is reconciled against. Never a runtime dependency. +cowprotocol = { version = "0.2.0", default-features = false } [features] # The body-type + codec slice ships by default; the `client` slice layers diff --git a/crates/cow-venue/data/classification.toml b/crates/cow-venue/data/classification.toml index 46507d48..f7910590 100644 --- a/crates/cow-venue/data/classification.toml +++ b/crates/cow-venue/data/classification.toml @@ -33,17 +33,26 @@ # # Relationship to `cowprotocol::ApiError::retry_hint()`. The upstream # `cowprotocol` crate (a shepherd-sdk dependency) also classifies -# orderbook `errorType`s, via `RetryHint`. This table is deliberately -# NOT delegated to it: it is shepherd's own, more conservative retry -# policy, kept as data of record here so a non-Rust author owns it and -# so the guest `client` slice stays free of the upstream error module. -# The two intentionally diverge on several types - e.g. this table drops -# `InvalidEip1271Signature`, `InsufficientBalance`, `InsufficientAllowance` -# and `InvalidAppData` where upstream retries or backs off, and backs off -# `TooManyLimitOrders` for 30s rather than an hour. These are ratified -# shepherd decisions (a permanent-looking contract rejection is dropped -# rather than retried every block); revisit them here, not by switching -# the source of truth to `RetryHint`. +# orderbook `errorType`s, via `RetryHint`. Ratified: this table, not +# `RetryHint`, is shepherd's classification source of truth. It is +# shepherd's own, more conservative retry policy, kept as data of record +# here so a non-Rust author owns it and so the guest `client` slice +# stays free of the upstream error module. The ratified divergences +# (a permanent-looking contract rejection is dropped rather than +# retried, and the limit-order backoff is shorter) are exactly: +# +# InvalidEip1271Signature drop upstream: retry next block +# InsufficientBalance drop upstream: backoff 10 min +# InsufficientAllowance drop upstream: backoff 10 min +# InvalidAppData drop upstream: backoff 60 s +# TooManyLimitOrders backoff 30 s upstream: backoff 1 h +# +# Every `error-type` below must name a member of the upstream orderbook +# errorType enum (`cowprotocol::OrderbookApiErrorType`). Parity tests +# reject phantom types and pin the divergence set to the list above, so +# both a data edit and an upstream policy change force re-ratification. +# Revisit policy here, not by switching the source of truth to +# `RetryHint`. # --- Transient: retry on the next block ------------------------------ @@ -51,10 +60,6 @@ error-type = "InsufficientFee" action = "try-next-block" -[[entry]] -error-type = "PriceExceedsMarketPrice" -action = "try-next-block" - # --- Throttle: wait, then retry -------------------------------------- # The account already holds the maximum number of open limit orders. A diff --git a/crates/cow-venue/src/classification.rs b/crates/cow-venue/src/classification.rs index 73d0e71f..aabda91f 100644 --- a/crates/cow-venue/src/classification.rs +++ b/crates/cow-venue/src/classification.rs @@ -235,6 +235,66 @@ mod tests { ); } + /// Every listed `error-type` names a member of the upstream + /// orderbook errorType enum, in its exact wire spelling: no phantom + /// rows. + #[test] + fn every_row_names_a_real_error_type() { + let entries = parse_and_validate(CLASSIFICATION_TOML).expect("shipped data is valid"); + for entry in &entries { + let kind = cowprotocol::OrderbookApiErrorType::from(entry.error_type.as_str()); + assert!( + !matches!(kind, cowprotocol::OrderbookApiErrorType::Unknown(_)), + "phantom errorType {}", + entry.error_type, + ); + assert_eq!(kind.as_str(), entry.error_type, "wire spelling"); + } + } + + /// The table's divergence from upstream `retry_hint()` is exactly + /// the ratified set in the data header. A data edit or an upstream + /// policy change lands here and forces re-ratification. + #[test] + fn divergence_from_upstream_is_exactly_the_ratified_set() { + const RATIFIED: [&str; 5] = [ + "InsufficientAllowance", + "InsufficientBalance", + "InvalidAppData", + "InvalidEip1271Signature", + "TooManyLimitOrders", + ]; + let entries = parse_and_validate(CLASSIFICATION_TOML).expect("shipped data is valid"); + let mut divergent: Vec<&str> = Vec::new(); + for entry in &entries { + let api = cowprotocol::ApiError { + error_type: entry.error_type.clone(), + description: String::new(), + data: None, + }; + // Project the upstream hint into the table's model; a hint + // variant this projection does not know is a divergence. + let upstream = match api.retry_hint() { + cowprotocol::RetryHint::Retry => Some((RetryAction::TryNextBlock, false)), + cowprotocol::RetryHint::Backoff { seconds } => { + Some((RetryAction::Backoff { seconds }, false)) + } + cowprotocol::RetryHint::Drop => Some((RetryAction::Drop, false)), + cowprotocol::RetryHint::AlreadySubmitted => Some((RetryAction::TryNextBlock, true)), + _ => None, + }; + let shepherd = ( + classify(&entry.error_type), + is_already_submitted(&entry.error_type), + ); + if upstream != Some(shepherd) { + divergent.push(&entry.error_type); + } + } + divergent.sort_unstable(); + assert_eq!(divergent, RATIFIED); + } + /// A non-Rust reader sees the same file as plain data: parsing it /// with the untyped TOML value model (no Rust schema) exposes the /// entries and their fields, proving any TOML library reads it. diff --git a/crates/shepherd-sdk/src/cow/error.rs b/crates/shepherd-sdk/src/cow/error.rs index 3f676861..a5e0de03 100644 --- a/crates/shepherd-sdk/src/cow/error.rs +++ b/crates/shepherd-sdk/src/cow/error.rs @@ -170,14 +170,11 @@ mod tests { } #[test] - fn retriable_kinds_yield_try_next_block() { - for kind in ["InsufficientFee", "PriceExceedsMarketPrice"] { - assert_eq!( - classify_api_error(&rejection(kind)), - RetryAction::TryNextBlock, - "{kind}", - ); - } + fn retriable_kind_yields_try_next_block() { + assert_eq!( + classify_api_error(&rejection("InsufficientFee")), + RetryAction::TryNextBlock, + ); } /// A throttle errorType backs off rather than retrying next block,