Skip to content

Commit

Permalink
Avoid looking up 0x000 tx_hash (#2358)
Browse files Browse the repository at this point in the history
# Description
While the logic introduced in #2123 is sound the concern brought up by
Dusan
[here](#2123 (comment))
caused issues down the line.
Specifically looking up the `0x000` tx_hash will always fail and would
get logged as an error in the node proxy which surfaced
[here](cowprotocol/infrastructure#1062 (comment))

Over all it's probably best to avoid issuing requests we know will fail
even it the system is able to handle an error on those.

# Changes
Make `InFlightOrders` optional and don't fetch the tx_receipt when it
has not been initialized yet.
I decided to use `tokio::sync::Mutex` instead of `std::sync::Mutex`
because calling a future while holding the lock made the code cleaner
and since there is no contention on the lock due to the (currently)
sequential nature of auctioneering holding the lock while fetching the
tx receipt is not an issue.
  • Loading branch information
MartinquaXD authored Feb 2, 2024
1 parent 6213928 commit 4bb2e59
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions crates/autopilot/src/run_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@ use {
shared::token_list::AutoUpdatingTokenList,
std::{
collections::{BTreeMap, HashMap, HashSet},
sync::{Arc, Mutex},
sync::Arc,
time::{Duration, Instant},
},
tokio::sync::Mutex,
tracing::Instrument,
web3::types::TransactionReceipt,
};
Expand All @@ -48,7 +49,7 @@ pub struct RunLoop {
pub score_cap: U256,
pub max_settlement_transaction_wait: Duration,
pub solve_deadline: Duration,
pub in_flight_orders: Arc<Mutex<InFlightOrders>>,
pub in_flight_orders: Arc<Mutex<Option<InFlightOrders>>>,
pub liveness: Arc<Liveness>,
}

Expand Down Expand Up @@ -429,10 +430,10 @@ impl RunLoop {
.map_err(SettleError::Failure)?
.tx_hash;

*self.in_flight_orders.lock().unwrap() = InFlightOrders {
*self.in_flight_orders.lock().await = Some(InFlightOrders {
tx_hash,
orders: solved.orders.keys().copied().collect(),
};
});

let order_uids = solved.orders.keys().copied().collect();
self.persistence
Expand All @@ -445,8 +446,11 @@ impl RunLoop {
/// Removes orders that are currently being settled to avoid solvers trying
/// to fill an order a second time.
async fn remove_in_flight_orders(&self, mut auction: domain::Auction) -> domain::Auction {
let prev_settlement = self.in_flight_orders.lock().unwrap().tx_hash;
let tx_receipt = self.eth.transaction_receipt(prev_settlement).await;
let Some(in_flight) = &*self.in_flight_orders.lock().await else {
return auction;
};

let tx_receipt = self.eth.transaction_receipt(in_flight.tx_hash).await;

let prev_settlement_block = match tx_receipt {
Ok(Some(TransactionReceipt {
Expand All @@ -460,11 +464,10 @@ impl RunLoop {

if auction.latest_settlement_block < prev_settlement_block {
// Auction was built before the in-flight orders were processed.
let in_flight_orders = self.in_flight_orders.lock().unwrap();
auction
.orders
.retain(|o| !in_flight_orders.orders.contains(&o.uid));
tracing::debug!(orders = ?in_flight_orders.orders, "filtered out in-flight orders");
.retain(|o| !in_flight.orders.contains(&o.uid));
tracing::debug!(orders = ?in_flight.orders, "filtered out in-flight orders");
}

auction
Expand Down

0 comments on commit 4bb2e59

Please sign in to comment.