papyrus_base_layer: extract check and cycle URLs into a helper function with logging

Author: guy-starkware

crates/papyrus_base_layer/src/cyclic_base_layer_wrapper.rs

@@ -2,6 +2,7 @@ use std::ops::RangeInclusive;
 
 use async_trait::async_trait;
 use starknet_api::block::BlockHashAndNumber;
+use tracing::info;
 use url::Url;
 
 use crate::{BaseLayerContract, L1BlockHeader, L1BlockNumber, L1BlockReference, L1Event};
@@ -18,6 +19,52 @@ impl<B: BaseLayerContract + Send + Sync> CyclicBaseLayerWrapper<B> {
     pub fn new(base_layer: B) -> Self {
         Self { base_layer }
     }
+
+    // Check the result of a function call to the base layer. If it fails, cycle the URL and signal
+    // the caller that we should try again (by returning None).
+    async fn cycle_url_on_error<ReturnType: std::fmt::Debug>(
+        &mut self,
+        start_url: &Url,
+        result: Result<ReturnType, B::Error>,
+    ) -> Option<Result<ReturnType, B::Error>> {
+        // In case we succeed, just return the (successful) result.
+        if result.is_ok() {
+            return Some(result);
+        }
+        // Get the current URL (return error in case it fails to get it).
+        let current_url_result = self.base_layer.get_url().await;
+        let Ok(current_url) = current_url_result else {
+            return Some(Err(current_url_result.expect_err("result is checked at let-else")));
+        };
+        // Otherwise, cycle the URL so we can try again. Return error in case it fails to cycle.
+        let cycle_url_result = self.base_layer.cycle_provider_url().await;
+        let Ok(()) = cycle_url_result else {
+            return Some(Err(cycle_url_result.expect_err("result is checked at let-else")));
+        };
+        // Get the new URL (return error in case it fails to get it).
+        let new_url_result = self.base_layer.get_url().await;
+        let Ok(new_url) = new_url_result else {
+            return Some(Err(new_url_result.expect_err("result is checked at let-else")));
+        };
+        info!(
+            "Cycling URL from {:?} to {:?}",
+            to_safe_string(&current_url),
+            to_safe_string(&new_url)
+        );
+
+        // If we've cycled back to the start URL, we need to return the last error we got.
+        if &new_url == start_url {
+            info!(
+                "Cycled back to start URL {:?}, returning error {:?}.",
+                to_safe_string(start_url),
+                result
+            );
+            return Some(result);
+        }
+        // If we cycled but still haven't reached the start URL, we return None to signal that we
+        // should try again with the new URL.
+        None
+    }
 }
 
 #[async_trait]
@@ -31,12 +78,8 @@ impl<B: BaseLayerContract + Send + Sync> BaseLayerContract for CyclicBaseLayerWr
         let start_url = self.base_layer.get_url().await?;
         loop {
             let result = self.base_layer.get_proved_block_at(l1_block).await;
-            if result.is_ok() {
-                return result;
-            }
-            self.base_layer.cycle_provider_url().await?;
-            if self.base_layer.get_url().await? == start_url {
-                return result;
+            if let Some(result) = self.cycle_url_on_error(&start_url, result).await {
+                return result; // Could return a success or an error.
             }
         }
     }
@@ -45,12 +88,8 @@ impl<B: BaseLayerContract + Send + Sync> BaseLayerContract for CyclicBaseLayerWr
         let start_url = self.base_layer.get_url().await?;
         loop {
             let result = self.base_layer.latest_l1_block_number().await;
-            if result.is_ok() {
-                return result;
-            }
-            self.base_layer.cycle_provider_url().await?;
-            if self.base_layer.get_url().await? == start_url {
-                return result;
+            if let Some(result) = self.cycle_url_on_error(&start_url, result).await {
+                return result; // Could return a success or an error.
             }
         }
     }
@@ -62,12 +101,8 @@ impl<B: BaseLayerContract + Send + Sync> BaseLayerContract for CyclicBaseLayerWr
         let start_url = self.base_layer.get_url().await?;
         loop {
             let result = self.base_layer.l1_block_at(block_number).await;
-            if result.is_ok() {
-                return result;
-            }
-            self.base_layer.cycle_provider_url().await?;
-            if self.base_layer.get_url().await? == start_url {
-                return result;
+            if let Some(result) = self.cycle_url_on_error(&start_url, result).await {
+                return result; // Could return a success or an error.
             }
         }
     }
@@ -80,12 +115,8 @@ impl<B: BaseLayerContract + Send + Sync> BaseLayerContract for CyclicBaseLayerWr
         let start_url = self.base_layer.get_url().await?;
         loop {
             let result = self.base_layer.events(block_range.clone(), event_identifiers).await;
-            if result.is_ok() {
-                return result;
-            }
-            self.base_layer.cycle_provider_url().await?;
-            if self.base_layer.get_url().await? == start_url {
-                return result;
+            if let Some(result) = self.cycle_url_on_error(&start_url, result).await {
+                return result; // Could return a success or an error.
             }
         }
     }
