diff --git a/crates/driver/src/domain/competition/solution/mod.rs b/crates/driver/src/domain/competition/solution/mod.rs index f909d08a3d..6f78a064c6 100644 --- a/crates/driver/src/domain/competition/solution/mod.rs +++ b/crates/driver/src/domain/competition/solution/mod.rs @@ -522,6 +522,8 @@ pub mod error { NonBufferableTokensUsed(BTreeSet), #[error("invalid internalization: uninternalized solution fails to simulate")] FailingInternalization, + #[error("Gas estimate of {0:?} exceeded the per settlement limit of {1:?}")] + GasLimitExceeded(eth::Gas, eth::Gas), #[error("insufficient solver account Ether balance, required {0:?}")] SolverAccountInsufficientBalance(eth::Ether), #[error("attempted to merge settlements generated by different solvers")] diff --git a/crates/driver/src/domain/competition/solution/settlement.rs b/crates/driver/src/domain/competition/solution/settlement.rs index 44372c2f39..dc2ad56ed1 100644 --- a/crates/driver/src/domain/competition/solution/settlement.rs +++ b/crates/driver/src/domain/competition/solution/settlement.rs @@ -123,7 +123,7 @@ impl Settlement { ) .await?; let price = eth.gas_price().await?; - let gas = Gas::new(gas, eth.block_gas_limit(), price); + let gas = Gas::new(gas, eth.block_gas_limit(), price)?; // Ensure that the solver has sufficient balance for the settlement to be mined. if eth.balance(settlement.solver).await? < gas.required_balance() { @@ -337,18 +337,11 @@ pub struct Gas { impl Gas { /// Computes settlement gas parameters given estimates for gas and gas /// price. - pub fn new(estimate: eth::Gas, block_limit: eth::Gas, price: eth::GasPrice) -> Self { - // Specify a different gas limit than the estimated gas when executing a - // settlement transaction. This allows the transaction to be resilient - // to small variations in actual gas usage. - // Also, some solutions can have significant gas refunds that are refunded at - // the end of execution, so we want to increase gas limit enough so - // those solutions don't revert with out of gas error. - const GAS_LIMIT_FACTOR: f64 = 2.0; - let estimate_with_buffer = - eth::U256::from_f64_lossy(eth::U256::to_f64_lossy(estimate.into()) * GAS_LIMIT_FACTOR) - .into(); - + pub fn new( + estimate: eth::Gas, + block_limit: eth::Gas, + price: eth::GasPrice, + ) -> Result { // We don't allow for solutions to take up more than half of the block's gas // limit. This is to ensure that block producers attempt to include the // settlement transaction in the next block as long as it is reasonably @@ -360,12 +353,26 @@ impl Gas { // whose gas limit exceed the remaining space (without simulating the actual // gas required). let max_gas = eth::Gas(block_limit.0 / 2); + if estimate > max_gas { + return Err(solution::Error::GasLimitExceeded(estimate, max_gas)); + } + + // Specify a different gas limit than the estimated gas when executing a + // settlement transaction. This allows the transaction to be resilient + // to small variations in actual gas usage. + // Also, some solutions can have significant gas refunds that are refunded at + // the end of execution, so we want to increase gas limit enough so + // those solutions don't revert with out of gas error. + const GAS_LIMIT_FACTOR: f64 = 2.0; + let estimate_with_buffer = + eth::U256::from_f64_lossy(eth::U256::to_f64_lossy(estimate.into()) * GAS_LIMIT_FACTOR) + .into(); - Self { + Ok(Self { estimate, limit: std::cmp::min(max_gas, estimate_with_buffer), price, - } + }) } /// The balance required to ensure settlement execution with the given gas diff --git a/crates/driver/src/infra/notify/mod.rs b/crates/driver/src/infra/notify/mod.rs index 300dafb7e6..b01ac403ea 100644 --- a/crates/driver/src/infra/notify/mod.rs +++ b/crates/driver/src/infra/notify/mod.rs @@ -76,6 +76,10 @@ pub fn encoding_failed( } solution::Error::FailingInternalization => return, solution::Error::DifferentSolvers => return, + solution::Error::GasLimitExceeded(used, limit) => notification::Kind::DriverError(format!( + "Settlement gas limit exceeded: used {}, limit {}", + used.0, limit.0 + )), }; solver.notify(auction_id, Some(solution_id.clone()), notification); diff --git a/crates/driver/src/tests/cases/settle.rs b/crates/driver/src/tests/cases/settle.rs index aec5fda80d..867eea843f 100644 --- a/crates/driver/src/tests/cases/settle.rs +++ b/crates/driver/src/tests/cases/settle.rs @@ -1,10 +1,13 @@ -use crate::{ - domain::competition::order, - tests::{ - self, - cases::{EtherExt, DEFAULT_SOLVER_FEE}, - setup::{ab_order, ab_pool, ab_solution}, +use { + crate::{ + domain::competition::order, + tests::{ + self, + cases::{EtherExt, DEFAULT_SOLVER_FEE}, + setup::{ab_order, ab_pool, ab_solution}, + }, }, + web3::Transport, }; /// Run a matrix of tests for all meaningful combinations of order kind and @@ -73,33 +76,39 @@ async fn private_rpc_with_high_risk_solution() { err.kind("FailedToSubmit"); } -// /// Checks that we can settle transactions that have a gas limit higher than -// /// half the block size -// #[tokio::test] -// #[ignore] -// async fn high_gas_limit() { -// let test = tests::setup() -// .name("high gas limit") -// .pool(ab_pool()) -// .order(ab_order()) -// .solution(ab_solution().increase_gas(15_000_000)) -// .done() -// .await; +#[tokio::test] +#[ignore] +async fn too_much_gas() { + let test = tests::setup() + .name("too much gas") + .pool(ab_pool()) + .order(ab_order()) + .solution(ab_solution().increase_gas(6_000_000)) + .rpc_args(vec!["--gas-limit".into(), "10000000".into()]) + .done() + .await; + test.solve().await.ok().empty(); +} -// // Set to 30M for solving purposes -// test.web3() -// .transport() -// .execute("evm_setBlockGasLimit", vec![serde_json::json!(30_000_000)]) -// .await -// .unwrap(); +#[tokio::test] +#[ignore] +async fn high_gas_limit() { + let test = tests::setup() + .name("high gas limit") + .pool(ab_pool()) + .order(ab_order()) + .solution(ab_solution().increase_gas(4_000_000)) + .rpc_args(vec!["--gas-limit".into(), "10000000".into()]) + .done() + .await; -// test.solve().await.ok(); + test.solve().await.ok().orders(&[ab_order()]); -// // Assume validators downvoted gas limit to 29.9M, solution still settles -// test.web3() -// .transport() -// .execute("evm_setBlockGasLimit", vec![serde_json::json!(29_900_000)]) -// .await -// .unwrap(); -// test.settle().await.ok().await; -// } + // Assume validators downvoted gas limit, solution still settles + test.web3() + .transport() + .execute("evm_setBlockGasLimit", vec![serde_json::json!(9_000_000)]) + .await + .unwrap(); + test.settle().await.ok().await; +} diff --git a/crates/driver/src/tests/setup/blockchain.rs b/crates/driver/src/tests/setup/blockchain.rs index c805e85e0f..7b5a8288e3 100644 --- a/crates/driver/src/tests/setup/blockchain.rs +++ b/crates/driver/src/tests/setup/blockchain.rs @@ -164,6 +164,7 @@ pub struct Config { pub trader_secret_key: SecretKey, pub solvers: Vec, pub settlement_address: Option, + pub rpc_args: Vec, } impl Blockchain { @@ -175,7 +176,7 @@ impl Blockchain { // should be happening from the primary_account of the node, will do this // later - let node = Node::new().await; + let node = Node::new(&config.rpc_args).await; let web3 = Web3::new(DynTransport::new( web3::transports::Http::new(&node.url()).expect("valid URL"), )); @@ -772,7 +773,7 @@ impl std::fmt::Debug for Node { impl Node { /// Spawn a new node instance. - async fn new() -> Self { + async fn new(extra_args: &[String]) -> Self { use tokio::io::AsyncBufReadExt as _; // Allow using some custom logic to spawn `anvil` by setting `ANVIL_COMMAND`. @@ -784,8 +785,7 @@ impl Node { .arg("0") // use 0 to let `anvil` use any open port .arg("--balance") .arg("1000000") - .arg("--gas-limit") - .arg("30000000") + .args(extra_args) .stdout(std::process::Stdio::piped()) .spawn() .unwrap(); diff --git a/crates/driver/src/tests/setup/mod.rs b/crates/driver/src/tests/setup/mod.rs index 98cc1be4e2..1e7b6c9e46 100644 --- a/crates/driver/src/tests/setup/mod.rs +++ b/crates/driver/src/tests/setup/mod.rs @@ -485,6 +485,7 @@ pub fn setup() -> Setup { enable_simulation: true, settlement_address: Default::default(), mempools: vec![Mempool::Public], + rpc_args: vec!["--gas-limit".into(), "10000000".into()], } } @@ -506,6 +507,8 @@ pub struct Setup { settlement_address: Option, /// Via which mempool the solutions should be submitted mempools: Vec, + /// Extra configuration for the RPC node + rpc_args: Vec, } /// The validity of a solution. @@ -763,6 +766,11 @@ impl Setup { self } + pub fn rpc_args(mut self, rpc_args: Vec) -> Self { + self.rpc_args = rpc_args; + self + } + /// Create the test: set up onchain contracts and pools, start a mock HTTP /// server for the solver and start the HTTP server for the driver. pub async fn done(self) -> Test { @@ -799,6 +807,7 @@ impl Setup { trader_secret_key, solvers: self.solvers.clone(), settlement_address: self.settlement_address, + rpc_args: self.rpc_args, }) .await; let mut solutions = Vec::new();