From 668b35b2ea9e61c3ace0be4bef37729aefcea3eb Mon Sep 17 00:00:00 2001 From: benthecarman Date: Sat, 30 May 2026 12:34:23 +0200 Subject: [PATCH] Detect nested v1 filesystem data FilesystemStoreV2 already rejected v1 data when a key file was found at the store root, but it did not inspect namespace directories. This missed v1 layouts such as primary/key, where v2 expects primary to contain secondary namespace directories. For example, an ldk-node store can contain a BDK descriptor below a namespace directory. The previous check would accept that directory as v2 data because the root contained only directories, leaving the incompatible descriptor file undetected. --- lightning-persister/src/fs_store/v2.rs | 75 ++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/lightning-persister/src/fs_store/v2.rs b/lightning-persister/src/fs_store/v2.rs index 2f79cae0da3..6154d22d35c 100644 --- a/lightning-persister/src/fs_store/v2.rs +++ b/lightning-persister/src/fs_store/v2.rs @@ -21,8 +21,8 @@ use std::sync::Arc; /// An error returned when constructing a [`FilesystemStoreV2`]. #[derive(Debug)] pub enum FilesystemStoreV2Error { - /// The data directory contains a file at the top level, indicating it was previously used - /// by [`FilesystemStore`] (v1). Contains the path of the offending file. + /// The data directory contains a file where v2 expects a namespace directory, indicating it + /// was previously used by [`FilesystemStore`] (v1). Contains the path of the offending file. /// /// [`FilesystemStore`]: crate::fs_store::v1::FilesystemStore V1DataDetected(PathBuf), @@ -35,7 +35,7 @@ impl fmt::Display for FilesystemStoreV2Error { match self { Self::V1DataDetected(path) => write!( f, - "Found file `{}` in the top-level data directory. \ + "Found file `{}` where FilesystemStoreV2 expects a namespace directory. \ This indicates the directory was previously used by FilesystemStore (v1). \ Please migrate your data or use a different directory.", path.display() @@ -97,18 +97,28 @@ impl FilesystemStoreV2 { /// Constructs a new [`FilesystemStoreV2`]. /// /// Returns [`FilesystemStoreV2Error::V1DataDetected`] if the data directory already exists - /// and contains files at the top level, which would indicate it was previously used by a - /// [`FilesystemStore`] (v1). The v2 store expects only directories (namespaces) at the top - /// level. + /// and contains files where v2 expects namespace directories, which would indicate it was + /// previously used by a [`FilesystemStore`] (v1). The v2 store expects only directories at + /// the top level and one level down. /// /// [`FilesystemStore`]: crate::fs_store::v1::FilesystemStore pub fn new(data_dir: PathBuf) -> Result { if data_dir.exists() { for entry in fs::read_dir(&data_dir)? { let entry = entry?; - if entry.file_type()?.is_file() { + let file_type = entry.file_type()?; + if file_type.is_file() { return Err(FilesystemStoreV2Error::V1DataDetected(entry.path())); } + + if file_type.is_dir() { + for child_entry in fs::read_dir(entry.path())? { + let child_entry = child_entry?; + if child_entry.file_type()?.is_file() { + return Err(FilesystemStoreV2Error::V1DataDetected(child_entry.path())); + } + } + } } } @@ -699,6 +709,7 @@ mod tests { fs::create_dir_all(&temp_path).unwrap(); // Create a file at the top level, as v1 would for an empty primary namespace + // and an empty secondary namespace. fs::write(temp_path.join("some_key"), b"data").unwrap(); // V2 construction should fail @@ -713,13 +724,59 @@ mod tests { // Clean up let _ = fs::remove_dir_all(&temp_path); + // Create a file one level down, as v1 would for a non-empty primary namespace + // and an empty secondary namespace. + fs::create_dir_all(temp_path.join("some_namespace")).unwrap(); + fs::write(temp_path.join("some_namespace").join("some_key"), b"data").unwrap(); + + match FilesystemStoreV2::new(temp_path.clone()) { + Err(FilesystemStoreV2Error::V1DataDetected(path)) => { + assert_eq!(path, temp_path.join("some_namespace").join("some_key")); + }, + Err(err) => panic!("Expected V1DataDetected, got {:?}", err), + Ok(_) => panic!("Expected error for directory with files one level down"), + } + + let _ = fs::remove_dir_all(&temp_path); + + // A v1 write with an empty primary namespace and non-empty secondary namespace + // is rejected by the KVStore API, but its filesystem layout would be the same + // one-level shape. + fs::create_dir_all(temp_path.join("some_secondary_namespace")).unwrap(); + fs::write(temp_path.join("some_secondary_namespace").join("some_key"), b"data").unwrap(); + + match FilesystemStoreV2::new(temp_path.clone()) { + Err(FilesystemStoreV2Error::V1DataDetected(path)) => { + assert_eq!(path, temp_path.join("some_secondary_namespace").join("some_key")); + }, + Err(err) => panic!("Expected V1DataDetected, got {:?}", err), + Ok(_) => panic!("Expected error for directory with files one level down"), + } + + let _ = fs::remove_dir_all(&temp_path); + // An empty directory should succeed fs::create_dir_all(&temp_path).unwrap(); let result = FilesystemStoreV2::new(temp_path.clone()); assert!(result.is_ok()); - // A directory with only subdirectories should succeed - fs::create_dir_all(temp_path.join("some_namespace")).unwrap(); + // A directory with only namespace subdirectories should succeed + fs::create_dir_all(temp_path.join("some_namespace").join("some_sub_namespace")).unwrap(); + let result = FilesystemStoreV2::new(temp_path.clone()); + assert!(result.is_ok()); + + // V1 data with non-empty primary and secondary namespaces has the same filesystem + // layout as valid v2 data, so construction must not reject this shape. + let fs_store = result.unwrap(); + KVStoreSync::write( + &fs_store, + "some_namespace", + "some_sub_namespace", + "some_key", + b"data".to_vec(), + ) + .unwrap(); + let result = FilesystemStoreV2::new(temp_path); assert!(result.is_ok()); }