Skip to content

Upgrade rand dependency, use os_rng for seed generation #683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tnull merged 3 commits into from
Nov 7, 2025
Merged

Upgrade rand dependency, use os_rng for seed generation #683

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76194c7
Upgrade `rand` to v0.9.2
tnull Oct 29, 2025
e173997
Use `os_rng` for seed/mnemonic generation
tnull Oct 29, 2025
7063f2c
No turbofishing for `user_channel_id`
tnull Oct 30, 2025
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
10 changes: 7 additions & 3 deletions src/io/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ use lightning::util::persist::{
};
use lightning::util::ser::{Readable, ReadableArgs, Writeable};
use lightning_types::string::PrintableString;
use rand::{rng, RngCore};
use rand::rngs::OsRng;
use rand::TryRngCore;

use super::*;
use crate::chain::ChainSource;
Expand Down Expand Up @@ -63,7 +64,7 @@ pub const EXTERNAL_PATHFINDING_SCORES_CACHE_KEY: &str = "external_pathfinding_sc
pub fn generate_entropy_mnemonic() -> Mnemonic {
// bip39::Mnemonic supports 256 bit entropy max
let mut entropy = [0; 32];
rng().fill_bytes(&mut entropy);
OsRng.try_fill_bytes(&mut entropy).expect("Failed to generate entropy");
Mnemonic::from_entropy(&entropy).unwrap()
}

Expand Down Expand Up @@ -96,7 +97,10 @@ where
Ok(key)
} else {
let mut key = [0; WALLET_KEYS_SEED_LEN];
rng().fill_bytes(&mut key);
OsRng.try_fill_bytes(&mut key).map_err(|e| {
log_error!(logger, "Failed to generate entropy: {}", e);
std::io::Error::new(std::io::ErrorKind::Other, "Failed to generate seed bytes")
})?;
Copy link
Contributor

@tankyleo tankyleo Oct 30, 2025

Choose a reason for hiding this comment

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

If we propagate the Err from the OsRng here should we also propagate the Err in generate_entropy_mnemonic above ?

Copy link
Contributor

@tankyleo tankyleo Oct 30, 2025

Choose a reason for hiding this comment

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

I guess I am looking for consistency here, but maybe the difference is a public API function vs a function internal to the crate ?

Copy link
Collaborator Author

@tnull tnull Oct 30, 2025

Choose a reason for hiding this comment

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

If we propagate the Err from the OsRng here should we also propagate the Err in generate_entropy_mnemonic above ?

Well, we would need to introduce an enum error type just for this case, which seems like overkill. It could even be argued that we should to panic in this case, as according to the docs of OsRng failing is only expected in case of system misconfiguration and we fail to actually provide sufficient entropy, i.e., every operation after this could be considered unsafe.

Copy link
Contributor

@tankyleo tankyleo Oct 30, 2025

Choose a reason for hiding this comment

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

hmm ok would you consider panicking here ? sounds to me like OsRng returning an Error is very rare, but if it ever does, user should probably stop what they are doing and consider running ldk-node on a different machine.

Copy link
Collaborator Author

@tnull tnull Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

hmm ok would you consider panicking here ? sounds to me like OsRng returning an Error is very rare, but if it ever does, user should probably stop what they are doing and consider running ldk-node on a different machine.

Well, the other path will happen on build only, so instead of panicking we'd just be erroring out before we did anything really.


if let Some(parent_dir) = Path::new(&keys_seed_path).parent() {
fs::create_dir_all(parent_dir).map_err(|e| {
Expand Down
Loading