← Watch feed
Make BlindedMessagePath creation infallible
Public commit record
63/100 · Adequate
What the developer wrote
Authored by Valentine Wallace
Make BlindedMessagePath creation infallible
The LDK codebase in general is comfortable panicking if the entropy source
provided to it is dysfunctional. Up until now we made an exception for blinded
path creation, where we would handle an error that could occur on mul_tweak
that could only occur if the session_priv provided was not actually random.
In comparable cases in onion_utils, we would panic instead.
In upcoming commits, we will be including blinded paths in outbound
revoke_and_ack messages as part of implementing async payments, where it is
difficult to handle failing back an HTLC if blinded path creation fails. Thus
we now have an incentive to make the blinded path creation methods infallible,
so do so here.
✓ Descriptive subject✓ Provides detailed explanatory context✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit changes how LDK builds private 'blinded' communication paths so that the creation process can no longer fail in normal use. Previously, a bad random number could cause path creation to return an error. Now the code will panic (crash the program) instead if that happens, matching how LDK already handles broken randomness elsewhere. The change is described by the developers as a cleanup to simplify upcoming async-payments work, not as a fix for an active security bug.
Recommended action
Treat as a routine API refactor rather than a security patch. Reviewers should verify that callers no longer need to handle the removed error case and that the panic message is acceptable for production behavior. No urgent action required.
Security signals we found
01
Removal of error propagation for cryptographic operations
02
Introduction of panic on secp256k1 mul_tweak failure
03
Change in public API from fallible to infallible constructors
04
Commit message frames change as design consistency, not security fix
Evidence from the diff
The patch removes Result/Option error returns from BlindedMessagePath and related blinded-path constructors, making them infallible. The only real failure mode being removed is a secp256k1::Error from PublicKey::mul_tweak when the session_priv RNG produces an invalid scalar. That condition is now handled with .expect(“RNG is busted”), i.e., a panic. Call sites are updated to drop .unwrap()/.ok() handling. The commit message explicitly states LDK is ‘comfortable panicking if the entropy source provided to it is dysfunctional’ and that the motivation is to simplify async-payments message flows, not to fix a vulnerability.
Changed components
lightning/src/blinded_path/message.rslightning/src/blinded_path/payment.rslightning/src/blinded_path/utils.rslightning/src/onion_message/messenger.rslightning-dns-resolver/src/lib.rsInspect captured patch +58 / −94
diff --git a/lightning-dns-resolver/src/lib.rs b/lightning-dns-resolver/src/lib.rs
index fc591c8..3d8e1b3 100644
--- a/lightning-dns-resolver/src/lib.rs
+++ b/lightning-dns-resolver/src/lib.rs
@@ -239,8 +239,7 @@ mod test {
context,
&keys,
secp_ctx,
- )
- .unwrap()])
+ )])
}
}
impl Deref for DirectlyConnectedRouter {
@@ -349,8 +348,7 @@ mod test {
query_context,
&*payer_keys,
&secp_ctx,
- )
- .unwrap();
+ );
payer.pending_messages.lock().unwrap().push((
DNSResolverMessage::DNSSECQuery(msg),
MessageSendInstructions::WithSpecifiedReplyPath {
diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs
index 8105bb2..7d721cd 100644
--- a/lightning/src/blinded_path/message.rs
+++ b/lightning/src/blinded_path/message.rs
@@ -57,23 +57,19 @@ impl BlindedMessagePath {
pub fn one_hop<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
recipient_node_id: PublicKey, local_node_receive_key: ReceiveAuthKey,
context: MessageContext, entropy_source: ES, secp_ctx: &Secp256k1<T>,
- ) -> Result<Self, ()>
+ ) -> Self
where
ES::Target: EntropySource,
{
Self::new(&[], recipient_node_id, local_node_receive_key, context, entropy_source, secp_ctx)
}
- /// Create a path for an onion message, to be forwarded along `node_pks`. The last node
- /// pubkey in `node_pks` will be the destination node.
- ///
- /// Errors if no hops are provided or if `node_pk`(s) are invalid.
- // TODO: make all payloads the same size with padding + add dummy hops
+ /// Create a path for an onion message, to be forwarded along `node_pks`.
pub fn new<ES: Deref, T: secp256k1::Signing + secp256k1::Verification>(
intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
local_node_receive_key: ReceiveAuthKey, context: MessageContext, entropy_source: ES,
secp_ctx: &Secp256k1<T>,
- ) -> Result<Self, ()>
+ ) -> Self
where
ES::Target: EntropySource,
{
@@ -96,7 +92,7 @@ impl BlindedMessagePath {
intermediate_nodes: &[MessageForwardNode], recipient_node_id: PublicKey,
dummy_hop_count: usize, local_node_receive_key: ReceiveAuthKey, context: MessageContext,
entropy_source: ES, secp_ctx: &Secp256k1<T>,
- ) -> Result<Self, ()>
+ ) -> Self
where
ES::Target: EntropySource,
{
@@ -107,7 +103,7 @@ impl BlindedMessagePath {
let blinding_secret =
SecretKey::from_slice(&blinding_secret_bytes[..]).expect("RNG is busted");
- Ok(Self(BlindedPath {
+ Self(BlindedPath {
introduction_node,
blinding_point: PublicKey::from_secret_key(secp_ctx, &blinding_secret),
blinded_hops: blinded_hops(
@@ -118,9 +114,8 @@ impl BlindedMessagePath {
context,
&blinding_secret,
local_node_receive_key,
- )
- .map_err(|_| ())?,
- }))
+ ),
+ })
}
/// Attempts to a use a compact representation for the [`IntroductionNode`] by using a directed
@@ -669,7 +664,7 @@ pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
secp_ctx: &Secp256k1<T>, intermediate_nodes: &[MessageForwardNode],
recipient_node_id: PublicKey, dummy_hop_count: usize, context: MessageContext,
session_priv: &SecretKey, local_node_receive_key: ReceiveAuthKey,
-) -> Result<Vec<BlindedHop>, secp256k1::Error> {
+) -> Vec<BlindedHop> {
let dummy_count = cmp::min(dummy_hop_count, MAX_DUMMY_HOPS_COUNT);
let pks = intermediate_nodes
.iter()
diff --git a/lightning/src/blinded_path/payment.rs b/lightning/src/blinded_path/payment.rs
index 96913ef..4ae10f7 100644
--- a/lightning/src/blinded_path/payment.rs
+++ b/lightning/src/blinded_path/payment.rs
@@ -116,7 +116,6 @@ impl BlindedPaymentPath {
/// Create a blinded path for a payment, to be forwarded along `intermediate_nodes`.
///
/// Errors if:
- /// * a provided node id is invalid
/// * [`BlindedPayInfo`] calculation results in an integer overflow
/// * any unknown features are required in the provided [`ForwardTlvs`]
// TODO: make all payloads the same size with padding + add dummy hops
@@ -151,8 +150,7 @@ impl BlindedPaymentPath {
payee_node_id,
payee_tlvs,
&blinding_secret,
- )
- .map_err(|_| ())?,
+ ),
},
payinfo: blinded_payinfo,
})
@@ -663,7 +661,7 @@ pub(crate) const PAYMENT_PADDING_ROUND_OFF: usize = 30;
pub(super) fn blinded_hops<T: secp256k1::Signing + secp256k1::Verification>(
secp_ctx: &Secp256k1<T>, intermediate_nodes: &[PaymentForwardNode], payee_node_id: PublicKey,
payee_tlvs: ReceiveTlvs, session_priv: &SecretKey,
-) -> Result<Vec<BlindedHop>, secp256k1::Error> {
+) -> Vec<BlindedHop> {
let pks = intermediate_nodes
.iter()
.map(|node| (node.node_id, None))
diff --git a/lightning/src/blinded_path/utils.rs b/lightning/src/blinded_path/utils.rs
index 3956fd9..5fc359a 100644
--- a/lightning/src/blinded_path/utils.rs
+++ b/lightning/src/blinded_path/utils.rs
@@ -51,10 +51,8 @@ macro_rules! build_keys_helper {
hmac.input(encrypted_data_ss.as_ref());
Hmac::from_engine(hmac).to_byte_array()
};
- pk.mul_tweak(
- $secp_ctx,
- &Scalar::from_be_bytes(hop_pk_blinding_factor).unwrap(),
- )?
+ pk.mul_tweak($secp_ctx, &Scalar::from_be_bytes(hop_pk_blinding_factor).unwrap())
+ .expect("RNG is busted")
};
let onion_packet_ss = SharedSecret::new(&blinded_hop_pk, &onion_packet_pubkey_priv);
@@ -84,9 +82,9 @@ macro_rules! build_keys_helper {
Sha256::from_engine(sha).to_byte_array()
};
- msg_blinding_point_priv = msg_blinding_point_priv.mul_tweak(
- &Scalar::from_be_bytes(msg_blinding_point_blinding_factor).unwrap(),
- )?;
+ msg_blinding_point_priv = msg_blinding_point_priv
+ .mul_tweak(&Scalar::from_be_bytes(msg_blinding_point_blinding_factor).unwrap())
+ .expect("RNG is busted");
msg_blinding_point =
PublicKey::from_secret_key($secp_ctx, &msg_blinding_point_priv);
@@ -96,9 +94,9 @@ macro_rules! build_keys_helper {
sha.input(onion_packet_ss.as_ref());
Sha256::from_engine(sha).to_byte_array()
};
- onion_packet_pubkey_priv = onion_packet_pubkey_priv.mul_tweak(
- &Scalar::from_be_bytes(onion_packet_pubkey_blinding_factor).unwrap(),
- )?;
+ onion_packet_pubkey_priv = onion_packet_pubkey_priv
+ .mul_tweak(&Scalar::from_be_bytes(onion_packet_pubkey_blinding_factor).unwrap())
+ .expect("RNG is busted");
onion_packet_pubkey =
PublicKey::from_secret_key($secp_ctx, &onion_packet_pubkey_priv);
};
@@ -109,8 +107,7 @@ macro_rules! build_keys_helper {
pub(crate) fn construct_keys_for_onion_message<'a, T, I, F>(
secp_ctx: &Secp256k1<T>, unblinded_path: I, destination: Destination, session_priv: &SecretKey,
mut callback: F,
-) -> Result<(), secp256k1::Error>
-where
+) where
T: secp256k1::Signing + secp256k1::Verification,
I: Iterator<Item = PublicKey>,
F: FnMut(SharedSecret, PublicKey, [u8; 32], Option<PublicKey>, Option<Vec<u8>>),
@@ -134,13 +131,11 @@ where
}
},
}
- Ok(())
}
fn construct_keys_for_blinded_path<'a, T, I, F, H>(
secp_ctx: &Secp256k1<T>, unblinded_path: I, session_priv: &SecretKey, mut callback: F,
-) -> Result<(), secp256k1::Error>
-where
+) where
T: secp256k1::Signing + secp256k1::Verification,
H: Borrow<PublicKey>,
I: Iterator<Item = H>,
@@ -151,7 +146,6 @@ where
for pk in unblinded_path {
build_keys_in_loop!(pk, false, None);
}
- Ok(())
}
struct PublicKeyWithTlvs<W: Writeable> {
@@ -168,7 +162,7 @@ impl<W: Writeable> Borrow<PublicKey> for PublicKeyWithTlvs<W> {
pub(crate) fn construct_blinded_hops<'a, T, I, W>(
secp_ctx: &Secp256k1<T>, unblinded_path: I, session_priv: &SecretKey,
-) -> Result<Vec<BlindedHop>, secp256k1::Error>
+) -> Vec<BlindedHop>
where
T: secp256k1::Signing + secp256k1::Verification,
I: Iterator<Item = ((PublicKey, Option<ReceiveAuthKey>), W)>,
@@ -194,8 +188,8 @@ where
),
});
},
- )?;
- Ok(blinded_hops)
+ );
+ blinded_hops
}
/// Encrypt TLV payload to be used as a [`crate::blinded_path::BlindedHop::encrypted_payload`].
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index 8db7930..a8e7af2 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -1552,7 +1552,7 @@ fn route_blinding_spec_test_vector() {
];
let mut dave_eve_blinded_hops = blinded_path::utils::construct_blinded_hops(
&secp_ctx, path.into_iter(), &dave_eve_session_priv,
- ).unwrap();
+ );
// Concatenate an additional Bob -> Carol blinded path to the Eve -> Dave blinded path.
let bob_carol_session_priv = secret_from_hex("0202020202020202020202020202020202020202020202020202020202020202");
@@ -1563,7 +1563,7 @@ fn route_blinding_spec_test_vector() {
];
let bob_carol_blinded_hops = blinded_path::utils::construct_blinded_hops(
&secp_ctx, path.into_iter(), &bob_carol_session_priv,
- ).unwrap();
+ );
let mut blinded_hops = bob_carol_blinded_hops;
blinded_hops.append(&mut dave_eve_blinded_hops);
@@ -2030,7 +2030,7 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
let path = [((carol_node_id, None), WithoutLength(&carol_unblinded_tlvs))];
blinded_path::utils::construct_blinded_hops(
&secp_ctx, path.into_iter(), &carol_alice_trampoline_session_priv,
- ).unwrap()
+ )
} else {
let payee_tlvs = blinded_path::payment::TrampolineForwardTlvs {
next_trampoline: alice_node_id,
@@ -2051,7 +2051,7 @@ fn do_test_trampoline_single_hop_receive(success: bool) {
let path = [((carol_node_id, None), WithoutLength(&carol_unblinded_tlvs))];
blinded_path::utils::construct_blinded_hops(
&secp_ctx, path.into_iter(), &carol_alice_trampoline_session_priv,
- ).unwrap()
+ )
};
let route = Route {
@@ -2255,7 +2255,7 @@ fn test_trampoline_unblinded_receive() {
let carol_blinding_point = PublicKey::from_secret_key(&secp_ctx, &carol_alice_trampoline_session_priv);
let carol_blinded_hops = blinded_path::utils::construct_blinded_hops(
&secp_ctx, path.into_iter(), &carol_alice_trampoline_session_priv,
- ).unwrap();
+ );
let route = Route {
paths: vec![Path {
diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs
index 4bec3dc..1cbea9f 100644
--- a/lightning/src/onion_message/functional_tests.rs
+++ b/lightning/src/onion_message/functional_tests.rs
@@ -437,8 +437,7 @@ fn one_blinded_hop() {
let entropy = &*nodes[1].entropy_source;
let receive_key = nodes[1].messenger.node_signer.get_receive_auth_key();
let blinded_path =
- BlindedMessagePath::new(&[], nodes[1].node_id, receive_key, context, entropy, &secp_ctx)
- .unwrap();
+ BlindedMessagePath::new(&[], nodes[1].node_id, receive_key, context, entropy, &secp_ctx);
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap();
@@ -463,8 +462,7 @@ fn blinded_path_with_dummy_hops() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
// Ensure that dummy hops are added to the blinded path.
assert_eq!(blinded_path.blinded_hops().len(), 6);
let destination = Destination::BlindedPath(blinded_path);
@@ -492,8 +490,7 @@ fn two_unblinded_two_blinded() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let path = OnionMessagePath {
intermediate_nodes: vec![nodes[1].node_id, nodes[2].node_id],
destination: Destination::BlindedPath(blinded_path),
@@ -525,8 +522,7 @@ fn three_blinded_hops() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
@@ -553,8 +549,7 @@ fn async_response_over_one_blinded_hop() {
let entropy = &*nodes[1].entropy_source;
let receive_key = nodes[1].messenger.node_signer.get_receive_auth_key();
let reply_path =
- BlindedMessagePath::new(&[], nodes[1].node_id, receive_key, context, entropy, &secp_ctx)
- .unwrap();
+ BlindedMessagePath::new(&[], nodes[1].node_id, receive_key, context, entropy, &secp_ctx);
// 4. Create a responder using the reply path for Alice.
let responder = Some(Responder::new(reply_path));
@@ -595,8 +590,7 @@ fn async_response_with_reply_path_succeeds() {
let entropy = &*bob.entropy_source;
let receive_key = bob.messenger.node_signer.get_receive_auth_key();
let reply_path =
- BlindedMessagePath::new(&[], bob.node_id, receive_key, context, entropy, &secp_ctx)
- .unwrap();
+ BlindedMessagePath::new(&[], bob.node_id, receive_key, context, entropy, &secp_ctx);
// Alice asynchronously responds to Bob, expecting a response back from him.
let responder = Responder::new(reply_path);
@@ -638,8 +632,7 @@ fn async_response_with_reply_path_fails() {
let entropy = &*bob.entropy_source;
let receive_key = bob.messenger.node_signer.get_receive_auth_key();
let reply_path =
- BlindedMessagePath::new(&[], bob.node_id, receive_key, context, entropy, &secp_ctx)
- .unwrap();
+ BlindedMessagePath::new(&[], bob.node_id, receive_key, context, entropy, &secp_ctx);
// Alice tries to asynchronously respond to Bob, but fails because the nodes are unannounced and
// disconnected. Thus, a reply path could no be created for the response.
@@ -697,8 +690,7 @@ fn test_blinded_path_padding_for_full_length_path() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
assert!(is_padded(&blinded_path.blinded_hops(), MESSAGE_PADDING_ROUND_OFF));
@@ -734,8 +726,7 @@ fn test_blinded_path_no_padding_for_compact_path() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
assert!(!is_padded(&blinded_path.blinded_hops(), MESSAGE_PADDING_ROUND_OFF));
}
@@ -762,8 +753,7 @@ fn we_are_intro_node() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
@@ -784,8 +774,7 @@ fn we_are_intro_node() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
@@ -814,8 +803,7 @@ fn invalid_blinded_path_error() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
blinded_path.clear_blinded_hops();
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
@@ -850,8 +838,7 @@ fn reply_path() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
nodes[0]
.messenger
.send_onion_message_using_path(path, test_msg.clone(), Some(reply_path))
@@ -878,8 +865,7 @@ fn reply_path() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let intermediate_nodes = [
MessageForwardNode { node_id: nodes[2].node_id, short_channel_id: None },
@@ -895,8 +881,7 @@ fn reply_path() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let instructions = MessageSendInstructions::WithSpecifiedReplyPath { destination, reply_path };
nodes[0].messenger.send_onion_message(test_msg, instructions).unwrap();
@@ -1000,8 +985,7 @@ fn requests_peer_connection_for_buffered_messages() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
@@ -1049,8 +1033,7 @@ fn drops_buffered_messages_waiting_for_peer_connection() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
@@ -1114,8 +1097,7 @@ fn intercept_offline_peer_oms() {
context,
entropy,
&secp_ctx,
- )
- .unwrap();
+ );
let destination = Destination::BlindedPath(blinded_path);
let instructions = MessageSendInstructions::WithoutReplyPath { destination };
diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs
index ede82b4..4fe2a63 100644
--- a/lightning/src/onion_message/messenger.rs
+++ b/lightning/src/onion_message/messenger.rs
@@ -272,7 +272,7 @@ where
/// ];
/// let context = MessageContext::Custom(Vec::new());
/// let receive_key = keys_manager.get_receive_auth_key();
-/// let blinded_path = BlindedMessagePath::new(&hops, your_node_id, receive_key, context, &keys_manager, &secp_ctx).unwrap();
+/// let blinded_path = BlindedMessagePath::new(&hops, your_node_id, receive_key, context, &keys_manager, &secp_ctx);
///
/// // Send a custom onion message to a blinded path.
/// let destination = Destination::BlindedPath(blinded_path);
@@ -627,16 +627,14 @@ where
.into_iter()
.map(|(peer, _, _)| build_path(&[peer]))
.take(MAX_PATHS)
- .collect::<Result<Vec<_>, _>>()
- .ok()
- .filter(|paths| !paths.is_empty())
- .or_else(|| {
- is_recipient_announced
- .then(|| build_path(&[]))
- .and_then(|result| result.ok())
- .map(|path| vec![path])
- })
- .ok_or(())?;
+ .collect::<Vec<_>>();
+ if paths.is_empty() {
+ if is_recipient_announced {
+ paths = vec![build_path(&[])];
+ } else {
+ return Err(());
+ }
+ }
// Sanity check: Ones the paths are created for the non-compact case, ensure
// each of them are of the length `PADDED_PATH_LENGTH`.
@@ -2464,8 +2462,7 @@ fn packet_payloads_and_keys<
mu,
});
},
- )
- .map_err(|e| SendError::Secp256k1(e))?;
+ );
if let Some(control_tlvs) = final_control_tlvs {
payloads.push((
Risk score
Our methodology →Why this scored 36/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.