Reject orders using too much gas

cowprotocol · Mar 20, 2024 · b0cb12e · b0cb12e
1 parent d12f0ca
commit b0cb12e
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 0 deletions.
diff --git a/crates/e2e/tests/e2e/hooks.rs b/crates/e2e/tests/e2e/hooks.rs
@@ -10,6 +10,7 @@ use {
         order::{Hook, OrderCreation, OrderCreationAppData, OrderKind},
         signature::{hashed_eip712_message, EcdsaSigningScheme, Signature},
     },
+    reqwest::StatusCode,
     secp256k1::SecretKey,
     serde_json::json,
     shared::ethrpc::Web3,
@@ -34,6 +35,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: 1_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;
 

diff --git a/crates/orderbook/openapi.yml b/crates/orderbook/openapi.yml
@@ -1187,6 +1187,7 @@ components:
               ZeroAmount,
               IncompatibleSigningScheme,
               TooManyLimitOrders,
+              TooMuchGas,
               UnsupportedBuyTokenDestination,
               UnsupportedSellTokenSource,
               UnsupportedOrderType,

diff --git a/crates/orderbook/src/api/post_order.rs b/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");

diff --git a/crates/orderbook/src/arguments.rs b/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(())
     }

diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs
@@ -455,6 +455,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()),
     );

diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs
@@ -164,6 +164,7 @@ pub enum ValidationError {
     ZeroAmount,
     IncompatibleSigningScheme,
     TooManyLimitOrders,
+    TooMuchGas,
     Other(anyhow::Error),
 }
 
@@ -255,6 +256,7 @@ pub struct OrderValidator {
     app_data_validator: crate::app_data::Validator,
     request_verified_quotes: bool,
     market_orders_deprecation_date: Option<chrono::DateTime<chrono::Utc>>,
+    max_gas_per_order: u64,
 }
 
 #[derive(Debug, Eq, PartialEq, Default)]
@@ -326,6 +328,7 @@ impl OrderValidator {
         code_fetcher: Arc<dyn CodeFetching>,
         app_data_validator: crate::app_data::Validator,
         market_orders_deprecation_date: Option<chrono::DateTime<chrono::Utc>>,
+        max_gas_per_order: u64,
     ) -> Self {
         Self {
             native_token,
@@ -343,6 +346,7 @@ impl OrderValidator {
             app_data_validator,
             request_verified_quotes: false,
             market_orders_deprecation_date,
+            max_gas_per_order,
         }
     }
 
@@ -728,6 +732,14 @@ impl OrderValidating for OrderValidator {
             }
         };
 
+        if quote.as_ref().is_some_and(|quote| {
+            // Quoted gas does not include additional gas for hooks
+            quote.data.fee_parameters.gas_amount as u64 + quote_parameters.additional_gas
+                > self.max_gas_per_order
+        }) {
+            return Err(ValidationError::TooMuchGas);
+        }
+
         let order = Order {
             metadata: OrderMetadata {
                 owner,
@@ -1061,6 +1073,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let result = validator
             .partial_validate(PreOrderData {
@@ -1208,6 +1221,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = || PreOrderData {
             valid_to: time::now_in_epoch_seconds()
@@ -1296,6 +1310,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
 
         let creation = OrderCreation {
@@ -1500,6 +1515,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
 
         let creation = OrderCreation {
@@ -1571,6 +1587,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
 
         let creation = OrderCreation {
@@ -1627,6 +1644,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -1685,6 +1703,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -1742,6 +1761,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -1794,6 +1814,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -1848,6 +1869,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -1906,6 +1928,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -1958,6 +1981,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
         let order = OrderCreation {
             valid_to: time::now_in_epoch_seconds() + 2,
@@ -2014,6 +2038,7 @@ mod tests {
             Arc::new(MockCodeFetching::new()),
             Default::default(),
             None,
+            u64::MAX,
         );
 
         let creation = OrderCreation {
@@ -2077,6 +2102,7 @@ mod tests {
                 Arc::new(MockCodeFetching::new()),
                 Default::default(),
                 None,
+                u64::MAX,
             );
 
             let order = OrderCreation {