Merge remote-tracking branch 'origin/main' into kill-bad-tokens-1

crates/autopilot/src/domain/auction/mod.rs

@@ -13,7 +13,6 @@ pub mod order;
 #[derive(Clone, Debug, PartialEq)]
 pub struct RawAuctionData {
     pub block: u64,
-    pub latest_settlement_block: u64,
     pub orders: Vec<Order>,
     pub prices: Prices,
     pub surplus_capturing_jit_order_owners: Vec<eth::Address>,
@@ -25,7 +24,6 @@ pub type Id = i64;
 pub struct Auction {
     pub id: Id,
     pub block: u64,
-    pub latest_settlement_block: u64,
     pub orders: Vec<Order>,
     pub prices: Prices,
     pub surplus_capturing_jit_order_owners: Vec<eth::Address>,
@@ -34,7 +32,6 @@ pub struct Auction {
 impl PartialEq for Auction {
     fn eq(&self, other: &Self) -> bool {
         self.block == other.block
-            && self.latest_settlement_block == other.latest_settlement_block
             && self.orders == other.orders
             && self.prices == other.prices
             && self.surplus_capturing_jit_order_owners == other.surplus_capturing_jit_order_owners

crates/autopilot/src/infra/persistence/dto/auction.rs

@@ -14,7 +14,6 @@ use {
 pub fn from_domain(auction: domain::RawAuctionData) -> RawAuctionData {
     RawAuctionData {
         block: auction.block,
-        latest_settlement_block: auction.latest_settlement_block,
         orders: auction
             .orders
             .into_iter()
@@ -38,7 +37,6 @@ pub fn from_domain(auction: domain::RawAuctionData) -> RawAuctionData {
 #[serde(rename_all = "camelCase")]
 pub struct RawAuctionData {
     pub block: u64,
-    pub latest_settlement_block: u64,
     pub orders: Vec<Order>,
     #[serde_as(as = "BTreeMap<_, HexOrDecimalU256>")]
     pub prices: BTreeMap<H160, U256>,
@@ -62,7 +60,6 @@ impl Auction {
         Ok(domain::Auction {
             id: self.id,
             block: self.auction.block,
-            latest_settlement_block: self.auction.latest_settlement_block,
             orders: self
                 .auction
                 .orders

crates/autopilot/src/infra/solvers/dto/reveal.rs

@@ -9,7 +9,6 @@ use {
 #[serde(rename_all = "camelCase")]
 pub struct Request {
     /// Unique ID of the solution (per driver competition), to reveal.
-    #[serde_as(as = "serde_with::DisplayFromStr")]
     pub solution_id: u64,
     /// Auction ID in which the specified solution ID is competing.
     #[serde_as(as = "Option<serde_with::DisplayFromStr>")]

crates/autopilot/src/infra/solvers/dto/settle.rs

@@ -9,7 +9,6 @@ use {
 #[serde(rename_all = "camelCase")]
 pub struct Request {
     /// Unique ID of the solution (per driver competition), to settle.
-    #[serde_as(as = "serde_with::DisplayFromStr")]
     pub solution_id: u64,
     /// The last block number in which the solution TX can be included
     pub submission_deadline_latest_block: u64,

crates/autopilot/src/infra/solvers/dto/solve.rs

@@ -170,7 +170,6 @@ pub enum Side {
 pub struct Solution {
     /// Unique ID of the solution (per driver competition), used to identify
     /// it in subsequent requests (reveal, settle).
-    #[serde(deserialize_with = "deserialize_solution_id")]
     pub solution_id: u64,
     #[serde_as(as = "HexOrDecimalU256")]
     pub score: U256,
@@ -182,37 +181,6 @@ pub struct Solution {
     pub gas: Option<u64>,
 }
 
-fn deserialize_solution_id<'de, D>(deserializer: D) -> Result<u64, D::Error>
-where
-    D: serde::Deserializer<'de>,
-{
-    struct SolutionIdVisitor;
-
-    impl serde::de::Visitor<'_> for SolutionIdVisitor {
-        type Value = u64;
-
-        fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-            formatter.write_str("a string or integer representing a solution ID")
-        }
-
-        fn visit_u64<E>(self, value: u64) -> Result<u64, E>
-        where
-            E: serde::de::Error,
-        {
-            Ok(value)
-        }
-
-        fn visit_str<E>(self, value: &str) -> Result<u64, E>
-        where
-            E: serde::de::Error,
-        {
-            value.parse::<u64>().map_err(serde::de::Error::custom)
-        }
-    }
-
-    deserializer.deserialize_any(SolutionIdVisitor)
-}
-
 #[derive(Clone, Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Response {

crates/autopilot/src/run_loop.rs

@@ -214,7 +214,6 @@ impl RunLoop {
         Some(domain::Auction {
             id,
             block: auction.block,
-            latest_settlement_block: auction.latest_settlement_block,
             orders: auction.orders,
             prices: auction.prices,
             surplus_capturing_jit_order_owners: auction.surplus_capturing_jit_order_owners,

crates/autopilot/src/solvable_orders.rs

@@ -267,7 +267,6 @@ impl SolvableOrdersCache {
             .collect::<Vec<_>>();
         let auction = domain::RawAuctionData {
             block,
-            latest_settlement_block: db_solvable_orders.latest_settlement_block,
             orders: orders
                 .into_iter()
                 .map(|order| {

crates/contracts/solidity/Trader.sol

@@ -96,20 +96,20 @@ contract Trader {
             }
         }
 
-        uint256 currentAllowance = IERC20(sellToken).allowance(address(this), address(settlementContract.vaultRelayer()));
+        address vaultRelayer = settlementContract.vaultRelayer();
+        uint256 currentAllowance = IERC20(sellToken).allowance(address(this), vaultRelayer);
         if (currentAllowance < sellAmount) {
             // Simulate an approval to the settlement contract so the user doesn't have to
             // spend gas on that just to get a quote. If they are happy with the quote and
             // want to create an order they will actually have to do the approvals, though.
             // We first reset the allowance to 0 since some ERC20 tokens (e.g. USDT)
             // require that due to this attack:
             // https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
-            // We catch reverts because we'll later assert the correct approval got set anyway.
-            try IERC20(sellToken).approve(address(settlementContract.vaultRelayer()), 0) {}
-            catch {}
-            try IERC20(sellToken).approve(address(settlementContract.vaultRelayer()), type(uint256).max) {}
-            catch {}
-            uint256 allowance = IERC20(sellToken).allowance(address(this), address(settlementContract.vaultRelayer()));
+            // In order to handle tokens which are not ERC20 compliant (like USDT) we have
+            // to use `safeApprove()` instead of the regular `approve()` here.
+            IERC20(sellToken).safeApprove(vaultRelayer, 0);
+            IERC20(sellToken).safeApprove(vaultRelayer, type(uint256).max);
+            uint256 allowance = IERC20(sellToken).allowance(address(this), vaultRelayer);
             require(allowance >= sellAmount, "trader did not give the required approvals");
         }
 

crates/database/src/solver_competition.rs

@@ -38,10 +38,14 @@ pub async fn load_by_id(
     id: AuctionId,
 ) -> Result<Option<LoadCompetition>, sqlx::Error> {
     const QUERY: &str = r#"
-SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE s.tx_hash IS NOT NULL), '{}') AS tx_hashes
+SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE so.block_number IS NOT NULL), '{}') AS tx_hashes
 FROM solver_competitions sc
 -- outer joins because the data might not have been indexed yet
 LEFT OUTER JOIN settlements s ON sc.id = s.auction_id
+-- exclude settlements from another environment for which observation is guaranteed to not exist
+LEFT OUTER JOIN settlement_observations so 
+    ON s.block_number = so.block_number 
+    AND s.log_index = so.log_index
 WHERE sc.id = $1
 GROUP BY sc.id
     ;"#;
@@ -52,10 +56,14 @@ pub async fn load_latest_competition(
     ex: &mut PgConnection,
 ) -> Result<Option<LoadCompetition>, sqlx::Error> {
     const QUERY: &str = r#"
-SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE s.tx_hash IS NOT NULL), '{}') AS tx_hashes
+SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE so.block_number IS NOT NULL), '{}') AS tx_hashes
 FROM solver_competitions sc
 -- outer joins because the data might not have been indexed yet
 LEFT OUTER JOIN settlements s ON sc.id = s.auction_id
+-- exclude settlements from another environment for which observation is guaranteed to not exist
+LEFT OUTER JOIN settlement_observations so 
+    ON s.block_number = so.block_number 
+    AND s.log_index = so.log_index
 GROUP BY sc.id
 ORDER BY sc.id DESC
 LIMIT 1
@@ -72,11 +80,17 @@ WITH competition AS (
     SELECT sc.id
     FROM solver_competitions sc
     JOIN settlements s ON sc.id = s.auction_id
+    JOIN settlement_observations so 
+        ON s.block_number = so.block_number 
+        AND s.log_index = so.log_index
     WHERE s.tx_hash = $1
 )
-SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE s.tx_hash IS NOT NULL), '{}') AS tx_hashes
+SELECT sc.json, sc.id, COALESCE(ARRAY_AGG(s.tx_hash) FILTER (WHERE so.block_number IS NOT NULL), '{}') AS tx_hashes
 FROM solver_competitions sc
 JOIN settlements s ON sc.id = s.auction_id
+JOIN settlement_observations so 
+    ON s.block_number = so.block_number 
+    AND s.log_index = so.log_index
 WHERE sc.id = (SELECT id FROM competition)
 GROUP BY sc.id
     ;"#;
@@ -336,7 +350,9 @@ mod tests {
         // non-existent auction returns none
         assert!(load_by_id(&mut db, 1).await.unwrap().is_none());
 
-        // insert two settlement events for the same auction id
+        // insert three settlement events for the same auction id, with one of them not
+        // having observation (in practice usually meaning it's from different
+        // environment)
         crate::events::insert_settlement(
             &mut db,
             &EventIndex {
@@ -350,6 +366,16 @@ mod tests {
         )
         .await
         .unwrap();
+        crate::settlement_observations::upsert(
+            &mut db,
+            crate::settlement_observations::Observation {
+                block_number: 0,
+                log_index: 0,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
         crate::events::insert_settlement(
             &mut db,
             &EventIndex {
@@ -363,12 +389,38 @@ mod tests {
         )
         .await
         .unwrap();
+        crate::settlement_observations::upsert(
+            &mut db,
+            crate::settlement_observations::Observation {
+                block_number: 0,
+                log_index: 1,
+                ..Default::default()
+            },
+        )
+        .await
+        .unwrap();
+        crate::events::insert_settlement(
+            &mut db,
+            &EventIndex {
+                block_number: 0,
+                log_index: 2,
+            },
+            &Settlement {
+                solver: Default::default(),
+                transaction_hash: ByteArray([2u8; 32]),
+            },
+        )
+        .await
+        .unwrap();
         crate::settlements::update_settlement_auction(&mut db, 0, 0, 0)
             .await
             .unwrap();
         crate::settlements::update_settlement_auction(&mut db, 0, 1, 0)
             .await
             .unwrap();
+        crate::settlements::update_settlement_auction(&mut db, 0, 2, 0)
+            .await
+            .unwrap();
 
         // load by id works, and finds two hashes
         let value_ = load_by_id(&mut db, 0).await.unwrap().unwrap();
@@ -389,6 +441,11 @@ mod tests {
             .unwrap()
             .unwrap();
         assert!(value_.tx_hashes.len() == 2);
+        // this one should not find any hashes since it's from another environment
+        let value_ = load_by_tx_hash(&mut db, &ByteArray([2u8; 32]))
+            .await
+            .unwrap();
+        assert!(value_.is_none());
     }
 
     #[tokio::test]

crates/driver/src/domain/mempools.rs

@@ -107,6 +107,24 @@ impl Mempools {
         let mut block_stream = into_stream(self.ethereum.current_block().clone());
         block_stream.next().await;
 
+        // The tx is simulated before submitting the solution to the competition, but a
+        // delay between that and the actual execution can cause the simulation to be
+        // invalid which doesn't make sense to submit to the mempool anymore.
+        if let Err(err) = self.ethereum.estimate_gas(tx).await {
+            if err.is_revert() {
+                tracing::info!(
+                    ?err,
+                    "settlement tx simulation reverted before submitting to the mempool"
+                );
+                return Err(Error::SimulationRevert);
+            } else {
+                tracing::warn!(
+                    ?err,
+                    "couldn't simulate tx before submitting to the mempool"
+                );
+            }
+        }
+
         let hash = mempool.submit(tx.clone(), settlement.gas, solver).await?;
         tracing::debug!(?hash, "submitted tx to the mempool");
 

crates/driver/src/infra/api/routes/mod.rs

@@ -15,34 +15,3 @@ pub(super) use {
     settle::settle,
     solve::{solve, AuctionError},
 };
-
-pub(crate) fn deserialize_solution_id<'de, D>(deserializer: D) -> Result<u64, D::Error>
-where
-    D: serde::Deserializer<'de>,
-{
-    struct SolutionIdVisitor;
-
-    impl serde::de::Visitor<'_> for SolutionIdVisitor {
-        type Value = u64;
-
-        fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-            formatter.write_str("a string or integer representing a solution ID")
-        }
-
-        fn visit_u64<E>(self, value: u64) -> Result<u64, E>
-        where
-            E: serde::de::Error,
-        {
-            Ok(value)
-        }
-
-        fn visit_str<E>(self, value: &str) -> Result<u64, E>
-        where
-            E: serde::de::Error,
-        {
-            value.parse::<u64>().map_err(serde::de::Error::custom)
-        }
-    }
-
-    deserializer.deserialize_any(SolutionIdVisitor)
-}

crates/driver/src/infra/api/routes/reveal/dto/reveal_request.rs

@@ -1,11 +1,10 @@
-use {super::super::super::deserialize_solution_id, serde::Deserialize, serde_with::serde_as};
+use {serde::Deserialize, serde_with::serde_as};
 
 #[serde_as]
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RevealRequest {
     /// Unique ID of the solution (per driver competition), to reveal.
-    #[serde(deserialize_with = "deserialize_solution_id")]
     pub solution_id: u64,
     /// Auction ID in which the specified solution ID is competing.
     #[serde_as(as = "Option<serde_with::DisplayFromStr>")]

crates/driver/src/infra/api/routes/settle/dto/settle_request.rs

@@ -1,11 +1,10 @@
-use {super::super::super::deserialize_solution_id, serde::Deserialize, serde_with::serde_as};
+use {serde::Deserialize, serde_with::serde_as};
 
 #[serde_as]
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct SettleRequest {
     /// Unique ID of the solution (per driver competition), to settle.
-    #[serde(deserialize_with = "deserialize_solution_id")]
     pub solution_id: u64,
     /// The last block number in which the solution TX can be included
     pub submission_deadline_latest_block: u64,

crates/driver/src/tests/cases/buy_eth.rs

@@ -16,5 +16,5 @@ async fn test() {
         .await;
 
     let id = test.solve().await.ok().orders(&[order]).id();
-    test.settle(&id).await.ok().await.eth_order_executed().await;
+    test.settle(id).await.ok().await.eth_order_executed().await;
 }