Reject too gas intensive solutions already in driver competition (#2563)

# Description
#2526 started limiting the amount of gas the driver allows per solution.
However, the limit was only applied after solutions had been ranked and
participated in the autopilot's auction. This lead to large settlements
effectively clogging the auction cycle as solvers would continue to win
the competition but then fail to submit.

This PR filters out such solutions already in the driver internal
ranking phase so that too large settlements never participate as
solution candidate.

# Changes
- [x] Make `Gas::new` return an error if gas limit is too high
- [x] Introduce a new solver error type that solver engines are notified
about.

## How to test
Adjusted high gas limit driver test

## Related Issues

Fixes #2511 (or rather makes it obsolete)

crates/driver/src/domain/competition/solution/mod.rs

@@ -522,6 +522,8 @@ pub mod error {
         NonBufferableTokensUsed(BTreeSet<TokenAddress>),
         #[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")]

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<Self, solution::Error> {
         // 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

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);

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;
+}

crates/driver/src/tests/setup/blockchain.rs

@@ -164,6 +164,7 @@ pub struct Config {
     pub trader_secret_key: SecretKey,
     pub solvers: Vec<super::Solver>,
     pub settlement_address: Option<eth::H160>,
+    pub rpc_args: Vec<String>,
 }
 
 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();

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<eth::H160>,
     /// Via which mempool the solutions should be submitted
     mempools: Vec<Mempool>,
+    /// Extra configuration for the RPC node
+    rpc_args: Vec<String>,
 }
 
 /// The validity of a solution.
@@ -763,6 +766,11 @@ impl Setup {
         self
     }
 
+    pub fn rpc_args(mut self, rpc_args: Vec<String>) -> 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();