From d13627dc3538e1d596bd8d667a010ca516ae5b34 Mon Sep 17 00:00:00 2001 From: Martin Magnus Date: Thu, 9 Jan 2025 16:11:55 +0100 Subject: [PATCH 01/11] Revert executing `eth_estimateGas` and `eth_createAccessList` on the latest block (#3227) While this PR would give us revert detection reproducible with tenderly it also leads to an overly optimistic revert protection. Meaning on average future reverts get detected too late which would result in solvers losing a lot of money. This was noticed by an external solver running the original PR in production and seeing their rewards vanish due to increased reverts. See this [thread](https://cowservices.slack.com/archives/C0690QX4R5X/p1736368423899369?thread_ts=1736279159.869429&cid=C0690QX4R5X) for reference. --- crates/driver/src/infra/blockchain/mod.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/crates/driver/src/infra/blockchain/mod.rs b/crates/driver/src/infra/blockchain/mod.rs index 6b85a00b08..a43a037bfe 100644 --- a/crates/driver/src/infra/blockchain/mod.rs +++ b/crates/driver/src/infra/blockchain/mod.rs @@ -182,10 +182,7 @@ impl Ethereum { .transport() .execute( "eth_createAccessList", - vec![ - serde_json::to_value(&tx).unwrap(), - serde_json::Value::String("latest".into()), - ], + vec![serde_json::to_value(&tx).unwrap()], ) .await?; if let Some(err) = json.get("error") { @@ -210,7 +207,7 @@ impl Ethereum { gas_price: self.simulation_gas_price().await, ..Default::default() }, - Some(ethcontract::BlockNumber::Latest), + None, ) .await .map(Into::into) From 162ebc47f79b3ec2f91a35e62a63fbcad42cfa94 Mon Sep 17 00:00:00 2001 From: ilya Date: Thu, 9 Jan 2025 16:55:34 +0000 Subject: [PATCH 02/11] Support rust 1.84.0 (#3229) Fixes compilation on the latest stable rust version. --- crates/autopilot/src/database/onchain_order_events/mod.rs | 4 ++-- .../src/sources/balancer_v2/swap/fixed_point/logexpmath.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/autopilot/src/database/onchain_order_events/mod.rs b/crates/autopilot/src/database/onchain_order_events/mod.rs index 9ae6051493..bcbabd2ebf 100644 --- a/crates/autopilot/src/database/onchain_order_events/mod.rs +++ b/crates/autopilot/src/database/onchain_order_events/mod.rs @@ -435,8 +435,8 @@ type GeneralOnchainOrderPlacementData = ( OnchainOrderPlacement, Order, ); -async fn parse_general_onchain_order_placement_data<'a>( - quoter: &'a dyn OrderQuoting, +async fn parse_general_onchain_order_placement_data( + quoter: &'_ dyn OrderQuoting, order_placement_events_and_quotes_zipped: Vec<(EthContractEvent, i64, i64)>, domain_separator: DomainSeparator, settlement_contract: H160, diff --git a/crates/shared/src/sources/balancer_v2/swap/fixed_point/logexpmath.rs b/crates/shared/src/sources/balancer_v2/swap/fixed_point/logexpmath.rs index 302053b7f6..15375bca70 100644 --- a/crates/shared/src/sources/balancer_v2/swap/fixed_point/logexpmath.rs +++ b/crates/shared/src/sources/balancer_v2/swap/fixed_point/logexpmath.rs @@ -223,8 +223,8 @@ mod tests { constant_x: fn(u32) -> I256, constant_a: fn(u32) -> I256, ) { + let re = Regex::new(r".* ([ax])([\d]+) = (\d+);.*$").unwrap(); for line in code.split('\n').filter(|line| !line.trim().is_empty()) { - let re = Regex::new(r".* ([ax])([\d]+) = (\d+);.*$").unwrap(); let cap = re.captures_iter(line).next().unwrap(); match &cap[1] { "x" => assert_eq!(&cap[3], format!("{}", constant_x(cap[2].parse().unwrap()))), From b46fc728dde8fc648108f117c3e13c84481f4dd5 Mon Sep 17 00:00:00 2001 From: ilya Date: Thu, 9 Jan 2025 17:18:10 +0000 Subject: [PATCH 03/11] Bad token detector metrics (#3228) # Description Adds Prometheus metrics for driver's bad token detector strategies, so it will be possible to show how many tokens were marked as `bad` by each solver and strategy. It would be helpful to identify solvers that perform as not expected and to analyze the work of each detection strategy individually. ## How to test Create 2 grafana panels: the one that shows detected tokens by solver and another - by strategy. Added `hotfix` label to deploy it asap. --- .../domain/competition/bad_tokens/metrics.rs | 20 +++++++++++++++++-- .../competition/bad_tokens/simulation.rs | 4 +++- crates/driver/src/infra/api/mod.rs | 1 + crates/driver/src/infra/observe/metrics.rs | 3 +++ crates/driver/src/infra/observe/mod.rs | 2 +- 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/crates/driver/src/domain/competition/bad_tokens/metrics.rs b/crates/driver/src/domain/competition/bad_tokens/metrics.rs index c6951fbea6..daaad1ee7c 100644 --- a/crates/driver/src/domain/competition/bad_tokens/metrics.rs +++ b/crates/driver/src/domain/competition/bad_tokens/metrics.rs @@ -1,6 +1,9 @@ use { super::Quality, - crate::domain::eth, + crate::{ + domain::eth, + infra::{observe::metrics, solver}, + }, dashmap::DashMap, std::{ sync::Arc, @@ -27,6 +30,7 @@ pub struct Detector { counter: Arc>, log_only: bool, token_freeze_time: Duration, + solver: solver::Name, } impl Detector { @@ -35,6 +39,7 @@ impl Detector { required_measurements: u32, log_only: bool, token_freeze_time: Duration, + solver: solver::Name, ) -> Self { Self { failure_ratio, @@ -42,6 +47,7 @@ impl Detector { counter: Default::default(), log_only, token_freeze_time, + solver, } } @@ -120,6 +126,10 @@ impl Detector { tokens = ?new_unsupported_tokens, "mark tokens as unsupported" ); + metrics::get() + .bad_tokens_detected + .with_label_values(&[&self.solver.0, "metrics"]) + .inc_by(new_unsupported_tokens.len() as u64); } } } @@ -133,7 +143,13 @@ mod tests { #[tokio::test] async fn unfreeze_bad_tokens() { const FREEZE_DURATION: Duration = Duration::from_millis(50); - let detector = Detector::new(0.5, 2, false, FREEZE_DURATION); + let detector = Detector::new( + 0.5, + 2, + false, + FREEZE_DURATION, + solver::Name("mysolver".to_string()), + ); let token_a = eth::TokenAddress(eth::ContractAddress(H160([1; 20]))); let token_b = eth::TokenAddress(eth::ContractAddress(H160([2; 20]))); diff --git a/crates/driver/src/domain/competition/bad_tokens/simulation.rs b/crates/driver/src/domain/competition/bad_tokens/simulation.rs index dd8eb5e0f4..9b593b8726 100644 --- a/crates/driver/src/domain/competition/bad_tokens/simulation.rs +++ b/crates/driver/src/domain/competition/bad_tokens/simulation.rs @@ -8,7 +8,7 @@ use { }, eth, }, - infra, + infra::{self, observe::metrics}, }, futures::FutureExt, model::interaction::InteractionData, @@ -100,6 +100,8 @@ impl Detector { } Ok(TokenQuality::Bad { reason }) => { tracing::debug!(reason, token=?sell_token.0, "cache token as unsupported"); + // All solvers share the same cache for the simulation detector, so there is no need to specify the solver name here. + metrics::get().bad_tokens_detected.with_label_values(&["any", "simulation"]).inc(); inner .cache .update_quality(sell_token, false, now); diff --git a/crates/driver/src/infra/api/mod.rs b/crates/driver/src/infra/api/mod.rs index de7897cfa8..ccbdc5a178 100644 --- a/crates/driver/src/infra/api/mod.rs +++ b/crates/driver/src/infra/api/mod.rs @@ -84,6 +84,7 @@ impl Api { bad_token_config.metrics_strategy_required_measurements, bad_token_config.metrics_strategy_log_only, bad_token_config.metrics_strategy_token_freeze_time, + name.clone(), )); } diff --git a/crates/driver/src/infra/observe/metrics.rs b/crates/driver/src/infra/observe/metrics.rs index 330e250867..237206c668 100644 --- a/crates/driver/src/infra/observe/metrics.rs +++ b/crates/driver/src/infra/observe/metrics.rs @@ -19,6 +19,9 @@ pub struct Metrics { /// The results of the mempool submission. #[metric(labels("mempool", "result"))] pub mempool_submission: prometheus::IntCounterVec, + /// How many tokens detected by specific solver and strategy. + #[metric(labels("solver", "strategy"))] + pub bad_tokens_detected: prometheus::IntCounterVec, } /// Setup the metrics registry. diff --git a/crates/driver/src/infra/observe/mod.rs b/crates/driver/src/infra/observe/mod.rs index 0dff50d918..9714f80264 100644 --- a/crates/driver/src/infra/observe/mod.rs +++ b/crates/driver/src/infra/observe/mod.rs @@ -28,7 +28,7 @@ use { url::Url, }; -mod metrics; +pub mod metrics; /// Setup the observability. The log argument configures the tokio tracing /// framework. From d1972cde1eff3a7fd8fc07a2e4c95a91b4543481 Mon Sep 17 00:00:00 2001 From: ilya Date: Thu, 9 Jan 2025 17:55:52 +0000 Subject: [PATCH 04/11] [TRIVIAL] Hotfix GH job typo fix (#3230) There was a typo in the GH token name. --- .github/workflows/hotfix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/hotfix.yml b/.github/workflows/hotfix.yml index 5a71a19b07..bd65cd97b7 100644 --- a/.github/workflows/hotfix.yml +++ b/.github/workflows/hotfix.yml @@ -17,7 +17,7 @@ jobs: - name: Check out uses: actions/checkout@v4 with: - token: "${{ secrets.HOTFIX_ACTION_JOB }}" + token: "${{ secrets.HOTFIX_ACTION_TOKEN }}" fetch-depth: 0 - name: Configure git From d9a7621a37a2377f76884613d787cad81aaa0744 Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Fri, 10 Jan 2025 10:59:28 +0100 Subject: [PATCH 05/11] Remove full_fee_amount and solver_fee (#3223) # Description As part of fee breakdown refactor, the time has come to remove `full_fee_amount` and `solver_fee` from order API. These fields were relevant more than 2 years ago when orders with signed fee > 0 existed and when subsidies for that fee were calculated. Will wait for confirmation from frontend that these fields are no longer used anywhere, before merging. # Changes - [ ] Removed fields from the API and all dependent code paths. ## How to test Existing unit tests, e2e tests, postgresql tests. --- .../src/database/onchain_order_events/mod.rs | 3 --- crates/database/src/jit_orders.rs | 2 +- crates/database/src/orders.rs | 13 ++------- crates/model/src/order.rs | 27 ------------------- crates/orderbook/openapi.yml | 4 --- crates/orderbook/src/database/orders.rs | 7 ----- crates/shared/src/db_order_conversions.rs | 13 --------- crates/shared/src/order_validation.rs | 1 - crates/shared/src/remaining_amounts.rs | 7 ----- .../solver/src/liquidity/order_converter.rs | 1 - database/README.md | 1 - database/sql/V078__drop_full_fee_amount.sql | 2 ++ 12 files changed, 5 insertions(+), 76 deletions(-) create mode 100644 database/sql/V078__drop_full_fee_amount.sql diff --git a/crates/autopilot/src/database/onchain_order_events/mod.rs b/crates/autopilot/src/database/onchain_order_events/mod.rs index bcbabd2ebf..d96f740983 100644 --- a/crates/autopilot/src/database/onchain_order_events/mod.rs +++ b/crates/autopilot/src/database/onchain_order_events/mod.rs @@ -616,7 +616,6 @@ fn convert_onchain_order_placement( settlement_contract: ByteArray(settlement_contract.0), sell_token_balance: sell_token_source_into(order_data.sell_token_balance), buy_token_balance: buy_token_destination_into(order_data.buy_token_balance), - full_fee_amount: u256_to_big_decimal(&order_data.fee_amount), cancellation_timestamp: None, class: match order_data.fee_amount.is_zero() { true => OrderClass::Limit, @@ -925,7 +924,6 @@ mod test { settlement_contract: ByteArray(settlement_contract.0), sell_token_balance: sell_token_source_into(expected_order_data.sell_token_balance), buy_token_balance: buy_token_destination_into(expected_order_data.buy_token_balance), - full_fee_amount: u256_to_big_decimal(&expected_order_data.fee_amount), cancellation_timestamp: None, }; assert_eq!(onchain_order_placement, expected_onchain_order_placement); @@ -1036,7 +1034,6 @@ mod test { settlement_contract: ByteArray(settlement_contract.0), sell_token_balance: sell_token_source_into(expected_order_data.sell_token_balance), buy_token_balance: buy_token_destination_into(expected_order_data.buy_token_balance), - full_fee_amount: u256_to_big_decimal(&U256::zero()), cancellation_timestamp: None, }; assert_eq!(onchain_order_placement, expected_onchain_order_placement); diff --git a/crates/database/src/jit_orders.rs b/crates/database/src/jit_orders.rs index b17453d6a1..ef77a5ed04 100644 --- a/crates/database/src/jit_orders.rs +++ b/crates/database/src/jit_orders.rs @@ -18,7 +18,7 @@ use { pub const SELECT: &str = r#" o.uid, o.owner, o.creation_timestamp, o.sell_token, o.buy_token, o.sell_amount, o.buy_amount, -o.valid_to, o.app_data, o.fee_amount, o.fee_amount AS full_fee_amount, o.kind, o.partially_fillable, o.signature, +o.valid_to, o.app_data, o.fee_amount, o.kind, o.partially_fillable, o.signature, o.receiver, o.signing_scheme, '\x9008d19f58aabd9ed0d60971565aa8510560ab41'::bytea AS settlement_contract, o.sell_token_balance, o.buy_token_balance, 'liquidity'::OrderClass AS class, (SELECT COALESCE(SUM(t.buy_amount), 0) FROM trades t WHERE t.order_uid = o.uid) AS sum_buy, diff --git a/crates/database/src/orders.rs b/crates/database/src/orders.rs index a0c4d11da9..ba3b663e80 100644 --- a/crates/database/src/orders.rs +++ b/crates/database/src/orders.rs @@ -96,7 +96,6 @@ pub struct Order { pub settlement_contract: Address, pub sell_token_balance: SellTokenSource, pub buy_token_balance: BuyTokenDestination, - pub full_fee_amount: BigDecimal, pub cancellation_timestamp: Option>, pub class: OrderClass, } @@ -140,11 +139,10 @@ INSERT INTO orders ( settlement_contract, sell_token_balance, buy_token_balance, - full_fee_amount, cancellation_timestamp, class ) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20) "#; pub async fn insert_order_and_ignore_conflicts( @@ -183,7 +181,6 @@ async fn insert_order_execute_sqlx( .bind(order.settlement_contract) .bind(order.sell_token_balance) .bind(order.buy_token_balance) - .bind(&order.full_fee_amount) .bind(order.cancellation_timestamp) .bind(order.class) .execute(ex) @@ -473,7 +470,6 @@ pub struct FullOrder { pub valid_to: i64, pub app_data: AppId, pub fee_amount: BigDecimal, - pub full_fee_amount: BigDecimal, pub kind: OrderKind, pub class: OrderClass, pub partially_fillable: bool, @@ -547,7 +543,7 @@ impl FullOrder { // that with the current amount of data this wouldn't be better. pub const SELECT: &str = r#" o.uid, o.owner, o.creation_timestamp, o.sell_token, o.buy_token, o.sell_amount, o.buy_amount, -o.valid_to, o.app_data, o.fee_amount, o.full_fee_amount, o.kind, o.partially_fillable, o.signature, +o.valid_to, o.app_data, o.fee_amount, o.kind, o.partially_fillable, o.signature, o.receiver, o.signing_scheme, o.settlement_contract, o.sell_token_balance, o.buy_token_balance, o.class, (SELECT COALESCE(SUM(t.buy_amount), 0) FROM trades t WHERE t.order_uid = o.uid) AS sum_buy, @@ -880,7 +876,6 @@ mod tests { assert_eq!(order.valid_to, full_order.valid_to); assert_eq!(order.app_data, full_order.app_data); assert_eq!(order.fee_amount, full_order.fee_amount); - assert_eq!(order.full_fee_amount, full_order.full_fee_amount); assert_eq!(order.kind, full_order.kind); assert_eq!(order.class, full_order.class); assert_eq!(order.partially_fillable, full_order.partially_fillable); @@ -1364,10 +1359,6 @@ mod tests { order_with_quote.full_order.fee_amount, full_order.fee_amount ); - assert_eq!( - order_with_quote.full_order.full_fee_amount, - full_order.full_fee_amount - ); assert_eq!(order_with_quote.full_order.kind, full_order.kind); assert_eq!(order_with_quote.full_order.class, full_order.class); assert_eq!( diff --git a/crates/model/src/order.rs b/crates/model/src/order.rs index 6aa3e4b78b..d06712d518 100644 --- a/crates/model/src/order.rs +++ b/crates/model/src/order.rs @@ -212,8 +212,6 @@ pub struct OrderData { /// as they should only ever be used to improve the price of a regular order /// and should not be settled on their own. /// This is 0 for limit orders as their fee gets taken from the surplus. - /// This is equal to `OrderMetadata::full_fee_amount` except for old orders - /// where the subsidy was applied (at the time when we used the subsidies). #[serde_as(as = "HexOrDecimalU256")] pub fee_amount: U256, pub kind: OrderKind, @@ -685,27 +683,6 @@ pub struct OrderMetadata { #[serde(flatten)] pub class: OrderClass, pub settlement_contract: H160, - /// This is `fee_amount` for liquidity orders. See comment on `fee_amount` - /// for the reasoning. - /// For market/limit orders it's the gas used of the best trade - /// execution we could find while quoting converted to an equivalent - /// `sell_token` amount. - /// Does not take partial fill into account. - /// - /// [TO BE DEPRECATED] - #[serde_as(as = "HexOrDecimalU256")] - pub full_fee_amount: U256, - /// The fee amount that should be used for objective value computations. - /// - /// This is different than the actual signed fee in that it - /// does not have any subsidies applied and may be scaled by a constant - /// factor to make matching orders more valuable from an objective value - /// perspective. - /// Does not take partial fill into account. - /// - /// [TO BE DEPRECATED] - #[serde_as(as = "HexOrDecimalU256")] - pub solver_fee: U256, #[serde(default, skip_serializing_if = "Option::is_none")] pub ethflow_data: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -1028,8 +1005,6 @@ mod tests { "executedSurplusFee": "1", "executedFee": "1", "executedFeeToken": "0x000000000000000000000000000000000000000a", - "fullFeeAmount": "115792089237316195423570985008687907853269984665640564039457584007913129639935", - "solverFee": "115792089237316195423570985008687907853269984665640564039457584007913129639935", "kind": "buy", "class": "limit", "partiallyFillable": false, @@ -1064,8 +1039,6 @@ mod tests { invalidated: true, status: OrderStatus::Open, settlement_contract: H160::from_low_u64_be(2), - full_fee_amount: U256::MAX, - solver_fee: U256::MAX, full_app_data: Some("123".to_string()), ..Default::default() }, diff --git a/crates/orderbook/openapi.yml b/crates/orderbook/openapi.yml index aa4c2da3ac..aea622a4a1 100644 --- a/crates/orderbook/openapi.yml +++ b/crates/orderbook/openapi.yml @@ -912,10 +912,6 @@ components: description: Order status. allOf: - $ref: "#/components/schemas/OrderStatus" - fullFeeAmount: - description: Amount that the signed fee would be without subsidies. - allOf: - - $ref: "#/components/schemas/TokenAmount" isLiquidityOrder: description: |- Liquidity orders are functionally the same as normal smart contract diff --git a/crates/orderbook/src/database/orders.rs b/crates/orderbook/src/database/orders.rs index 51bce8a8c6..cc12d71d93 100644 --- a/crates/orderbook/src/database/orders.rs +++ b/crates/orderbook/src/database/orders.rs @@ -207,7 +207,6 @@ async fn insert_order(order: &Order, ex: &mut PgConnection) -> Result<(), Insert settlement_contract: ByteArray(order.metadata.settlement_contract.0), sell_token_balance: sell_token_source_into(order.data.sell_token_balance), buy_token_balance: buy_token_destination_into(order.data.buy_token_balance), - full_fee_amount: u256_to_big_decimal(&order.metadata.full_fee_amount), cancellation_timestamp: None, }; @@ -632,11 +631,6 @@ fn full_order_into_model_order(order: FullOrder) -> Result { is_liquidity_order: class == OrderClass::Liquidity, class, settlement_contract: H160(order.settlement_contract.0), - full_fee_amount: big_decimal_to_u256(&order.full_fee_amount) - .context("full_fee_amount is not U256")?, - // Initialize unscaled and scale later when required. - solver_fee: big_decimal_to_u256(&order.full_fee_amount) - .context("solver_fee is not U256")?, ethflow_data, onchain_user, onchain_order_data, @@ -730,7 +724,6 @@ mod tests { valid_to: valid_to_timestamp.timestamp(), app_data: ByteArray([0; 32]), fee_amount: BigDecimal::default(), - full_fee_amount: BigDecimal::default(), kind: DbOrderKind::Sell, class: DbOrderClass::Liquidity, partially_fillable: true, diff --git a/crates/shared/src/db_order_conversions.rs b/crates/shared/src/db_order_conversions.rs index 44e2ae3320..c6e94610ff 100644 --- a/crates/shared/src/db_order_conversions.rs +++ b/crates/shared/src/db_order_conversions.rs @@ -59,18 +59,7 @@ pub fn full_order_into_model_order(order: database::orders::FullOrder) -> Result sender: onchain_user, placement_error: onchain_placement_error, }); - let full_fee_amount = - big_decimal_to_u256(&order.full_fee_amount).context("full_fee_amount is not U256")?; let fee_amount = big_decimal_to_u256(&order.fee_amount).context("fee_amount is not U256")?; - let solver_fee = match &class { - // Liquidity orders should never have a fee unless the owner bribes the protocol by setting - // one themselves. - OrderClass::Liquidity => fee_amount, - // We can't use `surplus_fee` or `fee_amount` here because those values include subsidies. - // All else being equal a solver would then prefer including an unsubsidized order over a - // subsidized one which we don't want. - OrderClass::Limit | OrderClass::Market => full_fee_amount, - }; let metadata = OrderMetadata { creation_date: order.creation_timestamp, @@ -100,8 +89,6 @@ pub fn full_order_into_model_order(order: database::orders::FullOrder) -> Result is_liquidity_order: class == OrderClass::Liquidity, class, settlement_contract: H160(order.settlement_contract.0), - full_fee_amount, - solver_fee, ethflow_data, onchain_user, onchain_order_data, diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 05b2368238..e53211bb3f 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -713,7 +713,6 @@ impl OrderValidating for OrderValidator { creation_date: chrono::offset::Utc::now(), uid, settlement_contract, - full_fee_amount: data.fee_amount, class, full_app_data: match order.app_data { OrderCreationAppData::Both { full, .. } diff --git a/crates/shared/src/remaining_amounts.rs b/crates/shared/src/remaining_amounts.rs index a9b2a32a7f..d7da34706b 100644 --- a/crates/shared/src/remaining_amounts.rs +++ b/crates/shared/src/remaining_amounts.rs @@ -196,10 +196,6 @@ mod tests { partially_fillable: false, ..Default::default() }, - metadata: OrderMetadata { - full_fee_amount: 42.into(), - ..Default::default() - }, ..Default::default() } .into(); @@ -220,7 +216,6 @@ mod tests { }, metadata: OrderMetadata { executed_sell_amount_before_fees: 0.into(), - full_fee_amount: 13.into(), ..Default::default() }, ..Default::default() @@ -243,7 +238,6 @@ mod tests { }, metadata: OrderMetadata { executed_sell_amount_before_fees: 90.into(), - full_fee_amount: 200.into(), ..Default::default() }, ..Default::default() @@ -265,7 +259,6 @@ mod tests { }, metadata: OrderMetadata { executed_buy_amount: 9_u32.into(), - full_fee_amount: 200.into(), ..Default::default() }, ..Default::default() diff --git a/crates/solver/src/liquidity/order_converter.rs b/crates/solver/src/liquidity/order_converter.rs index 5425d557be..66894bbf1d 100644 --- a/crates/solver/src/liquidity/order_converter.rs +++ b/crates/solver/src/liquidity/order_converter.rs @@ -334,7 +334,6 @@ pub mod tests { }, metadata: OrderMetadata { executed_sell_amount_before_fees: 10.into(), - solver_fee: 60.into(), ..Default::default() }, ..Default::default() diff --git a/database/README.md b/database/README.md index 8b8980b95c..f3f6eb2d80 100644 --- a/database/README.md +++ b/database/README.md @@ -270,7 +270,6 @@ Column | Type | Nullable | Details settlement\_contract | bytea | not null | address of the contract that should be used to settle this order sell\_token\_balance | [enum](#selltokensource) | not null | defines how sell\_tokens need to be transferred into the settlement contract buy\_token\_balance | [enum](#buytokendestination) | not null | defined how buy\_tokens need to be transferred back to the user - full\_fee\_amount | numeric | not null | estimated execution cost in sell\_token of this order class | [enum](#orderclass) | not null | determines which special trade semantics will apply to the execution of this order diff --git a/database/sql/V078__drop_full_fee_amount.sql b/database/sql/V078__drop_full_fee_amount.sql new file mode 100644 index 0000000000..dff60d3f43 --- /dev/null +++ b/database/sql/V078__drop_full_fee_amount.sql @@ -0,0 +1,2 @@ +ALTER TABLE orders +DROP COLUMN full_fee_amount; From 49eb712fb0265b36389af4dffc6cd4a167a53799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Strug?= <47604705+mstrug@users.noreply.github.com> Date: Mon, 13 Jan 2025 12:36:23 +0100 Subject: [PATCH 06/11] [TRIVIAL] Maintaining quote verified flag when creating an order (#3234) # Description Changed order `verified` flag to be used from quote original value when order is crated from that quote. The need for this change is that ~98% of quotes are verified successfully but most of the created `order_quotes` from that orders are not verified. ## How to test Updated e2e test to verify this particular case (it fails before applying this PR). --- crates/e2e/tests/e2e/place_order_with_quote.rs | 8 ++++---- crates/shared/src/order_quoting.rs | 4 +--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/crates/e2e/tests/e2e/place_order_with_quote.rs b/crates/e2e/tests/e2e/place_order_with_quote.rs index 09d3bb55d3..6902fced4e 100644 --- a/crates/e2e/tests/e2e/place_order_with_quote.rs +++ b/crates/e2e/tests/e2e/place_order_with_quote.rs @@ -67,6 +67,7 @@ async fn place_order_with_quote(web3: Web3) { let quote_response = services.submit_quote("e_request).await.unwrap(); tracing::debug!(?quote_response); assert!(quote_response.id.is_some()); + assert!(quote_response.verified); let quote_metadata = crate::database::quote_metadata(services.db(), quote_response.id.unwrap()).await; @@ -99,9 +100,8 @@ async fn place_order_with_quote(web3: Web3) { &database::byte_array::ByteArray(order_uid.0), ) .await + .unwrap() .unwrap(); - assert!(order_quote.is_some()); - // compare quote metadata and order quote metadata - let order_quote_metadata = order_quote.unwrap().metadata; - assert_eq!(quote_metadata.unwrap().0, order_quote_metadata); + assert_eq!(quote_response.verified, order_quote.verified); + assert_eq!(quote_metadata.unwrap().0, order_quote.metadata); } diff --git a/crates/shared/src/order_quoting.rs b/crates/shared/src/order_quoting.rs index 60a78a141b..ccad1b6cc3 100644 --- a/crates/shared/src/order_quoting.rs +++ b/crates/shared/src/order_quoting.rs @@ -195,9 +195,7 @@ impl TryFrom for QuoteData { expiration: row.expiration_timestamp, quote_kind: row.quote_kind, solver: H160(row.solver.0), - // Even if the quote was verified at the time of creation - // it might no longer be accurate. - verified: false, + verified: row.verified, metadata: row.metadata.try_into()?, }) } From 01f35f8cbc8fc45bd4b09a1907312d36ef220560 Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Tue, 14 Jan 2025 13:53:58 +0100 Subject: [PATCH 07/11] [TRIVIAL] Adjust message for breaking API changes (#3235) # Description https://github.com/cowprotocol/services/pull/3223 caused breaking the SAFE client even though they did not actually use the field. In the future, all API changes (without exception) need to be communicated with 1. Frontend team 2. Safe team # Changes - [ ] Adjusted the message --- .github/nitpicks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/nitpicks.yml b/.github/nitpicks.yml index 38c77b3a5b..b97c0f4c54 100644 --- a/.github/nitpicks.yml +++ b/.github/nitpicks.yml @@ -7,7 +7,7 @@ Reminder: Please consider backward compatibility when modifying the API specification. If breaking changes are unavoidable, ensure: - You explicitly pointed out breaking changes. - - You communicate the changes to affected teams. + - You communicate the changes to affected teams (at least Frontend team and SAFE team). - You provide proper versioning and migration mechanisms. pathFilter: - "**/openapi.yml" From 2725d5c099a3f9c0fff5bec9705f3ebbc6420ad3 Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Wed, 15 Jan 2025 13:26:06 +0100 Subject: [PATCH 08/11] Use traces to extract the input for `settlement::Observer` (#3233) # Description Implements first part of https://github.com/cowprotocol/services/issues/3177 # Changes - [ ] Uses `trace_transaction` function to get traces and extract the calldata of `settle` call ## How to test Existing tests show that no regression bugs were introduced. Final e2e test should cover this part of the code. Although by manually testing the `trace_transaction` I am pretty sure the code is correct. --- crates/autopilot/src/domain/eth/mod.rs | 11 +- crates/autopilot/src/domain/settlement/mod.rs | 287 +++++++++++++++++- .../src/domain/settlement/observer.rs | 15 +- .../src/domain/settlement/transaction/mod.rs | 48 ++- crates/autopilot/src/infra/blockchain/mod.rs | 20 +- 5 files changed, 347 insertions(+), 34 deletions(-) diff --git a/crates/autopilot/src/domain/eth/mod.rs b/crates/autopilot/src/domain/eth/mod.rs index 9b187526ba..29ca478683 100644 --- a/crates/autopilot/src/domain/eth/mod.rs +++ b/crates/autopilot/src/domain/eth/mod.rs @@ -312,6 +312,13 @@ pub struct TradeEvent { pub order_uid: domain::OrderUid, } +/// A trace of a Call type of action. +#[derive(Debug, Clone, Default)] +pub struct TraceCall { + pub to: Address, + pub input: Calldata, +} + /// Any type of on-chain transaction. #[derive(Debug, Clone, Default)] pub struct Transaction { @@ -319,8 +326,6 @@ pub struct Transaction { pub hash: TxId, /// The address of the sender of the transaction. pub from: Address, - /// The call data of the transaction. - pub input: Calldata, /// The block number of the block that contains the transaction. pub block: BlockNo, /// The timestamp of the block that contains the transaction. @@ -329,4 +334,6 @@ pub struct Transaction { pub gas: Gas, /// The effective gas price of the transaction. pub gas_price: EffectiveGasPrice, + /// Traces of all Calls contained in the transaction. + pub trace_calls: Vec, } diff --git a/crates/autopilot/src/domain/settlement/mod.rs b/crates/autopilot/src/domain/settlement/mod.rs index 7908789719..dbcb4e14c4 100644 --- a/crates/autopilot/src/domain/settlement/mod.rs +++ b/crates/autopilot/src/domain/settlement/mod.rs @@ -29,8 +29,6 @@ pub struct Settlement { gas: eth::Gas, /// The effective gas price of the settlement transaction. gas_price: eth::EffectiveGasPrice, - /// The address of the solver that submitted the settlement transaction. - solver: eth::Address, /// The block number of the block that contains the settlement transaction. block: eth::BlockNo, /// The associated auction. @@ -146,7 +144,6 @@ impl Settlement { .collect(); Ok(Self { - solver: settled.solver, block: settled.block, gas: settled.gas, gas_price: settled.gas_price, @@ -216,14 +213,246 @@ impl From for Error { #[cfg(test)] mod tests { use { - crate::{ - domain, - domain::{auction, eth}, - }, + crate::domain::{self, auction, eth}, hex_literal::hex, std::collections::{HashMap, HashSet}, }; + // https://etherscan.io/tx/0x030623e438f28446329d8f4ff84db897907fcac59b9943b31b7be66f23c877af + // A transfer transaction that emits a settlement event, but it's not actually a + // swap. + #[test] + fn not_a_swap() { + let calldata = hex!( + " + 13d79a0b + 0000000000000000000000000000000000000000000000000000000000000080 + 00000000000000000000000000000000000000000000000000000000000000a0 + 00000000000000000000000000000000000000000000000000000000000000c0 + 00000000000000000000000000000000000000000000000000000000000000e0 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000080 + 00000000000000000000000000000000000000000000000000000000000017a0 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000011 + 0000000000000000000000000000000000000000000000000000000000000220 + 0000000000000000000000000000000000000000000000000000000000000360 + 00000000000000000000000000000000000000000000000000000000000004a0 + 00000000000000000000000000000000000000000000000000000000000005e0 + 0000000000000000000000000000000000000000000000000000000000000720 + 0000000000000000000000000000000000000000000000000000000000000860 + 00000000000000000000000000000000000000000000000000000000000009a0 + 0000000000000000000000000000000000000000000000000000000000000ae0 + 0000000000000000000000000000000000000000000000000000000000000c20 + 0000000000000000000000000000000000000000000000000000000000000d60 + 0000000000000000000000000000000000000000000000000000000000000ea0 + 0000000000000000000000000000000000000000000000000000000000000fe0 + 0000000000000000000000000000000000000000000000000000000000001120 + 0000000000000000000000000000000000000000000000000000000000001260 + 00000000000000000000000000000000000000000000000000000000000013a0 + 00000000000000000000000000000000000000000000000000000000000014e0 + 0000000000000000000000000000000000000000000000000000000000001620 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000388674df3c7f96ac76c6fa06813b758322b5b64ce14bf46f0f9b4ec6f2 + d015ff9a9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000383f19e12409d5913e40bfde35a1607a1a43f1f9d26e76dd2d9d409cc7 + 50125f769008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 00000038a91a488e41e46dedbe71ae0c0d9f41be8de9f45ffc62271970362777 + 9e302e8d9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 0000003820621f141681025ffcbce03b51c3ea78c93da8739df9202210647968 + 6d2efad59008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000385846bbbcee39039678e529973cf6962b90d49663d5ff49220adfb240 + 4805d11e9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000388a608a3e6fd6f95797d376ffa35ae077d78440b627dce2b3d2278e04 + 8d37f7c09008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 00000038436b1ae2723b42982a435ede8f92033fbdf1505e5dac97c1099b5ffd + fc1debda9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 00000038c31724565adeb2d0e4d6a6980f45b9fd91bb0b858fa8c8ada300e697 + 45d1ab379008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 0000003812789b110b2bf7353d054d978dd30972f2f33e54c4c7b93a1f992d20 + fd2f29619008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 00000038f527dd3ad4938713bff3fb23a62d7b454caf40bc80f3eeed77c3d269 + d9ff8eb69008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000388a63a1ed3d671e94aafe29b9ad479340ee04ad1fb5796c73cb71b25a + e8e2b0a49008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000384acca1a9694919daee72c0f20b64a2c0103750c26ce9b25cc864609f + 2f4977269008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 00000038bdbeba9388a200bfa8c3f153c31a539622e2994a2cc59bfdc8c562a3 + 2f20b9919008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000383649cc08adbf44018756fec310e9348d64139f33530166ef53f1834f + 2f5c8c469008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 0000003864bee671badbb222a43006744c89b70a368ca34a356dcf41f755923e + aab5b4029008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 00000000000000000000000000000000000000000000000000000000000000a4 + ec6cb13f00000000000000000000000000000000000000000000000000000000 + 0000004000000000000000000000000000000000000000000000000000000000 + 0000000100000000000000000000000000000000000000000000000000000000 + 000000382285ea60762c9a58d407e5aa3b8c3287628290793bc7260b0e0324ed + 6f8bb1269008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 + 0000000000000000000000000000000000000000000000000000000000000000 + 000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2 + 0000000000000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000060 + 0000000000000000000000000000000000000000000000000000000000000044 + a9059cbb000000000000000000000000a03be496e67ec29bc62f01a428683d7f + 9c2049300000000000000000000000000000000000000000000000002136c5d9 + c1570aef00000000000000000000000000000000000000000000000000000000 + 0000000000000000000000000000000000000000000000000000000000000000" + ) + .to_vec(); + + let domain_separator = eth::DomainSeparator(hex!( + "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" + )); + let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( + "9008d19f58aabd9ed0d60971565aa8510560ab41" + ))); + let transaction = super::transaction::Transaction::try_new( + &domain::eth::Transaction { + trace_calls: vec![domain::eth::TraceCall { + to: settlement_contract, + input: calldata.into(), + }], + ..Default::default() + }, + &domain_separator, + settlement_contract, + ) + .unwrap_err(); + + // These transfer transactions don't have the auction_id attached so overall bad + // calldata is expected + assert!(matches!( + transaction, + super::transaction::Error::Decoding(_) + )); + } + // https://etherscan.io/tx/0xc48dc0d43ffb43891d8c3ad7bcf05f11465518a2610869b20b0b4ccb61497634 #[test] fn settlement() { @@ -306,12 +535,19 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let transaction = super::transaction::Transaction::new( + let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( + "9008d19f58aabd9ed0d60971565aa8510560ab41" + ))); + let transaction = super::transaction::Transaction::try_new( &domain::eth::Transaction { - input: calldata.into(), + trace_calls: vec![domain::eth::TraceCall { + to: settlement_contract, + input: calldata.into(), + }], ..Default::default() }, &domain_separator, + settlement_contract, ) .unwrap(); @@ -442,12 +678,19 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let transaction = super::transaction::Transaction::new( + let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( + "9008d19f58aabd9ed0d60971565aa8510560ab41" + ))); + let transaction = super::transaction::Transaction::try_new( &domain::eth::Transaction { - input: calldata.into(), + trace_calls: vec![domain::eth::TraceCall { + to: settlement_contract, + input: calldata.into(), + }], ..Default::default() }, &domain_separator, + settlement_contract, ) .unwrap(); @@ -610,12 +853,19 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let transaction = super::transaction::Transaction::new( + let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( + "9008d19f58aabd9ed0d60971565aa8510560ab41" + ))); + let transaction = super::transaction::Transaction::try_new( &domain::eth::Transaction { - input: calldata.into(), + trace_calls: vec![domain::eth::TraceCall { + to: settlement_contract, + input: calldata.into(), + }], ..Default::default() }, &domain_separator, + settlement_contract, ) .unwrap(); @@ -784,12 +1034,19 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let transaction = super::transaction::Transaction::new( + let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( + "9008d19f58aabd9ed0d60971565aa8510560ab41" + ))); + let transaction = super::transaction::Transaction::try_new( &domain::eth::Transaction { - input: calldata.into(), + trace_calls: vec![domain::eth::TraceCall { + to: settlement_contract, + input: calldata.into(), + }], ..Default::default() }, &domain_separator, + settlement_contract, ) .unwrap(); diff --git a/crates/autopilot/src/domain/settlement/observer.rs b/crates/autopilot/src/domain/settlement/observer.rs index 631539f9ef..d142646728 100644 --- a/crates/autopilot/src/domain/settlement/observer.rs +++ b/crates/autopilot/src/domain/settlement/observer.rs @@ -66,7 +66,8 @@ impl Observer { let transaction = match self.eth.transaction(event.transaction).await { Ok(transaction) => { let separator = self.eth.contracts().settlement_domain_separator(); - settlement::Transaction::new(&transaction, separator) + let settlement_contract = self.eth.contracts().settlement().address().into(); + settlement::Transaction::try_new(&transaction, separator, settlement_contract) } Err(err) => { tracing::warn!(hash = ?event.transaction, ?err, "no tx found"); @@ -95,7 +96,17 @@ impl Observer { (auction_id, settlement) } Err(err) => { - tracing::warn!(hash = ?event.transaction, ?err, "invalid settlement transaction"); + match err { + settlement::transaction::Error::MissingCalldata => { + tracing::error!(hash = ?event.transaction, ?err, "invalid settlement transaction") + } + settlement::transaction::Error::MissingAuctionId + | settlement::transaction::Error::Decoding(_) + | settlement::transaction::Error::SignatureRecover(_) + | settlement::transaction::Error::OrderUidRecover(_) => { + tracing::warn!(hash = ?event.transaction, ?err, "invalid settlement transaction") + } + } // default values so we don't get stuck on invalid settlement transactions (0.into(), None) } diff --git a/crates/autopilot/src/domain/settlement/transaction/mod.rs b/crates/autopilot/src/domain/settlement/transaction/mod.rs index 3b0e3f4540..d5877d00db 100644 --- a/crates/autopilot/src/domain/settlement/transaction/mod.rs +++ b/crates/autopilot/src/domain/settlement/transaction/mod.rs @@ -1,6 +1,10 @@ -use crate::{ - boundary, - domain::{self, auction::order, eth}, +use { + crate::{ + boundary, + domain::{self, auction::order, eth}, + }, + ethcontract::common::FunctionExt, + std::sync::LazyLock, }; mod tokenized; @@ -12,8 +16,6 @@ pub struct Transaction { pub hash: eth::TxId, /// The associated auction id. pub auction_id: domain::auction::Id, - /// The address of the solver that submitted the transaction. - pub solver: eth::Address, /// The block number of the block that contains the transaction. pub block: eth::BlockNo, /// The timestamp of the block that contains the transaction. @@ -27,18 +29,33 @@ pub struct Transaction { } impl Transaction { - pub fn new( + pub fn try_new( transaction: ð::Transaction, domain_separator: ð::DomainSeparator, + settlement_contract: eth::Address, ) -> Result { + // find trace call to settlement contract + let calldata = transaction + .trace_calls + .iter() + .find(|trace| is_settlement_trace(trace, settlement_contract)) + .map(|trace| trace.input.clone()) + // all transactions emitting settlement events should have a /settle call, + // otherwise it's an execution client bug + .ok_or(Error::MissingCalldata)?; + /// Number of bytes that may be appended to the calldata to store an /// auction id. const META_DATA_LEN: usize = 8; - let (data, metadata) = transaction - .input - .0 - .split_at(transaction.input.0.len() - META_DATA_LEN); + let (data, metadata) = calldata.0.split_at( + calldata + .0 + .len() + .checked_sub(META_DATA_LEN) + // should contain at least 4 bytes for function selector and 8 bytes for auction id + .ok_or(Error::MissingCalldata)?, + ); let metadata: Option<[u8; META_DATA_LEN]> = metadata.try_into().ok(); let auction_id = metadata .map(crate::domain::auction::Id::from_be_bytes) @@ -46,7 +63,6 @@ impl Transaction { Ok(Self { hash: transaction.hash, auction_id, - solver: transaction.from, block: transaction.block, timestamp: transaction.timestamp, gas: transaction.gas, @@ -116,6 +132,14 @@ impl Transaction { } } +fn is_settlement_trace(trace: ð::TraceCall, settlement_contract: eth::Address) -> bool { + static SETTLE_FUNCTION_SELECTOR: LazyLock<[u8; 4]> = LazyLock::new(|| { + let abi = &contracts::GPv2Settlement::raw_contract().interface.abi; + abi.function("settle").unwrap().selector() + }); + trace.to == settlement_contract && trace.input.0.starts_with(&*SETTLE_FUNCTION_SELECTOR) +} + /// Trade containing onchain observable data specific to a settlement /// transaction. #[derive(Debug, Clone)] @@ -152,6 +176,8 @@ pub struct ClearingPrices { #[derive(Debug, thiserror::Error)] pub enum Error { + #[error("settle calldata must exist for all transactions emitting settlement event")] + MissingCalldata, #[error("missing auction id")] MissingAuctionId, #[error(transparent)] diff --git a/crates/autopilot/src/infra/blockchain/mod.rs b/crates/autopilot/src/infra/blockchain/mod.rs index 6b8f3234cc..79f0ef1d75 100644 --- a/crates/autopilot/src/infra/blockchain/mod.rs +++ b/crates/autopilot/src/infra/blockchain/mod.rs @@ -103,9 +103,10 @@ impl Ethereum { } pub async fn transaction(&self, hash: eth::TxId) -> Result { - let (transaction, receipt) = tokio::try_join!( + let (transaction, receipt, traces) = tokio::try_join!( self.web3.eth().transaction(hash.0.into()), - self.web3.eth().transaction_receipt(hash.0) + self.web3.eth().transaction_receipt(hash.0), + self.web3.trace().transaction(hash.0), )?; let transaction = transaction.ok_or(Error::TransactionNotFound)?; let receipt = receipt.ok_or(Error::TransactionNotFound)?; @@ -121,13 +122,15 @@ impl Ethereum { .block(block_hash.into()) .await? .ok_or(Error::TransactionNotFound)?; - into_domain(transaction, receipt, block.timestamp).map_err(Error::IncompleteTransactionData) + into_domain(transaction, receipt, traces, block.timestamp) + .map_err(Error::IncompleteTransactionData) } } fn into_domain( transaction: web3::types::Transaction, receipt: web3::types::TransactionReceipt, + traces: Vec, timestamp: U256, ) -> anyhow::Result { Ok(eth::Transaction { @@ -136,7 +139,6 @@ fn into_domain( .from .ok_or(anyhow::anyhow!("missing from"))? .into(), - input: transaction.input.0.into(), block: receipt .block_number .ok_or(anyhow::anyhow!("missing block_number"))? @@ -151,6 +153,16 @@ fn into_domain( .ok_or(anyhow::anyhow!("missing effective_gas_price"))? .into(), timestamp: timestamp.as_u32(), + trace_calls: traces + .into_iter() + .filter_map(|trace| match trace.action { + web3::types::Action::Call(call) => Some(eth::TraceCall { + to: call.to.into(), + input: call.input.0.into(), + }), + _ => None, + }) + .collect(), }) } From b4e2bbdab467f0be1556769c0f49099d0cc0d77b Mon Sep 17 00:00:00 2001 From: Dusan Stanivukovic Date: Thu, 16 Jan 2025 09:08:44 +0100 Subject: [PATCH 09/11] =?UTF-8?q?Revert=20"Use=20traces=20to=20extract=20t?= =?UTF-8?q?he=20input=20for=20`settlement::Observer`=20(#=E2=80=A6=20(#323?= =?UTF-8?q?7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Reverts https://github.com/cowprotocol/services/pull/3233 None of the Arbitrum nodes supports `trace_transaction`. Supposedly, `arbtrace_transaction` should be used instead, but I couldn't make it work. Also, not all backup nodes (read alchemy) for other networks (base, gc) support `trace_transaction`. I noticed that alchemy supports `debug_traceTransaction` that could be used instead. Overall, I should have checked this before merging the PR. I decided to revert this PR since, right now, Arbitrum staging is unable to work properly, and also, even with specific/per_chain implementation I am not sure I will be able to make it work with all PRIMARY/BACKUP nodes on ALL networks. This needs throughout testing first. --- crates/autopilot/src/domain/eth/mod.rs | 11 +- crates/autopilot/src/domain/settlement/mod.rs | 287 +----------------- .../src/domain/settlement/observer.rs | 15 +- .../src/domain/settlement/transaction/mod.rs | 48 +-- crates/autopilot/src/infra/blockchain/mod.rs | 20 +- 5 files changed, 34 insertions(+), 347 deletions(-) diff --git a/crates/autopilot/src/domain/eth/mod.rs b/crates/autopilot/src/domain/eth/mod.rs index 29ca478683..9b187526ba 100644 --- a/crates/autopilot/src/domain/eth/mod.rs +++ b/crates/autopilot/src/domain/eth/mod.rs @@ -312,13 +312,6 @@ pub struct TradeEvent { pub order_uid: domain::OrderUid, } -/// A trace of a Call type of action. -#[derive(Debug, Clone, Default)] -pub struct TraceCall { - pub to: Address, - pub input: Calldata, -} - /// Any type of on-chain transaction. #[derive(Debug, Clone, Default)] pub struct Transaction { @@ -326,6 +319,8 @@ pub struct Transaction { pub hash: TxId, /// The address of the sender of the transaction. pub from: Address, + /// The call data of the transaction. + pub input: Calldata, /// The block number of the block that contains the transaction. pub block: BlockNo, /// The timestamp of the block that contains the transaction. @@ -334,6 +329,4 @@ pub struct Transaction { pub gas: Gas, /// The effective gas price of the transaction. pub gas_price: EffectiveGasPrice, - /// Traces of all Calls contained in the transaction. - pub trace_calls: Vec, } diff --git a/crates/autopilot/src/domain/settlement/mod.rs b/crates/autopilot/src/domain/settlement/mod.rs index dbcb4e14c4..7908789719 100644 --- a/crates/autopilot/src/domain/settlement/mod.rs +++ b/crates/autopilot/src/domain/settlement/mod.rs @@ -29,6 +29,8 @@ pub struct Settlement { gas: eth::Gas, /// The effective gas price of the settlement transaction. gas_price: eth::EffectiveGasPrice, + /// The address of the solver that submitted the settlement transaction. + solver: eth::Address, /// The block number of the block that contains the settlement transaction. block: eth::BlockNo, /// The associated auction. @@ -144,6 +146,7 @@ impl Settlement { .collect(); Ok(Self { + solver: settled.solver, block: settled.block, gas: settled.gas, gas_price: settled.gas_price, @@ -213,246 +216,14 @@ impl From for Error { #[cfg(test)] mod tests { use { - crate::domain::{self, auction, eth}, + crate::{ + domain, + domain::{auction, eth}, + }, hex_literal::hex, std::collections::{HashMap, HashSet}, }; - // https://etherscan.io/tx/0x030623e438f28446329d8f4ff84db897907fcac59b9943b31b7be66f23c877af - // A transfer transaction that emits a settlement event, but it's not actually a - // swap. - #[test] - fn not_a_swap() { - let calldata = hex!( - " - 13d79a0b - 0000000000000000000000000000000000000000000000000000000000000080 - 00000000000000000000000000000000000000000000000000000000000000a0 - 00000000000000000000000000000000000000000000000000000000000000c0 - 00000000000000000000000000000000000000000000000000000000000000e0 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 0000000000000000000000000000000000000000000000000000000000000080 - 00000000000000000000000000000000000000000000000000000000000017a0 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000011 - 0000000000000000000000000000000000000000000000000000000000000220 - 0000000000000000000000000000000000000000000000000000000000000360 - 00000000000000000000000000000000000000000000000000000000000004a0 - 00000000000000000000000000000000000000000000000000000000000005e0 - 0000000000000000000000000000000000000000000000000000000000000720 - 0000000000000000000000000000000000000000000000000000000000000860 - 00000000000000000000000000000000000000000000000000000000000009a0 - 0000000000000000000000000000000000000000000000000000000000000ae0 - 0000000000000000000000000000000000000000000000000000000000000c20 - 0000000000000000000000000000000000000000000000000000000000000d60 - 0000000000000000000000000000000000000000000000000000000000000ea0 - 0000000000000000000000000000000000000000000000000000000000000fe0 - 0000000000000000000000000000000000000000000000000000000000001120 - 0000000000000000000000000000000000000000000000000000000000001260 - 00000000000000000000000000000000000000000000000000000000000013a0 - 00000000000000000000000000000000000000000000000000000000000014e0 - 0000000000000000000000000000000000000000000000000000000000001620 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000388674df3c7f96ac76c6fa06813b758322b5b64ce14bf46f0f9b4ec6f2 - d015ff9a9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000383f19e12409d5913e40bfde35a1607a1a43f1f9d26e76dd2d9d409cc7 - 50125f769008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 00000038a91a488e41e46dedbe71ae0c0d9f41be8de9f45ffc62271970362777 - 9e302e8d9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 0000003820621f141681025ffcbce03b51c3ea78c93da8739df9202210647968 - 6d2efad59008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000385846bbbcee39039678e529973cf6962b90d49663d5ff49220adfb240 - 4805d11e9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000388a608a3e6fd6f95797d376ffa35ae077d78440b627dce2b3d2278e04 - 8d37f7c09008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 00000038436b1ae2723b42982a435ede8f92033fbdf1505e5dac97c1099b5ffd - fc1debda9008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 00000038c31724565adeb2d0e4d6a6980f45b9fd91bb0b858fa8c8ada300e697 - 45d1ab379008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 0000003812789b110b2bf7353d054d978dd30972f2f33e54c4c7b93a1f992d20 - fd2f29619008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 00000038f527dd3ad4938713bff3fb23a62d7b454caf40bc80f3eeed77c3d269 - d9ff8eb69008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000388a63a1ed3d671e94aafe29b9ad479340ee04ad1fb5796c73cb71b25a - e8e2b0a49008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000384acca1a9694919daee72c0f20b64a2c0103750c26ce9b25cc864609f - 2f4977269008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 00000038bdbeba9388a200bfa8c3f153c31a539622e2994a2cc59bfdc8c562a3 - 2f20b9919008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000383649cc08adbf44018756fec310e9348d64139f33530166ef53f1834f - 2f5c8c469008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 0000003864bee671badbb222a43006744c89b70a368ca34a356dcf41f755923e - aab5b4029008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000009008d19f58aabd9ed0d60971565aa8510560ab41 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 00000000000000000000000000000000000000000000000000000000000000a4 - ec6cb13f00000000000000000000000000000000000000000000000000000000 - 0000004000000000000000000000000000000000000000000000000000000000 - 0000000100000000000000000000000000000000000000000000000000000000 - 000000382285ea60762c9a58d407e5aa3b8c3287628290793bc7260b0e0324ed - 6f8bb1269008d19f58aabd9ed0d60971565aa8510560ab41678716a000000000 - 0000000000000000000000000000000000000000000000000000000000000000 - 000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2 - 0000000000000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000060 - 0000000000000000000000000000000000000000000000000000000000000044 - a9059cbb000000000000000000000000a03be496e67ec29bc62f01a428683d7f - 9c2049300000000000000000000000000000000000000000000000002136c5d9 - c1570aef00000000000000000000000000000000000000000000000000000000 - 0000000000000000000000000000000000000000000000000000000000000000" - ) - .to_vec(); - - let domain_separator = eth::DomainSeparator(hex!( - "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" - )); - let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( - "9008d19f58aabd9ed0d60971565aa8510560ab41" - ))); - let transaction = super::transaction::Transaction::try_new( - &domain::eth::Transaction { - trace_calls: vec![domain::eth::TraceCall { - to: settlement_contract, - input: calldata.into(), - }], - ..Default::default() - }, - &domain_separator, - settlement_contract, - ) - .unwrap_err(); - - // These transfer transactions don't have the auction_id attached so overall bad - // calldata is expected - assert!(matches!( - transaction, - super::transaction::Error::Decoding(_) - )); - } - // https://etherscan.io/tx/0xc48dc0d43ffb43891d8c3ad7bcf05f11465518a2610869b20b0b4ccb61497634 #[test] fn settlement() { @@ -535,19 +306,12 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( - "9008d19f58aabd9ed0d60971565aa8510560ab41" - ))); - let transaction = super::transaction::Transaction::try_new( + let transaction = super::transaction::Transaction::new( &domain::eth::Transaction { - trace_calls: vec![domain::eth::TraceCall { - to: settlement_contract, - input: calldata.into(), - }], + input: calldata.into(), ..Default::default() }, &domain_separator, - settlement_contract, ) .unwrap(); @@ -678,19 +442,12 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( - "9008d19f58aabd9ed0d60971565aa8510560ab41" - ))); - let transaction = super::transaction::Transaction::try_new( + let transaction = super::transaction::Transaction::new( &domain::eth::Transaction { - trace_calls: vec![domain::eth::TraceCall { - to: settlement_contract, - input: calldata.into(), - }], + input: calldata.into(), ..Default::default() }, &domain_separator, - settlement_contract, ) .unwrap(); @@ -853,19 +610,12 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( - "9008d19f58aabd9ed0d60971565aa8510560ab41" - ))); - let transaction = super::transaction::Transaction::try_new( + let transaction = super::transaction::Transaction::new( &domain::eth::Transaction { - trace_calls: vec![domain::eth::TraceCall { - to: settlement_contract, - input: calldata.into(), - }], + input: calldata.into(), ..Default::default() }, &domain_separator, - settlement_contract, ) .unwrap(); @@ -1034,19 +784,12 @@ mod tests { let domain_separator = eth::DomainSeparator(hex!( "c078f884a2676e1345748b1feace7b0abee5d00ecadb6e574dcdd109a63e8943" )); - let settlement_contract = eth::Address(eth::H160::from_slice(&hex!( - "9008d19f58aabd9ed0d60971565aa8510560ab41" - ))); - let transaction = super::transaction::Transaction::try_new( + let transaction = super::transaction::Transaction::new( &domain::eth::Transaction { - trace_calls: vec![domain::eth::TraceCall { - to: settlement_contract, - input: calldata.into(), - }], + input: calldata.into(), ..Default::default() }, &domain_separator, - settlement_contract, ) .unwrap(); diff --git a/crates/autopilot/src/domain/settlement/observer.rs b/crates/autopilot/src/domain/settlement/observer.rs index d142646728..631539f9ef 100644 --- a/crates/autopilot/src/domain/settlement/observer.rs +++ b/crates/autopilot/src/domain/settlement/observer.rs @@ -66,8 +66,7 @@ impl Observer { let transaction = match self.eth.transaction(event.transaction).await { Ok(transaction) => { let separator = self.eth.contracts().settlement_domain_separator(); - let settlement_contract = self.eth.contracts().settlement().address().into(); - settlement::Transaction::try_new(&transaction, separator, settlement_contract) + settlement::Transaction::new(&transaction, separator) } Err(err) => { tracing::warn!(hash = ?event.transaction, ?err, "no tx found"); @@ -96,17 +95,7 @@ impl Observer { (auction_id, settlement) } Err(err) => { - match err { - settlement::transaction::Error::MissingCalldata => { - tracing::error!(hash = ?event.transaction, ?err, "invalid settlement transaction") - } - settlement::transaction::Error::MissingAuctionId - | settlement::transaction::Error::Decoding(_) - | settlement::transaction::Error::SignatureRecover(_) - | settlement::transaction::Error::OrderUidRecover(_) => { - tracing::warn!(hash = ?event.transaction, ?err, "invalid settlement transaction") - } - } + tracing::warn!(hash = ?event.transaction, ?err, "invalid settlement transaction"); // default values so we don't get stuck on invalid settlement transactions (0.into(), None) } diff --git a/crates/autopilot/src/domain/settlement/transaction/mod.rs b/crates/autopilot/src/domain/settlement/transaction/mod.rs index d5877d00db..3b0e3f4540 100644 --- a/crates/autopilot/src/domain/settlement/transaction/mod.rs +++ b/crates/autopilot/src/domain/settlement/transaction/mod.rs @@ -1,10 +1,6 @@ -use { - crate::{ - boundary, - domain::{self, auction::order, eth}, - }, - ethcontract::common::FunctionExt, - std::sync::LazyLock, +use crate::{ + boundary, + domain::{self, auction::order, eth}, }; mod tokenized; @@ -16,6 +12,8 @@ pub struct Transaction { pub hash: eth::TxId, /// The associated auction id. pub auction_id: domain::auction::Id, + /// The address of the solver that submitted the transaction. + pub solver: eth::Address, /// The block number of the block that contains the transaction. pub block: eth::BlockNo, /// The timestamp of the block that contains the transaction. @@ -29,33 +27,18 @@ pub struct Transaction { } impl Transaction { - pub fn try_new( + pub fn new( transaction: ð::Transaction, domain_separator: ð::DomainSeparator, - settlement_contract: eth::Address, ) -> Result { - // find trace call to settlement contract - let calldata = transaction - .trace_calls - .iter() - .find(|trace| is_settlement_trace(trace, settlement_contract)) - .map(|trace| trace.input.clone()) - // all transactions emitting settlement events should have a /settle call, - // otherwise it's an execution client bug - .ok_or(Error::MissingCalldata)?; - /// Number of bytes that may be appended to the calldata to store an /// auction id. const META_DATA_LEN: usize = 8; - let (data, metadata) = calldata.0.split_at( - calldata - .0 - .len() - .checked_sub(META_DATA_LEN) - // should contain at least 4 bytes for function selector and 8 bytes for auction id - .ok_or(Error::MissingCalldata)?, - ); + let (data, metadata) = transaction + .input + .0 + .split_at(transaction.input.0.len() - META_DATA_LEN); let metadata: Option<[u8; META_DATA_LEN]> = metadata.try_into().ok(); let auction_id = metadata .map(crate::domain::auction::Id::from_be_bytes) @@ -63,6 +46,7 @@ impl Transaction { Ok(Self { hash: transaction.hash, auction_id, + solver: transaction.from, block: transaction.block, timestamp: transaction.timestamp, gas: transaction.gas, @@ -132,14 +116,6 @@ impl Transaction { } } -fn is_settlement_trace(trace: ð::TraceCall, settlement_contract: eth::Address) -> bool { - static SETTLE_FUNCTION_SELECTOR: LazyLock<[u8; 4]> = LazyLock::new(|| { - let abi = &contracts::GPv2Settlement::raw_contract().interface.abi; - abi.function("settle").unwrap().selector() - }); - trace.to == settlement_contract && trace.input.0.starts_with(&*SETTLE_FUNCTION_SELECTOR) -} - /// Trade containing onchain observable data specific to a settlement /// transaction. #[derive(Debug, Clone)] @@ -176,8 +152,6 @@ pub struct ClearingPrices { #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("settle calldata must exist for all transactions emitting settlement event")] - MissingCalldata, #[error("missing auction id")] MissingAuctionId, #[error(transparent)] diff --git a/crates/autopilot/src/infra/blockchain/mod.rs b/crates/autopilot/src/infra/blockchain/mod.rs index 79f0ef1d75..6b8f3234cc 100644 --- a/crates/autopilot/src/infra/blockchain/mod.rs +++ b/crates/autopilot/src/infra/blockchain/mod.rs @@ -103,10 +103,9 @@ impl Ethereum { } pub async fn transaction(&self, hash: eth::TxId) -> Result { - let (transaction, receipt, traces) = tokio::try_join!( + let (transaction, receipt) = tokio::try_join!( self.web3.eth().transaction(hash.0.into()), - self.web3.eth().transaction_receipt(hash.0), - self.web3.trace().transaction(hash.0), + self.web3.eth().transaction_receipt(hash.0) )?; let transaction = transaction.ok_or(Error::TransactionNotFound)?; let receipt = receipt.ok_or(Error::TransactionNotFound)?; @@ -122,15 +121,13 @@ impl Ethereum { .block(block_hash.into()) .await? .ok_or(Error::TransactionNotFound)?; - into_domain(transaction, receipt, traces, block.timestamp) - .map_err(Error::IncompleteTransactionData) + into_domain(transaction, receipt, block.timestamp).map_err(Error::IncompleteTransactionData) } } fn into_domain( transaction: web3::types::Transaction, receipt: web3::types::TransactionReceipt, - traces: Vec, timestamp: U256, ) -> anyhow::Result { Ok(eth::Transaction { @@ -139,6 +136,7 @@ fn into_domain( .from .ok_or(anyhow::anyhow!("missing from"))? .into(), + input: transaction.input.0.into(), block: receipt .block_number .ok_or(anyhow::anyhow!("missing block_number"))? @@ -153,16 +151,6 @@ fn into_domain( .ok_or(anyhow::anyhow!("missing effective_gas_price"))? .into(), timestamp: timestamp.as_u32(), - trace_calls: traces - .into_iter() - .filter_map(|trace| match trace.action { - web3::types::Action::Call(call) => Some(eth::TraceCall { - to: call.to.into(), - input: call.input.0.into(), - }), - _ => None, - }) - .collect(), }) } From 50f7c9a6e0c1e8b5fbac341e0b92291b78e8d10f Mon Sep 17 00:00:00 2001 From: Martin Magnus Date: Thu, 16 Jan 2025 09:22:33 +0100 Subject: [PATCH 10/11] Flashloan appdata (#3236) # Description Fixes: https://github.com/cowprotocol/services/issues/3214 # Changes Extend appdata format with `flashloan` field explained in the linked issue. ## How to test added an e2e test that shows it's now possible to create an order that contains flashloan hints in the appdata. --- crates/app-data/src/app_data.rs | 21 +++++- crates/e2e/tests/e2e/app_data.rs | 112 +++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/crates/app-data/src/app_data.rs b/crates/app-data/src/app_data.rs index c8cd70f5b5..7f12a00723 100644 --- a/crates/app-data/src/app_data.rs +++ b/crates/app-data/src/app_data.rs @@ -1,7 +1,7 @@ use { crate::{app_data_hash::hash_full_app_data, AppDataHash, Hooks}, anyhow::{anyhow, Context, Result}, - primitive_types::H160, + primitive_types::{H160, U256}, serde::{de, Deserialize, Deserializer, Serialize, Serializer}, std::{fmt, fmt::Display}, }; @@ -24,6 +24,24 @@ pub struct ProtocolAppData { pub signer: Option, pub replaced_order: Option, pub partner_fee: Option, + pub flashloan: Option, +} + +/// Contains information to hint at how a solver could make +/// use of flashloans to settle the associated order. +/// Since using flashloans introduces a bunch of complexities +/// all these hints are not binding for the solver. +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] +pub struct Flashloan { + /// Which contract to request the flashloan from. + lender: Option, + /// Who should receive the borrowed tokens. If this is not + /// set the order owner will get the tokens. + borrower: Option, + /// Which token to flashloan. + token: H160, + /// How much of the token to flashloan. + amount: U256, } #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq)] @@ -217,6 +235,7 @@ impl From for ProtocolAppData { signer: None, replaced_order: None, partner_fee: None, + flashloan: None, } } } diff --git a/crates/e2e/tests/e2e/app_data.rs b/crates/e2e/tests/e2e/app_data.rs index b6e9bbac14..4972c81c3f 100644 --- a/crates/e2e/tests/e2e/app_data.rs +++ b/crates/e2e/tests/e2e/app_data.rs @@ -20,6 +20,12 @@ async fn local_node_app_data() { run_test(app_data).await; } +#[tokio::test] +#[ignore] +async fn local_node_app_data_full_format() { + run_test(app_data_full_format).await; +} + // Test that orders can be placed with the new app data format. async fn app_data(web3: Web3) { let mut onchain = OnchainComponents::deploy(web3).await; @@ -177,3 +183,109 @@ async fn app_data(web3: Web3) { .unwrap_err(); dbg!(err); } + +/// Tests that orders can be placed with an app data JSON containing +/// all supported features. +async fn app_data_full_format(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 [token_a, token_b] = onchain + .deploy_tokens_with_weth_uni_v2_pools(to_wei(1_000), to_wei(1_000)) + .await; + + token_a.mint(trader.address(), to_wei(10)).await; + tx!( + trader.account(), + token_a.approve(onchain.contracts().allowance, to_wei(10)) + ); + + let mut valid_to: u32 = model::time::now_in_epoch_seconds() + 300; + let mut create_order = |app_data| { + let order = OrderCreation { + app_data, + sell_token: token_a.address(), + sell_amount: to_wei(2), + buy_token: token_b.address(), + buy_amount: to_wei(1), + valid_to, + kind: OrderKind::Sell, + ..Default::default() + } + .sign( + EcdsaSigningScheme::Eip712, + &onchain.contracts().domain_separator, + SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()), + ); + // Adjust valid to make sure we get unique UIDs. + valid_to += 1; + order + }; + + let services = Services::new(&onchain).await; + services.start_protocol(solver).await; + + // Unknown hashes are not accepted. + let order0 = create_order(OrderCreationAppData::Hash { + hash: AppDataHash([0; 32]), + }); + let order_uid = services.create_order(&order0).await.unwrap(); + + // appdata using all features + let app_data = format!( + r#"{{ + "version": "0.9.0", + "appCode": "CoW Swap", + "environment": "barn", + "metadata": {{ + "quote": {{ + "slippageBps": "50" + }}, + "hooks": {{ + "pre": [ + {{ + "target": "0x0000000000000000000000000000000000000000", + "callData": "0x12345678", + "gasLimit": "21000" + }} + ], + "post": [ + {{ + "target": "0x0000000000000000000000000000000000000000", + "callData": "0x12345678", + "gasLimit": "21000" + }} + ] + }}, + "flashloan": {{ + "lender": "0x1111111111111111111111111111111111111111", + "borrower": "0x2222222222222222222222222222222222222222", + "token": "0x3333333333333333333333333333333333333333", + "amount": "1234" + }}, + "signer": "{:?}", + "replacedOrder": {{ + "uid": "{}" + }}, + "partnerFee": {{ + "bps": 1, + "recipient": "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + }} + }} + }}"#, + trader.address(), + order_uid + ); + + let app_data_hash = AppDataHash(hash_full_app_data(app_data.as_bytes())); + let order1 = create_order(OrderCreationAppData::Full { + full: app_data.to_string(), + }); + let uid = services.create_order(&order1).await.unwrap(); + let order1_ = services.get_order(&uid).await.unwrap(); + assert_eq!(order1_.data.app_data, app_data_hash); + assert_eq!(order1_.metadata.full_app_data, Some(app_data.to_string())); + + let app_data_ = services.get_app_data(app_data_hash).await.unwrap(); + assert_eq!(app_data_, app_data); +} From 395d605a821adbcf6f27fd7ed9125c9eb0c11838 Mon Sep 17 00:00:00 2001 From: Martin Magnus Date: Thu, 16 Jan 2025 14:52:45 +0100 Subject: [PATCH 11/11] Support faking balances for more tokens (#3238) # Description To verify more quotes we introduced some logic to figure out which storage slots need to be overwritten to fake a token balance for a given owner. That way our quote verification simulation will not revert when a user currently doesn't hold the necessary amounts of token which improves UX a lot. While looking into some quote we couldn't verify ([simulation](https://dashboard.tenderly.co/cow-protocol/production/simulator/e3456eaf-90e6-4212-9758-deef4b80948f?trace=0.3.0.3.0.0.0.5.0.0.1.0.0.0.0)) it became apparent that our current strategy to detect which storage slot needs to get overwritten doesn't work for some tokens. The current logic assumes a "normal" contract where member variables are laid out using the default solidity logic. This is not sufficient to detect proxy contracts which store their state at a hardcoded address since we can't predict where the author decided to put their member variables in their custom memory layout. However, most of the time people use libraries to handle that logic for them. These libraries hardcode the necessary information to find the member variable storing the balances in the token so we can just add support by probing for these known hardcoded values too. Kudos to @fedgiac for helping me debug the problem with the original proxy contract. # Changes This PR adds support for 2 libraries: OpenZeppelin and Solady. The OpenZeppeline implementation simply puts the `balances` member variable at a hardcoded position so we can use the existing encoding logic with the hardcoded address we found the in their source code. Solady is a bit different since it doesn't hash 64 bytes but instead only hashes 48 bytes to compute the storage slot of an addresses balance. For this I introduced another enum variant. This PR also fixes a bug with the original implementation. The original implementation works by calling `token.balanceOf(address)` with 25 storage slots. Each slot gets overwritten with a specific value. When `balanceOf()` returns that value we recover which storage slot we originally overwrote. This idea makes a lot of sense but had a problem with the very first override. The amount we override was computed with `U256::from(u64::from_be_bytes([i; 8]))` where `i` is an index variable. That means the first override always tried to set the balance to `0`. If you call `token.balanceOf(random_address)` it will return 0 because the address doesn't have any balances. The original logic interpreted this `0` as the sentinel `0` value it originally overwrote and therefore returned that the `balances` mapping lives in the `0th` storage slot. To fix that we now compute the sentinel values with `U256::from(u64::from_be_bytes([i + 1; 8]))` to ensure that we always search for non-zero values to detect the correct storage override. Note that this error only surfaced when none of the storage slots we tried to override was the correct one. If we were able to override the storage slot `balanceOf()` would no longer return `0` to cause the false positive. ## How to test added an ignored unit test detecting 1 storage slot of each variant: 1. standard solidity mapping + standard solidity layout 2. standard solidity mapping + hardcoded OpenZeppelin storage slot 3. custom Solady mapping Also there was already 1 e2e test that already tested the original logic. --- .../trade_verifier/balance_overrides.rs | 105 ++++++++--- .../balance_overrides/detector.rs | 167 +++++++++++++----- 2 files changed, 206 insertions(+), 66 deletions(-) diff --git a/crates/shared/src/price_estimation/trade_verifier/balance_overrides.rs b/crates/shared/src/price_estimation/trade_verifier/balance_overrides.rs index d1f85c03d3..d12d3c9621 100644 --- a/crates/shared/src/price_estimation/trade_verifier/balance_overrides.rs +++ b/crates/shared/src/price_estimation/trade_verifier/balance_overrides.rs @@ -86,7 +86,8 @@ impl Display for TokenConfiguration { fn fmt(&self, f: &mut Formatter) -> fmt::Result { let format_entry = |f: &mut Formatter, (addr, strategy): (&Address, &Strategy)| match strategy { - Strategy::Mapping { slot } => write!(f, "{addr:?}@{slot}"), + Strategy::SolidityMapping { slot } => write!(f, "{addr:?}@{slot}"), + Strategy::SoladyMapping => write!(f, "SoladyMapping({addr:?})"), }; let mut entries = self.0.iter(); @@ -121,7 +122,7 @@ impl FromStr for TokenConfiguration { .context("expected {addr}@{slot} format")?; Ok(( addr.parse()?, - Strategy::Mapping { + Strategy::SolidityMapping { slot: slot.parse()?, }, )) @@ -151,7 +152,7 @@ pub struct BalanceOverrideRequest { } /// Balance override strategy for a token. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum Strategy { /// Balance override strategy for tokens whose balances are stored in a /// direct Solidity mapping from token holder to balance amount in the @@ -160,29 +161,40 @@ pub enum Strategy { /// The strategy is configured with the storage slot [^1] of the mapping. /// /// [^1]: - Mapping { slot: U256 }, + SolidityMapping { slot: U256 }, + /// Strategy computing storage slot for balances based on the Solady library + /// [^1]. + /// + /// [^1]: + SoladyMapping, } impl Strategy { /// Computes the storage slot and value to override for a particular token /// holder and amount. fn state_override(&self, holder: &Address, amount: &U256) -> (H256, H256) { - match self { - Self::Mapping { slot } => { - let key = { - let mut buf = [0; 64]; - buf[12..32].copy_from_slice(holder.as_fixed_bytes()); - slot.to_big_endian(&mut buf[32..64]); - H256(signing::keccak256(&buf)) - }; - let value = { - let mut buf = [0; 32]; - amount.to_big_endian(&mut buf); - H256(buf) - }; - (key, value) + let key = match self { + Self::SolidityMapping { slot } => { + let mut buf = [0; 64]; + buf[12..32].copy_from_slice(holder.as_fixed_bytes()); + slot.to_big_endian(&mut buf[32..64]); + H256(signing::keccak256(&buf)) } - } + Self::SoladyMapping => { + let mut buf = [0; 32]; + buf[0..20].copy_from_slice(holder.as_fixed_bytes()); + buf[28..32].copy_from_slice(&[0x87, 0xa2, 0x11, 0xa2]); + H256(signing::keccak256(&buf)) + } + }; + + let value = { + let mut buf = [0; 32]; + amount.to_big_endian(&mut buf); + H256(buf) + }; + + (key, value) } } @@ -264,7 +276,7 @@ mod tests { async fn balance_override_computation() { let balance_overrides = BalanceOverrides { hardcoded: hashmap! { - addr!("DEf1CA1fb7FBcDC777520aa7f396b4E015F497aB") => Strategy::Mapping { + addr!("DEf1CA1fb7FBcDC777520aa7f396b4E015F497aB") => Strategy::SolidityMapping { slot: U256::from(0), }, }, @@ -290,7 +302,7 @@ mod tests { // You can verify the state override computation is correct by running: // ``` - // curl -X POST $RPC -H 'Content-Type: application/data' --data '{ + // curl -X POST $RPC -H 'Content-Type: application/json' --data '{ // "jsonrpc": "2.0", // "id": 0, // "method": "eth_call", @@ -327,4 +339,55 @@ mod tests { None, ); } + + #[tokio::test] + async fn balance_override_computation_solady() { + let balance_overrides = BalanceOverrides { + hardcoded: hashmap! { + addr!("0000000000c5dc95539589fbd24be07c6c14eca4") => Strategy::SoladyMapping, + }, + ..Default::default() + }; + + assert_eq!( + balance_overrides + .state_override(BalanceOverrideRequest { + token: addr!("0000000000c5dc95539589fbd24be07c6c14eca4"), + holder: addr!("d8dA6BF26964aF9D7eEd9e03E53415D37aA96045"), + amount: 0x42_u64.into(), + }) + .await, + Some(StateOverride { + state_diff: Some(hashmap! { + H256(hex!("f6a6656ed2d14bad3cdd3e8871db3f535a136a1b6cd5ae2dced8eb813f3d4e4f")) => + H256(hex!("0000000000000000000000000000000000000000000000000000000000000042")), + }), + ..Default::default() + }), + ); + + // You can verify the state override computation is correct by running: + // ``` + // curl -X POST $RPC -H 'Content-Type: application/json' --data '{ + // "jsonrpc": "2.0", + // "id": 0, + // "method": "eth_call", + // "params": [ + // { + // "to": "0x0000000000c5dc95539589fbd24be07c6c14eca4", + // "data": "0x70a08231000000000000000000000000d8dA6BF26964aF9D7eEd9e03E53415D37aA96045" + // }, + // "latest", + // { + // "0x0000000000c5dc95539589fbd24be07c6c14eca4": { + // "stateDiff": { + // "f6a6656ed2d14bad3cdd3e8871db3f535a136a1b6cd5ae2dced8eb813f3d4e4f": + // "0x0000000000000000000000000000000000000000000000000000000000000042" + // } + // } + // } + // ] + // }' + // ``` + } } diff --git a/crates/shared/src/price_estimation/trade_verifier/balance_overrides/detector.rs b/crates/shared/src/price_estimation/trade_verifier/balance_overrides/detector.rs index d8795b7ce4..257d3ef4d5 100644 --- a/crates/shared/src/price_estimation/trade_verifier/balance_overrides/detector.rs +++ b/crates/shared/src/price_estimation/trade_verifier/balance_overrides/detector.rs @@ -2,12 +2,13 @@ use { super::Strategy, crate::code_simulation::{CodeSimulating, SimulationError}, contracts::{dummy_contract, ERC20}, - ethcontract::{Address, U256}, + ethcontract::{Address, H256, U256}, ethrpc::extensions::StateOverride, maplit::hashmap, std::{ + collections::HashMap, fmt::{self, Debug, Formatter}, - sync::Arc, + sync::{Arc, LazyLock}, }, thiserror::Error, web3::{signing::keccak256, types::CallRequest}, @@ -22,9 +23,6 @@ pub struct Detector { } impl Detector { - /// Number of different slots to try out. - const TRIES: u8 = 25; - /// Creates a new balance override detector. pub fn new(simulator: Arc) -> Self { Self { simulator } @@ -34,49 +32,15 @@ impl Detector { /// Returns an `Err` if it cannot detect the strategy or an internal /// simulation fails. pub async fn detect(&self, token: Address) -> Result { - // This is a pretty unsophisticated strategy where we basically try a - // bunch of different slots and see which one sticks. We try balance - // mappings for the first `TRIES` slots; each with a unique value. - let mut tries = (0..Self::TRIES).map(|i| { - let strategy = Strategy::Mapping { - slot: U256::from(i), - }; - // Use an exact value which isn't too large or too small. This helps - // not have false positives for cases where the token balances in - // some other denomination from the actual token balance (such as - // stETH for example) and not run into issues with overflows. - let amount = U256::from(u64::from_be_bytes([i; 8])); - - (strategy, amount) - }); - - // On a technical note, Ethereum public addresses are, for the most - // part, generated by taking the 20 last bytes of a Keccak-256 hash (for - // things like contract creation, public address derivation from a - // Secp256k1 public key, etc.), so we use one for our heuristics from a - // 32-byte digest with no know pre-image, to prevent any weird - // interactions with the weird tokens of the world. - let holder = { - let mut address = Address::default(); - address.0.copy_from_slice(&keccak256(b"Moo!")[12..]); - address.0[19] = address.0[19].wrapping_sub(1); - address - }; - let token = dummy_contract!(ERC20, token); let call = CallRequest { to: Some(token.address()), - data: token.methods().balance_of(holder).m.tx.data, + data: token.methods().balance_of(*HOLDER).m.tx.data, ..Default::default() }; let overrides = hashmap! { token.address() => StateOverride { - state_diff: Some( - tries - .clone() - .map(|(strategy, amount)| strategy.state_override(&holder, &amount)) - .collect(), - ), + state_diff: Some(STORAGE_OVERRIDES.clone()), ..Default::default() }, }; @@ -86,13 +50,87 @@ impl Detector { .then(|| U256::from_big_endian(&output)) .ok_or(DetectionError::Decode)?; - let strategy = tries - .find_map(|(strategy, amount)| (amount == balance).then_some(strategy)) - .ok_or(DetectionError::NotFound)?; - Ok(strategy) + TESTED_STRATEGIES + .iter() + .find_map(|helper| (helper.balance == balance).then_some(helper.strategy.clone())) + .ok_or(DetectionError::NotFound) + } +} + +/// Contains all the information we need to determine which state override +/// was successful. +struct StrategyHelper { + /// strategy that was used to compute the state override + strategy: Strategy, + /// balance amount the strategy wrote into the storage + balance: U256, +} + +impl StrategyHelper { + fn new(strategy: Strategy, index: u8) -> Self { + Self { + strategy, + // Use an exact value which isn't too large or too small. This helps + // not have false positives for cases where the token balances in + // some other denomination from the actual token balance (such as + // stETH for example) and not run into issues with overflows. + // We also make sure that we avoid 0 because `balanceOf()` returns + // 0 by default so we can't use it to detect successful state overrides. + balance: U256::from(u64::from_be_bytes([index + 1; 8])), + } } } +/// Storage slot based on OpenZeppelin's ERC20Upgradeable contract [^1]. +/// +/// [^1]: +static OPEN_ZEPPELIN_ERC20_UPGRADEABLE: &str = + "52c63247e1f47db19d5ce0460030c497f067ca4cebf71ba98eeadabe20bace00"; + +/// Address which we try to override the balances for. +static HOLDER: LazyLock
= LazyLock::new(|| { + // On a technical note, Ethereum public addresses are, for the most + // part, generated by taking the 20 last bytes of a Keccak-256 hash (for + // things like contract creation, public address derivation from a + // Secp256k1 public key, etc.), so we use one for our heuristics from a + // 32-byte digest with no know pre-image, to prevent any weird + // interactions with the weird tokens of the world. + let mut address = Address::default(); + address.0.copy_from_slice(&keccak256(b"Moo!")[12..]); + address.0[19] = address.0[19].wrapping_sub(1); + address +}); + +/// All the strategies we use to detect where a token stores the balances. +static TESTED_STRATEGIES: LazyLock> = LazyLock::new(|| { + const FIRST_N_SLOTS: u8 = 25; + + // This is a pretty unsophisticated strategy where we basically try a + // bunch of different slots and see which one sticks. We try balance + // mappings for the first `TRIES` slots; each with a unique value. + (0..FIRST_N_SLOTS).map(|i| { + let strategy = Strategy::SolidityMapping { slot: U256::from(i) }; + StrategyHelper::new(strategy, i) + }) + // Afterwards we try hardcoded storage slots based on popular utility + // libraries like OpenZeppelin. + .chain((FIRST_N_SLOTS..).zip([ + Strategy::SolidityMapping{ slot: U256::from(OPEN_ZEPPELIN_ERC20_UPGRADEABLE) }, + Strategy::SoladyMapping, + ]).map(|(index, strategy)| { + StrategyHelper::new(strategy, index) + })) + .collect() +}); + +/// Storage overrides (storage_slot, value) for all tested strategies. +static STORAGE_OVERRIDES: LazyLock> = LazyLock::new(|| { + TESTED_STRATEGIES + .iter() + .map(|helper| helper.strategy.state_override(&HOLDER, &helper.balance)) + .collect::>() +}); + impl Debug for Detector { fn fmt(&self, f: &mut Formatter) -> fmt::Result { f.debug_struct("Detector") @@ -111,3 +149,42 @@ pub enum DetectionError { #[error(transparent)] Simulation(#[from] SimulationError), } + +#[cfg(test)] +mod tests { + use {super::*, ethrpc::create_env_test_transport, web3::Web3}; + + /// Tests that we can detect storage slots by probing the first + /// n slots or by checking hardcoded known slots. + /// Set `NODE_URL` environment to a mainnet RPC URL. + #[ignore] + #[tokio::test] + async fn detects_storage_slots() { + let detector = Detector { + simulator: Arc::new(Web3::new(create_env_test_transport())), + }; + + let storage = detector + .detect(addr!("c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2")) + .await + .unwrap(); + assert_eq!(storage, Strategy::SolidityMapping { slot: 3.into() }); + + let storage = detector + .detect(addr!("4956b52ae2ff65d74ca2d61207523288e4528f96")) + .await + .unwrap(); + assert_eq!( + storage, + Strategy::SolidityMapping { + slot: U256::from(OPEN_ZEPPELIN_ERC20_UPGRADEABLE), + } + ); + + let storage = detector + .detect(addr!("0000000000c5dc95539589fbd24be07c6c14eca4")) + .await + .unwrap(); + assert_eq!(storage, Strategy::SoladyMapping); + } +}