diff --git a/crates/driver/src/domain/competition/auction.rs b/crates/driver/src/domain/competition/auction.rs index e97717bf00..2d205a32e7 100644 --- a/crates/driver/src/domain/competition/auction.rs +++ b/crates/driver/src/domain/competition/auction.rs @@ -1,8 +1,19 @@ use { - crate::domain::{ - competition::{self, solution}, - eth, + super::order, + crate::{ + domain::{ + competition::{self, solution}, + eth::{self}, + }, + infra::{ + blockchain, + observe::{self}, + Ethereum, + }, }, + futures::future::join_all, + itertools::Itertools, + primitive_types::U256, std::collections::HashMap, thiserror::Error, }; @@ -23,26 +34,53 @@ pub struct Auction { } impl Auction { - pub fn new( + pub async fn new( id: Option, - mut orders: Vec, + orders: Vec, tokens: impl Iterator, - gas_price: eth::GasPrice, deadline: Deadline, - weth: eth::WethAddress, - ) -> Result { + eth: &Ethereum, + ) -> Result { let tokens = Tokens(tokens.map(|token| (token.address, token)).collect()); // Ensure that tokens are included for each order. + let weth = eth.contracts().weth_address(); if !orders.iter().all(|order| { tokens.0.contains_key(&order.buy.token.wrap(weth)) && tokens.0.contains_key(&order.sell.token.wrap(weth)) }) { - return Err(InvalidTokens); + return Err(Error::InvalidTokens); } - // Sort orders such that most likely to be fulfilled come first. - orders.sort_by_key(|order| { + Ok(Self { + id, + orders, + tokens, + gas_price: eth.gas_price().await?, + deadline, + }) + } + + /// [`None`] if this auction applies to a quote. See + /// [`crate::domain::quote`]. + pub fn id(&self) -> Option { + self.id + } + + /// The orders for the auction. + pub fn orders(&self) -> &[competition::Order] { + &self.orders + } + + /// Prioritize the orders such that those which are more likely to be + /// fulfilled come before less likely orders. Filter out orders which + /// the trader doesn't have enough balance to pay for. + /// + /// Prioritization is skipped during quoting. It's only used during + /// competition. + pub async fn prioritize(mut self, eth: &Ethereum) -> Self { + // Sort orders so that most likely to be fulfilled come first. + self.orders.sort_by_key(|order| { // Market orders are preferred over limit orders, as the expectation is that // they should be immediately fulfillable. Liquidity orders come last, as they // are the most niche and rarely used. @@ -55,31 +93,112 @@ impl Auction { class, // If the orders are of the same kind, then sort by likelihood of fulfillment // based on token prices. - order.likelihood(&tokens), + order.likelihood(&self.tokens), )) }); - // TODO Filter out orders based on user balance + // Fetch balances of each token for each trader. + // Has to be separate closure due to compiler bug. + let f = |order: &competition::Order| -> (order::Trader, eth::TokenAddress) { + (order.trader(), order.sell.token) + }; + let tokens_by_trader = self.orders.iter().map(f).unique(); + let mut balances: HashMap< + (order::Trader, eth::TokenAddress), + Result, + > = join_all(tokens_by_trader.map(|(trader, token)| async move { + let contract = eth.erc20(token); + let balance = contract.balance(trader.into()).await; + ((trader, token), balance) + })) + .await + .into_iter() + .collect(); + + self.orders.retain(|order| { + // TODO: We should use balance fetching that takes interactions into account + // from `crates/shared/src/account_balances/simulation.rs` instead of hardcoding + // an Ethflow exception. https://github.com/cowprotocol/services/issues/1595 + if Some(order.signature.signer.0) == eth.contracts().ethflow_address().map(|a| a.0) { + return true; + } + + let remaining_balance = match balances + .get_mut(&(order.trader(), order.sell.token)) + .unwrap() + { + Ok(balance) => &mut balance.0, + Err(err) => { + let reason = observe::OrderExcludedFromAuctionReason::CouldNotFetchBalance(err); + observe::order_excluded_from_auction(order, reason); + return false; + } + }; - Ok(Self { - id, - orders, - tokens, - gas_price, - deadline, - }) - } + fn max_fill(order: &competition::Order) -> anyhow::Result { + use { + anyhow::Context, + shared::remaining_amounts::{Order as RemainingOrder, Remaining}, + }; + + let remaining = Remaining::from_order(&RemainingOrder { + kind: match order.side { + order::Side::Buy => model::order::OrderKind::Buy, + order::Side::Sell => model::order::OrderKind::Sell, + }, + buy_amount: order.buy.amount.0, + sell_amount: order.sell.amount.0, + fee_amount: order.fee.user.0, + executed_amount: match order.partial { + order::Partial::Yes { executed } => executed.0, + order::Partial::No => 0.into(), + }, + partially_fillable: match order.partial { + order::Partial::Yes { .. } => true, + order::Partial::No => false, + }, + }) + .context("Remaining::from_order")?; + let sell = remaining + .remaining(order.sell.amount.0) + .context("remaining_sell")?; + let fee = remaining + .remaining(order.fee.user.0) + .context("remaining_fee")?; + sell.checked_add(fee).context("add sell and fee") + } + + let max_fill = match max_fill(order) { + Ok(balance) => balance, + Err(err) => { + let reason = + observe::OrderExcludedFromAuctionReason::CouldNotCalculateRemainingAmount( + &err, + ); + observe::order_excluded_from_auction(order, reason); + return false; + } + }; - /// [`None`] if this auction applies to a quote. See - /// [`crate::domain::quote`]. - pub fn id(&self) -> Option { - self.id - } + let used_balance = match order.is_partial() { + true => { + if *remaining_balance == 0.into() { + return false; + } + max_fill.min(*remaining_balance) + } + false => { + if *remaining_balance < max_fill { + return false; + } + max_fill + } + }; + *remaining_balance -= used_balance; + true + }); - /// The orders for the auction. The orders are sorted such that those which - /// are more likely to be fulfilled come before less likely orders. - pub fn orders(&self) -> &[competition::Order] { - &self.orders + self } /// The tokens used in the auction. @@ -224,10 +343,14 @@ pub struct DeadlineExceeded; #[error("invalid auction id")] pub struct InvalidId; -#[derive(Debug, Error)] -#[error("invalid auction tokens")] -pub struct InvalidTokens; - #[derive(Debug, Error)] #[error("price cannot be zero")] pub struct InvalidPrice; + +#[derive(Debug, Error)] +pub enum Error { + #[error("invalid auction tokens")] + InvalidTokens, + #[error("blockchain error: {0:?}")] + Blockchain(#[from] blockchain::Error), +} diff --git a/crates/driver/src/domain/competition/order/mod.rs b/crates/driver/src/domain/competition/order/mod.rs index e21cc01ee6..f5ed51712a 100644 --- a/crates/driver/src/domain/competition/order/mod.rs +++ b/crates/driver/src/domain/competition/order/mod.rs @@ -103,6 +103,10 @@ impl Order { } } + pub fn trader(&self) -> Trader { + Trader(self.signature.signer) + } + pub fn is_partial(&self) -> bool { matches!(self.partial, Partial::Yes { .. }) } @@ -285,6 +289,16 @@ pub enum BuyTokenBalance { Internal, } +/// The address which placed the order. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct Trader(eth::Address); + +impl From for eth::Address { + fn from(value: Trader) -> Self { + value.0 + } +} + /// A just-in-time order. JIT orders are added at solving time by the solver to /// generate a more optimal solution for the auction. Very similar to a regular /// [`Order`]. diff --git a/crates/driver/src/domain/quote.rs b/crates/driver/src/domain/quote.rs index 282f550082..f69d5e7cb0 100644 --- a/crates/driver/src/domain/quote.rs +++ b/crates/driver/src/domain/quote.rs @@ -75,12 +75,10 @@ impl Order { tokens: &infra::tokens::Fetcher, ) -> Result { let liquidity = liquidity.fetch(&self.liquidity_pairs()).await; - let gas_price = eth.gas_price().await?; let timeout = self.deadline.timeout()?; - let fake_auction = self - .fake_auction(gas_price, eth.contracts().weth_address(), tokens) - .await; - let solutions = solver.solve(&fake_auction, &liquidity, timeout).await?; + let solutions = solver + .solve(&self.fake_auction(eth, tokens).await?, &liquidity, timeout) + .await?; Quote::new( eth, self, @@ -95,10 +93,9 @@ impl Order { async fn fake_auction( &self, - gas_price: eth::GasPrice, - weth: eth::WethAddress, + eth: &Ethereum, tokens: &infra::tokens::Fetcher, - ) -> competition::Auction { + ) -> Result { let tokens = tokens.get(&[self.buy().token, self.sell().token]).await; let buy_token_metadata = tokens.get(&self.buy().token); @@ -146,11 +143,14 @@ impl Order { }, ] .into_iter(), - gas_price.effective().into(), Default::default(), - weth, + eth, ) - .unwrap() + .await + .map_err(|err| match err { + auction::Error::InvalidTokens => panic!("fake auction with invalid tokens"), + auction::Error::Blockchain(e) => e.into(), + }) } /// The asset being bought, or [`eth::U256::one`] if this is a sell, to diff --git a/crates/driver/src/infra/api/error.rs b/crates/driver/src/infra/api/error.rs index 7ce098986a..3e684e60e5 100644 --- a/crates/driver/src/infra/api/error.rs +++ b/crates/driver/src/infra/api/error.rs @@ -84,7 +84,7 @@ impl From for (hyper::StatusCode, axum::Json) api::routes::AuctionError::InvalidAuctionId => Kind::InvalidAuctionId, api::routes::AuctionError::MissingSurplusFee => Kind::MissingSurplusFee, api::routes::AuctionError::InvalidTokens => Kind::InvalidTokens, - api::routes::AuctionError::GasPrice(_) => Kind::Unknown, + api::routes::AuctionError::Blockchain(_) => Kind::Unknown, }; error.into() } diff --git a/crates/driver/src/infra/api/routes/solve/dto/auction.rs b/crates/driver/src/infra/api/routes/solve/dto/auction.rs index 40e8a48ef1..6fb1793e83 100644 --- a/crates/driver/src/infra/api/routes/solve/dto/auction.rs +++ b/crates/driver/src/infra/api/routes/solve/dto/auction.rs @@ -131,10 +131,10 @@ impl Auction { trusted: token.trusted, } }), - eth.gas_price().await.map_err(Error::GasPrice)?, self.deadline.into(), - eth.contracts().weth_address(), + eth, ) + .await .map_err(Into::into) } } @@ -147,8 +147,8 @@ pub enum Error { MissingSurplusFee, #[error("invalid tokens in auction")] InvalidTokens, - #[error("error getting gas price")] - GasPrice(#[source] crate::infra::blockchain::Error), + #[error("blockchain error: {0:?}")] + Blockchain(#[source] crate::infra::blockchain::Error), } impl From for Error { @@ -157,9 +157,12 @@ impl From for Error { } } -impl From for Error { - fn from(_value: auction::InvalidTokens) -> Self { - Self::InvalidTokens +impl From for Error { + fn from(value: auction::Error) -> Self { + match value { + auction::Error::InvalidTokens => Self::InvalidTokens, + auction::Error::Blockchain(err) => Self::Blockchain(err), + } } } diff --git a/crates/driver/src/infra/api/routes/solve/mod.rs b/crates/driver/src/infra/api/routes/solve/mod.rs index a77fdfa371..57439e15c4 100644 --- a/crates/driver/src/infra/api/routes/solve/mod.rs +++ b/crates/driver/src/infra/api/routes/solve/mod.rs @@ -27,6 +27,7 @@ async fn route( .tap_err(|err| { observe::invalid_dto(err, "auction"); })?; + let auction = auction.prioritize(state.eth()).await; observe::auction(&auction); let competition = state.competition(); let result = competition.solve(&auction).await; diff --git a/crates/driver/src/infra/blockchain/contracts.rs b/crates/driver/src/infra/blockchain/contracts.rs index 9b9146effe..0a9b449e6f 100644 --- a/crates/driver/src/infra/blockchain/contracts.rs +++ b/crates/driver/src/infra/blockchain/contracts.rs @@ -9,12 +9,14 @@ pub use crate::boundary::contracts::{GPv2Settlement, IUniswapLikeRouter, ERC20, pub struct Contracts { settlement: contracts::GPv2Settlement, weth: contracts::WETH9, + ethflow: Option, } #[derive(Debug, Default, Clone, Copy)] pub struct Addresses { pub settlement: Option, pub weth: Option, + pub ethflow: Option, } impl Contracts { @@ -32,7 +34,16 @@ impl Contracts { .unwrap() .into(); let weth = contracts::WETH9::at(web3, address); - Self { settlement, weth } + + // Not doing deployment information because there are separate Ethflow contracts + // for staging and production. + let ethflow = addresses.ethflow; + + Self { + settlement, + weth, + ethflow, + } } pub fn settlement(&self) -> &contracts::GPv2Settlement { @@ -46,6 +57,10 @@ impl Contracts { pub fn weth_address(&self) -> eth::WethAddress { self.weth.address().into() } + + pub fn ethflow_address(&self) -> Option { + self.ethflow + } } /// Returns the address of a contract for the specified network, or `None` if diff --git a/crates/driver/src/infra/config/file/load.rs b/crates/driver/src/infra/config/file/load.rs index 814fb3593e..599e32c201 100644 --- a/crates/driver/src/infra/config/file/load.rs +++ b/crates/driver/src/infra/config/file/load.rs @@ -235,6 +235,7 @@ pub async fn load(network: &blockchain::Network, path: &Path) -> infra::Config { contracts: blockchain::contracts::Addresses { settlement: config.contracts.gp_v2_settlement.map(Into::into), weth: config.contracts.weth.map(Into::into), + ethflow: config.contracts.ethflow.map(Into::into), }, disable_access_list_simulation: config.disable_access_list_simulation, disable_gas_simulation: config.disable_gas_simulation.map(Into::into), diff --git a/crates/driver/src/infra/config/file/mod.rs b/crates/driver/src/infra/config/file/mod.rs index 3ecae2ac70..d4afc47084 100644 --- a/crates/driver/src/infra/config/file/mod.rs +++ b/crates/driver/src/infra/config/file/mod.rs @@ -184,6 +184,11 @@ struct ContractsConfig { /// Override the default address of the WETH contract. weth: Option, + + /// Sets the Ethflow contract address. Without this we cannot detect Ethflow + /// orders, which leads to such orders not being solved because it appears + /// that the user doesn't have enough sell token balance. + ethflow: Option, } #[derive(Debug, Deserialize)] diff --git a/crates/driver/src/infra/observe/mod.rs b/crates/driver/src/infra/observe/mod.rs index 211b2b2b34..6abea639aa 100644 --- a/crates/driver/src/infra/observe/mod.rs +++ b/crates/driver/src/infra/observe/mod.rs @@ -15,7 +15,7 @@ use { Solution, Solved, }, - eth, + eth::{self}, quote::{self, Quote}, Liquidity, }, @@ -183,8 +183,8 @@ pub fn settled(solver: &solver::Name, result: &Result) { match result { - Ok(reveal) => { - tracing::info!(?reveal, "solved auction"); + Ok(solved) => { + tracing::info!(?solved, "solved auction"); metrics::get() .solutions .with_label_values(&[solver.as_str(), "Success"]) @@ -313,3 +313,16 @@ fn competition_error(err: &competition::Error) -> &'static str { competition::Error::Solver(solver::Error::Dto(_)) => "SolverDtoError", } } + +#[derive(Debug)] +pub enum OrderExcludedFromAuctionReason<'a> { + CouldNotFetchBalance(&'a crate::infra::blockchain::Error), + CouldNotCalculateRemainingAmount(&'a anyhow::Error), +} + +pub fn order_excluded_from_auction( + order: &competition::Order, + reason: OrderExcludedFromAuctionReason, +) { + tracing::trace!(uid=?order.uid, ?reason,"order excluded from auction"); +} diff --git a/crates/driver/src/tests/cases/mod.rs b/crates/driver/src/tests/cases/mod.rs index c5034a356c..97e3718059 100644 --- a/crates/driver/src/tests/cases/mod.rs +++ b/crates/driver/src/tests/cases/mod.rs @@ -7,7 +7,7 @@ pub mod internalization; pub mod merge_settlements; pub mod multiple_solutions; pub mod negative_scores; -pub mod order_sorting; +pub mod order_prioritization; pub mod quote; pub mod risk; pub mod settle; diff --git a/crates/driver/src/tests/cases/order_prioritization.rs b/crates/driver/src/tests/cases/order_prioritization.rs new file mode 100644 index 0000000000..09857a2bbb --- /dev/null +++ b/crates/driver/src/tests/cases/order_prioritization.rs @@ -0,0 +1,75 @@ +use crate::tests::setup::{ab_order, ab_pool, ab_solution, setup, Order}; + +/// Test that orders are sorted correctly before being sent to the solver: +/// market orders come before limit orders, and orders that are more likely to +/// fulfill come before orders that are less likely (according to token prices +/// in ETH). +#[tokio::test] +#[ignore] +async fn sorting() { + let test = setup() + .pool(ab_pool()) + // Orders with better price ratios come first. + .order( + ab_order() + .reduce_amount(1000000000000000u128.into()), + ) + .order(ab_order().rename("second order")) + // Limit orders come after market orders. + .order( + ab_order() + .rename("third order") + .limit() + .reduce_amount(1000000000000000u128.into()), + ) + .order(ab_order().rename("fourth order").limit()) + .solution(ab_solution()) + .done() + .await; + + // Only check that the solve endpoint can be called successfully, which means + // that the solver received the orders sorted. + test.solve().await.ok(); +} + +/// If a user does not have enough tokens to settle all their orders filter out +/// the least likely to settle ones that go over the user's budget. +#[tokio::test] +#[ignore] +async fn filtering() { + let test = setup() + .pool(ab_pool()) + // Orders with better price ratios come first. + .order( + ab_order() + .reduce_amount(1000000000000000u128.into()), + ) + .order(ab_order().rename("second order")) + // Filter out the next order, because the trader doesn't have enough balance to cover it. + .order( + ab_order() + .rename("third order") + .multiply_amount(100000000000000000u128.into()) + .filtered() + ) + // Filter out the next order. It can't be fulfilled due to the balance that is required to + // fulfill the previous orders. + .order( + Order { + sell_amount: 4999899999900002000000000000000u128.into(), + surplus_factor: 1.into(), + ..ab_order() + } + .rename("fourth order") + .unfunded() + .filtered() + .limit() + ) + .solution(ab_solution()) + .done() + .await; + + // Only check that the solve endpoint can be called successfully, which means + // that the solver received the orders sorted. + test.solve().await.ok(); +} diff --git a/crates/driver/src/tests/cases/order_sorting.rs b/crates/driver/src/tests/cases/order_sorting.rs deleted file mode 100644 index 813ddf0fd9..0000000000 --- a/crates/driver/src/tests/cases/order_sorting.rs +++ /dev/null @@ -1,36 +0,0 @@ -use crate::tests::{ - setup, - setup::{ab_order, ab_pool, ab_solution}, -}; - -/// Test that orders are sorted correctly before being sent to the solver: -/// market orders come before limit orders, and orders that are more likely to -/// fulfill come before orders that are less likely (according to token prices -/// in ETH). -#[tokio::test] -#[ignore] -async fn test() { - let test = setup() - .pool(ab_pool()) - // Orders with better price ratios come first. - .order( - ab_order() - .reduce_amount(1000000000000000u128.into()), - ) - .order(ab_order().rename("second order")) - // Limit orders come after market orders. - .order( - ab_order() - .rename("third order") - .limit() - .reduce_amount(1000000000000000u128.into()), - ) - .order(ab_order().rename("fourth order").limit()) - .solution(ab_solution()) - .done() - .await; - - // Only check that the solve endpoint can be called successfully, which means - // that the solver received the orders sorted. - test.solve().await.ok(); -} diff --git a/crates/driver/src/tests/setup/blockchain.rs b/crates/driver/src/tests/setup/blockchain.rs index cb15f1b5b9..7f2cf04135 100644 --- a/crates/driver/src/tests/setup/blockchain.rs +++ b/crates/driver/src/tests/setup/blockchain.rs @@ -1,7 +1,10 @@ use { super::{Asset, Order}, crate::{ - domain::{competition::order, eth}, + domain::{ + competition::order, + eth::{self, ContractAddress}, + }, infra::time, tests::{self, boundary}, }, @@ -38,6 +41,7 @@ pub struct Blockchain { pub tokens: HashMap<&'static str, contracts::ERC20Mintable>, pub weth: contracts::WETH9, pub settlement: contracts::GPv2Settlement, + pub ethflow: Option, pub domain_separator: boundary::DomainSeparator, pub geth: Geth, pub pairs: Vec, @@ -472,6 +476,7 @@ impl Blockchain { settlement, domain_separator, weth, + ethflow: None, web3, web3_url: geth.url(), geth, @@ -545,7 +550,7 @@ impl Blockchain { ); if order.sell_token == "WETH" { todo!("deposit trader funds into the weth contract, none of the tests do this yet") - } else { + } else if order.funded { wait_for( &self.web3, self.tokens @@ -553,7 +558,7 @@ impl Blockchain { .unwrap() .mint( self.trader_address, - eth::U256::from(2) * quote.sell + order.user_fee, + eth::U256::from(100000000000u64) * quote.sell + order.user_fee, ) .from(trader_account.clone()) .send(), diff --git a/crates/driver/src/tests/setup/mod.rs b/crates/driver/src/tests/setup/mod.rs index 7594d8c2ce..20b1dbd94c 100644 --- a/crates/driver/src/tests/setup/mod.rs +++ b/crates/driver/src/tests/setup/mod.rs @@ -105,6 +105,12 @@ pub struct Order { /// Override the executed amount of the order. Useful for testing liquidity /// orders. Otherwise [`execution_diff`] is probably more suitable. pub executed: Option, + + /// Should this order be filtered out before being sent to the solver? + pub filtered: bool, + /// Should the trader account be funded with enough tokens to place this + /// order? True by default. + pub funded: bool, } impl Order { @@ -113,7 +119,7 @@ impl Order { Self { name, ..self } } - /// Reduce the amount of this order by the given amount. + /// Reduce the sell amount of this order by the given amount. pub fn reduce_amount(self, diff: eth::U256) -> Self { Self { sell_amount: self.sell_amount - diff, @@ -121,6 +127,14 @@ impl Order { } } + /// Multiply the sell amount of this order by the given factor. + pub fn multiply_amount(self, mult: eth::U256) -> Self { + Self { + sell_amount: self.sell_amount * mult, + ..self + } + } + /// Ensure that this order generates no surplus, and therefore most likely /// has a negative score. pub fn no_surplus(self) -> Self { @@ -182,6 +196,24 @@ impl Order { } } + /// Mark that this order should be filtered out before being sent to the + /// solver. + pub fn filtered(self) -> Self { + Self { + filtered: true, + ..self + } + } + + /// Mark that the trader should not be funded with tokens that are needed to + /// place this order. + pub fn unfunded(self) -> Self { + Self { + funded: false, + ..self + } + } + fn surplus_fee(&self) -> eth::U256 { match self.kind { order::Kind::Limit { surplus_fee: _ } => self.solver_fee.unwrap_or_default(), @@ -207,6 +239,8 @@ impl Default for Order { surplus_factor: DEFAULT_SURPLUS_FACTOR.into(), execution_diff: Default::default(), executed: Default::default(), + filtered: Default::default(), + funded: true, } } } diff --git a/crates/driver/src/tests/setup/solver.rs b/crates/driver/src/tests/setup/solver.rs index 2a2bb57741..b76b29cc58 100644 --- a/crates/driver/src/tests/setup/solver.rs +++ b/crates/driver/src/tests/setup/solver.rs @@ -36,7 +36,7 @@ impl Solver { pub async fn new(config: Config<'_>) -> Self { let mut solutions_json = Vec::new(); let mut orders_json = Vec::new(); - for quote in config.quoted_orders { + for quote in config.quoted_orders.iter().filter(|q| !q.order.filtered) { // ETH orders get unwrapped into WETH by the driver before being passed to the // solver. let sell_token = if quote.order.sell_token == "ETH" { @@ -199,6 +199,7 @@ impl Solver { Addresses { settlement: Some(config.blockchain.settlement.address().into()), weth: Some(config.blockchain.weth.address().into()), + ethflow: config.blockchain.ethflow, }, ) .await