Reject orders using too much gas (#2551)

# Description
We are seeing inefficiency with some ERC1271 orders on Gnosis that are
using a large amount of gas reducing settlement throughput for other
traders. This PR limits the amount of gas an order can consume
(including signature validation and hooks)

# Changes
- [x] Introduce a new error type too much gas
- [x] Return this error if the total gas estimate (including hooks)
exceeds a certain threshold

Considerations:
- Should we also block native ETH flow orders below a certain validation
limit?
- Should we already warn during quoting when the order is likely going
to exceed the gas limit?

Both of these would make the PR more involved while not directly
addressing the somewhat pressing issue we have on Gnosis Chain.

## How to test
Test for hooks. I'm also working on a test for ERC1271 orders, however
it's a bit more involved because we need a fake verifier contract that
uses a lot of gas

<!--
## Related Issues

Fixes #
-->

crates/contracts/artifacts/GasHog.json

@@ -0,0 +1 @@
+{"abi":[{"inputs":[{"internalType":"contract ERC20","name":"token","type":"address"},{"internalType":"address","name":"spender","type":"address"},{"internalType":"uint256","name":"amount","type":"uint256"}],"name":"approve","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"bytes32","name":"order","type":"bytes32"},{"internalType":"bytes","name":"signature","type":"bytes"}],"name":"isValidSignature","outputs":[{"internalType":"bytes4","name":"","type":"bytes4"}],"stateMutability":"view","type":"function"}],"bytecode":"0x608060405234801561001057600080fd5b50610318806100206000396000f3fe608060405234801561001057600080fd5b50600436106100365760003560e01c80631626ba7e1461003b578063e1f21c6714610083575b600080fd5b61004e6100493660046101d0565b610098565b6040517fffffffff00000000000000000000000000000000000000000000000000000000909116815260200160405180910390f35b610096610091366004610271565b610143565b005b6000805a905060006100ac848601866102b2565b90507fce7d7369855be79904099402d83db6d6ab8840dcd5c086e062cd1ca0c8111dfc5b815a6100dc90856102cb565b101561010b576040805160208101839052016040516020818303038152906040528051906020012090506100d0565b86810361011757600080fd5b507f1626ba7e000000000000000000000000000000000000000000000000000000009695505050505050565b6040517f095ea7b300000000000000000000000000000000000000000000000000000000815273ffffffffffffffffffffffffffffffffffffffff83811660048301526024820183905284169063095ea7b390604401600060405180830381600087803b1580156101b357600080fd5b505af11580156101c7573d6000803e3d6000fd5b50505050505050565b6000806000604084860312156101e557600080fd5b83359250602084013567ffffffffffffffff8082111561020457600080fd5b818601915086601f83011261021857600080fd5b81358181111561022757600080fd5b87602082850101111561023957600080fd5b6020830194508093505050509250925092565b73ffffffffffffffffffffffffffffffffffffffff8116811461026e57600080fd5b50565b60008060006060848603121561028657600080fd5b83356102918161024c565b925060208401356102a18161024c565b929592945050506040919091013590565b6000602082840312156102c457600080fd5b5035919050565b81810381811115610305577f4e487b7100000000000000000000000000000000000000000000000000000000600052601160045260246000fd5b9291505056fea164736f6c6343000811000a","deployedBytecode":"0x608060405234801561001057600080fd5b50600436106100365760003560e01c80631626ba7e1461003b578063e1f21c6714610083575b600080fd5b61004e6100493660046101d0565b610098565b6040517fffffffff00000000000000000000000000000000000000000000000000000000909116815260200160405180910390f35b610096610091366004610271565b610143565b005b6000805a905060006100ac848601866102b2565b90507fce7d7369855be79904099402d83db6d6ab8840dcd5c086e062cd1ca0c8111dfc5b815a6100dc90856102cb565b101561010b576040805160208101839052016040516020818303038152906040528051906020012090506100d0565b86810361011757600080fd5b507f1626ba7e000000000000000000000000000000000000000000000000000000009695505050505050565b6040517f095ea7b300000000000000000000000000000000000000000000000000000000815273ffffffffffffffffffffffffffffffffffffffff83811660048301526024820183905284169063095ea7b390604401600060405180830381600087803b1580156101b357600080fd5b505af11580156101c7573d6000803e3d6000fd5b50505050505050565b6000806000604084860312156101e557600080fd5b83359250602084013567ffffffffffffffff8082111561020457600080fd5b818601915086601f83011261021857600080fd5b81358181111561022757600080fd5b87602082850101111561023957600080fd5b6020830194508093505050509250925092565b73ffffffffffffffffffffffffffffffffffffffff8116811461026e57600080fd5b50565b60008060006060848603121561028657600080fd5b83356102918161024c565b925060208401356102a18161024c565b929592945050506040919091013590565b6000602082840312156102c457600080fd5b5035919050565b81810381811115610305577f4e487b7100000000000000000000000000000000000000000000000000000000600052601160045260246000fd5b9291505056fea164736f6c6343000811000a","devdoc":{"methods":{}},"userdoc":{"methods":{}}}

crates/contracts/build.rs

@@ -691,6 +691,9 @@ fn main() {
 
     // Test Contract for incrementing arbitrary counters.
     generate_contract("Counter");
+
+    // Test Contract for using up a specified amount of gas.
+    generate_contract("GasHog");
 }
 
 fn generate_contract(name: &str) {

crates/contracts/solidity/Makefile

@@ -19,7 +19,7 @@ CONTRACTS := \
 	Trader.sol
 ARTIFACTS := $(patsubst %.sol,$(ARTIFACTDIR)/%.json,$(CONTRACTS))
 
-TEST_CONTRACTS := Counter.sol
+TEST_CONTRACTS := Counter.sol GasHog.sol
 TEST_ARTIFACTS := $(patsubst %.sol,$(ARTIFACTDIR)/%.json,$(TEST_CONTRACTS))
 
 .PHONY: artifacts
@@ -58,11 +58,11 @@ $(TARGETDIR)/%.abi: %.sol
 		$(SOLC) \
 		$(SOLFLAGS) -o /target $<
 
-$(TARGETDIR)/%.abi: test/%.sol
+$(TARGETDIR)/%.abi: tests/%.sol
 	@mkdir -p $(TARGETDIR)
 	@echo solc $(SOLFLAGS) -o /target $(notdir $<)
 	@$(DOCKER) run -it --rm \
-		-v "$(abspath .)/test:/contracts" -w "/contracts" \
+		-v "$(abspath .)/tests:/contracts" -w "/contracts" \
 		-v "$(abspath $(TARGETDIR)):/target" \
 		$(SOLC) \
 		$(SOLFLAGS) -o /target $(notdir $<)

crates/contracts/solidity/tests/GasHog.sol

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.17;
+
+interface ERC20 {
+    function approve(address spender, uint amount) external;
+}
+
+/// @title Helper contract to simulate gas intensive ERC1271 signatures
+contract GasHog {
+    function isValidSignature(bytes32 order, bytes calldata signature) public view returns (bytes4) {
+        uint start = gasleft();
+        uint target = abi.decode(signature, (uint));
+        bytes32 hash = keccak256("go");
+        while (start - gasleft() < target) {
+            hash = keccak256(abi.encode(hash));
+        }
+        // Assert the impossible so that the compiler doesn't optimise the loop away
+        require(hash != order);
+
+        // ERC1271 Magic Value
+        return 0x1626ba7e;
+    }
+
+    function approve(ERC20 token, address spender, uint amount) external {
+        token.approve(spender, amount);
+    }
+}

crates/contracts/src/lib.rs

@@ -86,6 +86,7 @@ pub mod support {
 pub mod test {
     include_contracts! {
         Counter;
+        GasHog;
     }
 }
 

crates/e2e/src/setup/services.rs

@@ -62,6 +62,12 @@ impl ServicesBuilder {
     }
 }
 
+#[derive(Default)]
+pub struct ExtraServiceArgs {
+    pub api: Vec<String>,
+    pub autopilot: Vec<String>,
+}
+
 /// Wrapper over offchain services.
 /// Exposes various utility methods for tests.
 pub struct Services<'a> {
@@ -103,11 +109,6 @@ impl<'a> Services<'a> {
             "--baseline-sources=None".to_string(),
             "--network-block-interval=1s".to_string(),
             "--solver-competition-auth=super_secret_key".to_string(),
-            format!(
-                "--custom-univ2-baseline-sources={:?}|{:?}",
-                self.contracts.uniswap_v2_router.address(),
-                self.contracts.default_pool_code(),
-            ),
             format!(
                 "--settlement-contract-address={:?}",
                 self.contracts.gp_settlement.address()
@@ -168,6 +169,11 @@ impl<'a> Services<'a> {
 
     /// Starts a basic version of the protocol with a single baseline solver.
     pub async fn start_protocol(&self, solver: TestAccount) {
+        self.start_protocol_with_args(Default::default(), solver)
+            .await;
+    }
+
+    pub async fn start_protocol_with_args(&self, args: ExtraServiceArgs, solver: TestAccount) {
         let solver_endpoint =
             colocation::start_baseline_solver(self.contracts.weth.address()).await;
         colocation::start_driver(
@@ -180,15 +186,26 @@ impl<'a> Services<'a> {
         );
         self.start_autopilot(
             None,
-            vec![
-                "--drivers=test_solver|http://localhost:11088/test_solver".to_string(),
-                "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver"
-                    .to_string(),
-            ],
+            [
+                vec![
+                    "--drivers=test_solver|http://localhost:11088/test_solver".to_string(),
+                    "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver"
+                        .to_string(),
+                ],
+                args.autopilot,
+            ]
+            .concat(),
         );
-        self.start_api(vec![
-            "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver".to_string(),
-        ])
+        self.start_api(
+            [
+                vec![
+                    "--price-estimation-drivers=test_quoter|http://localhost:11088/test_solver"
+                        .to_string(),
+                ],
+                args.api,
+            ]
+            .concat(),
+        )
         .await;
     }
 

crates/e2e/tests/e2e/hooks.rs

@@ -11,6 +11,7 @@ use {
         order::{OrderCreation, OrderCreationAppData, OrderKind},
         signature::{hashed_eip712_message, EcdsaSigningScheme, Signature},
     },
+    reqwest::StatusCode,
     secp256k1::SecretKey,
     serde_json::json,
     shared::ethrpc::Web3,
@@ -35,6 +36,65 @@ async fn local_node_partial_fills() {
     run_test(partial_fills).await;
 }
 
+#[tokio::test]
+#[ignore]
+async fn local_node_gas_limit() {
+    run_test(gas_limit).await;
+}
+
+async fn gas_limit(web3: Web3) {
+    let mut onchain = OnchainComponents::deploy(web3).await;
+
+    let [solver] = onchain.make_solvers(to_wei(1)).await;
+    let [trader] = onchain.make_accounts(to_wei(1)).await;
+    let cow = onchain
+        .deploy_cow_weth_pool(to_wei(1_000_000), to_wei(1_000), to_wei(1_000))
+        .await;
+
+    // Fund trader accounts and approve relayer
+    cow.fund(trader.address(), to_wei(5)).await;
+    tx!(
+        trader.account(),
+        cow.approve(onchain.contracts().allowance, to_wei(5))
+    );
+
+    let services = Services::new(onchain.contracts()).await;
+    services.start_protocol(solver).await;
+
+    let order = OrderCreation {
+        sell_token: cow.address(),
+        sell_amount: to_wei(4),
+        buy_token: onchain.contracts().weth.address(),
+        buy_amount: to_wei(3),
+        valid_to: model::time::now_in_epoch_seconds() + 300,
+        kind: OrderKind::Sell,
+        app_data: OrderCreationAppData::Full {
+            full: json!({
+                "metadata": {
+                    "hooks": {
+                        "pre": [Hook {
+                            target: trader.address(),
+                            call_data: Default::default(),
+                            gas_limit: 10_000_000,
+                        }],
+                        "post": [],
+                    },
+                },
+            })
+            .to_string(),
+        },
+        ..Default::default()
+    }
+    .sign(
+        EcdsaSigningScheme::Eip712,
+        &onchain.contracts().domain_separator,
+        SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()),
+    );
+    let error = services.create_order(&order).await.unwrap_err();
+    assert_eq!(error.0, StatusCode::BAD_REQUEST);
+    assert!(error.1.contains("TooMuchGas"));
+}
+
 async fn allowance(web3: Web3) {
     let mut onchain = OnchainComponents::deploy(web3).await;
 

crates/e2e/tests/e2e/smart_contract_orders.rs

@@ -1,10 +1,14 @@
 use {
-    e2e::setup::{safe::Safe, *},
+    e2e::{
+        setup::{safe::Safe, *},
+        tx,
+    },
     ethcontract::{Bytes, H160, U256},
     model::{
         order::{OrderCreation, OrderCreationAppData, OrderKind, OrderStatus, OrderUid},
         signature::Signature,
     },
+    reqwest::StatusCode,
     shared::ethrpc::Web3,
 };
 
@@ -14,6 +18,12 @@ async fn local_node_smart_contract_orders() {
     run_test(smart_contract_orders).await;
 }
 
+#[tokio::test]
+#[ignore]
+async fn local_node_max_gas_limit() {
+    run_test(erc1271_gas_limit).await;
+}
+
 async fn smart_contract_orders(web3: Web3) {
     let mut onchain = OnchainComponents::deploy(web3.clone()).await;
 
@@ -149,3 +159,55 @@ async fn smart_contract_orders(web3: Web3) {
         .expect("Couldn't fetch native token balance");
     assert_eq!(balance, U256::from(7_975_363_406_512_003_608_u128));
 }
+
+async fn erc1271_gas_limit(web3: Web3) {
+    let mut onchain = OnchainComponents::deploy(web3.clone()).await;
+
+    let [solver] = onchain.make_solvers(to_wei(1)).await;
+    let trader = contracts::test::GasHog::builder(&web3)
+        .deploy()
+        .await
+        .unwrap();
+
+    let cow = onchain
+        .deploy_cow_weth_pool(to_wei(1_000_000), to_wei(1_000), to_wei(1_000))
+        .await;
+
+    // Fund trader accounts and approve relayer
+    cow.fund(trader.address(), to_wei(5)).await;
+    tx!(
+        solver.account(),
+        trader.approve(cow.address(), onchain.contracts().allowance, to_wei(10))
+    );
+
+    let services = Services::new(onchain.contracts()).await;
+    services
+        .start_protocol_with_args(
+            ExtraServiceArgs {
+                api: vec!["--max-gas-per-order=1000000".to_string()],
+                ..Default::default()
+            },
+            solver,
+        )
+        .await;
+
+    // Use 1M gas units during signature verification
+    let mut signature = [0; 32];
+    U256::exp10(6).to_big_endian(&mut signature);
+
+    let order = OrderCreation {
+        sell_token: cow.address(),
+        sell_amount: to_wei(4),
+        buy_token: onchain.contracts().weth.address(),
+        buy_amount: to_wei(3),
+        valid_to: model::time::now_in_epoch_seconds() + 300,
+        kind: OrderKind::Sell,
+        signature: Signature::Eip1271(signature.to_vec()),
+        from: Some(trader.address()),
+        ..Default::default()
+    };
+
+    let error = services.create_order(&order).await.unwrap_err();
+    assert_eq!(error.0, StatusCode::BAD_REQUEST);
+    assert!(error.1.contains("TooMuchGas"));
+}

crates/orderbook/openapi.yml

@@ -1187,6 +1187,7 @@ components:
               ZeroAmount,
               IncompatibleSigningScheme,
               TooManyLimitOrders,
+              TooMuchGas,
               UnsupportedBuyTokenDestination,
               UnsupportedSellTokenSource,
               UnsupportedOrderType,

crates/orderbook/src/api/post_order.rs

@@ -229,6 +229,10 @@ impl IntoWarpReply for ValidationErrorWrapper {
                 error("TooManyLimitOrders", "Too many limit orders"),
                 StatusCode::BAD_REQUEST,
             ),
+            ValidationError::TooMuchGas => with_status(
+                error("TooMuchGas", "Executing order requires too many gas units"),
+                StatusCode::BAD_REQUEST,
+            ),
 
             ValidationError::Other(err) => {
                 tracing::error!(?err, "ValidationErrorWrapper");

crates/orderbook/src/arguments.rs

@@ -143,6 +143,10 @@ pub struct Arguments {
     /// Set the maximum size in bytes of order app data.
     #[clap(long, env, default_value = "8192")]
     pub app_data_size_limit: usize,
+
+    /// The maximum gas amount a single order can use for getting settled.
+    #[clap(long, env, default_value = "8000000")]
+    pub max_gas_per_order: u64,
 }
 
 impl std::fmt::Display for Arguments {
@@ -173,6 +177,7 @@ impl std::fmt::Display for Arguments {
             hooks_contract_address,
             app_data_size_limit,
             db_url,
+            max_gas_per_order,
         } = self;
 
         write!(f, "{}", shared)?;
@@ -237,6 +242,7 @@ impl std::fmt::Display for Arguments {
             &hooks_contract_address.map(|a| format!("{a:?}")),
         )?;
         writeln!(f, "app_data_size_limit: {}", app_data_size_limit)?;
+        writeln!(f, "max_gas_per_order: {}", max_gas_per_order)?;
 
         Ok(())
     }

crates/orderbook/src/run.rs

@@ -463,6 +463,7 @@ pub async fn run(args: Arguments) {
             Arc::new(CachedCodeFetcher::new(Arc::new(web3.clone()))),
             app_data_validator.clone(),
             args.shared.market_orders_deprecation_date,
+            args.max_gas_per_order,
         )
         .with_verified_quotes(args.price_estimation.trade_simulator.is_some()),
     );

crates/shared/src/order_quoting.rs

@@ -60,7 +60,7 @@ impl QuoteParameters {
         }
     }
 
-    fn additional_cost(&self) -> u64 {
+    pub fn additional_cost(&self) -> u64 {
         self.signing_scheme
             .additional_gas_amount()
             .saturating_add(self.additional_gas)
@@ -279,7 +279,7 @@ impl QuoteSearchParameters {
     }
 
     /// Returns additional gas costs incurred by the quote.
-    fn additional_cost(&self) -> u64 {
+    pub fn additional_cost(&self) -> u64 {
         self.signing_scheme
             .additional_gas_amount()
             .saturating_add(self.additional_gas)