fix: coderabbit review fixes

dash-spv-ffi/src/platform_integration.rs

@@ -171,9 +171,9 @@ pub unsafe extern "C" fn ffi_dash_spv_get_quorum_public_key(
                 );
             }
 
-            // Copy the public key directly from the global index
-            let pubkey_ptr = public_key as *const _ as *const u8;
-            std::ptr::copy_nonoverlapping(pubkey_ptr, out_pubkey, QUORUM_PUBKEY_SIZE);
+            // Get the public key's canonical 48-byte representation safely
+            let pubkey_bytes: &[u8; 48] = public_key.as_ref();
+            std::ptr::copy_nonoverlapping(pubkey_bytes.as_ptr(), out_pubkey, QUORUM_PUBKEY_SIZE);
 
             // Return success
             FFIResult {

dash-spv-ffi/tests/test_platform_integration_safety.rs

@@ -357,10 +357,11 @@ fn test_error_string_lifecycle() {
 fn test_handle_lifecycle() {
     unsafe {
         // Test null handle operations
-        let null_handle = ptr::null_mut();
+        let null_client: *mut FFIDashSpvClient = ptr::null_mut();
+        let null_handle: *mut CoreSDKHandle = ptr::null_mut();
 
         // Getting core handle from null client
-        let handle = ffi_dash_spv_get_core_handle(null_handle);
+        let handle = ffi_dash_spv_get_core_handle(null_client);
         assert!(handle.is_null());
 
         // Releasing null handle should be safe

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

@@ -172,6 +172,8 @@ mod tests {
             current_filter_tip: None,
             masternode_engine: None,
             last_masternode_diff_height: None,
+            sync_base_height: 0,
+            synced_from_checkpoint: false,
         };
 
         let ffi_state = FFIChainState::from(state);
@@ -213,6 +215,10 @@ mod tests {
             pending_filter_requests: 0,
             filter_request_timeouts: u64::MAX,
             filter_requests_retried: u64::MAX,
+            connected_peers: 0,
+            total_peers: 0,
+            header_height: 0,
+            filter_height: 0,
         };
 
         let ffi_stats = FFISpvStats::from(stats);

dash-spv/src/network/message_handler.rs

@@ -92,11 +92,11 @@ impl MessageHandler {
                 self.stats.qrinfo_messages += 1;
                 MessageHandleResult::QRInfo(qr_info)
             }
-            NetworkMessage::GetQRInfo(_) => {
+            NetworkMessage::GetQRInfo(req) => {
                 // We don't serve QRInfo requests, only make them
                 tracing::warn!("Received unexpected GetQRInfo request");
                 self.stats.other_messages += 1;
-                MessageHandleResult::Unhandled(message)
+                MessageHandleResult::Unhandled(NetworkMessage::GetQRInfo(req))
             }
             other => {
                 self.stats.other_messages += 1;

dash-spv/src/sync/chainlock_validation.rs

@@ -223,20 +223,38 @@ impl ChainLockValidator {
         // Get the masternode list at or before the height
         let mn_list_height = engine.masternode_lists.range(..=height).rev().next().map(|(h, _)| *h);
 
-        if let Some(list_height) = mn_list_height {
-            if let Some(mn_list) = engine.masternode_lists.get(&list_height) {
-                // Find the chain lock quorum
-                if let Some(quorums) = mn_list.quorums.get(&self.config.required_llmq_type) {
-                    // Get the most recent quorum
-                    if let Some((quorum_hash, entry)) = quorums.iter().next() {
-                        // Get public key from the quorum entry
-                        return Ok(Some((*quorum_hash, entry.quorum_entry.quorum_public_key)));
-                    }
-                }
-            }
-        }
-
-        Ok(None)
+        let list_height = mn_list_height.ok_or_else(|| {
+            SyncError::Validation(format!(
+                "No masternode list found at or before height {}",
+                height
+            ))
+        })?;
+
+        let mn_list = engine.masternode_lists.get(&list_height).ok_or_else(|| {
+            SyncError::Validation(format!(
+                "Masternode list not found at height {}",
+                list_height
+            ))
+        })?;
+
+        // Find the chain lock quorum
+        let quorums = mn_list.quorums.get(&self.config.required_llmq_type).ok_or_else(|| {
+            SyncError::Validation(format!(
+                "No quorums found for LLMQ type {:?} at masternode list height {}",
+                self.config.required_llmq_type, list_height
+            ))
+        })?;
+
+        // Get the most recent quorum
+        let (quorum_hash, entry) = quorums.iter().next().ok_or_else(|| {
+            SyncError::Validation(format!(
+                "No quorum entries found for LLMQ type {:?}",
+                self.config.required_llmq_type
+            ))
+        })?;
+
+        // Get public key from the quorum entry
+        Ok(Some((*quorum_hash, entry.quorum_entry.quorum_public_key)))
     }
 
     /// Verify chain lock signature using the engine's built-in verification

dash-spv/src/sync/sequential/mod.rs

@@ -1823,7 +1823,7 @@ impl SequentialSyncManager {
             // If we have masternodes enabled, request masternode list updates for ChainLock validation
             if self.config.enable_masternodes {
                 // For ChainLock validation, we need masternode lists at (block_height - CHAINLOCK_VALIDATION_MASTERNODE_OFFSET)
-                // So we request the masternode diff for this new block to maintain our rolling window
+                // We request the masternode diff for each new block (not just offset blocks) to maintain a complete rolling window
                 let base_block_hash = if height > 0 {
                     // Get the previous block hash
                     storage

dash-spv/src/sync/sequential/phases.rs

@@ -546,17 +546,22 @@ impl SyncPhase {
         if let SyncPhase::DownloadingMnList {
             sync_strategy:
                 Some(HybridSyncStrategy::EngineDiscovery {
+                    qr_info_requests,
                     qr_info_completed,
                     ..
                 }),
             requests_completed,
+            requests_total,
             last_progress,
             ..
         } = self
         {
-            *qr_info_completed += 1;
-            *requests_completed += 1;
-            *last_progress = Instant::now();
+            // Only increment if we haven't reached the planned total
+            if *qr_info_completed < *qr_info_requests {
+                *qr_info_completed += 1;
+                *requests_completed = (*requests_completed + 1).min(*requests_total);
+                *last_progress = Instant::now();
+            }
         }
     }
 
@@ -565,19 +570,24 @@ impl SyncPhase {
         if let SyncPhase::DownloadingMnList {
             sync_strategy:
                 Some(HybridSyncStrategy::EngineDiscovery {
+                    mn_diff_requests,
                     mn_diff_completed,
                     ..
                 }),
             requests_completed,
+            requests_total,
             diffs_processed,
             last_progress,
             ..
         } = self
         {
-            *mn_diff_completed += 1;
-            *requests_completed += 1;
-            *diffs_processed += 1; // Backward compatibility
-            *last_progress = Instant::now();
+            // Only increment if we haven't reached the planned total
+            if *mn_diff_completed < *mn_diff_requests {
+                *mn_diff_completed += 1;
+                *requests_completed = (*requests_completed + 1).min(*requests_total);
+                *diffs_processed += 1; // Backward compatibility
+                *last_progress = Instant::now();
+            }
         }
     }
 

dash-spv/src/sync/validation.rs

@@ -233,14 +233,15 @@ impl ValidationEngine {
 
         // Validate masternode list diffs
         for diff in &qr_info.mn_list_diff_list {
+            let block_height = engine.block_container.get_height(&diff.block_hash).unwrap_or(0);
             match self.validate_mn_list_diff(diff, engine) {
                 Ok(true) => items_validated += 1,
                 Ok(false) => errors.push(ValidationError::InvalidMnListDiff(
-                    0, // We don't have block height in MnListDiff
+                    block_height,
                     "Validation failed".to_string(),
                 )),
                 Err(e) => errors.push(ValidationError::InvalidMnListDiff(
-                    0, // We don't have block height in MnListDiff
+                    block_height,
                     e.to_string(),
                 )),
             }
@@ -277,7 +278,8 @@ impl ValidationEngine {
         diff: &MnListDiff,
         engine: &MasternodeListEngine,
     ) -> SyncResult<bool> {
-        let cache_key = ValidationCacheKey::MasternodeList(0); // Use 0 as we don't have block height
+        let block_height = engine.block_container.get_height(&diff.block_hash).unwrap_or(0);
+        let cache_key = ValidationCacheKey::MasternodeList(block_height);
 
         // Check cache
         if let Some(cached) = self.get_cached_result(&cache_key) {
@@ -297,11 +299,11 @@ impl ValidationEngine {
     fn perform_mn_list_diff_validation(
         &self,
         diff: &MnListDiff,
-        _engine: &MasternodeListEngine,
+        engine: &MasternodeListEngine,
     ) -> SyncResult<bool> {
         // Check if we have the base list
-        // Note: We can't check by height as MnListDiff doesn't contain block height
-        // We would need to look up the height from the block hash
+        // We can resolve block height from the diff's block hash using the engine's block container
+        let block_height = engine.block_container.get_height(&diff.block_hash).unwrap_or(0);
 
         // Validate merkle root matches
         // TODO: Implement merkle root validation

dash-spv/src/sync/validation_test.rs

@@ -3,7 +3,7 @@
 #[cfg(test)]
 mod tests {
     use crate::client::ClientConfig;
-    use crate::storage::MemoryStorage;
+    use crate::storage::MemoryStorageManager;
     use crate::sync::chainlock_validation::{ChainLockValidationConfig, ChainLockValidator};
     use crate::sync::masternodes::MasternodeSyncManager;
     use crate::sync::validation::{ValidationConfig, ValidationEngine};
@@ -136,7 +136,7 @@ mod tests {
     async fn test_qr_info_validation() {
         let config = create_test_config();
         let _sync_manager = MasternodeSyncManager::new(&config);
-        let _storage = MemoryStorage::new();
+        let _storage = MemoryStorageManager::new().await.expect("Failed to create MemoryStorageManager");
 
         // Create mock QRInfo
         let _qr_info = create_mock_qr_info();

dash-spv/tests/chainlock_validation_test.rs

@@ -288,7 +288,7 @@ async fn test_chainlock_queue_and_process_flow() {
     let storage_path = temp_dir.path().to_path_buf();
 
     // Create storage
-    let storage = Box::new(DiskStorageManager::new(storage_path).await.unwrap());
+    let mut storage = Box::new(DiskStorageManager::new(storage_path).await.unwrap());
     let network = Box::new(MockNetworkManager::new());
 
     // Create client config
@@ -325,7 +325,6 @@ async fn test_chainlock_queue_and_process_flow() {
 
     // Process pending (will fail validation but clear the queue)
     let chain_state = ChainState::new();
-    let storage = client.storage();
     let _ = chainlock_manager.validate_pending_chainlocks(&chain_state, &mut *storage).await;
 
     // Verify queue is cleared
@@ -363,13 +362,12 @@ async fn test_chainlock_manager_cache_operations() {
 
     // Add test headers
     let genesis = genesis_block(Network::Dash).header;
-    let storage = client.storage();
+    let mut storage = client.storage();
     // storage.store_header(&genesis, 0).await.unwrap();
 
     // Create and process a ChainLock
     let chain_lock = create_test_chainlock(0, genesis.block_hash());
     let chain_state = ChainState::new();
-    let storage = client.storage();
     let _ =
         chainlock_manager.process_chain_lock(chain_lock.clone(), &chain_state, &mut *storage).await;