From d54d2319f3a4408538b8578aef5b091ec2e24431 Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Mon, 2 Dec 2024 12:04:31 +0100 Subject: [PATCH] Sepolia fix for Cow AMMs (#3110) # Description Signature verification is now part of order creation for cow amm orders. # Changes - [ ] isValidSignature call is executed as part of creating the cow amm order ## How to test Cow amm e2e test should confirm no new issues are introduced. Fixes # https://github.com/cowprotocol/services/issues/3102 --------- Co-authored-by: MartinquaXD --- crates/cow-amm/src/amm.rs | 35 +++++++++++++++++-- .../driver/src/domain/competition/auction.rs | 20 +++++++++-- crates/driver/src/infra/blockchain/mod.rs | 4 +++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/cow-amm/src/amm.rs b/crates/cow-amm/src/amm.rs index 9bb6385d51..64c3965acf 100644 --- a/crates/cow-amm/src/amm.rs +++ b/crates/cow-amm/src/amm.rs @@ -1,13 +1,15 @@ use { - anyhow::Result, + anyhow::{Context, Result}, app_data::AppDataHash, contracts::CowAmmLegacyHelper, ethcontract::{errors::MethodError, Address, Bytes, U256}, model::{ interaction::InteractionData, order::{BuyTokenDestination, OrderData, OrderKind, SellTokenSource}, - signature::Signature, + signature::{hashed_eip712_message, Signature}, + DomainSeparator, }, + shared::signature_validator::{SignatureCheck, SignatureValidating}, }; #[derive(Clone, Debug)] @@ -25,8 +27,8 @@ impl Amm { let tradeable_tokens = helper.tokens(address).call().await?; Ok(Self { - helper: helper.clone(), address, + helper: helper.clone(), tradeable_tokens, }) } @@ -50,6 +52,33 @@ impl Amm { self.convert_orders_reponse(order, signature, pre_interactions, post_interactions) } + /// Generates a template order to rebalance the AMM but also verifies that + /// the signature is actually valid to protect against buggy helper + /// contracts. + pub async fn validated_template_order( + &self, + prices: Vec, + validator: &dyn SignatureValidating, + domain_separator: &DomainSeparator, + ) -> Result { + let template = self.template_order(prices).await?; + + // A buggy helper contract could return a signature that is actually not valid. + // To avoid issues caused by that we check the validity of the signature. + let hash = hashed_eip712_message(domain_separator, &template.order.hash_struct()); + validator + .validate_signature_and_get_additional_gas(SignatureCheck { + signer: self.address, + hash, + signature: template.signature.to_bytes(), + interactions: template.pre_interactions.clone(), + }) + .await + .context("invalid signature")?; + + Ok(template) + } + /// Converts a successful response of the CowAmmHelper into domain types. /// Can be used for any contract that correctly implements the CoW AMM /// helper interface. diff --git a/crates/driver/src/domain/competition/auction.rs b/crates/driver/src/domain/competition/auction.rs index 8764767e6f..59cddff203 100644 --- a/crates/driver/src/domain/competition/auction.rs +++ b/crates/driver/src/domain/competition/auction.rs @@ -14,6 +14,7 @@ use { futures::future::{join_all, BoxFuture, FutureExt, Shared}, itertools::Itertools, model::{order::OrderKind, signature::Signature}, + shared::signature_validator::{Contracts, SignatureValidating}, std::{ collections::{HashMap, HashSet}, sync::{Arc, Mutex}, @@ -133,6 +134,7 @@ struct Inner { /// Order sorting strategies should be in the same order as the /// `order_priority_strategies` from the driver's config. order_sorting_strategies: Vec>, + signature_validator: Arc, } type BalanceGroup = (order::Trader, eth::TokenAddress, order::SellTokenBalance); @@ -171,6 +173,7 @@ impl AuctionProcessor { let rt = tokio::runtime::Handle::current(); let tokens: Tokens = auction.tokens().clone(); + let signature_validator = lock.signature_validator.clone(); let cow_amms = auction.surplus_capturing_jit_order_owners.clone(); let mut orders = auction.orders.clone(); let solver = *solver; @@ -180,7 +183,7 @@ impl AuctionProcessor { // and we don't want to block the runtime for too long. let fut = tokio::task::spawn_blocking(move || { let start = std::time::Instant::now(); - orders.extend(rt.block_on(Self::cow_amm_orders(ð, &tokens, &cow_amms))); + orders.extend(rt.block_on(Self::cow_amm_orders(ð, &tokens, &cow_amms, signature_validator.as_ref()))); sorting::sort_orders(&mut orders, &tokens, &solver, &order_comparators); let mut balances = rt.block_on(async { Self::fetch_balances(ð, &orders).await }); @@ -329,8 +332,11 @@ impl AuctionProcessor { eth: &Ethereum, tokens: &Tokens, eligible_for_surplus: &HashSet, + signature_validator: &dyn SignatureValidating, ) -> Vec { let cow_amms = eth.contracts().cow_amm_registry().amms().await; + let domain_separator = eth.contracts().settlement_domain_separator(); + let domain_separator = model::DomainSeparator(domain_separator.0); let results: Vec<_> = futures::future::join_all( cow_amms .into_iter() @@ -356,7 +362,7 @@ impl AuctionProcessor { Some((amm, prices)) }) .map(|(cow_amm, prices)| async move { - (*cow_amm.address(), cow_amm.template_order(prices).await) + (*cow_amm.address(), cow_amm.validated_template_order(prices, signature_validator, &domain_separator).await) }), ) .await; @@ -461,11 +467,21 @@ impl AuctionProcessor { }; order_sorting_strategies.push(comparator); } + + let signature_validator = shared::signature_validator::validator( + eth.web3(), + Contracts { + settlement: eth.contracts().settlement().address(), + vault_relayer: eth.contracts().vault_relayer().0, + }, + ); + Self(Arc::new(Mutex::new(Inner { auction: Id(0), fut: futures::future::pending().boxed().shared(), eth, order_sorting_strategies, + signature_validator, }))) } } diff --git a/crates/driver/src/infra/blockchain/mod.rs b/crates/driver/src/infra/blockchain/mod.rs index 9115d203a0..e2c930a1a9 100644 --- a/crates/driver/src/infra/blockchain/mod.rs +++ b/crates/driver/src/infra/blockchain/mod.rs @@ -254,6 +254,10 @@ impl Ethereum { .ok() .map(|gas| gas.effective().0 .0) } + + pub fn web3(&self) -> &DynWeb3 { + &self.web3 + } } impl fmt::Debug for Ethereum {