From b0cb12e5b9f6f250320ab981c60c4507fe12a196 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Wed, 20 Mar 2024 21:27:03 +0100 Subject: [PATCH] Reject orders using too much gas --- crates/e2e/tests/e2e/hooks.rs | 60 ++++++++++++++++++++++++++ crates/orderbook/openapi.yml | 1 + crates/orderbook/src/api/post_order.rs | 4 ++ crates/orderbook/src/arguments.rs | 6 +++ crates/orderbook/src/run.rs | 1 + crates/shared/src/order_validation.rs | 26 +++++++++++ 6 files changed, 98 insertions(+) diff --git a/crates/e2e/tests/e2e/hooks.rs b/crates/e2e/tests/e2e/hooks.rs index 3d9bd939dc..753c0c313b 100644 --- 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 index 88ec1e3ff8..d168889dad 100644 --- 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 index cf68214b49..23a5a4de9f 100644 --- 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 index 0ba214617f..c33442c178 100644 --- 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 index 40bd5aad0e..847aa5d86a 100644 --- 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 index f7939b8b93..3eae44dfd3 100644 --- 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>, + max_gas_per_order: u64, } #[derive(Debug, Eq, PartialEq, Default)] @@ -326,6 +328,7 @@ impl OrderValidator { code_fetcher: Arc, app_data_validator: crate::app_data::Validator, market_orders_deprecation_date: Option>, + 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 {