diff --git a/src/domain/dex/mod.rs b/src/domain/dex/mod.rs index 3f56c26..31f4c3c 100644 --- a/src/domain/dex/mod.rs +++ b/src/domain/dex/mod.rs @@ -109,12 +109,18 @@ impl Swap { sell_token: Option, simulator: &infra::dex::Simulator, ) -> Option { - let gas = match simulator.gas(order.owner(), &self).await { - Ok(value) => value, - Err(err) => { - tracing::warn!(?err, "gas simulation failed"); - return None; + let gas = if order.class == order::Class::Limit { + match simulator.gas(order.owner(), &self).await { + Ok(value) => value, + Err(err) => { + tracing::warn!(?err, "gas simulation failed"); + return None; + } } + } else { + // We are fine with just using heuristic gas for market orders, + // since it doesn't really play a role in the final solution. + self.gas }; let allowance = self.allowance(); diff --git a/src/domain/eth/mod.rs b/src/domain/eth/mod.rs index e8b6a91..ed1b6d0 100644 --- a/src/domain/eth/mod.rs +++ b/src/domain/eth/mod.rs @@ -45,20 +45,8 @@ impl From for Ether { } } -impl std::ops::Add for Gas { - type Output = Self; - - fn add(self, rhs: SignedGas) -> Self::Output { - if rhs.0.is_positive() { - Self(self.0.saturating_add(rhs.0.into())) - } else { - Self(self.0.saturating_sub(rhs.0.abs().into())) - } - } -} - /// Gas amount. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Default)] pub struct Gas(pub U256); impl std::ops::Add for Gas { @@ -69,16 +57,6 @@ impl std::ops::Add for Gas { } } -/// Like [`Gas`] but can be negative to encode a gas discount. -#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] -pub struct SignedGas(i64); - -impl From for SignedGas { - fn from(value: i64) -> Self { - Self(value) - } -} - /// A 256-bit rational type. pub type Rational = num::rational::Ratio; @@ -107,26 +85,3 @@ pub struct Tx { pub input: Bytes>, pub access_list: AccessList, } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn add_gas_offset() { - let gas = |value: u128| Gas(value.into()); - let offset = SignedGas::from; - - // saturating sub - assert_eq!(gas(100) + offset(-101), gas(0)); - - // regular sub - assert_eq!(gas(100) + offset(-90), gas(10)); - - // saturating add - assert_eq!(Gas(U256::MAX) + offset(100), Gas(U256::MAX)); - - // regular add - assert_eq!(gas(100) + offset(100), gas(200)); - } -} diff --git a/src/domain/order.rs b/src/domain/order.rs index 413a477..0e1e515 100644 --- a/src/domain/order.rs +++ b/src/domain/order.rs @@ -36,7 +36,7 @@ impl Order { /// Returns `true` if the order expects a solver-computed fee. pub fn solver_determines_fee(&self) -> bool { - self.class != Class::Liquidity && self.fee.0.is_zero() + self.class == Class::Limit } } diff --git a/src/domain/solution.rs b/src/domain/solution.rs index 08cc03a..07fb86e 100644 --- a/src/domain/solution.rs +++ b/src/domain/solution.rs @@ -111,13 +111,14 @@ pub struct Single { pub output: eth::Asset, /// The swap interactions for the single order settlement. pub interactions: Vec, - /// The estimated gas needed for swapping the sell amount to buy amount - /// already including the additional overhead of calling the settlement - /// contract. + /// The estimated gas needed for swapping the sell amount to buy amount. pub gas: eth::Gas, } impl Single { + /// An approximation for the overhead of executing a trade in a settlement. + const SETTLEMENT_OVERHEAD: u64 = 106_391; + /// Creates a full solution for a single order solution given gas and sell /// token prices. pub fn into_solution( @@ -143,7 +144,13 @@ impl Single { // full order fee as well as a solver computed fee. Note that this // is fine for now, since there is no way to create limit orders // with non-zero fees. - Fee::Surplus(sell_token?.ether_value(eth::Ether(swap.0.checked_mul(gas_price.0 .0)?))?) + Fee::Surplus( + sell_token?.ether_value(eth::Ether( + swap.0 + .checked_add(Self::SETTLEMENT_OVERHEAD.into())? + .checked_mul(gas_price.0 .0)?, + ))?, + ) } else { Fee::Protocol }; @@ -191,7 +198,7 @@ impl Single { ]), trades: vec![Trade::Fulfillment(Fulfillment::new(order, executed, fee)?)], interactions, - gas: Some(self.gas), + gas: Some(eth::Gas(Self::SETTLEMENT_OVERHEAD.into()) + self.gas), }) } } diff --git a/src/domain/solver/dex/mod.rs b/src/domain/solver/dex/mod.rs index 756e33c..998bff3 100644 --- a/src/domain/solver/dex/mod.rs +++ b/src/domain/solver/dex/mod.rs @@ -54,7 +54,6 @@ impl Dex { &config.node_url, config.contracts.settlement, config.contracts.authenticator, - config.solution_gas_offset, ), slippage: config.slippage, concurrent_requests: config.concurrent_requests, diff --git a/src/infra/config/dex/file.rs b/src/infra/config/dex/file.rs index 77464bd..576bd8b 100644 --- a/src/infra/config/dex/file.rs +++ b/src/infra/config/dex/file.rs @@ -57,11 +57,6 @@ struct Config { #[serde(with = "humantime_serde", default = "default_max_back_off")] max_back_off: Duration, - /// Units of gas that get added to the gas estimate for executing a - /// computed trade route to arrive at a gas estimate for a whole settlement. - #[serde(default)] - solution_gas_offset: i64, - /// Settings specific to the wrapped dex API. dex: toml::Value, } @@ -143,7 +138,6 @@ pub async fn load(path: &Path) -> (super::Config, T) { config.max_back_off, ) .unwrap(), - solution_gas_offset: config.solution_gas_offset.into(), }; (config, dex) } diff --git a/src/infra/config/dex/mod.rs b/src/infra/config/dex/mod.rs index 1e8f770..4b47f04 100644 --- a/src/infra/config/dex/mod.rs +++ b/src/infra/config/dex/mod.rs @@ -26,5 +26,4 @@ pub struct Config { pub concurrent_requests: NonZeroUsize, pub smallest_partial_fill: eth::Ether, pub rate_limiting_strategy: RateLimitingStrategy, - pub solution_gas_offset: eth::SignedGas, } diff --git a/src/infra/dex/simulator.rs b/src/infra/dex/simulator.rs index 1fcf02d..d17c2bf 100644 --- a/src/infra/dex/simulator.rs +++ b/src/infra/dex/simulator.rs @@ -15,9 +15,6 @@ pub struct Simulator { web3: ethrpc::Web3, settlement: eth::ContractAddress, authenticator: eth::ContractAddress, - /// Configurable offset to adjust for systematic under- or - /// over-estimating of gas costs. - gas_offset: eth::SignedGas, } impl Simulator { @@ -26,13 +23,11 @@ impl Simulator { url: &reqwest::Url, settlement: eth::ContractAddress, authenticator: eth::ContractAddress, - gas_offset: eth::SignedGas, ) -> Self { Self { web3: blockchain::rpc(url), settlement, authenticator, - gas_offset, } } @@ -112,9 +107,9 @@ impl Simulator { "could not simulate dex swap to get gas used; fall back to gas estimate provided \ by dex API" ); - swap.gas + self.gas_offset + swap.gas } else { - eth::Gas(gas) + self.gas_offset + eth::Gas(gas) }) } } diff --git a/src/tests/balancer/market_order.rs b/src/tests/balancer/market_order.rs index bff56fd..835e3e8 100644 --- a/src/tests/balancer/market_order.rs +++ b/src/tests/balancer/market_order.rs @@ -44,12 +44,7 @@ async fn sell() { }]) .await; - let node = tests::mock::node::constant_gas_estimate(195283).await; - let engine = tests::SolverEngine::new( - "balancer", - balancer::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("balancer", balancer::config(&api.address)).await; let solution = engine .solve(json!({ @@ -204,12 +199,7 @@ async fn buy() { }]) .await; - let node = tests::mock::node::constant_gas_estimate(195283).await; - let engine = tests::SolverEngine::new( - "balancer", - balancer::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("balancer", balancer::config(&api.address)).await; let solution = engine .solve(json!({ diff --git a/src/tests/balancer/mod.rs b/src/tests/balancer/mod.rs index fefcd72..c71bf72 100644 --- a/src/tests/balancer/mod.rs +++ b/src/tests/balancer/mod.rs @@ -14,15 +14,3 @@ endpoint = 'http://{solver_addr}/sor' ", )) } - -/// Creates a temporary file containing the config of the given solver and a -/// node. -pub fn config_with_node(solver_addr: &SocketAddr, node: &SocketAddr) -> tests::Config { - tests::Config::String(format!( - r" -node-url = 'http://{node}' -[dex] -endpoint = 'http://{solver_addr}/sor' - ", - )) -} diff --git a/src/tests/dex/partial_fill.rs b/src/tests/dex/partial_fill.rs index e232542..feba85b 100644 --- a/src/tests/dex/partial_fill.rs +++ b/src/tests/dex/partial_fill.rs @@ -122,7 +122,18 @@ async fn tested_amounts_adjust_depending_on_response() { ]) .await; - let simulation_node = mock::node::constant_gas_estimate(195283).await; + let simulation_node = mock::http::setup(vec![mock::http::Expectation::Post { + path: mock::http::Path::Any, + req: mock::http::RequestBody::Any, + res: { + json!({ + "id": 1, + "jsonrpc": "2.0", + "result": "0x0000000000000000000000000000000000000000000000000000000000015B3C" + }) + }, + }]) + .await; let config = tests::Config::String(format!( r" @@ -453,7 +464,23 @@ async fn moves_surplus_fee_to_buy_token() { ]) .await; - let simulation_node = mock::node::constant_gas_estimate(195283).await; + let simulation_node = mock::http::setup(vec![mock::http::Expectation::Post { + path: mock::http::Path::Any, + req: mock::http::RequestBody::Any, + res: { + json!({ + "id": 1, + "jsonrpc": "2.0", + // If the simulation logic returns 0 it means that the user did not have the + // required balance. This could be caused by a pre-interaction that acquires the + // necessary sell_token before the trade which is currently not supported by the + // simulation loic. + // In that case we fall back to the heuristic gas price we had in the past. + "result": "0x0000000000000000000000000000000000000000000000000000000000000000" + }) + }, + }]) + .await; let config = tests::Config::String(format!( r" @@ -738,12 +765,7 @@ async fn market() { }]) .await; - let node = mock::node::constant_gas_estimate(195283).await; - let engine = tests::SolverEngine::new( - "balancer", - balancer::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("balancer", balancer::config(&api.address)).await; let solution = engine .solve(json!({ diff --git a/src/tests/mock/mod.rs b/src/tests/mock/mod.rs index b906737..3883215 100644 --- a/src/tests/mock/mod.rs +++ b/src/tests/mock/mod.rs @@ -1,2 +1 @@ pub mod http; -pub mod node; diff --git a/src/tests/mock/node.rs b/src/tests/mock/node.rs deleted file mode 100644 index ddc2e84..0000000 --- a/src/tests/mock/node.rs +++ /dev/null @@ -1,19 +0,0 @@ -use crate::{ - domain::eth, - tests::mock::http::{setup, Expectation, Path, RequestBody, ServerHandle}, -}; - -/// Returns a node that will always return the given number as a U256 -/// which will internally be used as a gas estimate for the proposed -/// solution. -pub async fn constant_gas_estimate(gas: u64) -> ServerHandle { - setup(vec![Expectation::Post { - path: Path::Any, - req: RequestBody::Any, - res: serde_json::json!({ - "result": format!("{:#066X}", eth::U256::from(gas)), - "id": 0, - }), - }]) - .await -} diff --git a/src/tests/oneinch/market_order.rs b/src/tests/oneinch/market_order.rs index 4e3c851..b0ce55c 100644 --- a/src/tests/oneinch/market_order.rs +++ b/src/tests/oneinch/market_order.rs @@ -119,12 +119,7 @@ async fn sell() { ]) .await; - let node = tests::mock::node::constant_gas_estimate(206391).await; - let engine = tests::SolverEngine::new( - "oneinch", - super::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("oneinch", super::config(&api.address)).await; let solution = engine .solve(json!({ diff --git a/src/tests/oneinch/mod.rs b/src/tests/oneinch/mod.rs index 5dcb7e7..efdf31a 100644 --- a/src/tests/oneinch/mod.rs +++ b/src/tests/oneinch/mod.rs @@ -16,16 +16,3 @@ exclude-liquidity = ['UNISWAP_V3', 'PMM4'] ", )) } - -/// Creates a temporary file containing the config of the given solver and node. -pub fn config_with_node(solver_addr: &SocketAddr, node: &SocketAddr) -> tests::Config { - tests::Config::String(format!( - r" -node-url = 'http://{node}' -[dex] -chain-id = '1' -endpoint = 'http://{solver_addr}' -exclude-liquidity = ['UNISWAP_V3', 'PMM4'] - ", - )) -} diff --git a/src/tests/paraswap/market_order.rs b/src/tests/paraswap/market_order.rs index 55a09d0..a7274c3 100644 --- a/src/tests/paraswap/market_order.rs +++ b/src/tests/paraswap/market_order.rs @@ -148,12 +148,7 @@ async fn sell() { ]) .await; - let node = tests::mock::node::constant_gas_estimate(348691).await; - let engine = tests::SolverEngine::new( - "paraswap", - paraswap::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("paraswap", paraswap::config(&api.address)).await; let solution = engine .solve(json!({ @@ -405,12 +400,7 @@ async fn buy() { ]) .await; - let node = tests::mock::node::constant_gas_estimate(213326).await; - let engine = tests::SolverEngine::new( - "paraswap", - paraswap::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("paraswap", paraswap::config(&api.address)).await; let solution = engine .solve(json!({ diff --git a/src/tests/paraswap/mod.rs b/src/tests/paraswap/mod.rs index 5f33dad..c14dfad 100644 --- a/src/tests/paraswap/mod.rs +++ b/src/tests/paraswap/mod.rs @@ -17,17 +17,3 @@ partner = 'cow' ", )) } - -/// Creates a temporary file containing the config of the given solver and node. -pub fn config_with_node(solver_addr: &SocketAddr, node: &SocketAddr) -> tests::Config { - tests::Config::String(format!( - r" -node-url = 'http://{node}' -[dex] -endpoint = 'http://{solver_addr}' -exclude-dexs = ['UniswapV2'] -address = '0xE0B3700e0aadcb18ed8d4BFF648Bc99896a18ad1' -partner = 'cow' - ", - )) -} diff --git a/src/tests/zeroex/market_order.rs b/src/tests/zeroex/market_order.rs index 1815d11..d2821c8 100644 --- a/src/tests/zeroex/market_order.rs +++ b/src/tests/zeroex/market_order.rs @@ -88,12 +88,7 @@ async fn sell() { }]) .await; - let node = mock::node::constant_gas_estimate(234277).await; - let engine = tests::SolverEngine::new( - "zeroex", - zeroex::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("zeroex", zeroex::config(&api.address)).await; let solution = engine .solve(json!({ @@ -272,12 +267,7 @@ async fn buy() { }]) .await; - let node = mock::node::constant_gas_estimate(217391).await; - let engine = tests::SolverEngine::new( - "zeroex", - zeroex::config_with_node(&api.address, &node.address), - ) - .await; + let engine = tests::SolverEngine::new("zeroex", zeroex::config(&api.address)).await; let solution = engine .solve(json!({ diff --git a/src/tests/zeroex/mod.rs b/src/tests/zeroex/mod.rs index d5c5279..cbc9f5c 100644 --- a/src/tests/zeroex/mod.rs +++ b/src/tests/zeroex/mod.rs @@ -5,21 +5,7 @@ mod not_found; mod options; mod out_of_price; -/// Creates a temporary file containing the config of the given solver and node. -pub fn config_with_node(solver_addr: &SocketAddr, node: &SocketAddr) -> tests::Config { - tests::Config::String(format!( - r" -node-url = 'http://{node}' -[dex] -endpoint = 'http://{solver_addr}/swap/v1/' -api-key = 'SUPER_SECRET_API_KEY' - ", - )) -} - /// Creates a temporary file containing the config of the given solver. -/// Does not have access to a node so only suitable for tests that not rely on -/// that. pub fn config(solver_addr: &SocketAddr) -> tests::Config { tests::Config::String(format!( r" diff --git a/src/tests/zeroex/options.rs b/src/tests/zeroex/options.rs index 4b7898a..0749149 100644 --- a/src/tests/zeroex/options.rs +++ b/src/tests/zeroex/options.rs @@ -180,10 +180,9 @@ async fn test() { }]) .await; - let node = mock::node::constant_gas_estimate(234277).await; let config = tests::Config::String(format!( r" -node-url = 'http://{}' +node-url = 'http://localhost:8545' relative-slippage = '0.1' [dex] endpoint = 'http://{}/swap/v1/' @@ -193,7 +192,7 @@ affiliate = '0x0123456789012345678901234567890123456789' enable-rfqt = true enable-slippage-protection = true ", - node.address, api.address + api.address )); let engine = tests::SolverEngine::new("zeroex", config).await;