Skip to content

Commit

Permalink
Revert cow amm mitigation (#2434)
Browse files Browse the repository at this point in the history
# Description
Now that the vulnerability was fixed on a smart contract level we no
longer need the mitigation in the backend.

# Changes
Reverts #2402
  • Loading branch information
MartinquaXD authored Feb 26, 2024
1 parent 60b7210 commit 5f013b8
Show file tree
Hide file tree
Showing 10 changed files with 3 additions and 103 deletions.
5 changes: 0 additions & 5 deletions crates/autopilot/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,6 @@ pub struct Arguments {
/// `order_events` database table.
#[clap(long, env, default_value = "30d", value_parser = humantime::parse_duration)]
pub order_events_cleanup_threshold: Duration,

#[clap(long, env, use_value_delimiter = true)]
pub cow_amms: Vec<H160>,
}

impl std::fmt::Display for Arguments {
Expand Down Expand Up @@ -262,7 +259,6 @@ impl std::fmt::Display for Arguments {
auction_update_interval,
max_settlement_transaction_wait,
s3,
cow_amms,
} = self;

write!(f, "{}", shared)?;
Expand Down Expand Up @@ -339,7 +335,6 @@ impl std::fmt::Display for Arguments {
max_settlement_transaction_wait
)?;
writeln!(f, "s3: {:?}", s3)?;
writeln!(f, "cow_amms: {:?}", cow_amms)?;
Ok(())
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/autopilot/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,6 @@ pub async fn run(args: Arguments) {
.try_into()
.expect("limit order price factor can't be converted to BigDecimal"),
domain::ProtocolFee::new(args.fee_policy.clone()),
args.cow_amms.into_iter().collect(),
);
solvable_orders_cache
.update(block)
Expand Down
47 changes: 0 additions & 47 deletions crates/autopilot/src/solvable_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use {
number::conversions::u256_to_big_decimal,
primitive_types::{H160, H256, U256},
prometheus::{IntCounter, IntCounterVec, IntGauge, IntGaugeVec},
rand::seq::SliceRandom,
shared::{
account_balances::{BalanceFetching, Query},
bad_token::BadTokenDetecting,
Expand Down Expand Up @@ -78,9 +77,6 @@ pub struct SolvableOrdersCache {
weth: H160,
limit_order_price_factor: BigDecimal,
protocol_fee: domain::ProtocolFee,
// TODO: remove ASAP since this we only use this for a mitigation that
// should be implemented on a smart contract level.
cow_amms: HashSet<H160>,
}

type Balances = HashMap<Query, U256>;
Expand All @@ -105,7 +101,6 @@ impl SolvableOrdersCache {
weth: H160,
limit_order_price_factor: BigDecimal,
protocol_fee: domain::ProtocolFee,
cow_amms: HashSet<H160>,
) -> Arc<Self> {
let self_ = Arc::new(Self {
min_order_validity_period,
Expand All @@ -123,7 +118,6 @@ impl SolvableOrdersCache {
weth,
limit_order_price_factor,
protocol_fee,
cow_amms,
});
tokio::task::spawn(
update_task(Arc::downgrade(&self_), update_interval, current_block)
Expand Down Expand Up @@ -159,10 +153,6 @@ impl SolvableOrdersCache {
let removed = counter.checkpoint("invalid_signature", &orders);
invalid_order_uids.extend(removed);

let orders = filter_duplicate_cow_amm_orders(orders, &self.cow_amms);
let removed = counter.checkpoint("duplicate_cow_amm_order", &orders);
invalid_order_uids.extend(removed);

let orders = filter_unsupported_tokens(orders, self.bad_token_detector.as_ref()).await?;
let removed = counter.checkpoint("unsupported_token", &orders);
invalid_order_uids.extend(removed);
Expand Down Expand Up @@ -529,43 +519,6 @@ async fn filter_unsupported_tokens(
Ok(orders)
}

/// Enforces that for all CoW AMMs at most 1 order is in the auction.
/// This is needed to protect against a known attack vector.
fn filter_duplicate_cow_amm_orders(mut orders: Vec<Order>, cow_amms: &HashSet<H160>) -> Vec<Order> {
let mut amm_orders = HashMap::<H160, Vec<&Order>>::new();
for order in &orders {
let owner = order.metadata.owner;
if cow_amms.contains(&owner) {
amm_orders.entry(owner).or_default().push(order);
}
}
let canonical_amm_orders: HashSet<OrderUid> = amm_orders
.into_iter()
.map(|(owner, orders)| {
let uid = orders
.choose(&mut rand::thread_rng())
.expect("every group contains at least 1 order")
.metadata
.uid;
if orders.len() > 1 {
// TODO: find better heuristic to pick "canonical" order
tracing::warn!(
num = orders.len(),
amm = ?owner,
order = ?uid,
"multiple orders for the same CoW AMM; picked random order"
);
}
uid
})
.collect();

orders.retain(|o| {
!cow_amms.contains(&o.metadata.owner) || canonical_amm_orders.contains(&o.metadata.uid)
});
orders
}

/// Filter out limit orders which are far enough outside the estimated native
/// token price.
fn filter_mispriced_limit_orders(
Expand Down
32 changes: 0 additions & 32 deletions crates/driver/src/domain/competition/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,38 +103,6 @@ impl Competition {
}
});

let solutions = solutions.filter(|solution| {
let mut amm_orders = HashSet::<eth::Address>::new();

for trade in solution.trades() {
let solution = solution.id();

let owner = match trade {
solution::Trade::Fulfillment(f) => {
let uid = f.order().uid;
let Some(order) = auction.orders().iter().find(|o| o.uid == uid) else {
tracing::warn!(?uid, ?solution, "order not in original auction");
return false;
};
order.trader().0
}
solution::Trade::Jit(j) => j.order().signature.signer,
};

let is_cow_amm_order = self.eth.contracts().cow_amms().contains(&owner);
if is_cow_amm_order && !amm_orders.insert(owner) {
tracing::warn!(
amm = ?owner,
?solution,
"solution contains more than 1 order for the same CoW AMM"
);
return false;
}
}

true
});

// Encode solutions into settlements (streamed).
let encoded = solutions
.map(|solution| async move {
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/domain/competition/order/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ pub enum BuyTokenBalance {

/// The address which placed the order.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct Trader(pub eth::Address);
pub struct Trader(eth::Address);

impl From<Trader> for eth::Address {
fn from(value: Trader) -> Self {
Expand Down
10 changes: 1 addition & 9 deletions crates/driver/src/infra/blockchain/contracts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use {
crate::{domain::eth, infra::blockchain::Ethereum},
ethcontract::dyns::DynWeb3,
std::collections::HashSet,
thiserror::Error,
};

Expand All @@ -11,14 +10,12 @@ pub struct Contracts {
vault_relayer: eth::ContractAddress,
vault: contracts::BalancerV2Vault,
weth: contracts::WETH9,
cow_amms: HashSet<eth::Address>,
}

#[derive(Debug, Default, Clone)]
#[derive(Debug, Default, Clone, Copy)]
pub struct Addresses {
pub settlement: Option<eth::ContractAddress>,
pub weth: Option<eth::ContractAddress>,
pub cow_amms: Option<HashSet<eth::Address>>,
}

impl Contracts {
Expand Down Expand Up @@ -56,7 +53,6 @@ impl Contracts {
vault_relayer,
vault,
weth,
cow_amms: addresses.cow_amms.unwrap_or_default(),
})
}

Expand All @@ -79,10 +75,6 @@ impl Contracts {
pub fn weth_address(&self) -> eth::WethAddress {
self.weth.address().into()
}

pub fn cow_amms(&self) -> &HashSet<eth::Address> {
&self.cow_amms
}
}

/// Returns the address of a contract for the specified network, or `None` if
Expand Down
4 changes: 0 additions & 4 deletions crates/driver/src/infra/config/file/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,6 @@ pub async fn load(chain: eth::ChainId, path: &Path) -> infra::Config {
contracts: blockchain::contracts::Addresses {
settlement: config.contracts.gp_v2_settlement.map(Into::into),
weth: config.contracts.weth.map(Into::into),
cow_amms: config
.contracts
.cow_amms
.map(|contracts| contracts.into_iter().map(eth::Address).collect()),
},
disable_access_list_simulation: config.disable_access_list_simulation,
disable_gas_simulation: config.disable_gas_simulation.map(Into::into),
Expand Down
2 changes: 0 additions & 2 deletions crates/driver/src/infra/config/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,6 @@ struct ContractsConfig {

/// Override the default address of the WETH contract.
weth: Option<eth::H160>,

cow_amms: Option<Vec<eth::H160>>,
}

#[derive(Debug, Deserialize)]
Expand Down
2 changes: 1 addition & 1 deletion crates/driver/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ async fn ethereum(config: &infra::Config, ethrpc: blockchain::Rpc) -> Ethereum {
.await
.expect("initialize gas price estimator"),
);
Ethereum::new(ethrpc, config.contracts.clone(), gas).await
Ethereum::new(ethrpc, config.contracts, gas).await
}

fn solvers(config: &config::Config, eth: &Ethereum) -> Vec<Solver> {
Expand Down
1 change: 0 additions & 1 deletion crates/driver/src/tests/setup/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ impl Solver {
Addresses {
settlement: Some(config.blockchain.settlement.address().into()),
weth: Some(config.blockchain.weth.address().into()),
cow_amms: Some(Default::default()),
},
gas,
)
Expand Down

0 comments on commit 5f013b8

Please sign in to comment.