refactor: Make synced_from_checkpoint based on sync_base_height (#226)

Author: xdustinface

dash-spv-ffi/tests/unit/test_type_conversions.rs

@@ -171,7 +171,6 @@ mod tests {
             masternode_engine: None,
             last_masternode_diff_height: None,
             sync_base_height: 0,
-            synced_from_checkpoint: false,
         };
 
         let ffi_state = FFIChainState::from(state);

dash-spv/src/chain/fork_detector.rs

@@ -108,10 +108,7 @@ impl ForkDetector {
         // Check if this connects to the main chain (creates new fork)
         if let Ok(Some(height)) = storage.get_header_height(&prev_hash) {
             // Check if this would create a fork from before our checkpoint
-            if chain_state.synced_from_checkpoint
-                && chain_state.sync_base_height > 0
-                && height < chain_state.sync_base_height
-            {
+            if chain_state.synced_from_checkpoint() && height < chain_state.sync_base_height {
                 tracing::warn!(
                         "Rejecting header that would create fork from height {} (before checkpoint base {}). \
                         This likely indicates headers from genesis were received during checkpoint sync.",
@@ -140,11 +137,7 @@ impl ForkDetector {
         for (height, state_header) in chain_state.headers.iter().enumerate() {
             if prev_hash == state_header.block_hash() {
                 // Calculate the actual blockchain height for this index
-                let actual_height = if chain_state.synced_from_checkpoint {
-                    chain_state.sync_base_height + (height as u32)
-                } else {
-                    height as u32
-                };
+                let actual_height = chain_state.sync_base_height + (height as u32);
 
                 // This connects to a header in chain state but not in storage
                 // Treat it as extending main chain if it's the tip

dash-spv/src/chain/fork_detector_test.rs

@@ -32,7 +32,6 @@ mod tests {
         let mut chain_state = ChainState::new();
 
         // Simulate checkpoint sync from height 1000
-        chain_state.synced_from_checkpoint = true;
         chain_state.sync_base_height = 1000;
 
         // Add a checkpoint header at height 1000

dash-spv/src/chain/reorg.rs

@@ -103,7 +103,7 @@ impl ReorgManager {
 
         // Check reorg depth - account for checkpoint sync
         let reorg_depth = if let Some(state) = chain_state {
-            if state.synced_from_checkpoint && state.sync_base_height > 0 {
+            if state.synced_from_checkpoint() {
                 // During checkpoint sync, both current_tip.height and fork.fork_height
                 // should be interpreted relative to sync_base_height
 

dash-spv/src/chain/reorg_test.rs

@@ -99,7 +99,6 @@ mod tests {
         let storage = MemoryStorage::new();
 
         // Simulate checkpoint sync from height 50000
-        chain_state.synced_from_checkpoint = true;
         chain_state.sync_base_height = 50000;
 
         // Current tip at height 50100

dash-spv/src/client/lifecycle.rs

@@ -210,7 +210,7 @@ impl<
                     tracing::error!("Failed to load headers into sync manager: {}", e);
                     // For checkpoint sync, this is critical
                     let state = self.state.read().await;
-                    if state.synced_from_checkpoint {
+                    if state.synced_from_checkpoint() {
                         return Err(SpvError::Sync(e));
                     }
                     // For normal sync, we can continue as headers will be re-synced
@@ -398,11 +398,7 @@ impl<
                         );
 
                         // Update the sync manager's cached flags from the checkpoint-initialized state
-                        self.sync_manager.update_chain_state_cache(
-                            true,
-                            checkpoint.height,
-                            headers_len,
-                        );
+                        self.sync_manager.update_chain_state_cache(checkpoint.height, headers_len);
                         tracing::info!(
                             "Updated sync manager with checkpoint-initialized chain state"
                         );

dash-spv/src/client/sync_coordinator.rs

@@ -917,12 +917,7 @@ impl<
         let target_height = saved_state.chain_tip.height;
 
         // Determine first height to load. Skip genesis (already present) unless we started from a checkpoint base.
-        let mut current_height =
-            if saved_state.synced_from_checkpoint && saved_state.sync_base_height > 0 {
-                saved_state.sync_base_height
-            } else {
-                1u32
-            };
+        let mut current_height = saved_state.sync_base_height.max(1);
 
         while current_height <= target_height {
             let end_height = (current_height + BATCH_SIZE - 1).min(target_height);

dash-spv/src/storage/disk/state.rs

@@ -21,7 +21,7 @@ impl DiskStorageManager {
 
         // First store all headers
         // For checkpoint sync, we need to store headers starting from the checkpoint height
-        if state.synced_from_checkpoint && state.sync_base_height > 0 && !state.headers.is_empty() {
+        if state.synced_from_checkpoint() && !state.headers.is_empty() {
             // Store headers starting from the checkpoint height
             self.store_headers_from_height(&state.headers, state.sync_base_height).await?;
         } else {
@@ -38,7 +38,6 @@ impl DiskStorageManager {
             "current_filter_tip": state.current_filter_tip,
             "last_masternode_diff_height": state.last_masternode_diff_height,
             "sync_base_height": state.sync_base_height,
-            "synced_from_checkpoint": state.synced_from_checkpoint,
         });
 
         let path = self.base_path.join("state/chain.json");
@@ -63,11 +62,7 @@ impl DiskStorageManager {
 
         // Load all headers
         if let Some(tip_height) = self.get_tip_height().await? {
-            let range_start = if state.synced_from_checkpoint && state.sync_base_height > 0 {
-                state.sync_base_height
-            } else {
-                0
-            };
+            let range_start = state.sync_base_height;
             state.headers = self.load_headers(range_start..tip_height + 1).await?;
         }
 
@@ -88,8 +83,6 @@ impl DiskStorageManager {
         // Load checkpoint sync fields
         state.sync_base_height =
             value.get("sync_base_height").and_then(|v| v.as_u64()).map(|h| h as u32).unwrap_or(0);
-        state.synced_from_checkpoint =
-            value.get("synced_from_checkpoint").and_then(|v| v.as_bool()).unwrap_or(false);
 
         Ok(Some(state))
     }
@@ -829,7 +822,6 @@ mod tests {
         // Set sync base height so storage interprets heights as blockchain heights
         let mut base_state = ChainState::new();
         base_state.sync_base_height = checkpoint_height;
-        base_state.synced_from_checkpoint = true;
         storage.store_chain_state(&base_state).await?;
 
         // Verify headers are stored at correct blockchain heights
@@ -861,7 +853,6 @@ mod tests {
         // Store chain state to persist sync_base_height
         let mut chain_state = ChainState::new();
         chain_state.sync_base_height = checkpoint_height;
-        chain_state.synced_from_checkpoint = true;
         storage.store_chain_state(&chain_state).await?;
 
         // Force save to disk

dash-spv/src/storage/sync_state.rs

@@ -42,9 +42,6 @@ pub struct PersistentSyncState {
 
     /// Base height when syncing from a checkpoint (0 if syncing from genesis).
     pub sync_base_height: u32,
-
-    /// Whether the chain was synced from a checkpoint rather than genesis.
-    pub synced_from_checkpoint: bool,
 }
 
 /// Chain tip information.
@@ -195,7 +192,6 @@ impl PersistentSyncState {
                 .map(|work| format!("{:?}", work))
                 .unwrap_or_else(|| String::from("0")),
             sync_base_height: chain_state.sync_base_height,
-            synced_from_checkpoint: chain_state.synced_from_checkpoint,
         })
     }
 
@@ -324,6 +320,10 @@ impl PersistentSyncState {
         }
     }
 
+    pub fn synced_from_checkpoint(&self) -> bool {
+        self.sync_base_height > 0
+    }
+
     /// Get the best checkpoint to use for recovery.
     pub fn get_best_checkpoint(&self) -> Option<&SyncCheckpoint> {
         self.checkpoints.iter().rev().find(|cp| cp.validated)
@@ -378,7 +378,6 @@ mod tests {
             saved_at: SystemTime::now(),
             chain_work: String::new(),
             sync_base_height: 0,
-            synced_from_checkpoint: false,
         };
 
         // Valid state

dash-spv/src/sync/headers/manager.rs

@@ -56,8 +56,7 @@ pub struct HeaderSyncManager<S: StorageManager, N: NetworkManager> {
     syncing_headers: bool,
     last_sync_progress: std::time::Instant,
     headers2_failed: bool,
-    // Cached flags for quick access without locking
-    cached_synced_from_checkpoint: bool,
+    // Cached flag for quick access without locking
     cached_sync_base_height: u32,
 }
 
@@ -94,7 +93,6 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
             syncing_headers: false,
             last_sync_progress: std::time::Instant::now(),
             headers2_failed: false,
-            cached_synced_from_checkpoint: false,
             cached_sync_base_height: 0,
             _phantom_s: std::marker::PhantomData,
             _phantom_n: std::marker::PhantomData,
@@ -106,13 +104,11 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
         // First, try to load the persisted chain state which may contain sync_base_height
         if let Ok(Some(stored_chain_state)) = storage.load_chain_state().await {
             tracing::info!(
-                "Loaded chain state from storage with sync_base_height: {}, synced_from_checkpoint: {}",
+                "Loaded chain state from storage with sync_base_height: {}",
                 stored_chain_state.sync_base_height,
-                stored_chain_state.synced_from_checkpoint
             );
             // Update our chain state with the loaded one to preserve sync_base_height
             {
-                self.cached_synced_from_checkpoint = stored_chain_state.synced_from_checkpoint;
                 self.cached_sync_base_height = stored_chain_state.sync_base_height;
                 let mut cs = self.chain_state.write().await;
                 *cs = stored_chain_state;
@@ -128,7 +124,7 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
         let Some(tip_height) = tip_height else {
             tracing::debug!("No headers found in storage");
             // If we're syncing from a checkpoint, this is expected
-            if self.is_synced_from_checkpoint() && self.get_sync_base_height() > 0 {
+            if self.is_synced_from_checkpoint() {
                 tracing::info!("No headers in storage for checkpoint sync - this is expected");
                 return Ok(0);
             }
@@ -147,15 +143,8 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
         const BATCH_SIZE: u32 = 10_000;
         let mut loaded_count = 0u32;
 
-        // Determine the first blockchain height we need to load.
         // For checkpoint syncs we start at the checkpoint base; otherwise we skip genesis (already present).
-        let base_height = if self.is_synced_from_checkpoint() && self.get_sync_base_height() > 0 {
-            self.get_sync_base_height()
-        } else {
-            1
-        };
-
-        let mut current_height = base_height;
+        let mut current_height = self.get_sync_base_height().max(1);
 
         while current_height <= tip_height {
             let end_height = (current_height + BATCH_SIZE - 1).min(tip_height);
@@ -406,7 +395,7 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
             Some(hash) => {
                 // When syncing from a checkpoint, we need to create a proper locator
                 // that helps the peer understand we want headers AFTER this point
-                if self.is_synced_from_checkpoint() && self.get_sync_base_height() > 0 {
+                if self.is_synced_from_checkpoint() {
                     // For checkpoint sync, only include the checkpoint hash
                     // Including genesis would allow peers to fall back to sending headers from genesis
                     // if they don't recognize the checkpoint, which is exactly what we want to avoid
@@ -651,9 +640,8 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
 
         tracing::info!("Preparing header synchronization");
         tracing::info!(
-            "Chain state before prepare_sync: sync_base_height={}, synced_from_checkpoint={}, headers_count={}",
+            "Chain state before prepare_sync: sync_base_height={}, headers_count={}",
             self.get_sync_base_height(),
-            self.is_synced_from_checkpoint(),
             self.chain_state.read().await.headers.len()
         );
 
@@ -679,8 +667,8 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
             }
         } else {
             tracing::info!(
-                "Not syncing from checkpoint or no tip height. synced_from_checkpoint={}, current_tip_height={:?}",
-                self.is_synced_from_checkpoint(),
+                "Not syncing from checkpoint or no tip height. sync_base_height={}, current_tip_height={:?}",
+                self.get_sync_base_height(),
                 current_tip_height
             );
             current_tip_height
@@ -769,12 +757,7 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
                 } else {
                     // Get the current tip hash from storage
                     // When syncing from checkpoint, the storage height is different from effective height
-                    let storage_height = if self.is_synced_from_checkpoint() {
-                        // The actual storage height is effective_height - sync_base_height
-                        height.saturating_sub(self.get_sync_base_height())
-                    } else {
-                        height
-                    };
+                    let storage_height = height.saturating_sub(self.get_sync_base_height());
 
                     let tip_header = storage.get_header(storage_height).await.map_err(|e| {
                         SyncError::Storage(format!(
@@ -855,7 +838,7 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
             let recovery_base_hash = match current_tip_height {
                 None => {
                     // No headers in storage - check if we're syncing from a checkpoint
-                    if self.is_synced_from_checkpoint() && self.get_sync_base_height() > 0 {
+                    if self.is_synced_from_checkpoint() {
                         // Use the checkpoint hash from chain state
                         if !self.chain_state.read().await.headers.is_empty() {
                             let checkpoint_hash =
@@ -1038,27 +1021,13 @@ impl<S: StorageManager + Send + Sync + 'static, N: NetworkManager + Send + Sync
 
     /// Whether we're syncing from a checkpoint
     pub fn is_synced_from_checkpoint(&self) -> bool {
-        self.cached_synced_from_checkpoint
+        self.cached_sync_base_height > 0
     }
 
     /// Update cached flags and totals based on an external state snapshot
-    pub fn update_cached_from_state_snapshot(
-        &mut self,
-        synced_from_checkpoint: bool,
-        sync_base_height: u32,
-        headers_len: u32,
-    ) {
-        self.cached_synced_from_checkpoint = synced_from_checkpoint;
+    pub fn update_cached_from_state_snapshot(&mut self, sync_base_height: u32, headers_len: u32) {
         self.cached_sync_base_height = sync_base_height;
         // Absolute blockchain tip height = base + headers_len - 1 (if any headers exist)
-        self.total_headers_synced = if headers_len == 0 {
-            if synced_from_checkpoint {
-                sync_base_height
-            } else {
-                0
-            }
-        } else {
-            sync_base_height.saturating_add(headers_len).saturating_sub(1)
-        };
+        self.total_headers_synced = sync_base_height.saturating_add(headers_len).saturating_sub(1);
     }
 }