diff --git a/crates/driver/src/domain/competition/bad_tokens/cache.rs b/crates/driver/src/domain/competition/bad_tokens/cache.rs index 88f11fcd8e..cbaf8b537b 100644 --- a/crates/driver/src/domain/competition/bad_tokens/cache.rs +++ b/crates/driver/src/domain/competition/bad_tokens/cache.rs @@ -25,7 +25,7 @@ struct CacheEntry { /// when the decision on the token quality was made last_updated: Instant, /// whether the token is supported or not - quality: Quality, + is_supported: bool, } impl Cache { @@ -39,23 +39,21 @@ impl Cache { } /// Updates whether or not a token should be considered supported. - pub fn update_quality(&self, token: eth::TokenAddress, quality: Quality, now: Instant) { + pub fn update_quality(&self, token: eth::TokenAddress, is_supported: bool, now: Instant) { self.0 .cache .entry(token) .and_modify(|value| { - if quality == Quality::Unsupported - || now.duration_since(value.last_updated) > self.0.max_age - { + if !is_supported || now.duration_since(value.last_updated) > self.0.max_age { // Only update the value if the cached value is outdated by now or // if the new value is "Unsupported". This means on conflicting updates // we err on the conservative side and assume a token is unsupported. - value.quality = quality; + value.is_supported = is_supported; } value.last_updated = now; }) .or_insert_with(|| CacheEntry { - quality, + is_supported, last_updated: now, }); } @@ -69,9 +67,15 @@ impl Cache { /// Returns the quality of the token if the cached value has not expired /// yet. - pub fn get_quality(&self, token: ð::TokenAddress, now: Instant) -> Option { - let token = self.0.cache.get(token)?; + pub fn get_quality(&self, token: ð::TokenAddress, now: Instant) -> Quality { + let Some(token) = self.0.cache.get(token) else { + return Quality::Unknown; + }; let still_valid = now.duration_since(token.last_updated) > self.0.max_age; - still_valid.then_some(token.quality) + match (still_valid, token.is_supported) { + (false, _) => Quality::Unknown, + (true, true) => Quality::Supported, + (true, false) => Quality::Unsupported, + } } } diff --git a/crates/driver/src/domain/competition/bad_tokens/metrics.rs b/crates/driver/src/domain/competition/bad_tokens/metrics.rs index 43e444dca2..de81d417b4 100644 --- a/crates/driver/src/domain/competition/bad_tokens/metrics.rs +++ b/crates/driver/src/domain/competition/bad_tokens/metrics.rs @@ -45,19 +45,26 @@ impl Detector { } } - pub fn get_quality(&self, token: ð::TokenAddress, now: Instant) -> Option { - let stats = self.counter.get(token)?; + pub fn get_quality(&self, token: ð::TokenAddress, now: Instant) -> Quality { + let Some(stats) = self.counter.get(token) else { + return Quality::Unknown; + }; + if stats .flagged_unsupported_at .is_some_and(|t| now.duration_since(t) > self.token_freeze_time) { // Sometimes tokens only cause issues temporarily. If the token's freeze - // period expired we give it another chance to see if it still behaves badly. - return None; + // period expired we pretend we don't have enough information to give it + // another chance. If it still behaves badly it will get frozen immediately. + return Quality::Unknown; } let is_unsupported = self.stats_indicate_unsupported(&stats); - (!self.log_only && is_unsupported).then_some(Quality::Unsupported) + match !self.log_only && is_unsupported { + true => Quality::Unsupported, + false => Quality::Supported, + } } fn stats_indicate_unsupported(&self, stats: &TokenStatistics) -> bool { @@ -128,28 +135,23 @@ mod tests { let token_a = eth::TokenAddress(eth::ContractAddress(H160([1; 20]))); let token_b = eth::TokenAddress(eth::ContractAddress(H160([2; 20]))); + let token_quality = || detector.get_quality(&token_a, Instant::now()); - // token is reported as supported while we don't have enough measurements - assert_eq!(detector.get_quality(&token_a, Instant::now()), None); + // token is reported as unknown while we don't have enough measurements + assert_eq!(token_quality(), Quality::Unknown); detector.update_tokens(&[(token_a, token_b)], true); - assert_eq!(detector.get_quality(&token_a, Instant::now()), None); + assert_eq!(token_quality(), Quality::Unknown); detector.update_tokens(&[(token_a, token_b)], true); // after we got enough measurements the token gets marked as bad - assert_eq!( - detector.get_quality(&token_a, Instant::now()), - Some(Quality::Unsupported) - ); + assert_eq!(token_quality(), Quality::Unsupported); - // after the freeze period is over the token gets reported as good again + // after the freeze period is over the token gets reported as unknown again tokio::time::sleep(FREEZE_DURATION).await; - assert_eq!(detector.get_quality(&token_a, Instant::now()), None); + assert_eq!(token_quality(), Quality::Unknown); // after an unfreeze another bad measurement is enough to freeze it again detector.update_tokens(&[(token_a, token_b)], true); - assert_eq!( - detector.get_quality(&token_a, Instant::now()), - Some(Quality::Unsupported) - ); + assert_eq!(token_quality(), Quality::Unsupported); } } diff --git a/crates/driver/src/domain/competition/bad_tokens/mod.rs b/crates/driver/src/domain/competition/bad_tokens/mod.rs index d724bea442..ab241183d5 100644 --- a/crates/driver/src/domain/competition/bad_tokens/mod.rs +++ b/crates/driver/src/domain/competition/bad_tokens/mod.rs @@ -23,6 +23,9 @@ pub enum Quality { /// contract - see /// * probably tons of other reasons Unsupported, + /// The detection strategy does not have enough data to make an informed + /// decision. + Unknown, } #[derive(Default)] @@ -67,26 +70,23 @@ impl Detector { let buy = self.get_token_quality(order.buy.token, now); match (sell, buy) { // both tokens supported => keep order - (Some(Quality::Supported), Some(Quality::Supported)) => Either::Left(order), + (Quality::Supported, Quality::Supported) => Either::Left(order), // at least 1 token unsupported => drop order - (Some(Quality::Unsupported), _) | (_, Some(Quality::Unsupported)) => { - Either::Right(order.uid) - } + (Quality::Unsupported, _) | (_, Quality::Unsupported) => Either::Right(order.uid), // sell token quality is unknown => keep order if token is supported - (None, _) => { + (Quality::Unknown, _) => { let Some(detector) = &self.simulation_detector else { // we can't determine quality => assume order is good return Either::Left(order); }; - let quality = detector.determine_sell_token_quality(&order, now).await; - match quality { - Some(Quality::Supported) => Either::Left(order), + match detector.determine_sell_token_quality(&order, now).await { + Quality::Supported => Either::Left(order), _ => Either::Right(order.uid), } } // buy token quality is unknown => keep order (because we can't // determine quality and assume it's good) - (_, None) => Either::Left(order), + (_, Quality::Unknown) => Either::Left(order), } }); let (supported_orders, removed_uids): (Vec<_>, Vec<_>) = join_all(token_quality_checks) @@ -120,22 +120,27 @@ impl Detector { } } - fn get_token_quality(&self, token: eth::TokenAddress, now: Instant) -> Option { - if let Some(quality) = self.hardcoded.get(&token) { - return Some(*quality); + fn get_token_quality(&self, token: eth::TokenAddress, now: Instant) -> Quality { + match self.hardcoded.get(&token) { + None | Some(Quality::Unknown) => (), + Some(quality) => return *quality, } - if let Some(detector) = &self.simulation_detector { - if let Some(quality) = detector.get_quality(&token, now) { - return Some(quality); - } + if let Some(Quality::Unsupported) = self + .simulation_detector + .as_ref() + .map(|d| d.get_quality(&token, now)) + { + return Quality::Unsupported; } - if let Some(metrics) = &self.metrics { - return metrics.get_quality(&token, now); + if let Some(Quality::Unsupported) = + self.metrics.as_ref().map(|m| m.get_quality(&token, now)) + { + return Quality::Unsupported; } - None + Quality::Unknown } } diff --git a/crates/driver/src/domain/competition/bad_tokens/simulation.rs b/crates/driver/src/domain/competition/bad_tokens/simulation.rs index fe3291c306..dd8eb5e0f4 100644 --- a/crates/driver/src/domain/competition/bad_tokens/simulation.rs +++ b/crates/driver/src/domain/competition/bad_tokens/simulation.rs @@ -32,7 +32,7 @@ pub struct Detector(Arc); struct Inner { cache: Cache, detector: TraceCallDetectorRaw, - sharing: BoxRequestSharing>, + sharing: BoxRequestSharing, } impl Detector { @@ -49,14 +49,11 @@ impl Detector { /// Simulates how the sell token behaves during transfers. Assumes that /// the order owner has the required sell token balance and approvals /// set. - pub async fn determine_sell_token_quality( - &self, - order: &Order, - now: Instant, - ) -> Option { + pub async fn determine_sell_token_quality(&self, order: &Order, now: Instant) -> Quality { let cache = &self.0.cache; - if let Some(quality) = cache.get_quality(&order.sell.token, now) { - return Some(quality); + let quality = cache.get_quality(&order.sell.token, now); + if quality != Quality::Unknown { + return quality; } // The simulation detector gets used by multiple solvers at the same time @@ -93,20 +90,20 @@ impl Detector { match result { Err(err) => { tracing::debug!(?err, token=?sell_token.0, "failed to determine token quality"); - None + Quality::Unknown } Ok(TokenQuality::Good) => { inner .cache - .update_quality(sell_token, Quality::Supported, now); - Some(Quality::Supported) + .update_quality(sell_token, true, now); + Quality::Supported } Ok(TokenQuality::Bad { reason }) => { tracing::debug!(reason, token=?sell_token.0, "cache token as unsupported"); inner .cache - .update_quality(sell_token, Quality::Unsupported, now); - Some(Quality::Unsupported) + .update_quality(sell_token, false, now); + Quality::Unsupported } } }