Skip to content

Commit

Permalink
Sepolia fix for Cow AMMs (#3110)
Browse files Browse the repository at this point in the history
# Description
Signature verification is now part of order creation for cow amm orders.

# Changes
<!-- List of detailed changes (how the change is accomplished) -->

- [ ] 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 #
#3102

---------

Co-authored-by: MartinquaXD <[email protected]>
  • Loading branch information
sunce86 and MartinquaXD authored Dec 2, 2024
1 parent 2fec574 commit d54d231
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 5 deletions.
35 changes: 32 additions & 3 deletions crates/cow-amm/src/amm.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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,
})
}
Expand All @@ -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<U256>,
validator: &dyn SignatureValidating,
domain_separator: &DomainSeparator,
) -> Result<TemplateOrder> {
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.
Expand Down
20 changes: 18 additions & 2 deletions crates/driver/src/domain/competition/auction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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<Arc<dyn sorting::SortingStrategy>>,
signature_validator: Arc<dyn SignatureValidating>,
}

type BalanceGroup = (order::Trader, eth::TokenAddress, order::SellTokenBalance);
Expand Down Expand Up @@ -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;
Expand All @@ -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(&eth, &tokens, &cow_amms)));
orders.extend(rt.block_on(Self::cow_amm_orders(&eth, &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(&eth, &orders).await });
Expand Down Expand Up @@ -329,8 +332,11 @@ impl AuctionProcessor {
eth: &Ethereum,
tokens: &Tokens,
eligible_for_surplus: &HashSet<eth::Address>,
signature_validator: &dyn SignatureValidating,
) -> Vec<Order> {
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()
Expand All @@ -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;
Expand Down Expand Up @@ -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,
})))
}
}
Expand Down
4 changes: 4 additions & 0 deletions crates/driver/src/infra/blockchain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit d54d231

Please sign in to comment.