Make entropy source configurable, default to mnemonic by tnull · Pull Request #197 · lightningdevkit/ldk-server

tnull · 2026-04-21T13:16:01Z

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 · 2026-04-21T13:16:05Z

👋 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.

benthecarman · 2026-04-21T19:01:18Z

concept ack

tnull · 2026-05-19T12:53:37Z

Should be good for review.

Anyitechs

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?

benthecarman · 2026-05-19T18:28:13Z

+	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))?;


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

Done in a fixup.

lorenzolfm · 2026-05-21T23:59:08Z

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:

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

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 · 2026-05-22T05:59:17Z

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.

benthecarman

Lgtm on squash

tankyleo · 2026-05-22T16:38:47Z

we should be able to at some point even drop the legacy keys_seed behavior entirely.

I'm happy to set the legacy keys seed explicitly, and would love to keep this behavior around long-term.

tnull · 2026-05-23T09:09:35Z

we should be able to at some point even drop the legacy keys_seed behavior entirely.

I'm happy to set the legacy keys seed explicitly, and would love to keep this behavior around long-term.

Mhh, I mean its not much code, but more generally we're not providing compatibility guarantees prior to 0.1 final release, and the 3 of us running on mainnet can be expected to migrate once. Before we cut the release we should also take another good look at the codebase if there is any tech debt already that we want to remove while we're free to break API and backwards compat. (cc @benthecarman).

tankyleo · 2026-05-23T17:34:47Z

Mhh, I mean its not much code, but more generally we're not providing compatibility guarantees prior to 0.1 final release, and the 3 of us running on mainnet can be expected to migrate once.

I have no problem with required migrations prior to the release. Rather, I do not want to take a dependency on BIP39 to run ldk-server :)

tnull · 2026-05-24T16:43:07Z

I have no problem with required migrations prior to the release. Rather, I do not want to take a dependency on BIP39 to run ldk-server :)

Okay, would love to learn why that is, but let's take that discussion offline.

tnull · 2026-05-24T16:44:43Z

Lgtm on squash

Squashed without further changes.

benthecarman · 2026-06-01T12:25:49Z

I think we discussed offline just removing the old file type completely. Do you still want to do that here?

Previously ldk-server loaded node entropy from a raw 64-byte file at `<storage_dir>/keys_seed`, which is opaque and cannot be imported into standard wallets. Use a BIP39 mnemonic at `<storage_dir>/keys_mnemonic` instead, so operators can back up their node identity as a 24-word phrase and recover on-chain funds with BIP39-compatible tooling. Do not retain raw `keys_seed` loading as a fallback or configurable entropy source. Stale `keys_seed` files are ignored, and removed `[node.entropy]` config and CLI options are rejected, keeping startup behavior unambiguous for new and upgraded nodes. Co-Authored-By: HAL 9000

Update the operator-facing docs to describe `keys_mnemonic` as the node master secret and default entropy file. Remove the legacy `keys_seed` backup guidance so the documentation matches the mnemonic-only startup behavior. This keeps recovery guidance aligned with the entropy source ldk-server actually loads and avoids suggesting that raw seed fallback remains supported. Co-Authored-By: HAL 9000

tnull · 2026-06-01T14:13:42Z

I think we discussed offline just removing the old file type completely. Do you still want to do that here?

Done, dropped support for keys_seed.

tnull marked this pull request as draft April 21, 2026 13:16

tnull force-pushed the 2026-04-mnemonic-seed branch from c95f8ab to e849521 Compare May 19, 2026 12:50

tnull marked this pull request as ready for review May 19, 2026 12:53

ldk-reviews-bot requested a review from benthecarman May 19, 2026 12:54

tnull force-pushed the 2026-04-mnemonic-seed branch from e849521 to a26361f Compare May 19, 2026 12:58

Anyitechs reviewed May 19, 2026

View reviewed changes

Comment thread ldk-server/src/util/config.rs Outdated

benthecarman requested changes May 19, 2026

View reviewed changes

tnull force-pushed the 2026-04-mnemonic-seed branch from a26361f to bcb430a Compare May 21, 2026 14:01

tnull requested a review from benthecarman May 21, 2026 14:07

benthecarman requested changes May 21, 2026

View reviewed changes

Comment thread ldk-server/src/util/entropy.rs Outdated

Comment thread ldk-server/src/util/entropy.rs Outdated

tnull force-pushed the 2026-04-mnemonic-seed branch from bcb430a to 367e8d2 Compare May 22, 2026 06:52

tnull requested a review from benthecarman May 22, 2026 06:52

tnull force-pushed the 2026-04-mnemonic-seed branch from 367e8d2 to ade2e30 Compare May 22, 2026 06:59

benthecarman reviewed May 22, 2026

View reviewed changes

tnull force-pushed the 2026-04-mnemonic-seed branch from ade2e30 to 36f89b6 Compare May 24, 2026 16:44

tnull requested a review from benthecarman June 1, 2026 12:19

tnull added 2 commits June 1, 2026 16:09

tnull force-pushed the 2026-04-mnemonic-seed branch from 36f89b6 to b1ebfe8 Compare June 1, 2026 14:12

benthecarman approved these changes Jun 1, 2026

View reviewed changes

benthecarman merged commit 639298b into lightningdevkit:main Jun 1, 2026
10 checks passed

Conversation

tnull commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

benthecarman commented Apr 21, 2026

Uh oh!

tnull commented May 19, 2026

Uh oh!

Anyitechs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benthecarman May 19, 2026

Choose a reason for hiding this comment

Uh oh!

tnull May 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lorenzolfm commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tnull commented May 22, 2026

Uh oh!

benthecarman left a comment

Choose a reason for hiding this comment

Uh oh!

tankyleo commented May 22, 2026

Uh oh!

tnull commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tankyleo commented May 23, 2026

Uh oh!

tnull commented May 24, 2026

Uh oh!

tnull commented May 24, 2026

Uh oh!

benthecarman commented Jun 1, 2026

Uh oh!

tnull commented Jun 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tnull commented Apr 21, 2026 •

edited

Loading

ldk-reviews-bot commented Apr 21, 2026 •

edited

Loading

lorenzolfm commented May 21, 2026 •

edited

Loading

tnull commented May 23, 2026 •

edited

Loading