add test for old epoch removal

crates/sui-core/src/narwhal_manager/mod.rs

@@ -187,7 +187,7 @@ impl NarwhalManager {
         let drop_boundary = epoch - 1;
 
         // Get all the epoch stores in the base path directory
-        let files = match fs::read_dir(storage_base_path.clone()) {
+        let files = match fs::read_dir(storage_base_path) {
             Ok(f) => f,
             Err(e) => {
                 tracing::error!("Narwhal Manager cannot read the files in the storage path directory for epoch cleanup: {:?}", e);
@@ -208,8 +208,13 @@ impl NarwhalManager {
                 }
             };
 
-            let file_epoch_string = f.file_name().to_str().unwrap().to_owned(); // todo:remove unwrap
-            let file_epoch = match file_epoch_string.parse::<u64>() {
+            let name = f.file_name();
+            let file_epoch_string = match name.to_str() {
+                Some(f) => f,
+                None => continue,
+            };
+
+            let file_epoch = match file_epoch_string.to_owned().parse::<u64>() {
                 Ok(f) => f,
                 Err(e) => {
                     tracing::error!("Narwhal Manager could not parse file in storage path into epoch for cleanup: {:?}",e);

crates/sui-core/src/unit_tests/narwhal_manager_tests.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::authority::AuthorityState;
+use crate::consensus_validator::SuiTxValidator;
 use crate::narwhal_manager::{NarwhalConfiguration, NarwhalManager};
 use bytes::Bytes;
 use fastcrypto::bls12381;
@@ -12,6 +13,8 @@ use narwhal_executor::ExecutionState;
 use narwhal_types::{ConsensusOutput, TransactionProto, TransactionsClient};
 use narwhal_worker::TrivialTransactionValidator;
 use prometheus::Registry;
+use std::fs;
+use std::path::PathBuf;
 use std::sync::Arc;
 use std::time::Duration;
 use test_utils::authority::test_and_configure_authority_configs;
@@ -217,3 +220,58 @@ async fn test_narwhal_manager() {
         _ = tr_shutdown.send(());
     }
 }
+
+#[tokio::test]
+async fn test_remove_old_epoch_data() {
+    // Create the storage paths
+    let base_path_string = "/tmp/test_nw_manager_storage_path".to_owned();
+
+    let mut base_path = PathBuf::new();
+    base_path.push(base_path_string.clone());
+
+    let mut path_12 = PathBuf::new();
+    path_12.push(base_path_string.clone() + "/12");
+    let mut path_98 = base_path.clone();
+    path_98.push(base_path_string.clone() + "/98");
+    let mut path_99 = base_path.clone();
+    path_99.push(base_path_string.clone() + "/99");
+    let mut path_100 = base_path.clone();
+    path_100.push(base_path_string.clone() + "/100");
+
+    // Remove the directories created next in case it wasn't cleaned up before the last test run terminated
+    _ = fs::remove_dir(path_12.clone());
+    _ = fs::remove_dir(path_98.clone());
+    _ = fs::remove_dir(path_99.clone());
+    _ = fs::remove_dir(path_100.clone());
+    _ = fs::remove_dir(base_path.clone());
+
+    // Create some epoch directories
+    fs::create_dir(base_path.clone()).unwrap();
+    fs::create_dir(path_12.clone()).unwrap();
+    fs::create_dir(path_98.clone()).unwrap();
+    fs::create_dir(path_99.clone()).unwrap();
+    fs::create_dir(path_100.clone()).unwrap();
+
+    // With the current epoch of 100, remove old epochs
+    NarwhalManager::<SuiTxValidator>::remove_old_epoch_data(base_path.clone(), 100).await;
+
+    // Now ensure the epoch directories older than 100 were removed
+    let files = fs::read_dir(base_path_string).unwrap();
+
+    let mut epochs_left = Vec::new();
+    for file_res in files {
+        let file_epoch_string = file_res.unwrap().file_name().to_str().unwrap().to_owned();
+        let file_epoch = file_epoch_string.parse::<u64>().unwrap();
+        epochs_left.push(file_epoch);
+    }
+
+    // Remove the directories we created before the test possibly terminates
+    _ = fs::remove_dir(path_12);
+    _ = fs::remove_dir(path_98);
+    _ = fs::remove_dir(path_99);
+    _ = fs::remove_dir(path_100);
+    _ = fs::remove_dir(base_path);
+
+    assert_eq!(epochs_left.len(), 1);
+    assert_eq!(epochs_left[0], 100);
+}

narwhal/storage/src/node_store.rs

@@ -108,5 +108,4 @@ impl NodeStorage {
             consensus_store,
         }
     }
-
 }