Skip to content

Make entropy source configurable, default to mnemonic#197

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

Make entropy source configurable, default to mnemonic#197
tnull wants to merge 5 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

👋 Thanks for assigning @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 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?

Comment thread ldk-server/src/util/config.rs Outdated
Comment thread ldk-server/src/util/entropy.rs Outdated
Comment thread ldk-server/src/util/config.rs Outdated
Comment thread ldk-server/src/util/entropy.rs Outdated
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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done in a fixup.

@tnull tnull force-pushed the 2026-04-mnemonic-seed branch from a26361f to bcb430a Compare May 21, 2026 14:01
@tnull tnull requested a review from benthecarman May 21, 2026 14:07
Comment thread ldk-server/src/util/entropy.rs Outdated
Comment thread ldk-server/src/util/entropy.rs Outdated
@lorenzolfm
Copy link
Copy Markdown

lorenzolfm commented May 21, 2026

Hi @tnull. I think that this change doesn't do the

We however detect the old behavior and will keep using keys_seed if it is present.

Part you described.

I've did the following to test:

  1. Ran ldk-server on main, let the software generate entropy for me.
 [INFO  ldk_node] Starting up LDK Node with node ID 0235b4adee490f4aba6bcb9dfb5d145d132625adb8229b08fea6bc295ef292577f on network: regtest
 [INFO  ldk_server] NODE_URI: 0235b4adee...77f@127.0.0.1:9735
 [INFO  ldk_server] gRPC service listening on 127.0.0.1:3536
  1. Stopped the node, switched to this branch, build and ran. Without setting anything entropy related in the config file.
 [INFO  ldk_server::util::entropy] Generated new BIP39 mnemonic at /tmp/ldk-repro-pr197/ldk-storage/keys_mnemonic. Back up this file securely — it is required to recover on-chain funds.
 [ERROR ldk_node::builder] Failed to set up wallet: Descriptor mismatch for External keychain:
         loaded   wpkh([c0ff2044/84'/1'/0']tpubDCLz...)#7y546d6z 
         expected wpkh([2bd63f70/84'/1'/0']tpubDCDx...)#d5ncgc2j 
 [ERROR ldk_server] Failed to build LDK Node: Failed to setup onchain wallet.

It seems that:

  • existing keys_seed is ignored
  • A brand-new mnemonic is silently written to disk before any safety check runs
  • The node fails to start with a nasty error message

Also, if I may, I'm not sure that the best path is ignoring any key, as it may confuse the user on what key he is actually using. Maybe the best path forward is fail to boot not only if both config options are present, but if the two files are present or something like that

@tnull
Copy link
Copy Markdown
Collaborator Author

tnull commented May 22, 2026

Hi @tnull. I think that this change doesn't do the

We however detect the old behavior and will keep using keys_seed if it is present.

Part you described.

Well, it used to, but Ben explicitly requested the backwards compatibility logic to be dropped in #197 (comment) as there are currently only ~5 nodes actually deployed for testing purposes and if this is the new default behavior prior to the actual release, we should be able to at some point even drop the legacy keys_seed behavior entirely.

tnull and others added 5 commits May 22, 2026 08:08
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.


Co-Authored-By: HAL 9000
Review feedback preferred a single explicit entropy source over

two mutually exclusive Options.

Convert the config to an enum and add CLI and environment

overrides so non-TOML deployments can select the entropy path.

Require legacy raw seed users to opt in via seed_file so

default startup consistently uses the mnemonic path.

Set permissions before the final sync so file contents and

permission metadata are flushed together.

Co-Authored-By: HAL 9000
Node entropy mnemonic creation should share the existing private-file write path instead of keeping a separate implementation, while the legacy seed filename should stay scoped to the tests that need it.

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.


Co-Authored-By: HAL 9000
Review feedback removed the automatic legacy keys_seed fallback.

Update the config docs and sample template so operators know an

existing raw seed must be selected explicitly.

Document the new CLI and environment names next to the TOML

fields so non-file configuration is discoverable.

Co-Authored-By: HAL 9000
@tnull tnull force-pushed the 2026-04-mnemonic-seed branch from bcb430a to 367e8d2 Compare May 22, 2026 06:52
@tnull tnull requested a review from benthecarman May 22, 2026 06:52
@tnull tnull force-pushed the 2026-04-mnemonic-seed branch from 367e8d2 to ade2e30 Compare May 22, 2026 06:59
Copy link
Copy Markdown
Collaborator

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

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

Lgtm on squash

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.

5 participants