@@ -97,12 +128,8 @@ impl<B: BaseLayerContract + Send + Sync> BaseLayerContract for CyclicBaseLayerWr
         let start_url = self.base_layer.get_url().await?;
         loop {
             let result = self.base_layer.get_block_header(block_number).await;
-            if result.is_ok() {
-                return result;
-            }
-            self.base_layer.cycle_provider_url().await?;
-            if self.base_layer.get_url().await? == start_url {
-                return result;
+            if let Some(result) = self.cycle_url_on_error(&start_url, result).await {
+                return result; // Could return a success or an error.
             }
         }
     }
@@ -126,3 +153,8 @@ impl<B: BaseLayerContract + Send + Sync> BaseLayerContract for CyclicBaseLayerWr
         self.base_layer.cycle_provider_url().await
     }
 }
+
+fn to_safe_string(url: &Url) -> String {
+    // We print only the hostnames to avoid leaking the API keys.
+    url.host().map_or_else(|| "no host in url!".to_string(), |host| host.to_string())
+}

crates/papyrus_base_layer/src/cyclic_base_layer_wrapper_test.rs

@@ -30,7 +30,7 @@ const NUM_URLS: usize = 2;
 async fn cycle_get_proved_block_at(#[case] num_failing_calls: usize) {
     // Setup.
     let success = num_failing_calls < NUM_URLS;
-    let expected_num_url_calls = num_failing_calls + 1;
+    let expected_num_url_calls = num_failing_calls * 2 + 1;
     let num_url_calls_made = Arc::new(AtomicUsize::new(0));
     let num_url_calls_made_clone = num_url_calls_made.clone();
     let num_url_calls_made_clone2 = num_url_calls_made.clone();
@@ -73,7 +73,7 @@ async fn cycle_get_proved_block_at(#[case] num_failing_calls: usize) {
 async fn cycle_latest_l1_block_number(#[case] num_failing_calls: usize) {
     // Setup.
     let success = num_failing_calls < NUM_URLS;
-    let expected_num_url_calls = num_failing_calls + 1;
+    let expected_num_url_calls = num_failing_calls * 2 + 1;
     let num_url_calls_made = Arc::new(AtomicUsize::new(0));
     let num_url_calls_made_clone = num_url_calls_made.clone();
     let num_url_calls_made_clone2 = num_url_calls_made.clone();
@@ -113,7 +113,7 @@ async fn cycle_latest_l1_block_number(#[case] num_failing_calls: usize) {
 async fn cycle_l1_block_at(#[case] num_failing_calls: usize) {
     // Setup.
     let success = num_failing_calls < NUM_URLS;
-    let expected_num_url_calls = num_failing_calls + 1;
+    let expected_num_url_calls = num_failing_calls * 2 + 1;
     let num_url_calls_made = Arc::new(AtomicUsize::new(0));
     let num_url_calls_made_clone = num_url_calls_made.clone();
     let num_url_calls_made_clone2 = num_url_calls_made.clone();
@@ -156,7 +156,7 @@ async fn cycle_l1_block_at(#[case] num_failing_calls: usize) {
 async fn cycle_events(#[case] num_failing_calls: usize) {
     // Setup.
     let success = num_failing_calls < NUM_URLS;
-    let expected_num_url_calls = num_failing_calls + 1;
+    let expected_num_url_calls = num_failing_calls * 2 + 1;
     let num_url_calls_made = Arc::new(AtomicUsize::new(0));
     let num_url_calls_made_clone = num_url_calls_made.clone();
     let num_url_calls_made_clone2 = num_url_calls_made.clone();
@@ -196,7 +196,7 @@ async fn cycle_events(#[case] num_failing_calls: usize) {
 async fn cycle_get_block_header(#[case] num_failing_calls: usize) {
     // Setup.
     let success = num_failing_calls < NUM_URLS;
-    let expected_num_url_calls = num_failing_calls + 1;
+    let expected_num_url_calls = num_failing_calls * 2 + 1;
     let num_url_calls_made = Arc::new(AtomicUsize::new(0));
     let num_url_calls_made_clone = num_url_calls_made.clone();
     let num_url_calls_made_clone2 = num_url_calls_made.clone();
@@ -255,7 +255,7 @@ async fn get_block_header_fails_after_cycle_error() {
     let mut base_layer = MockBaseLayerContract::new();
     base_layer
         .expect_get_url()
-        .times(1)
+        .times(2)
         .returning(move || Ok(Url::parse("http://first_endpoint").unwrap()));
     base_layer.expect_get_block_header().times(1).returning(move |_| Err(MockError::MockError));
     base_layer.expect_cycle_provider_url().times(1).returning(move || Err(MockError::MockError));