Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Quality::Unknown variant #3206

Merged
merged 3 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions crates/driver/src/domain/competition/bad_tokens/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
MartinquaXD marked this conversation as resolved.
Show resolved Hide resolved
}
value.last_updated = now;
})
.or_insert_with(|| CacheEntry {
quality,
is_supported,
last_updated: now,
});
}
Expand All @@ -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: &eth::TokenAddress, now: Instant) -> Option<Quality> {
let token = self.0.cache.get(token)?;
pub fn get_quality(&self, token: &eth::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,
}
}
}
38 changes: 20 additions & 18 deletions crates/driver/src/domain/competition/bad_tokens/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,26 @@ impl Detector {
}
}

pub fn get_quality(&self, token: &eth::TokenAddress, now: Instant) -> Option<Quality> {
let stats = self.counter.get(token)?;
pub fn get_quality(&self, token: &eth::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 {
Expand Down Expand Up @@ -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);
}
}
43 changes: 24 additions & 19 deletions crates/driver/src/domain/competition/bad_tokens/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ pub enum Quality {
/// contract - see <https://github.com/cowprotocol/services/pull/781>
/// * probably tons of other reasons
Unsupported,
/// The detection strategy does not have enough data to make an informed
/// decision.
Unknown,
}

#[derive(Default)]
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -120,22 +120,27 @@ impl Detector {
}
}

fn get_token_quality(&self, token: eth::TokenAddress, now: Instant) -> Option<Quality> {
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
}
}

Expand Down
23 changes: 10 additions & 13 deletions crates/driver/src/domain/competition/bad_tokens/simulation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub struct Detector(Arc<Inner>);
struct Inner {
cache: Cache,
detector: TraceCallDetectorRaw,
sharing: BoxRequestSharing<order::Uid, Option<Quality>>,
sharing: BoxRequestSharing<order::Uid, Quality>,
}

impl Detector {
Expand All @@ -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<Quality> {
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
Expand Down Expand Up @@ -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
}
}
}
Expand Down
Loading