fix: review feedback

core/lib/external_price_api/src/coingecko_api.rs

@@ -122,14 +122,6 @@ mod test {
     use super::*;
     use crate::tests::*;
 
-    fn auth_check(when: httpmock::When, api_key: Option<String>) -> httpmock::When {
-        if let Some(key) = api_key {
-            when.header(COINGECKO_AUTH_HEADER, key)
-        } else {
-            when
-        }
-    }
-
     fn get_mock_response(address: &str, price: f64) -> String {
         format!("{{\"{}\":{{\"eth\":{}}}}}", address, price)
     }
@@ -158,7 +150,7 @@ mod test {
 
             when = when.query_param("contract_addresses", address.clone());
             when = when.query_param("vs_currencies", ETH_ID);
-            auth_check(when, api_key);
+            api_key.map(|key| when.header(COINGECKO_AUTH_HEADER, key));
 
             if let Some(p) = price {
                 then.status(200).body(get_mock_response(&address, p));
@@ -176,7 +168,7 @@ mod test {
         ExternalPriceApiClientConfig {
             base_url: Some(base_url),
             api_key,
-            source: "FILLER".to_string(),
+            source: "coingecko".to_string(),
             client_timeout_ms: DEFAULT_TIMEOUT_MS,
             forced: None,
         }
@@ -202,35 +194,29 @@ mod test {
         }
     }
 
-    fn happy_day_setup_with_key(
-        server: &MockServer,
-        address: Address,
-        base_token_price: f64,
-    ) -> SetupResult {
-        happy_day_setup(
-            Some("test-key".to_string()),
-            server,
-            address,
-            base_token_price,
-        )
-    }
-
     #[tokio::test]
     async fn test_happy_day_with_api_key() {
-        happy_day_test(happy_day_setup_with_key).await
-    }
-
-    fn happy_day_setup_no_key(
-        server: &MockServer,
-        address: Address,
-        base_token_price: f64,
-    ) -> SetupResult {
-        happy_day_setup(None, server, address, base_token_price)
+        happy_day_test(
+            |server: &MockServer, address: Address, base_token_price: f64| {
+                happy_day_setup(
+                    Some("test-key".to_string()),
+                    server,
+                    address,
+                    base_token_price,
+                )
+            },
+        )
+        .await
     }
 
     #[tokio::test]
     async fn test_happy_day_with_no_api_key() {
-        happy_day_test(happy_day_setup_no_key).await
+        happy_day_test(
+            |server: &MockServer, address: Address, base_token_price: f64| {
+                happy_day_setup(None, server, address, base_token_price)
+            },
+        )
+        .await
     }
 
     fn error_404_setup(

core/lib/external_price_api/src/tests.rs

@@ -2,11 +2,19 @@ use std::str::FromStr;
 
 use chrono::Utc;
 use httpmock::MockServer;
-use zksync_types::{base_token_ratio::BaseTokenAPIRatio, Address};
+use zksync_types::Address;
 
-use crate::{utils::get_fraction, PriceAPIClient};
+use crate::PriceAPIClient;
 
 const TIME_TOLERANCE_MS: i64 = 100;
+/// Uniswap (UNI)
+const TEST_TOKEN_ADDRESS: &str = "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984";
+/// 1UNI = 0.00269ETH
+const TEST_TOKEN_PRICE_ETH: f64 = 0.00269;
+/// 1ETH = 371.74UNI; When converting gas price from ETH to UNI
+/// you need to multiply by this value. Thus, this should be equal to the ratio.
+const TEST_BASE_PRICE: f64 = 371.74;
+const PRICE_FLOAT_COMPARE_TOLERANCE: f64 = 0.1;
 
 pub(crate) struct SetupResult {
     pub(crate) client: Box<dyn PriceAPIClient>,
@@ -17,31 +25,27 @@ pub(crate) type SetupFn =
 
 pub(crate) async fn happy_day_test(setup: SetupFn) {
     let server = MockServer::start();
-    let address_str = "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984"; //Uniswap (UNI)
+    let address_str = TEST_TOKEN_ADDRESS;
     let address = Address::from_str(address_str).unwrap();
-    let base_token_price = 0.00269; //ETH costs one token
 
-    let SetupResult { client } = setup(&server, address, base_token_price);
+    // APIs return token price in ETH (ETH per 1 token)
+    let SetupResult { client } = setup(&server, address, TEST_TOKEN_PRICE_ETH);
     let api_price = client.fetch_ratio(address).await.unwrap();
 
-    let (num_in_eth, denom_in_eth) = get_fraction(base_token_price);
-    let (ratio_num, ratio_denom) = (denom_in_eth, num_in_eth);
-    assert!(((ratio_num.get() as f64) / (ratio_denom.get() as f64) - 371.74).abs() < 0.1);
-
-    assert_eq!(
-        BaseTokenAPIRatio {
-            numerator: ratio_num,
-            denominator: ratio_denom,
-            ratio_timestamp: api_price.ratio_timestamp,
-        },
-        api_price
+    // we expect the returned ratio to be such that when multiplying gas price in ETH you get gas
+    // price in base token. So we expect such ratio X that X Base = 1ETH
+    assert!(
+        ((api_price.numerator.get() as f64) / (api_price.denominator.get() as f64)
+            - TEST_BASE_PRICE)
+            .abs()
+            < PRICE_FLOAT_COMPARE_TOLERANCE
     );
     assert!((Utc::now() - api_price.ratio_timestamp).num_milliseconds() <= TIME_TOLERANCE_MS);
 }
 
 pub(crate) async fn error_test(setup: SetupFn) -> anyhow::Error {
     let server = MockServer::start();
-    let address_str = "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984";
+    let address_str = TEST_TOKEN_ADDRESS;
     let address = Address::from_str(address_str).unwrap();
 
     let SetupResult { client } = setup(&server, address, 1.0);

core/lib/external_price_api/src/utils.rs

@@ -5,10 +5,13 @@ use fraction::Fraction;
 /// Using the base token price and eth price, calculate the fraction of the base token to eth.
 pub fn get_fraction(ratio_f64: f64) -> (NonZeroU64, NonZeroU64) {
     let rate_fraction = Fraction::from(ratio_f64);
+    if rate_fraction.sign() == Some(fraction::Sign::Minus) {
+        panic!("number is negative");
+    }
 
-    let numerator = NonZeroU64::new(*rate_fraction.numer().expect("numerator is empty"))
+    let numerator = NonZeroU64::new(*rate_fraction.numer().expect("number is not rational"))
         .expect("numerator is zero");
-    let denominator = NonZeroU64::new(*rate_fraction.denom().expect("denominator is empty"))
+    let denominator = NonZeroU64::new(*rate_fraction.denom().expect("number is not rational"))
         .expect("denominator is zero");
 
     (numerator, denominator)
@@ -42,4 +45,28 @@ pub(crate) mod tests {
         assert_get_fraction_value(0.5, 1, 2);
         assert_get_fraction_value(3.1415, 6283, 2000);
     }
+
+    #[should_panic(expected = "numerator is zero")]
+    #[test]
+    fn test_zero_panics() {
+        get_fraction(0.0);
+    }
+
+    #[should_panic(expected = "number is negative")]
+    #[test]
+    fn test_negative() {
+        get_fraction(-1.0);
+    }
+
+    #[should_panic(expected = "number is not rational")]
+    #[test]
+    fn test_nan() {
+        get_fraction(f64::NAN);
+    }
+
+    #[should_panic(expected = "number is not rational")]
+    #[test]
+    fn test_infinity() {
+        get_fraction(f64::INFINITY);
+    }
 }