From 980f505c7613517f87b2358e7ffa67686083414e Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Tue, 5 Nov 2024 09:29:36 +0000 Subject: [PATCH 1/3] Ignore nonsensical native prices --- .../price_estimation/competition/native.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/shared/src/price_estimation/competition/native.rs b/crates/shared/src/price_estimation/competition/native.rs index 0600500fa1..2d1f17738b 100644 --- a/crates/shared/src/price_estimation/competition/native.rs +++ b/crates/shared/src/price_estimation/competition/native.rs @@ -4,6 +4,7 @@ use { native::{NativePriceEstimateResult, NativePriceEstimating}, PriceEstimationError, }, + anyhow::Context, futures::{future::BoxFuture, FutureExt}, model::order::OrderKind, primitive_types::H160, @@ -14,18 +15,25 @@ impl NativePriceEstimating for CompetitionEstimator BoxFuture<'_, NativePriceEstimateResult> { async move { let results = self - .produce_results(token, Result::is_ok, |e, q| e.estimate_native_price(q)) + .produce_results(token, is_result_usable, |e, q| e.estimate_native_price(q)) .await; let winner = results .into_iter() + .filter(|(_index, result)| is_result_usable(result)) .max_by(|a, b| compare_native_result(&a.1, &b.1)) - .expect("we get passed at least 1 result and did not filter out any of them"); + .context("could not get any native price")?; self.report_winner(&token, OrderKind::Buy, winner) } .boxed() } } +fn is_result_usable(result: &NativePriceEstimateResult) -> bool { + result + .as_ref() + .is_ok_and(|price| price.is_normal() && *price > 0.) +} + fn compare_native_result( a: &Result, b: &Result, @@ -123,4 +131,16 @@ mod tests { .await; assert_eq!(best, native_price(1.)); } + + /// Nonsensical prices like infinities, and non-positive values get ignored. + #[tokio::test] + async fn ignore_nonsensical_prices() { + let subnormal = f64::from_bits(1); + assert!(!subnormal.is_normal()); + + for price in [f64::NEG_INFINITY, -1., 0., f64::INFINITY, subnormal] { + let best = best_response(PriceRanking::MaxOutAmount, vec![native_price(price)]).await; + assert!(best.is_err()); + } + } } From 7e9d89962ac6ac49ee5a8018d21cc43db1bdf52c Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Tue, 5 Nov 2024 10:10:55 +0000 Subject: [PATCH 2/3] Fixup error handling --- .../price_estimation/competition/native.rs | 21 +++++++++++-------- crates/shared/src/price_estimation/native.rs | 12 ++++++++++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/crates/shared/src/price_estimation/competition/native.rs b/crates/shared/src/price_estimation/competition/native.rs index 2d1f17738b..53c4babab3 100644 --- a/crates/shared/src/price_estimation/competition/native.rs +++ b/crates/shared/src/price_estimation/competition/native.rs @@ -1,7 +1,7 @@ use { super::{compare_error, CompetitionEstimator}, crate::price_estimation::{ - native::{NativePriceEstimateResult, NativePriceEstimating}, + native::{is_price_malformed, NativePriceEstimateResult, NativePriceEstimating}, PriceEstimationError, }, anyhow::Context, @@ -15,11 +15,20 @@ impl NativePriceEstimating for CompetitionEstimator BoxFuture<'_, NativePriceEstimateResult> { async move { let results = self - .produce_results(token, is_result_usable, |e, q| e.estimate_native_price(q)) + .produce_results(token, Result::is_ok, |e, q| { + async move { + let res = e.estimate_native_price(q).await; + if res.as_ref().is_ok_and(|price| is_price_malformed(*price)) { + let err = anyhow::anyhow!("estimator returned malformed price"); + return Err(PriceEstimationError::EstimatorInternal(err)); + } + res + } + .boxed() + }) .await; let winner = results .into_iter() - .filter(|(_index, result)| is_result_usable(result)) .max_by(|a, b| compare_native_result(&a.1, &b.1)) .context("could not get any native price")?; self.report_winner(&token, OrderKind::Buy, winner) @@ -28,12 +37,6 @@ impl NativePriceEstimating for CompetitionEstimator bool { - result - .as_ref() - .is_ok_and(|price| price.is_normal() && *price > 0.) -} - fn compare_native_result( a: &Result, b: &Result, diff --git a/crates/shared/src/price_estimation/native.rs b/crates/shared/src/price_estimation/native.rs index dba3c8886b..6fb221795b 100644 --- a/crates/shared/src/price_estimation/native.rs +++ b/crates/shared/src/price_estimation/native.rs @@ -93,12 +93,22 @@ impl NativePriceEstimating for NativePriceEstimator { async move { let query = Arc::new(self.query(&token)); let estimate = self.inner.estimate(query.clone()).await?; - Ok(estimate.price_in_buy_token_f64(&query)) + let price = estimate.price_in_buy_token_f64(&query); + if !is_price_malformed(price) { + let err = anyhow::anyhow!("estimator returned malformed price: {price}"); + Err(PriceEstimationError::EstimatorInternal(err)) + } else { + Ok(price) + } } .boxed() } } +pub(crate) fn is_price_malformed(price: f64) -> bool { + !price.is_normal() || price <= 0. +} + #[cfg(test)] mod tests { use { From 3720065eb8b178e913d9a61cd70760842d751032 Mon Sep 17 00:00:00 2001 From: MartinquaXD Date: Tue, 5 Nov 2024 10:15:11 +0000 Subject: [PATCH 3/3] Fixup inverted check --- crates/shared/src/price_estimation/native.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/shared/src/price_estimation/native.rs b/crates/shared/src/price_estimation/native.rs index 6fb221795b..e0928ddb8d 100644 --- a/crates/shared/src/price_estimation/native.rs +++ b/crates/shared/src/price_estimation/native.rs @@ -94,7 +94,7 @@ impl NativePriceEstimating for NativePriceEstimator { let query = Arc::new(self.query(&token)); let estimate = self.inner.estimate(query.clone()).await?; let price = estimate.price_in_buy_token_f64(&query); - if !is_price_malformed(price) { + if is_price_malformed(price) { let err = anyhow::anyhow!("estimator returned malformed price: {price}"); Err(PriceEstimationError::EstimatorInternal(err)) } else {