diff --git a/crates/autopilot/src/infra/solvers/dto/solve.rs b/crates/autopilot/src/infra/solvers/dto/solve.rs index 7610832d7b..674d55cb98 100644 --- a/crates/autopilot/src/infra/solvers/dto/solve.rs +++ b/crates/autopilot/src/infra/solvers/dto/solve.rs @@ -17,7 +17,7 @@ use { observe::http_body::Measured, reqwest::{RequestBuilder, header::HeaderValue}, serde::{Deserialize, Serialize}, - serde_with::{DisplayFromStr, serde_as}, + serde_with::{DisplayFromStr, MapPreventDuplicates, serde_as}, std::{ borrow::Cow, collections::{HashMap, HashSet}, @@ -388,6 +388,9 @@ pub struct Solution { pub solution_id: u64, /// Address used by the driver to submit the settlement onchain. pub submission_address: Address, + /// A partially fillable order may be split across several solutions, but + /// a single solution settles each order exactly once. + #[serde_as(as = "MapPreventDuplicates<_, _>")] pub orders: HashMap, /// Deprecated: uniform clearing prices are no longer used by the /// autopilot. Kept here purely so we can detect and log drivers that @@ -450,6 +453,58 @@ mod tests { } } + fn order_uid(byte: u8) -> String { + const_hex::encode_prefixed([byte; 56]) + } + + fn traded_order() -> serde_json::Value { + serde_json::json!({ + "side": "sell", + "sellToken": format!("0x{:040x}", 1), + "buyToken": format!("0x{:040x}", 2), + "limitSell": "1000", + "limitBuy": "900", + "executedSell": "500", + "executedBuy": "450", + }) + } + + fn response_with_orders(orders: &[String]) -> String { + let orders = orders + .iter() + .map(|uid| format!("\"{uid}\": {}", traded_order())) + .join(","); + format!( + r#"{{"solutions": [{{ + "solutionId": 1, + "submissionAddress": "0x{:040x}", + "orders": {{{orders}}} + }}]}}"#, + 3 + ) + } + + #[test] + fn rejects_solutions_settling_the_same_order_twice() { + let uid = order_uid(1); + let response = response_with_orders(&[uid.clone(), uid.clone()]); + + let err = serde_json::from_str::(&response).unwrap_err(); + assert!( + err.to_string() + .contains("invalid entry: found duplicate key"), + "unexpected error: {err}" + ); + } + + #[test] + fn accepts_one_entry_per_order() { + let response = response_with_orders(&[order_uid(1), order_uid(2)]); + + let response = serde_json::from_str::(&response).unwrap(); + assert_eq!(response.solutions[0].orders.len(), 2); + } + #[test] fn compressed_request_round_trips() { let json = make_test_json(); diff --git a/crates/driver/openapi.yml b/crates/driver/openapi.yml index 5681b3abb4..da085c1f14 100644 --- a/crates/driver/openapi.yml +++ b/crates/driver/openapi.yml @@ -542,7 +542,10 @@ components: orders: description: > Mapping of order uid to net executed amount (including all - fees). + fees). Each order may appear at most once: a partially + fillable order can be split across multiple solutions, but not + across multiple entries of the same solution. Solutions that + violate this are rejected. additionalProperties: type: object properties: diff --git a/crates/solvers-dto/src/solution.rs b/crates/solvers-dto/src/solution.rs index 816486e47b..9eea432145 100644 --- a/crates/solvers-dto/src/solution.rs +++ b/crates/solvers-dto/src/solution.rs @@ -1,9 +1,9 @@ use { alloy_primitives::{Address, U256}, number::serialization::HexOrDecimalU256, - serde::{Deserialize, Serialize}, + serde::{Deserialize, Deserializer, Serialize, de}, serde_with::serde_as, - std::collections::HashMap, + std::collections::{HashMap, HashSet}, }; #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] @@ -27,13 +27,42 @@ pub enum SolverErrorCode { Other, } -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize)] #[serde(untagged)] pub enum SolverResponse { Solutions { solutions: Vec }, Error { error: SolverError }, } +/// Hand written because `#[serde(untagged)]` discards the error of every +/// variant it tries and reports only that none matched, which hides why a +/// solution was rejected from the solver we notify about it. +impl<'de> Deserialize<'de> for SolverResponse { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + #[derive(Deserialize)] + struct Raw { + solutions: Option>, + error: Option, + } + + match Raw::deserialize(deserializer)? { + Raw { + solutions: Some(solutions), + .. + } => Ok(Self::Solutions { solutions }), + Raw { + error: Some(error), .. + } => Ok(Self::Error { error }), + Raw { .. } => Err(de::Error::custom( + "expected either a `solutions` or an `error` field", + )), + } + } +} + impl Default for SolverResponse { fn default() -> Self { Self::Solutions { @@ -49,6 +78,7 @@ pub struct Solution { pub id: u64, #[serde_as(as = "HashMap<_, HexOrDecimalU256>")] pub prices: HashMap, + #[serde(deserialize_with = "deserialize_trades")] pub trades: Vec, #[serde(default)] pub pre_interactions: Vec, @@ -65,6 +95,30 @@ pub struct Solution { pub wrappers: Vec, } +/// A partially fillable order may be split across several solutions, but a +/// single solution settles each order exactly once. +fn deserialize_trades<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let trades = Vec::::deserialize(deserializer)?; + + let mut settled = HashSet::with_capacity(trades.len()); + for trade in &trades { + let Trade::Fulfillment(fulfillment) = trade else { + continue; + }; + if !settled.insert(&fulfillment.order) { + let uid = const_hex::encode_prefixed(fulfillment.order.0); + return Err(de::Error::custom(format!( + "order {uid} is settled by more than one trade" + ))); + } + } + + Ok(trades) +} + #[serde_as] #[derive(Clone, Debug, Serialize, Deserialize, Hash, Eq, PartialEq)] pub struct OrderUid(#[serde_as(as = "serde_ext::Hex")] pub [u8; 56]); @@ -300,6 +354,58 @@ mod tests { ); } + fn order_uid(byte: u8) -> String { + const_hex::encode_prefixed([byte; 56]) + } + + fn solution_with_fulfillments(orders: &[String]) -> serde_json::Value { + json!({ + "id": 0, + "prices": {}, + "trades": orders.iter().map(|order| json!({ + "kind": "fulfillment", + "order": order, + "executedAmount": "1000", + })).collect::>(), + "interactions": [], + }) + } + + #[test] + fn rejects_solutions_settling_the_same_order_twice() { + let order = order_uid(1); + let solution = solution_with_fulfillments(&[order.clone(), order.clone()]); + + let err = serde_json::from_value::(solution).unwrap_err(); + assert_eq!( + err.to_string(), + format!("order {order} is settled by more than one trade") + ); + } + + #[test] + fn accepts_one_trade_per_order() { + let solution = solution_with_fulfillments(&[order_uid(1), order_uid(2)]); + + let solution = serde_json::from_value::(solution).unwrap(); + assert_eq!(solution.trades.len(), 2); + } + + #[test] + fn duplicate_trade_error_survives_the_response_wrapper() { + let order = order_uid(1); + let response = json!({ + "solutions": [solution_with_fulfillments(&[order.clone(), order.clone()])], + }); + + let err = serde_json::from_value::(response).unwrap_err(); + assert!( + err.to_string() + .contains(&format!("order {order} is settled by more than one trade")), + "unexpected error: {err}" + ); + } + #[test] fn serializes_and_deserializes_error_responses() { let cases = vec![ diff --git a/crates/solvers/openapi.yml b/crates/solvers/openapi.yml index 2177a12881..e739e024bf 100644 --- a/crates/solvers/openapi.yml +++ b/crates/solvers/openapi.yml @@ -1009,7 +1009,10 @@ components: $ref: "#/components/schemas/U256" trades: description: | - CoW Protocol order trades included in the solution. + CoW Protocol order trades included in the solution. Each order may + appear at most once: a partially fillable order can be split across + multiple solutions, but not across multiple trades of the same + solution. Solutions that violate this are rejected. type: array items: $ref: "#/components/schemas/Trade"