Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Safely migrate to FileSystemStoreV2 by benthecarman · Pull Request #872 · lightningdevkit/ldk-node · GitHub
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/builder.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,7 +43,6 @@ use lightning::util::persist::{
use lightning::util::ser::ReadableArgs;
use lightning::util::sweep::OutputSweeper;
use lightning_dns_resolver::OMDomainResolver;
use lightning_persister::fs_store::v1::FilesystemStore;
use vss_client::headers::VssHeaderProvider;

use crate::chain::ChainSource;
Expand All@@ -59,8 +58,9 @@ use crate::fee_estimator::OnchainFeeEstimator;
use crate::gossip::GossipSource;
use crate::io::sqlite_store::SqliteStore;
use crate::io::utils::{
read_all_objects, read_event_queue, read_external_pathfinding_scores_from_cache,
read_network_graph, read_node_metrics, read_output_sweeper, read_peer_info, read_scorer,
open_or_migrate_fs_store, read_all_objects, read_event_queue,
read_external_pathfinding_scores_from_cache, read_network_graph, read_node_metrics,
read_output_sweeper, read_peer_info, read_scorer,
};
use crate::io::vss_store::VssStoreBuilder;
use crate::io::{
Expand DownExpand Up@@ -644,18 +644,19 @@ impl NodeBuilder {
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
///
/// If the storage directory contains data from a v1 filesystem store, it will be
/// automatically migrated to the v2 format.
///
/// [`FilesystemStoreV2`]: lightning_persister::fs_store::v2::FilesystemStoreV2
pub fn build_with_fs_store(&self, node_entropy: NodeEntropy) -> Result<Node, BuildError> {
let logger = setup_logger(&self.log_writer_config, &self.config)?;
let mut storage_dir_path: PathBuf = self.config.storage_dir_path.clone().into();
storage_dir_path.push("fs_store");

fs::create_dir_all(storage_dir_path.clone()).map_err(|e| {
log_error!(logger, "Failed to setup Filesystem store: {}", e);
BuildError::StoragePathAccessFailed
})?;
let kv_store = FilesystemStore::new(storage_dir_path);
let kv_store = open_or_migrate_fs_store(storage_dir_path)?;
self.build_with_store_and_logger(node_entropy, kv_store, logger)
}

Expand DownExpand Up@@ -1115,7 +1116,7 @@ impl ArcedNodeBuilder {
self.inner.read().expect("lock").build(*node_entropy).map(Arc::new)
}

/// Builds a [`Node`] instance with a [`FilesystemStore`] backend and according to the options
/// Builds a [`Node`] instance with a [`FilesystemStoreV2`] backend and according to the options
/// previously configured.
pub fn build_with_fs_store(
&self, node_entropy: Arc<NodeEntropy>,
Expand Down
272 changes: 263 additions & 9 deletions src/io/utils.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ use std::io::Write;
use std::ops::Deref;
#[cfg(unix)]
use std::os::unix::fs::OpenOptionsExt;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::{Arc, RwLock};

use bdk_chain::indexer::keychain_txout::ChangeSet as BdkIndexerChangeSet;
Expand All@@ -26,14 +26,16 @@ use lightning::routing::scoring::{
ChannelLiquidities, ProbabilisticScorer, ProbabilisticScoringDecayParameters,
};
use lightning::util::persist::{
KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET, KVSTORE_NAMESPACE_KEY_MAX_LEN,
NETWORK_GRAPH_PERSISTENCE_KEY, NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE,
NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_KEY,
OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE, OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE,
SCORER_PERSISTENCE_KEY, SCORER_PERSISTENCE_PRIMARY_NAMESPACE,
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
migrate_kv_store_data, KVStore, KVStoreSync, KVSTORE_NAMESPACE_KEY_ALPHABET,
KVSTORE_NAMESPACE_KEY_MAX_LEN, NETWORK_GRAPH_PERSISTENCE_KEY,
NETWORK_GRAPH_PERSISTENCE_PRIMARY_NAMESPACE, NETWORK_GRAPH_PERSISTENCE_SECONDARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_KEY, OUTPUT_SWEEPER_PERSISTENCE_PRIMARY_NAMESPACE,
OUTPUT_SWEEPER_PERSISTENCE_SECONDARY_NAMESPACE, SCORER_PERSISTENCE_KEY,
SCORER_PERSISTENCE_PRIMARY_NAMESPACE, SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::{FilesystemStoreV2, FilesystemStoreV2Error};
use lightning_types::string::PrintableString;

use super::*;
Expand All@@ -47,7 +49,7 @@ use crate::logger::{log_error, LdkLogger, Logger};
use crate::peer_store::PeerStore;
use crate::types::{Broadcaster, DynStore, KeysManager, Sweeper};
use crate::wallet::ser::{ChangeSetDeserWrapper, ChangeSetSerWrapper};
use crate::{Error, EventQueue, NodeMetrics};
use crate::{BuildError, Error, EventQueue, NodeMetrics};

pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_scores_cache";

Expand DownExpand Up@@ -619,10 +621,108 @@ pub(crate) fn read_bdk_wallet_change_set(
Ok(Some(change_set))
}

/// Opens a [`FilesystemStoreV2`], automatically migrating from v1 format if necessary.
///
/// If the directory contains v1 data (files at the top level), the data is migrated to v2 format
/// in a temporary directory, the original is renamed to `fs_store_v1_backup`, and the migrated
/// directory is moved into place.
pub(crate) fn open_or_migrate_fs_store(
storage_dir_path: PathBuf,
) -> Result<FilesystemStoreV2, BuildError> {
let parent_dir = storage_dir_path.parent().ok_or(BuildError::StoragePathAccessFailed)?;
fs::create_dir_all(parent_dir).map_err(|_| BuildError::StoragePathAccessFailed)?;
recover_incomplete_fs_store_migration(&storage_dir_path)?;
if !storage_dir_path.exists() {
fs::create_dir_all(storage_dir_path.clone())
.map_err(|_| BuildError::StoragePathAccessFailed)?;
}

match FilesystemStoreV2::new(storage_dir_path.clone()) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [P2] Detect v1 empty-secondary namespace data — /home/tnull/workspace/ldk-node/src/io/utils.rs:640-641
    When an old fs store only has v1 entries under a non-empty primary namespace with an empty secondary namespace, such as fs_store/bdk_wallet/descriptor before any top-level LDK files have been persisted, FilesystemStoreV2::new succeeds because it only rejects top-level files. Startup then
    skips migration and V2 looks under bdk_wallet/[empty]/descriptor, silently ignoring the existing wallet state and potentially missing historical funds; please detect these direct namespace files and migrate them too.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

migrate_kv_store_data does this already

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, no, the first files we create are the ones persisted by BDK, which all go in bdk_wallet. The FilesystemStoreV2::new logic only detects v1 if there are files on the top level, so wouldn't trigger migration of a wallet that was created but never started, i.e., didn't hit a first persistence of channelmanager, AFAIU.

@benthecarmanbenthecarmanMay 30, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will fix upstream

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok(store) => Ok(store),
Err(FilesystemStoreV2Error::V1DataDetected(_)) => {
// The directory contains v1 data, migrate to v2.
let mut v1_store = FilesystemStore::new(storage_dir_path.clone());

let v2_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v2_migrating");
fs::create_dir_all(v2_dir.clone()).map_err(|_| BuildError::StoragePathAccessFailed)?;
let mut v2_store = FilesystemStoreV2::new(v2_dir.clone())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

migrate_kv_store_data(&mut v1_store, &mut v2_store)
Comment thread
tnull marked this conversation as resolved.
.map_err(|_| BuildError::KVStoreSetupFailed)?;

// Swap directories: rename v1 out of the way, move v2 into place.
let backup_dir = fs_store_sibling_path(&storage_dir_path, "fs_store_v1_backup");
fs::rename(&storage_dir_path, &backup_dir)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex:

  • P1 /tmp/ldk-node-pr872/src/io/utils.rs:650, /tmp/ldk-node-pr872/src/builder.rs:659: the v1 to v2 migration is not crash-recoverable. After fs_store is renamed to fs_store_v1_backup, a crash or error before fs_store_v2_migrating is renamed into place leaves no fs_store. On the next startup,
    build_with_fs_store recreates an empty fs_store, FilesystemStoreV2::new succeeds, and the node can start against empty storage while the real data sits in backup/temp dirs. The migration helper should detect and recover incomplete states before creating a fresh store.

Seems we still might want to double-check we survive an ill-timed crash?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration defense and tests for all scenarios

.map_err(|_| BuildError::KVStoreSetupFailed)?;
fs::rename(&v2_dir, &storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can we add an fsync here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added


// fsync the renames
fs::File::open(parent_dir)
.and_then(|f| f.sync_all())
.map_err(|_| BuildError::KVStoreSetupFailed)?;

FilesystemStoreV2::new(storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)
},
Err(_) => Err(BuildError::KVStoreSetupFailed),
}
}

fn fs_store_sibling_path(storage_dir_path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = storage_dir_path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn recover_incomplete_fs_store_migration(storage_dir_path: &Path) -> Result<(), BuildError> {
let v2_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v2_migrating");
let backup_dir = fs_store_sibling_path(storage_dir_path, "fs_store_v1_backup");

if storage_dir_path.exists() {
if v2_dir.exists() {
// The original store is still in place, so a temp migration dir is from a crash before
// the rename step and can be discarded before retrying migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
return Ok(());
}

if backup_dir.exists() {
if v2_dir.exists() {
// Prefer retrying from the v1 backup instead of deciding here whether the temp v2 dir is
// usable. open_or_migrate_fs_store owns the actual v1-to-v2 migration.
fs::remove_dir_all(&v2_dir).map_err(|_| BuildError::KVStoreSetupFailed)?;
}
// The crash happened after moving v1 aside; restore it so normal startup can migrate it.
fs::rename(&backup_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
return Ok(());
}

if v2_dir.exists() {
// There is no v1 backup to retry from. Move the temp dir into place and let
// open_or_migrate_fs_store decide whether it is a valid v2 store.
fs::rename(&v2_dir, storage_dir_path).map_err(|_| BuildError::KVStoreSetupFailed)?;
}

Ok(())
}

#[cfg(test)]
mod tests {
use super::read_or_generate_seed_file;
use std::fs;
use std::path::{Path, PathBuf};

use lightning::util::persist::{migrate_kv_store_data, KVStoreSync};
use lightning_persister::fs_store::v1::FilesystemStore;
use lightning_persister::fs_store::v2::FilesystemStoreV2;

use super::test_utils::random_storage_path;
use super::{open_or_migrate_fs_store, read_or_generate_seed_file};

const TEST_PRIMARY_NAMESPACE: &str = "test_primary_namespace";
const TEST_SECONDARY_NAMESPACE: &str = "test_secondary_namespace";
const TEST_KEY: &str = "test_key";
const TEST_VALUE: &[u8] = b"test_value";

#[test]
fn generated_seed_is_readable() {
Expand All@@ -632,4 +732,158 @@ mod tests {
let read_seed_bytes = read_or_generate_seed_file(&rand_path.to_str().unwrap()).unwrap();
assert_eq!(expected_seed_bytes, read_seed_bytes);
}

#[test]
fn fs_store_migration_recovers_before_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v1_backup_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_after_v2_rename() {
let fs_store_path = fs_store_path();
let mut v1_store = write_v1_test_data(&fs_store_path);
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let mut v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
migrate_kv_store_data(&mut v1_store, &mut v2_store).unwrap();

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, &backup_path).unwrap();
fs::rename(&v2_migrating_path, &fs_store_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(backup_path.exists());
assert!(!v2_migrating_path.exists());
}

#[test]
fn fs_store_migration_recovers_backup_without_migrating_dir() {
let fs_store_path = fs_store_path();
write_v1_test_data(&fs_store_path);

let backup_path = sibling_path(&fs_store_path, "fs_store_v1_backup");
fs::rename(&fs_store_path, backup_path).unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!sibling_path(&fs_store_path, "fs_store_v1_backup").exists());
}

#[test]
fn fs_store_migration_recovers_unexpected_migrating_dir_without_backup() {
let fs_store_path = fs_store_path();
let v2_migrating_path = sibling_path(&fs_store_path, "fs_store_v2_migrating");
let v2_store = FilesystemStoreV2::new(v2_migrating_path.clone()).unwrap();
KVStoreSync::write(
&v2_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();

let migrated_store = open_or_migrate_fs_store(fs_store_path.clone()).unwrap();
assert_eq!(
KVStoreSync::read(
&migrated_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY
)
.unwrap(),
TEST_VALUE
);
assert!(fs_store_path.exists());
assert!(!v2_migrating_path.exists());
}

fn fs_store_path() -> PathBuf {
let mut fs_store_path = random_storage_path();
fs_store_path.push("fs_store");
fs_store_path
}

fn sibling_path(path: &Path, file_name: &str) -> PathBuf {
let mut sibling_path = path.to_path_buf();
sibling_path.set_file_name(file_name);
sibling_path
}

fn write_v1_test_data(fs_store_path: &Path) -> FilesystemStore {
let v1_store = FilesystemStore::new(fs_store_path.to_path_buf());
KVStoreSync::write(
&v1_store,
TEST_PRIMARY_NAMESPACE,
TEST_SECONDARY_NAMESPACE,
TEST_KEY,
TEST_VALUE.to_vec(),
)
.unwrap();
v1_store
}
}
4 changes: 4 additions & 0 deletions tests/common/mod.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -416,6 +416,7 @@ pub(crate) enum TestChainSource<'a> {
pub(crate) enum TestStoreType {
TestSyncStore,
Sqlite,
FilesystemStore,
}

impl Default for TestStoreType {
Expand DownExpand Up@@ -592,6 +593,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) ->
builder.build_with_store(config.node_entropy.into(), kv_store).unwrap()
},
TestStoreType::Sqlite => builder.build(config.node_entropy.into()).unwrap(),
TestStoreType::FilesystemStore => {
builder.build_with_fs_store(config.node_entropy.into()).unwrap()
},
};

if config.recovery_mode {
Expand Down
Loading
Loading