test: Base token price client, fix: ratio calculation multiplicative inverse error

Cargo.toml

@@ -128,6 +128,7 @@ google-cloud-storage = "0.20.0"
 governor = "0.4.2"
 hex = "0.4"
 http = "1.1"
+httpmock = "0.7.0"
 hyper = "1.3"
 iai = "0.1"
 insta = "1.29.0"

core/lib/external_price_api/Cargo.toml

@@ -24,3 +24,4 @@ rand.workspace = true
 zksync_config.workspace = true
 zksync_types.workspace = true
 tokio.workspace = true
+httpmock.workspace = true

core/lib/external_price_api/src/coingecko_api.rs

@@ -49,6 +49,7 @@ impl CoinGeckoPriceAPIClient {
         }
     }
 
+    /// returns ETH/BaseToken price of a token by address
     async fn get_token_price_by_address(&self, address: Address) -> anyhow::Result<f64> {
         let address_str = address_to_string(&address);
         let price_url = self
@@ -87,11 +88,13 @@ impl CoinGeckoPriceAPIClient {
 impl PriceAPIClient for CoinGeckoPriceAPIClient {
     async fn fetch_ratio(&self, token_address: Address) -> anyhow::Result<BaseTokenAPIRatio> {
         let base_token_in_eth = self.get_token_price_by_address(token_address).await?;
-        let (numerator, denominator) = get_fraction(base_token_in_eth);
+        let (num_in_eth, denom_in_eth) = get_fraction(base_token_in_eth);
+        // take reciprocal of price as returned price is ETH/BaseToken and BaseToken/ETH is needed
+        let (num_in_base, denom_in_base) = (denom_in_eth, num_in_eth);
 
         return Ok(BaseTokenAPIRatio {
-            numerator,
-            denominator,
+            numerator: num_in_base,
+            denominator: denom_in_base,
             ratio_timestamp: Utc::now(),
         });
     }
@@ -110,3 +113,174 @@ impl CoinGeckoPriceResponse {
             .and_then(|price| price.get(currency))
     }
 }
+
+#[cfg(test)]
+mod test {
+    use httpmock::MockServer;
+    use zksync_config::configs::external_price_api_client::DEFAULT_TIMEOUT_MS;
+
+    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)
+    }
+
+    #[test]
+    fn test_mock_response() {
+        // curl "https://api.coingecko.com/api/v3/simple/token_price/ethereum?contract_addresses=0x1f9840a85d5af5bf1d1762f925bdaddc4201f984&vs_currencies=eth"
+        // {"0x1f9840a85d5af5bf1d1762f925bdaddc4201f984":{"eth":0.00269512}}
+        assert_eq!(
+            get_mock_response("0x1f9840a85d5af5bf1d1762f925bdaddc4201f984", 0.00269512),
+            r#"{"0x1f9840a85d5af5bf1d1762f925bdaddc4201f984":{"eth":0.00269512}}"#
+        )
+    }
+
+    fn add_mock_by_address(
+        server: &MockServer,
+        // use string explicitly to verify that conversion of the address to string works as expected
+        address: String,
+        price: Option<f64>,
+        api_key: Option<String>,
+    ) {
+        server.mock(|mut when, then| {
+            when = when
+                .method(httpmock::Method::GET)
+                .path("/api/v3/simple/token_price/ethereum");
+
+            when = when.query_param("contract_addresses", address.clone());
+            when = when.query_param("vs_currencies", ETH_ID);
+            auth_check(when, api_key);
+
+            if let Some(p) = price {
+                then.status(200).body(get_mock_response(&address, p));
+            } else {
+                // requesting with invalid/unknown address results in empty json
+                // example:
+                // $ curl "https://api.coingecko.com/api/v3/simple/token_price/ethereum?contract_addresses=0x000000000000000000000000000000000000dead&vs_currencies=eth"
+                // {}
+                then.status(200).body("{}");
+            };
+        });
+    }
+
+    fn get_config(base_url: String, api_key: Option<String>) -> ExternalPriceApiClientConfig {
+        ExternalPriceApiClientConfig {
+            base_url: Some(base_url),
+            api_key,
+            source: "FILLER".to_string(),
+            client_timeout_ms: DEFAULT_TIMEOUT_MS,
+            forced: None,
+        }
+    }
+
+    fn happy_day_setup(
+        api_key: Option<String>,
+        server: &MockServer,
+        address: Address,
+        base_token_price: f64,
+    ) -> SetupResult {
+        add_mock_by_address(
+            server,
+            address_to_string(&address),
+            Some(base_token_price),
+            api_key.clone(),
+        );
+        SetupResult {
+            client: Box::new(CoinGeckoPriceAPIClient::new(get_config(
+                server.url(""),
+                api_key,
+            ))),
+        }
+    }
+
+    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)
+    }
+
+    #[tokio::test]
+    async fn test_happy_day_with_no_api_key() {
+        happy_day_test(happy_day_setup_no_key).await
+    }
+
+    fn error_404_setup(
+        server: &MockServer,
+        _address: Address,
+        _base_token_price: f64,
+    ) -> SetupResult {
+        // just don't add mock
+        SetupResult {
+            client: Box::new(CoinGeckoPriceAPIClient::new(get_config(
+                server.url(""),
+                Some("FILLER".to_string()),
+            ))),
+        }
+    }
+
+    #[tokio::test]
+    async fn test_error_404() {
+        let error_string = error_test(error_404_setup).await.to_string();
+        assert!(
+            error_string
+                .starts_with("Http error while fetching token price. Status: 404 Not Found"),
+            "Error was: {}",
+            &error_string
+        )
+    }
+
+    fn error_missing_setup(
+        server: &MockServer,
+        address: Address,
+        _base_token_price: f64,
+    ) -> SetupResult {
+        let api_key = Some("FILLER".to_string());
+
+        add_mock_by_address(server, address_to_string(&address), None, api_key.clone());
+        SetupResult {
+            client: Box::new(CoinGeckoPriceAPIClient::new(get_config(
+                server.url(""),
+                api_key,
+            ))),
+        }
+    }
+
+    #[tokio::test]
+    async fn test_error_missing() {
+        let error_string = error_test(error_missing_setup).await.to_string();
+        assert!(
+            error_string.starts_with("Price not found for token"),
+            "Error was: {}",
+            error_string
+        )
+    }
+}

