Skip to content

Detect nested v1 filesystem data#4649

Open
benthecarman wants to merge 1 commit into
lightningdevkit:mainfrom
benthecarman:fs-2-sub-dirs
Open

Detect nested v1 filesystem data#4649
benthecarman wants to merge 1 commit into
lightningdevkit:mainfrom
benthecarman:fs-2-sub-dirs

Conversation

@benthecarman
Copy link
Copy Markdown
Contributor

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.

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.
@benthecarman benthecarman requested a review from tnull May 30, 2026 10:40
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 30, 2026

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

I've thoroughly reviewed the entire PR diff and the surrounding code. The change extends the v1 data detection in FilesystemStoreV2::new() from checking only the root level to also checking one level down inside namespace directories. This correctly catches v1 layouts like primary/key where v2 expects primary/secondary_ns_dir/key.

I verified:

  • The v2 layout always places files at the third level (primary_ns_dir/secondary_ns_dir/key), so the two-level check cannot produce false positives on valid v2 data.
  • Temporary .tmp and .trash files from writes are created alongside the destination file (third level), so they won't trigger false positives.
  • The [empty] sentinel directory (used for empty namespaces in v2) is properly handled — it's a directory, so it's traversed but not flagged.
  • The ambiguous case (v1 with non-empty primary AND secondary) is correctly acknowledged as indistinguishable from v2 data and is not rejected.
  • Tests cover all relevant cases: files at root, files one level down, empty directories, nested namespace directories, and valid v2 data with written keys.

No issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants