apollo_l1_provider: rename catch_up_height and historic height

Author: guy-starkware

crates/apollo_l1_provider/src/bootstrapper.rs

@@ -20,7 +20,7 @@ use tracing::debug;
 /// Caches commits to be applied later. This flow is only relevant while the node is starting up.
 #[derive(Clone)]
 pub struct Bootstrapper {
-    pub catch_up_height: BlockNumber,
+    pub target_height: BlockNumber,
     pub sync_retry_interval: Duration,
     pub commit_block_backlog: Vec<CommitBlockBacklog>,
     pub l1_provider_client: SharedL1ProviderClient,
@@ -49,13 +49,13 @@ impl Bootstrapper {
             n_sync_health_check_failures: Default::default(),
             // This is overriden when starting the sync task (e.g., when provider starts
             // bootstrapping).
-            catch_up_height: BlockNumber(0),
+            target_height: BlockNumber(0),
         }
     }
 
     /// Check if the caller has caught up with the bootstrapper.
     pub fn is_caught_up(&self, current_provider_height: BlockNumber) -> bool {
-        let is_caught_up = current_provider_height > self.catch_up_height;
+        let is_caught_up = current_provider_height > self.target_height;
 
         self.sync_task_health_check(is_caught_up);
 
@@ -84,9 +84,9 @@ impl Bootstrapper {
     pub fn start_l2_sync(
         &mut self,
         current_provider_height: BlockNumber,
-        catch_up_height: BlockNumber,
+        target_height: BlockNumber,
     ) {
-        self.catch_up_height = catch_up_height;
+        self.target_height = target_height;
         // FIXME: spawning a task like this is evil.
         // However, we aren't using the task executor, so no choice :(
         // Once we start using a centralized threadpool, spawn through it instead of the
@@ -95,15 +95,15 @@ impl Bootstrapper {
             self.l1_provider_client.clone(),
             self.sync_client.clone(),
             current_provider_height,
-            catch_up_height,
+            target_height,
             self.sync_retry_interval,
         ));
 
         self.sync_task_handle = SyncTaskHandle::Started(sync_task_handle.into());
     }
 
-    pub fn catch_up_height(&self) -> BlockNumber {
-        self.catch_up_height
+    pub fn target_height(&self) -> BlockNumber {
+        self.target_height
     }
 
     fn sync_task_health_check(&self, is_caught_up: bool) {
@@ -129,7 +129,7 @@ impl Bootstrapper {
 
 impl PartialEq for Bootstrapper {
     fn eq(&self, other: &Self) -> bool {
-        self.catch_up_height == other.catch_up_height
+        self.target_height == other.target_height
             && self.commit_block_backlog == other.commit_block_backlog
     }
 }
@@ -139,7 +139,7 @@ impl Eq for Bootstrapper {}
 impl std::fmt::Debug for Bootstrapper {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Bootstrapper")
-            .field("catch_up_height", &self.catch_up_height)
+            .field("target_height", &self.target_height)
             .field("commit_block_backlog", &self.commit_block_backlog)
             .field("sync_task_handle", &self.sync_task_handle)
             .finish_non_exhaustive()
@@ -150,14 +150,14 @@ async fn l2_sync_task(
     l1_provider_client: SharedL1ProviderClient,
     sync_client: SharedStateSyncClient,
     mut current_height: BlockNumber,
-    catch_up_height: BlockNumber,
+    target_height: BlockNumber,
     retry_interval: Duration,
 ) {
-    while current_height <= catch_up_height {
+    while current_height <= target_height {
         // TODO(Gilad): add tracing instrument.
         debug!(
             "Syncing L1Provider with L2 height: {} to target height: {}",
-            current_height, catch_up_height
+            current_height, target_height
         );
         let block = sync_client.get_block(current_height).await.inspect_err(|err| debug!("{err}"));
 

crates/apollo_l1_provider/src/l1_provider.rs

@@ -29,6 +29,18 @@ use crate::L1ProviderConfig;
 #[path = "l1_provider_tests.rs"]
 pub mod l1_provider_tests;
 
+// Note on height specification:
+// - start_height: height of the next block after "historic" blocks, meaning blocks that were
+//   already proved and sent to L1, and will not be scraped. Transactions from blocks lower than
+//   start_height are already committed. The Provider is not interested in any block lower than
+//   start_height.
+// - current_height: height of the next block that the Provider expects to see. It means the
+//   provider has seen commits forall the previous blocks up to (not including) current_height.
+// - target_height: when bootstrapping, the height to which the bootstrapper will sync. After
+//   bootstrapping is done, we expect the current_height to be one above the target_height. If any
+//   more blocks are committed while bootstrapping, they are applied after the target_height, and
+//   the current_height will be set to one above the last block in the backlog.
+
 // TODO(Gilad): optimistic proposer support, will add later to keep things simple, but the design
 // here is compatible with it.
 #[derive(Debug, Clone)]
@@ -84,7 +96,7 @@ impl L1Provider {
     // Start the provider, get first-scrape events, start L2 sync.
     pub async fn initialize(
         &mut self,
-        historic_l2_height: BlockNumber,
+        start_height: BlockNumber,
         events: Vec<Event>,
     ) -> L1ProviderResult<()> {
         info!("Initializing l1 provider");
@@ -103,8 +115,8 @@ impl L1Provider {
         // The current_height is set to a very old height, that doesn't include any of the events
         // sent now, or to be scraped in the future. The provider will begin bootstrapping when the
         // batcher calls commit_block with a height above the current height.
-        self.start_height = Some(historic_l2_height);
-        self.current_height = historic_l2_height;
+        self.start_height = Some(start_height);
+        self.current_height = start_height;
         self.state = ProviderState::Pending;
         self.add_events(events)?;
 
@@ -310,7 +322,7 @@ impl L1Provider {
                 if self.state.is_uninitialized() {
                     warn!(
                         "Provider received a block height ({height}) while it is uninitialized. \
-                         Cannot start bootstrapping until getting the historic_height from the \
+                         Cannot start bootstrapping until getting the start_height from the \
                          scraper during the initialize call."
                     );
                 } else {

crates/apollo_l1_provider/src/test_utils.rs

@@ -43,7 +43,7 @@ macro_rules! make_bootstrapper {
                     committed_txs: [$(tx_hash!($tx)),*].into()
                 }),*
             ].into_iter().collect(),
-            catch_up_height: BlockNumber(0),
+            target_height: BlockNumber(0),
             l1_provider_client: Arc::new(FakeL1ProviderClient::default()),
             sync_client: Arc::new(MockStateSyncClient::default()),
             sync_task_handle: SyncTaskHandle::default(),
@@ -636,7 +636,7 @@ impl L1ProviderClient for FakeL1ProviderClient {
 
     async fn initialize(
         &self,
-        _last_historic_l2_height: BlockNumber,
+        _start_height: BlockNumber,
         _events: Vec<Event>,
     ) -> L1ProviderClientResult<()> {
         todo!()