core/lib/external_price_api/src/forced_price_client.rs

@@ -7,7 +7,7 @@ use zksync_types::{base_token_ratio::BaseTokenAPIRatio, Address};
 
 use crate::PriceAPIClient;
 
-// Struct for a a forced price "client" (conversion ratio is always a configured "forced" ratio).
+// Struct for a forced price "client" (conversion ratio is always a configured "forced" ratio).
 #[derive(Debug, Clone)]
 pub struct ForcedPriceClient {
     ratio: BaseTokenAPIRatio,

core/lib/external_price_api/src/lib.rs

@@ -1,5 +1,7 @@
 pub mod coingecko_api;
 pub mod forced_price_client;
+#[cfg(test)]
+mod tests;
 mod utils;
 
 use std::fmt;
@@ -11,6 +13,7 @@ use zksync_types::{base_token_ratio::BaseTokenAPIRatio, Address};
 #[async_trait]
 pub trait PriceAPIClient: Sync + Send + fmt::Debug + 'static {
     /// Returns the BaseToken<->ETH ratio for the input token address.
+    /// The returned unit is BaseToken/ETH. Example if 1 BaseToken = 0.002 ETH, then ratio is 500/1
     async fn fetch_ratio(&self, token_address: Address) -> anyhow::Result<BaseTokenAPIRatio>;
 }
 

core/lib/external_price_api/src/tests.rs

@@ -0,0 +1,52 @@
+use std::str::FromStr;
+
+use chrono::Utc;
+use httpmock::MockServer;
+use zksync_types::{base_token_ratio::BaseTokenAPIRatio, Address};
+
+use crate::{utils::get_fraction, PriceAPIClient};
+
+const TIME_TOLERANCE_MS: i64 = 100;
+
+pub(crate) struct SetupResult {
+    pub(crate) client: Box<dyn PriceAPIClient>,
+}
+
+pub(crate) type SetupFn =
+    fn(server: &MockServer, address: Address, base_token_price: f64) -> SetupResult;
+
+pub(crate) async fn happy_day_test(setup: SetupFn) {
+    let server = MockServer::start();
+    let address_str = "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984"; //Uniswap (UNI)
+    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);
+    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
+    );
+    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 = Address::from_str(address_str).unwrap();
+
+    let SetupResult { client } = setup(&server, address, 1.0);
+    let api_price = client.fetch_ratio(address).await;
+
+    assert!(api_price.is_err());
+    api_price.err().unwrap()
+}

core/lib/external_price_api/src/utils.rs

@@ -13,3 +13,33 @@ pub fn get_fraction(ratio_f64: f64) -> (NonZeroU64, NonZeroU64) {
 
     (numerator, denominator)
 }
+
+#[cfg(test)]
+pub(crate) mod tests {
+    use super::*;
+
+    fn assert_get_fraction_value(f: f64, num: u64, denum: u64) {
+        assert_eq!(
+            get_fraction(f),
+            (
+                NonZeroU64::try_from(num).unwrap(),
+                NonZeroU64::try_from(denum).unwrap()
+            )
+        );
+    }
+
+    #[allow(clippy::approx_constant)]
+    #[test]
+    fn test_float_to_fraction_conversion_as_expected() {
+        assert_get_fraction_value(1.0, 1, 1);
+        assert_get_fraction_value(1337.0, 1337, 1);
+        assert_get_fraction_value(0.1, 1, 10);
+        assert_get_fraction_value(3.141, 3141, 1000);
+        assert_get_fraction_value(1_000_000.0, 1_000_000, 1);
+        assert_get_fraction_value(3123.47, 312347, 100);
+        // below tests assume some not necessarily required behaviour of get_fraction
+        assert_get_fraction_value(0.2, 1, 5);
+        assert_get_fraction_value(0.5, 1, 2);
+        assert_get_fraction_value(3.1415, 6283, 2000);
+    }
+}

deny.toml

@@ -8,6 +8,7 @@ feature-depth = 1
 
 [advisories]
 ignore = [
+    "RUSTSEC-2024-0375", # atty dependency being unmaintained, dependency of clap and criterion, we would need to update to newer major of dependencies
     "RUSTSEC-2024-0320", # yaml_rust dependency being unmaintained, dependency in core, we should consider moving to yaml_rust2 fork
     "RUSTSEC-2020-0168", # mach dependency being unmaintained, dependency in consensus, we should consider moving to mach2 fork
     "RUSTSEC-2024-0370", # `cs_derive` needs to be updated to not rely on `proc-macro-error`