Skip to content

Make entropy source configurable, default to mnemonic#197

Open
tnull wants to merge 2 commits into
lightningdevkit:mainfrom
tnull:2026-04-mnemonic-seed
Open

Make entropy source configurable, default to mnemonic#197
tnull wants to merge 2 commits into
lightningdevkit:mainfrom
tnull:2026-04-mnemonic-seed

Conversation

@tnull
Copy link
Copy Markdown
Collaborator

@tnull tnull commented Apr 21, 2026

We switch to, by default, creating and reading a BIP39 mnemonic file rather than raw bytes. We however detect the old behavior and will keep using keys_seed if it is present.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 21, 2026

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

@tnull tnull marked this pull request as draft April 21, 2026 13:16
@benthecarman
Copy link
Copy Markdown
Collaborator

concept ack

@tnull tnull force-pushed the 2026-04-mnemonic-seed branch from c95f8ab to e849521 Compare May 19, 2026 12:50
@tnull
Copy link
Copy Markdown
Collaborator Author

tnull commented May 19, 2026

Should be good for review.

@tnull tnull marked this pull request as ready for review May 19, 2026 12:53
tnull added 2 commits May 19, 2026 14:56
Previously ldk-server unconditionally loaded its node entropy from a raw
64-byte file at `<storage_dir>/keys_seed`, which is opaque and cannot be
imported into other wallets. Switch the default to a BIP39 mnemonic
written to `<storage_dir>/keys_mnemonic`, so operators can back up their
node identity as a 24-word phrase and recover on-chain funds with any
standard BIP39-compatible wallet.

Add a `[node.entropy]` config section with two mutually exclusive
fields:

  - `mnemonic_file`: path to the BIP39 mnemonic file (defaults to
    `<storage_dir>/keys_mnemonic`).
  - `seed_file`: legacy raw-seed file path, for installs initialized
    before this change.

For backwards compatibility, if neither field is set and a `keys_seed`
file already exists at the storage root, ldk-server continues to use
it. No implicit migration: a raw 64-byte seed cannot be reversed back
into its source mnemonic.

Generated with the assistance of AI (Claude).

Co-Authored-By: HAL 9000
Update the example config and operator-facing docs to reflect the new
default of a BIP39 mnemonic at `<storage_dir>/keys_mnemonic`, the
mutually exclusive `[node.entropy]` options, and the legacy `keys_seed`
backwards-compatibility path. Update the backup table to list both
files so legacy operators don't lose track of their existing seed.

Generated with the assistance of AI (Claude).

Co-Authored-By: HAL 9000
@tnull tnull force-pushed the 2026-04-mnemonic-seed branch from e849521 to a26361f Compare May 19, 2026 12:58
Copy link
Copy Markdown
Contributor

@Anyitechs Anyitechs left a comment

Choose a reason for hiding this comment

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

Thanks!

Just curious, is there a reason the entropy source are only configurable via the toml file and not added to the env var as well?

node.async_payments_role.or(self.async_payments_role.clone());
self.rgs_server_url = node.rgs_server_url.or(self.rgs_server_url.clone());
if let Some(entropy) = node.entropy {
self.entropy.mnemonic_file =
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We use take() here and below because we immediately want to wipe the mnemonic_file/seed_file from memory after parsing, correct?

Comment on lines +34 to +42
let legacy_seed_path = storage_dir.join(LEGACY_SEED_FILE);
if entropy_config.mnemonic_file.is_none() && legacy_seed_path.exists() {
info!(
"Detected legacy raw seed file at {}; continuing to use it. New installs use a BIP39 mnemonic by default.",
legacy_seed_path.display()
);
return NodeEntropy::from_seed_path(legacy_seed_path.to_string_lossy().into_owned())
.map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e.to_string()));
}
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.

Add handling for the like 5 of us seems a bit weird. I'd rather just have us set the config for the legacy path and then not have to handle this in the code.

#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct EntropyConfig {
pub mnemonic_file: Option<String>,
pub seed_file: Option<String>,
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.

since we disallow both being Some this would work better as an enum imo

pub enum EntropyConfig {
  Mnemonic(String),
  SeedFile(String),
  Default, // default mnemonic path in storage dir
}

let mut f = fs::OpenOptions::new().create_new(true).write(true).mode(0o600).open(path)?;
writeln!(f, "{}", mnemonic)?;
f.sync_all()?;
fs::set_permissions(path, fs::Permissions::from_mode(0o600))?;
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.

shouldn't the f.sync_all() be after we set permissions?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants