Revert "fix: Use single file handle in atomic_write for Windows compatibility"

This reverts commit 7b7880a.

Author: xdustinface

dash-spv/src/chain/chainlock_manager.rs

@@ -336,7 +336,7 @@ impl ChainLockManager {
         let key = format!("chainlock_{}", chain_lock.block_height);
         let data = bincode::serialize(&chain_lock)
             .map_err(|e| StorageError::Serialization(e.to_string()))?;
-        storage.store_metadata(&key, data).await?;
+        storage.store_metadata(&key, &data).await?;
 
         Ok(())
     }

dash-spv/src/network/persist.rs

@@ -62,7 +62,7 @@ impl PeerStore {
         let json = serde_json::to_string_pretty(&saved)
             .map_err(|e| Error::Storage(StorageError::Serialization(e.to_string())))?;
 
-        atomic_write(&self.path, json.as_bytes().to_vec()).await.map_err(Error::Storage)?;
+        atomic_write(&self.path, json.as_bytes()).await.map_err(Error::Storage)?;
 
         log::debug!("Saved {} peers to {:?}", saved.peers.len(), self.path);
         Ok(())

dash-spv/src/network/reputation.rs

@@ -426,7 +426,7 @@ impl PeerReputationManager {
             .collect();
 
         let json = serde_json::to_string_pretty(&data)?;
-        atomic_write(path, json.as_bytes().to_vec()).await.map_err(std::io::Error::other)
+        atomic_write(path, json.as_bytes()).await.map_err(std::io::Error::other)
     }
 
     /// Load reputation data from persistent storage

dash-spv/src/storage/headers.rs

@@ -73,5 +73,5 @@ pub(super) async fn save_index_to_disk(
     let data = bincode::serialize(index)
         .map_err(|e| StorageError::WriteFailed(format!("Failed to serialize index: {}", e)))?;
 
-    atomic_write(path, data).await
+    atomic_write(path, &data).await
 }

dash-spv/src/storage/io.rs

@@ -1,6 +1,5 @@
 //! Low-level I/O utilities for reading and writing segment files.
 
-use std::io::Write;
 use std::path::{Path, PathBuf};
 
 use crate::error::{StorageError, StorageResult};
@@ -21,7 +20,7 @@ fn get_temp_path(path: &Path) -> PathBuf {
 
 /// Atomically write data to a file.
 /// Uses temporary file + sync + rename pattern for crash resilience.
-pub(crate) async fn atomic_write(path: &Path, data: Vec<u8>) -> StorageResult<()> {
+pub(crate) async fn atomic_write(path: &Path, data: &[u8]) -> StorageResult<()> {
     // Ensure parent directory exists
     if let Some(parent) = path.parent() {
         tokio::fs::create_dir_all(parent)
@@ -30,35 +29,32 @@ pub(crate) async fn atomic_write(path: &Path, data: Vec<u8>) -> StorageResult<()
     }
 
     let temp_path = get_temp_path(path);
-    let target_path = path.to_path_buf();
-
-    // Use blocking I/O to ensure file handle is fully closed before rename.
-    // On Windows, async file drops don't guarantee synchronous closure.
-    tokio::task::spawn_blocking(move || {
-        // Write to temp file
-        let write_result = (|| {
-            let mut file = std::fs::File::create(&temp_path)?;
-            file.write_all(&data)?;
-            file.sync_all()?;
-            // File handle closed here when `file` goes out of scope
-            Ok::<(), std::io::Error>(())
-        })();
-
-        if let Err(e) = write_result {
-            let _ = std::fs::remove_file(&temp_path);
-            return Err(StorageError::WriteFailed(format!("Failed to write temp file: {}", e)));
-        }
 
-        // Atomic rename
-        if let Err(e) = std::fs::rename(&temp_path, &target_path) {
-            let _ = std::fs::remove_file(&temp_path);
-            return Err(StorageError::WriteFailed(format!("Failed to rename temp file: {}", e)));
-        }
+    // Write to temporary file
+    let write_result = async {
+        tokio::fs::write(&temp_path, data).await?;
+
+        // Sync to disk - open the file and call sync_all
+        let file = tokio::fs::File::open(&temp_path).await?;
+        file.sync_all().await?;
+
+        Ok::<(), std::io::Error>(())
+    }
+    .await;
+
+    // Clean up temp file on error
+    if let Err(e) = write_result {
+        let _ = tokio::fs::remove_file(&temp_path).await;
+        return Err(StorageError::WriteFailed(format!("Failed to write temp file: {}", e)));
+    }
+
+    // Atomic rename
+    if let Err(e) = tokio::fs::rename(&temp_path, path).await {
+        let _ = tokio::fs::remove_file(&temp_path).await;
+        return Err(StorageError::WriteFailed(format!("Failed to rename temp file: {}", e)));
+    }
 
-        Ok(())
-    })
-    .await
-    .map_err(|e| StorageError::WriteFailed(format!("atomic_write join error: {}", e)))?
+    Ok(())
 }
 
 #[cfg(test)]
@@ -91,7 +87,7 @@ mod tests {
         let path = temp_dir.path().join("test.dat");
 
         let content = b"hello world";
-        atomic_write(&path, content.to_vec()).await.unwrap();
+        atomic_write(&path, content).await.unwrap();
 
         assert!(path.exists());
         let read_content = tokio::fs::read(&path).await.unwrap();
@@ -104,7 +100,7 @@ mod tests {
         let path = temp_dir.path().join("nested").join("dirs").join("test.dat");
 
         let content = b"nested content";
-        atomic_write(&path, content.to_vec()).await.unwrap();
+        atomic_write(&path, content).await.unwrap();
 
         assert!(path.exists());
         let read_content = tokio::fs::read(&path).await.unwrap();
@@ -121,7 +117,7 @@ mod tests {
 
         // Overwrite with atomic write
         let new_content = b"new content";
-        atomic_write(&path, new_content.to_vec()).await.unwrap();
+        atomic_write(&path, new_content).await.unwrap();
 
         let read_content = tokio::fs::read(&path).await.unwrap();
         assert_eq!(read_content, new_content);
@@ -132,7 +128,7 @@ mod tests {
         let temp_dir = TempDir::new().unwrap();
         let path = temp_dir.path().join("test.dat");
 
-        atomic_write(&path, b"data".to_vec()).await.unwrap();
+        atomic_write(&path, b"data").await.unwrap();
 
         // Check no .tmp files remain
         let mut entries = tokio::fs::read_dir(temp_dir.path()).await.unwrap();
@@ -158,7 +154,7 @@ mod tests {
         // Try to write to an invalid path (directory instead of file)
         let invalid_path = temp_dir.path().join("test.dat").join("invalid");
 
-        let result = atomic_write(&invalid_path, b"new content".to_vec()).await;
+        let result = atomic_write(&invalid_path, b"new content").await;
         assert!(result.is_err());
 
         // Original file should still have original content
@@ -177,7 +173,7 @@ mod tests {
         // Try to write to a path where parent "directory" is actually a file
         let invalid_path = blocker_path.join("subdir").join("file.dat");
 
-        let result = atomic_write(&invalid_path, b"data".to_vec()).await;
+        let result = atomic_write(&invalid_path, b"data").await;
         assert!(result.is_err());
 
         // No temp files should remain in the base temp dir
@@ -197,7 +193,7 @@ mod tests {
 
         // Write binary data with null bytes and various byte values
         let binary_data: Vec<u8> = (0u8..=255).collect();
-        atomic_write(&path, binary_data.clone()).await.unwrap();
+        atomic_write(&path, &binary_data).await.unwrap();
 
         let read_content = tokio::fs::read(&path).await.unwrap();
         assert_eq!(read_content, binary_data);
@@ -210,7 +206,7 @@ mod tests {
 
         // Write 1MB of data
         let large_data: Vec<u8> = (0..1_000_000).map(|i| (i % 256) as u8).collect();
-        atomic_write(&path, large_data.clone()).await.unwrap();
+        atomic_write(&path, &large_data).await.unwrap();
 
         let read_content = tokio::fs::read(&path).await.unwrap();
         assert_eq!(read_content.len(), large_data.len());
@@ -226,7 +222,7 @@ mod tests {
         // you cannot rename a file over an existing directory
         tokio::fs::create_dir(&path).await.unwrap();
 
-        let result = atomic_write(&path, b"data".to_vec()).await;
+        let result = atomic_write(&path, b"data").await;
         assert!(result.is_err());
 
         // The target directory should still exist

dash-spv/src/storage/mod.rs

@@ -114,7 +114,7 @@ pub trait StorageManager: Send + Sync {
     async fn load_filters(&self, range: Range<u32>) -> StorageResult<Vec<Vec<u8>>>;
 
     /// Store metadata.
-    async fn store_metadata(&mut self, key: &str, value: Vec<u8>) -> StorageResult<()>;
+    async fn store_metadata(&mut self, key: &str, value: &[u8]) -> StorageResult<()>;
 
     /// Load metadata.
     async fn load_metadata(&self, key: &str) -> StorageResult<Option<Vec<u8>>>;

dash-spv/src/storage/segments.rs

@@ -508,7 +508,7 @@ impl<I: Persistable> Segment<I> {
             })?;
         }
 
-        atomic_write(&path, buffer).await?;
+        atomic_write(&path, &buffer).await?;
 
         self.state = SegmentState::Clean;
         Ok(())

dash-spv/src/storage/state.rs

@@ -33,7 +33,7 @@ impl DiskStorageManager {
 
         let path = self.base_path.join("state/chain.json");
         let json = state_data.to_string();
-        atomic_write(&path, json.as_bytes().to_vec()).await?;
+        atomic_write(&path, json.as_bytes()).await?;
 
         Ok(())
     }
@@ -94,7 +94,7 @@ impl DiskStorageManager {
             ))
         })?;
 
-        atomic_write(&path, json.as_bytes().to_vec()).await?;
+        atomic_write(&path, json.as_bytes()).await?;
         Ok(())
     }
 
@@ -130,7 +130,7 @@ impl DiskStorageManager {
             ))
         })?;
 
-        atomic_write(&path, data).await?;
+        atomic_write(&path, &data).await?;
         tracing::debug!("Stored chain lock at height {}", height);
         Ok(())
     }
@@ -195,7 +195,7 @@ impl DiskStorageManager {
     }
 
     /// Store metadata.
-    pub async fn store_metadata(&mut self, key: &str, value: Vec<u8>) -> StorageResult<()> {
+    pub async fn store_metadata(&mut self, key: &str, value: &[u8]) -> StorageResult<()> {
         let path = self.base_path.join(format!("state/{}.dat", key));
         atomic_write(&path, value).await?;
         Ok(())
@@ -432,7 +432,7 @@ impl StorageManager for DiskStorageManager {
         self.filters.write().await.get_items(range).await
     }
 
-    async fn store_metadata(&mut self, key: &str, value: Vec<u8>) -> StorageResult<()> {
+    async fn store_metadata(&mut self, key: &str, value: &[u8]) -> StorageResult<()> {
         Self::store_metadata(self, key, value).await
     }
 

dash-spv/tests/segmented_storage_test.rs

@@ -308,7 +308,7 @@ async fn test_mixed_operations() {
     }
 
     // Store metadata
-    storage.store_metadata("test_key", b"test_value".to_vec()).await.unwrap();
+    storage.store_metadata("test_key", b"test_value").await.unwrap();
 
     // Verify everything
     assert_eq!(storage.get_tip_height().await.unwrap(), Some(74_999));