From bd466e57699b86607a80f47d35f03a8622d5daee Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Fri, 8 Dec 2023 11:52:12 +0100 Subject: [PATCH] Reveal settled order already in /solve Response (#1975) # Description Implements only the part > orders they intend to execute and their in/out amounts from https://github.com/cowprotocol/services/issues/1949 Orders are no longer sent as a reponse to `/reveal` but as a response to `/solve`. I also added in/out amounts for each order. --------- Co-authored-by: Felix Leupold --- crates/autopilot/src/driver_model.rs | 17 ++- crates/autopilot/src/run_loop.rs | 64 +++++---- crates/driver/src/domain/competition/mod.rs | 25 +++- .../domain/competition/solution/settlement.rs | 24 +++- .../src/domain/competition/solution/trade.rs | 43 +++++- crates/driver/src/domain/eth/mod.rs | 2 +- .../infra/api/routes/reveal/dto/revealed.rs | 8 +- .../src/infra/api/routes/solve/dto/solved.rs | 32 ++++- .../src/tests/cases/merge_settlements.rs | 8 +- .../src/tests/cases/multiple_solutions.rs | 16 ++- .../driver/src/tests/cases/negative_scores.rs | 8 +- .../src/tests/cases/score_competition.rs | 15 +- crates/driver/src/tests/setup/mod.rs | 136 +++++++++++------- .../e2e/partially_fillable_observed_score.rs | 12 +- crates/model/src/solver_competition.rs | 47 ++++-- .../src/database/solver_competition.rs | 2 +- crates/solver/src/driver.rs | 2 +- 17 files changed, 318 insertions(+), 143 deletions(-) diff --git a/crates/autopilot/src/driver_model.rs b/crates/autopilot/src/driver_model.rs index f2d195570f..5dd07e472a 100644 --- a/crates/autopilot/src/driver_model.rs +++ b/crates/autopilot/src/driver_model.rs @@ -60,6 +60,7 @@ pub mod solve { primitive_types::{H160, U256}, serde::{Deserialize, Serialize}, serde_with::{serde_as, DisplayFromStr}, + std::collections::HashMap, }; #[serde_as] @@ -136,6 +137,18 @@ pub mod solve { pub call_data: Vec, } + #[serde_as] + #[derive(Clone, Debug, Default, Deserialize)] + #[serde(rename_all = "camelCase", deny_unknown_fields)] + pub struct TradedAmounts { + /// The effective amount that left the user's wallet including all fees. + #[serde_as(as = "HexOrDecimalU256")] + pub sell_amount: U256, + /// The effective amount the user received after all fees. + #[serde_as(as = "HexOrDecimalU256")] + pub buy_amount: U256, + } + #[serde_as] #[derive(Clone, Debug, Default, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] @@ -148,6 +161,7 @@ pub mod solve { pub score: U256, /// Address used by the driver to submit the settlement onchain. pub submission_address: H160, + pub orders: HashMap, } #[derive(Clone, Debug, Default, Deserialize)] @@ -159,7 +173,7 @@ pub mod solve { pub mod reveal { use { - model::{bytes_hex, order::OrderUid}, + model::bytes_hex, serde::{Deserialize, Serialize}, serde_with::serde_as, }; @@ -186,7 +200,6 @@ pub mod reveal { #[derive(Clone, Debug, Default, Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct Response { - pub orders: Vec, pub calldata: Calldata, } } diff --git a/crates/autopilot/src/run_loop.rs b/crates/autopilot/src/run_loop.rs index 8f514e9bd2..9e983e5ab6 100644 --- a/crates/autopilot/src/run_loop.rs +++ b/crates/autopilot/src/run_loop.rs @@ -8,7 +8,7 @@ use { driver_model::{ reveal::{self, Request}, settle, - solve::{self, Class}, + solve::{self, Class, TradedAmounts}, }, solvable_orders::SolvableOrdersCache, }, @@ -34,7 +34,7 @@ use { rand::seq::SliceRandom, shared::{remaining_amounts, token_list::AutoUpdatingTokenList}, std::{ - collections::{BTreeMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, sync::{Arc, Mutex}, time::{Duration, Instant}, }, @@ -149,9 +149,8 @@ impl RunLoop { } }; - let events = revealed - .orders - .iter() + let events = solution + .order_ids() .map(|o| (*o, OrderEventLabel::Considered)) .collect::>(); self.database.store_order_events(&events).await; @@ -178,7 +177,7 @@ impl RunLoop { // Save order executions for all orders in the solution. Surplus fees for // limit orders will be saved after settling the order onchain. let mut order_executions = vec![]; - for order_id in &revealed.orders { + for order_id in solution.order_ids() { let auction_order = auction .orders .iter() @@ -239,22 +238,21 @@ impl RunLoop { solver_address: participant.solution.account, score: Some(Score::Solver(participant.solution.score.get())), ranking: Some(solutions.len() - index), + orders: participant + .solution + .orders() + .iter() + .map(|(id, order)| Order::Colocated { + id: *id, + sell_amount: order.sell_amount, + buy_amount: order.buy_amount, + }) + .collect(), // TODO: revisit once colocation is enabled (remove not populated // fields) Not all fields can be populated in the colocated world ..Default::default() }; if is_winner { - settlement.orders = revealed - .orders - .iter() - .map(|o| Order { - id: *o, - // TODO: revisit once colocation is enabled (remove not - // populated fields) Not all - // fields can be populated in the colocated world - ..Default::default() - }) - .collect(); settlement.call_data = revealed.calldata.internalized.clone(); settlement.uninternalized_call_data = Some(revealed.calldata.uninternalized.clone()); @@ -288,7 +286,7 @@ impl RunLoop { } tracing::info!(driver = %driver.name, "settling"); - match self.settle(driver, solution, &revealed).await { + match self.settle(driver, solution).await { Ok(()) => Metrics::settle_ok(driver), Err(err) => { Metrics::settle_err(driver, &err); @@ -385,6 +383,7 @@ impl RunLoop { id: solution.solution_id, account: solution.submission_address, score: NonZeroU256::new(solution.score).ok_or(ZeroScoreError)?, + orders: solution.orders, }) }) .collect()) @@ -414,15 +413,9 @@ impl RunLoop { /// Execute the solver's solution. Returns Ok when the corresponding /// transaction has been mined. - async fn settle( - &self, - driver: &Driver, - solved: &Solution, - revealed: &reveal::Response, - ) -> Result<(), SettleError> { - let events = revealed - .orders - .iter() + async fn settle(&self, driver: &Driver, solved: &Solution) -> Result<(), SettleError> { + let events = solved + .order_ids() .map(|uid| (*uid, OrderEventLabel::Executing)) .collect_vec(); self.database.store_order_events(&events).await; @@ -439,12 +432,12 @@ impl RunLoop { *self.in_flight_orders.lock().unwrap() = InFlightOrders { tx_hash, - orders: revealed.orders.iter().cloned().collect(), + orders: solved.orders.keys().copied().collect(), }; - let events = revealed + let events = solved .orders - .iter() + .keys() .map(|uid| (*uid, OrderEventLabel::Traded)) .collect_vec(); self.database.store_order_events(&events).await; @@ -583,6 +576,17 @@ struct Solution { id: u64, account: H160, score: NonZeroU256, + orders: HashMap, +} + +impl Solution { + pub fn order_ids(&self) -> impl Iterator { + self.orders.keys() + } + + pub fn orders(&self) -> &HashMap { + &self.orders + } } #[derive(Debug, thiserror::Error)] diff --git a/crates/driver/src/domain/competition/mod.rs b/crates/driver/src/domain/competition/mod.rs index 46ef2cf941..18fd581bb3 100644 --- a/crates/driver/src/domain/competition/mod.rs +++ b/crates/driver/src/domain/competition/mod.rs @@ -15,7 +15,10 @@ use { }, futures::{stream::FuturesUnordered, Stream, StreamExt}, itertools::Itertools, - std::{collections::HashSet, sync::Mutex}, + std::{ + collections::{HashMap, HashSet}, + sync::Mutex, + }, tap::TapFallible, }; @@ -171,7 +174,15 @@ impl Competition { let (mut score, settlement) = scores .into_iter() .max_by_key(|(score, _)| score.to_owned()) - .map(|(score, settlement)| (Solved { score }, settlement)) + .map(|(score, settlement)| { + ( + Solved { + score, + trades: settlement.orders(), + }, + settlement, + ) + }) .unzip(); *self.settlement.lock().unwrap() = settlement.clone(); @@ -217,7 +228,6 @@ impl Competition { .cloned() .ok_or(Error::SolutionNotAvailable)?; Ok(Revealed { - orders: settlement.orders(), internalized_calldata: settlement .calldata( self.eth.contracts().settlement(), @@ -335,6 +345,13 @@ async fn merge_settlements( #[derive(Debug)] pub struct Solved { pub score: Score, + pub trades: HashMap, +} + +#[derive(Debug, Default)] +pub struct Amounts { + pub sell: eth::TokenAmount, + pub buy: eth::TokenAmount, } /// Winning solution information revealed to the protocol by the driver before @@ -342,8 +359,6 @@ pub struct Solved { /// point. #[derive(Debug)] pub struct Revealed { - /// The orders solved by this solution. - pub orders: HashSet, /// The internalized calldata is the final calldata that appears onchain. pub internalized_calldata: Bytes>, /// The uninternalized calldata must be known so that the CoW solver team diff --git a/crates/driver/src/domain/competition/solution/settlement.rs b/crates/driver/src/domain/competition/solution/settlement.rs index 9c46cd22ef..f3a04a136b 100644 --- a/crates/driver/src/domain/competition/solution/settlement.rs +++ b/crates/driver/src/domain/competition/solution/settlement.rs @@ -303,12 +303,28 @@ impl Settlement { self.boundary.solver } - /// The settled user orders. - pub fn orders(&self) -> HashSet { + /// The settled user orders with their in/out amounts. + pub fn orders(&self) -> HashMap { self.solutions .values() - .flat_map(|solution| solution.user_trades().map(|trade| trade.order().uid)) - .collect() + .fold(Default::default(), |mut acc, solution| { + for trade in solution.user_trades() { + let order = acc.entry(trade.order().uid).or_default(); + order.sell = trade.sell_amount(&solution.prices).unwrap_or_else(|| { + // This should never happen, returning 0 is better than panicking, but we + // should still alert. + tracing::error!(uid = ?trade.order().uid, "could not compute sell_amount"); + 0.into() + }); + order.buy = trade.buy_amount(&solution.prices).unwrap_or_else(|| { + // This should never happen, returning 0 is better than panicking, but we + // should still alert. + tracing::error!(uid = ?trade.order().uid, "could not compute buy_amount"); + 0.into() + }); + } + acc + }) } /// Settlements have valid notify ID only if they are originated from a diff --git a/crates/driver/src/domain/competition/solution/trade.rs b/crates/driver/src/domain/competition/solution/trade.rs index 3f653476f3..67038e6299 100644 --- a/crates/driver/src/domain/competition/solution/trade.rs +++ b/crates/driver/src/domain/competition/solution/trade.rs @@ -1,6 +1,9 @@ -use crate::domain::{ - competition::{self, order}, - eth, +use { + crate::domain::{ + competition::{self, order}, + eth, + }, + std::collections::HashMap, }; /// A trade which executes an order as part of this solution. @@ -84,6 +87,40 @@ impl Fulfillment { Fee::Dynamic(fee) => fee, } } + + /// The effective amount that left the user's wallet including all fees. + pub fn sell_amount( + &self, + prices: &HashMap, + ) -> Option { + let before_fee = match self.order.side { + order::Side::Sell => self.executed.0, + order::Side::Buy => self + .executed + .0 + .checked_mul(*prices.get(&self.order.buy.token)?)? + .checked_div(*prices.get(&self.order.sell.token)?)?, + }; + Some(eth::TokenAmount( + before_fee.checked_add(self.solver_fee().0)?, + )) + } + + /// The effective amount the user received after all fees. + pub fn buy_amount( + &self, + prices: &HashMap, + ) -> Option { + let amount = match self.order.side { + order::Side::Buy => self.executed.0, + order::Side::Sell => self + .executed + .0 + .checked_mul(*prices.get(&self.order.sell.token)?)? + .checked_div(*prices.get(&self.order.buy.token)?)?, + }; + Some(eth::TokenAmount(amount)) + } } /// A fee that is charged for executing an order. diff --git a/crates/driver/src/domain/eth/mod.rs b/crates/driver/src/domain/eth/mod.rs index f339be6a19..1800a035b7 100644 --- a/crates/driver/src/domain/eth/mod.rs +++ b/crates/driver/src/domain/eth/mod.rs @@ -192,7 +192,7 @@ impl TokenAddress { /// An ERC20 token amount. /// /// https://eips.ethereum.org/EIPS/eip-20 -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct TokenAmount(pub U256); impl From for TokenAmount { diff --git a/crates/driver/src/infra/api/routes/reveal/dto/revealed.rs b/crates/driver/src/infra/api/routes/reveal/dto/revealed.rs index 0a9bca788e..1f256320b4 100644 --- a/crates/driver/src/infra/api/routes/reveal/dto/revealed.rs +++ b/crates/driver/src/infra/api/routes/reveal/dto/revealed.rs @@ -1,8 +1,5 @@ use { - crate::{ - domain::{competition, competition::order}, - util::serialize, - }, + crate::{domain::competition, util::serialize}, serde::Serialize, serde_with::serde_as, }; @@ -10,7 +7,6 @@ use { impl Revealed { pub fn new(reveal: competition::Revealed) -> Self { Self { - orders: reveal.orders.into_iter().map(Into::into).collect(), calldata: Calldata { internalized: reveal.internalized_calldata.into(), uninternalized: reveal.uninternalized_calldata.into(), @@ -23,8 +19,6 @@ impl Revealed { #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] pub struct Revealed { - #[serde_as(as = "Vec")] - orders: Vec<[u8; order::UID_LEN]>, calldata: Calldata, } diff --git a/crates/driver/src/infra/api/routes/solve/dto/solved.rs b/crates/driver/src/infra/api/routes/solve/dto/solved.rs index c765518baa..ef495c5093 100644 --- a/crates/driver/src/infra/api/routes/solve/dto/solved.rs +++ b/crates/driver/src/infra/api/routes/solve/dto/solved.rs @@ -1,11 +1,12 @@ use { crate::{ - domain::{competition, eth}, + domain::{competition, competition::order, eth}, infra::Solver, util::serialize, }, serde::Serialize, serde_with::serde_as, + std::collections::HashMap, }; impl Solved { @@ -31,10 +32,37 @@ impl Solution { solution_id, score: solved.score.0.get(), submission_address: solver.address().into(), + orders: solved + .trades + .into_iter() + .map(|(order_id, amounts)| { + ( + order_id.into(), + TradedAmounts { + sell_amount: amounts.sell.into(), + buy_amount: amounts.buy.into(), + }, + ) + }) + .collect(), } } } +#[serde_as] +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct TradedAmounts { + /// The effective amount that left the user's wallet including all fees. + #[serde_as(as = "serialize::U256")] + pub sell_amount: eth::U256, + /// The effective amount the user received after all fees. + #[serde_as(as = "serialize::U256")] + pub buy_amount: eth::U256, +} + +type OrderId = [u8; order::UID_LEN]; + #[serde_as] #[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] @@ -46,4 +74,6 @@ pub struct Solution { #[serde_as(as = "serialize::U256")] score: eth::U256, submission_address: eth::H160, + #[serde_as(as = "HashMap")] + orders: HashMap, } diff --git a/crates/driver/src/tests/cases/merge_settlements.rs b/crates/driver/src/tests/cases/merge_settlements.rs index 1430cbe2c6..f151f8e522 100644 --- a/crates/driver/src/tests/cases/merge_settlements.rs +++ b/crates/driver/src/tests/cases/merge_settlements.rs @@ -17,11 +17,11 @@ async fn possible() { .done() .await; - test.solve().await.ok(); - test.reveal() + test.solve() .await .ok() .orders(&[ab_order().name, cd_order().name]); + test.reveal().await.ok().calldata(); test.settle() .await // Even though the solver returned two solutions, the executed settlement is a @@ -54,7 +54,7 @@ async fn impossible() { // Only the first A-B order gets settled. - test.solve().await.ok(); - test.reveal().await.ok().orders(&[ab_order().name]); + test.solve().await.ok().orders(&[ab_order().name]); + test.reveal().await.ok().calldata(); test.settle().await.ok().await.ab_order_executed().await; } diff --git a/crates/driver/src/tests/cases/multiple_solutions.rs b/crates/driver/src/tests/cases/multiple_solutions.rs index 1875980bfa..dfac3c90d6 100644 --- a/crates/driver/src/tests/cases/multiple_solutions.rs +++ b/crates/driver/src/tests/cases/multiple_solutions.rs @@ -16,8 +16,12 @@ async fn valid() { .done() .await; - test.solve().await.ok().default_score(); - test.reveal().await.ok().orders(&[ab_order().name]); + test.solve() + .await + .ok() + .default_score() + .orders(&[ab_order().name]); + test.reveal().await.ok().calldata(); } /// Test that the invalid solution is discarded when the /solve endpoint @@ -33,6 +37,10 @@ async fn invalid() { .done() .await; - test.solve().await.ok().default_score(); - test.reveal().await.ok().orders(&[ab_order().name]); + test.solve() + .await + .ok() + .default_score() + .orders(&[ab_order().name]); + test.reveal().await.ok().calldata(); } diff --git a/crates/driver/src/tests/cases/negative_scores.rs b/crates/driver/src/tests/cases/negative_scores.rs index 06caea9ec2..d34c6c8c8d 100644 --- a/crates/driver/src/tests/cases/negative_scores.rs +++ b/crates/driver/src/tests/cases/negative_scores.rs @@ -36,6 +36,10 @@ async fn one_valid_solution() { }) .done() .await; - test.solve().await.ok().default_score(); - test.reveal().await.ok().orders(&[ab_order().name]); + test.solve() + .await + .ok() + .default_score() + .orders(&[ab_order().name]); + test.reveal().await.ok().calldata(); } diff --git a/crates/driver/src/tests/cases/score_competition.rs b/crates/driver/src/tests/cases/score_competition.rs index dbee014190..6ce2c66c2d 100644 --- a/crates/driver/src/tests/cases/score_competition.rs +++ b/crates/driver/src/tests/cases/score_competition.rs @@ -16,11 +16,10 @@ async fn solver_score_winner() { .done() .await; - assert_eq!( - test.solve().await.ok().score(), - 2902421280589416499u128.into() - ); - test.reveal().await.ok().orders(&[ab_order().name]); + let solve = test.solve().await.ok(); + assert_eq!(solve.score(), 2902421280589416499u128.into()); + solve.orders(&[ab_order().name]); + test.reveal().await.ok().calldata(); } #[tokio::test] @@ -36,6 +35,8 @@ async fn risk_adjusted_score_winner() { .done() .await; - assert!(test.solve().await.ok().score() != DEFAULT_SCORE_MIN.into()); - test.reveal().await.ok().orders(&[ab_order().name]); + let solve = test.solve().await.ok(); + assert!(solve.score() != DEFAULT_SCORE_MIN.into()); + solve.orders(&[ab_order().name]); + test.reveal().await.ok().calldata(); } diff --git a/crates/driver/src/tests/setup/mod.rs b/crates/driver/src/tests/setup/mod.rs index 09eef9c4d7..6078a396c3 100644 --- a/crates/driver/src/tests/setup/mod.rs +++ b/crates/driver/src/tests/setup/mod.rs @@ -624,7 +624,12 @@ impl Test { let status = res.status(); let body = res.text().await.unwrap(); tracing::debug!(?status, ?body, "got a response from /solve"); - Solve { status, body } + Solve { + status, + body, + fulfillments: &self.fulfillments, + blockchain: &self.blockchain, + } } /// Call the /reveal endpoint. @@ -643,12 +648,7 @@ impl Test { let status = res.status(); let body = res.text().await.unwrap(); tracing::debug!(?status, ?body, "got a response from /reveal"); - Reveal { - status, - body, - fulfillments: &self.fulfillments, - blockchain: &self.blockchain, - } + Reveal { status, body } } /// Call the /quote endpoint. @@ -745,24 +745,32 @@ impl Test { } /// A /solve response. -pub struct Solve { +pub struct Solve<'a> { status: StatusCode, body: String, + fulfillments: &'a [Fulfillment], + blockchain: &'a Blockchain, } -pub struct SolveOk { +pub struct SolveOk<'a> { body: String, + fulfillments: &'a [Fulfillment], + blockchain: &'a Blockchain, } -impl Solve { +impl<'a> Solve<'a> { /// Expect the /solve endpoint to have returned a 200 OK response. - pub fn ok(self) -> SolveOk { + pub fn ok(self) -> SolveOk<'a> { assert_eq!(self.status, hyper::StatusCode::OK); - SolveOk { body: self.body } + SolveOk { + body: self.body, + fulfillments: self.fulfillments, + blockchain: self.blockchain, + } } } -impl SolveOk { +impl<'a> SolveOk<'a> { fn solutions(&self) -> Vec { #[derive(serde::Deserialize)] struct Body { @@ -771,14 +779,22 @@ impl SolveOk { serde_json::from_str::(&self.body).unwrap().solutions } - /// Extracts the score from the response. Since response can contain - /// multiple solutions, it takes the score from the first solution. - pub fn score(&self) -> eth::U256 { + /// Extracts the first solution from the response. This is expected to be + /// always valid if there is a valid solution, as we expect from driver to + /// not send multiple solutions (yet). + fn solution(&self) -> serde_json::Value { let solutions = self.solutions(); assert_eq!(solutions.len(), 1); let solution = solutions[0].clone(); assert!(solution.is_object()); - assert_eq!(solution.as_object().unwrap().len(), 3); + assert_eq!(solution.as_object().unwrap().len(), 4); + solution + } + + /// Extracts the score from the response. Since response can contain + /// multiple solutions, it takes the score from the first solution. + pub fn score(&self) -> eth::U256 { + let solution = self.solution(); assert!(solution.get("score").is_some()); let score = solution.get("score").unwrap().as_str().unwrap(); eth::U256::from_dec_str(score).unwrap() @@ -804,35 +820,7 @@ impl SolveOk { pub fn empty(self) { assert!(self.solutions().is_empty()); } -} - -/// A /reveal response. -pub struct Reveal<'a> { - status: StatusCode, - body: String, - fulfillments: &'a [Fulfillment], - blockchain: &'a Blockchain, -} -impl<'a> Reveal<'a> { - /// Expect the /reveal endpoint to have returned a 200 OK response. - pub fn ok(self) -> RevealOk<'a> { - assert_eq!(self.status, hyper::StatusCode::OK); - RevealOk { - body: self.body, - fulfillments: self.fulfillments, - blockchain: self.blockchain, - } - } -} - -pub struct RevealOk<'a> { - body: String, - fulfillments: &'a [Fulfillment], - blockchain: &'a Blockchain, -} - -impl RevealOk<'_> { /// Check that the solution contains the expected orders. pub fn orders(self, order_names: &[&str]) -> Self { let expected_order_uids = order_names @@ -853,20 +841,58 @@ impl RevealOk<'_> { }) .sorted() .collect_vec(); + let solution = self.solution(); + assert!(solution.get("orders").is_some()); + let order_uids = serde_json::from_value::>( + solution.get("orders").unwrap().clone(), + ) + .unwrap() + .keys() + .cloned() + .sorted() + .collect_vec(); + assert_eq!(order_uids, expected_order_uids); + self + } +} + +/// A /reveal response. +pub struct Reveal { + status: StatusCode, + body: String, +} + +impl Reveal { + /// Expect the /reveal endpoint to have returned a 200 OK response. + pub fn ok(self) -> RevealOk { + assert_eq!(self.status, hyper::StatusCode::OK); + RevealOk { body: self.body } + } +} + +pub struct RevealOk { + body: String, +} + +impl RevealOk { + pub fn calldata(self) -> Self { let result: serde_json::Value = serde_json::from_str(&self.body).unwrap(); assert!(result.is_object()); - assert_eq!(result.as_object().unwrap().len(), 2); - assert!(result.get("orders").is_some()); - let order_uids = result - .get("orders") + assert_eq!(result.as_object().unwrap().len(), 1); + let calldata = result.get("calldata").unwrap().as_object().unwrap(); + assert_eq!(calldata.len(), 2); + assert!(!calldata + .get("internalized") .unwrap() - .as_array() + .as_str() .unwrap() - .iter() - .map(|order| order.as_str().unwrap().to_owned()) - .sorted() - .collect_vec(); - assert_eq!(order_uids, expected_order_uids); + .is_empty()); + assert!(!calldata + .get("uninternalized") + .unwrap() + .as_str() + .unwrap() + .is_empty()); self } } diff --git a/crates/e2e/tests/e2e/partially_fillable_observed_score.rs b/crates/e2e/tests/e2e/partially_fillable_observed_score.rs index 790fc8e6a6..3357cf6b7c 100644 --- a/crates/e2e/tests/e2e/partially_fillable_observed_score.rs +++ b/crates/e2e/tests/e2e/partially_fillable_observed_score.rs @@ -5,6 +5,7 @@ use { model::{ order::{OrderClass, OrderCreation, OrderKind}, signature::EcdsaSigningScheme, + solver_competition, }, secp256k1::SecretKey, shared::ethrpc::Web3, @@ -164,10 +165,11 @@ async fn test(web3: Web3) { assert!(solution_1.objective.fees > 0.); assert_ne!(solution_0.objective.fees, solution_1.objective.fees); - assert!(solution_0.orders[0].executed_amount > 0.into()); - assert!(solution_1.orders[0].executed_amount > 0.into()); - assert_ne!( - solution_0.orders[0].executed_amount, - solution_1.orders[0].executed_amount + assert!( + matches!(solution_0.orders[0], solver_competition::Order::Legacy{ executed_amount, ..} if executed_amount > 0.into()) ); + assert!( + matches!(solution_1.orders[0], solver_competition::Order::Legacy{ executed_amount, ..} if executed_amount > 0.into()) + ); + assert_ne!(solution_0.orders[0], solution_1.orders[0]); } diff --git a/crates/model/src/solver_competition.rs b/crates/model/src/solver_competition.rs index 9ea9f59a4d..cc45a35a03 100644 --- a/crates/model/src/solver_competition.rs +++ b/crates/model/src/solver_competition.rs @@ -155,12 +155,25 @@ impl Score { } #[serde_as] -#[derive(Clone, Debug, Default, Deserialize, Serialize, Eq, PartialEq)] -#[serde(rename_all = "camelCase")] -pub struct Order { - pub id: OrderUid, - #[serde_as(as = "HexOrDecimalU256")] - pub executed_amount: U256, +#[derive(Clone, Debug, Deserialize, Serialize, Eq, PartialEq)] +#[serde(untagged)] +pub enum Order { + #[serde(rename_all = "camelCase")] + Colocated { + id: OrderUid, + /// The effective amount that left the user's wallet including all fees. + #[serde_as(as = "HexOrDecimalU256")] + sell_amount: U256, + /// The effective amount the user received after all fees. + #[serde_as(as = "HexOrDecimalU256")] + buy_amount: U256, + }, + #[serde(rename_all = "camelCase")] + Legacy { + id: OrderUid, + #[serde_as(as = "HexOrDecimalU256")] + executed_amount: U256, + }, } #[cfg(test)] @@ -215,7 +228,12 @@ mod tests { "id": "0x3333333333333333333333333333333333333333333333333333333333333333\ 3333333333333333333333333333333333333333\ 33333333", - "executedAmount": "12", + "sellAmount": "12", + "buyAmount": "13", + }, + { + "id": "0x4444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444444", + "executedAmount": "14", } ], "callData": "0x13", @@ -259,10 +277,17 @@ mod tests { clearing_prices: btreemap! { H160([0x22; 20]) => 8.into(), }, - orders: vec![Order { - id: OrderUid([0x33; 56]), - executed_amount: 12.into(), - }], + orders: vec![ + Order::Colocated { + id: OrderUid([0x33; 56]), + sell_amount: 12.into(), + buy_amount: 13.into(), + }, + Order::Legacy { + id: OrderUid([0x44; 56]), + executed_amount: 14.into(), + }, + ], call_data: vec![0x13], uninternalized_call_data: Some(vec![0x13, 0x14]), }], diff --git a/crates/orderbook/src/database/solver_competition.rs b/crates/orderbook/src/database/solver_competition.rs index ca1f541e5d..1207d8f10d 100644 --- a/crates/orderbook/src/database/solver_competition.rs +++ b/crates/orderbook/src/database/solver_competition.rs @@ -184,7 +184,7 @@ mod tests { score: Default::default(), ranking: Some(1), clearing_prices: [Default::default()].into_iter().collect(), - orders: vec![Default::default()], + orders: vec![], call_data: vec![1, 2], uninternalized_call_data: Some(vec![1, 2, 3, 4]), }], diff --git a/crates/solver/src/driver.rs b/crates/solver/src/driver.rs index b65c6544c3..adb871020e 100644 --- a/crates/solver/src/driver.rs +++ b/crates/solver/src/driver.rs @@ -386,7 +386,7 @@ impl Driver { orders: rated_settlement .settlement .trades() - .map(|trade| solver_competition::Order { + .map(|trade| solver_competition::Order::Legacy { id: trade.order.metadata.uid, executed_amount: trade.executed_amount, })