Use HTLC CLTV instead of onion CLTV values for payment claim timer
What the developer wrote
Authored by Matt Corallo
Use HTLC CLTV instead of onion CLTV values for payment claim timer
When we receive an HTLC as a part of a claim, we validate that the
CLTV on the HTLC is >= the CLTV that the sender requested we
receive, but then we use the CLTV value that the sender requested
we receive as the deadline to claim the HTLC anyway.
This isn't generally all that interesting (they're always the same
unless the previous-hop node gave us "free CLTV"), but for
trampoline payments where we're both a trampoline hop and the
blinded intro point and the recipient, it means we end up allowing
ourselves less claim time than we actually have.
Instead, here, we just use the actual HTLC CLTV deadline.
What changed, and why it matters
This commit fixes a subtle timing bug in how Lightning payment deadlines are recorded. When a node receives a payment, it was writing down the deadline the sender asked it to honor rather than the actual deadline attached to the incoming payment. Normally those two numbers are the same, but in a special 'trampoline' routing case a node can end up giving itself less time than it really has to claim funds. The fix makes the node use the real deadline from the incoming payment instead.
Review and merge; backport to maintained release branches if the trampoline/blinded-path code is present. Monitor for any related claim-timeout failures in production. No urgent incident response indicated.
Security signals we found
Timing/deadline mismatch between validated HTLC CLTV and recorded claim timer
Affects trampoline + blinded-path recipient edge case
Could reduce available time to claim an HTLC, increasing risk of timeout/loss if chain congestion or disputes arise
No explicit memory safety, cryptographic, or remote-crash signal
Evidence from the diff
In create_recv_pending_htlc_info, the incoming_cltv_expiry field of PendingHTLCRouting::Receive and ReceiveKeysend variants was previously set to onion_cltv_expiry (the CLTV value requested by the sender in the onion payload). The patch changes both to cltv_expiry, the actual CLTV expiry on the received HTLC. The commit message notes that the HTLC CLTV is already validated to be >= the requested onion CLTV, so this change gives the receiving node the full, validated claim window rather than a potentially shorter requested window. This matters specifically for trampoline payments where the same node acts as trampoline hop, blinded intro point, and final recipient.
Changed components
lightning/src/ln/onion_payment.rslightning/src/ln/blinded_payment_tests.rsInspect captured patch +8 / −13
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index e8469ca..b945b89 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -981,11 +981,11 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
};
let amt_msat = 5000;
- let excess_final_cltv_delta_opt = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck {
- // Set the final CLTV expiry too low to trigger the failure in process_pending_htlc_forwards.
- Some(TEST_FINAL_CLTV as u16 - 2)
+ let required_final_cltv = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck {
+ // Set the final CLTV required much too high to trigger the failure in process_pending_htlc_forwards.
+ Some((TEST_FINAL_CLTV as u16) * 10)
} else { None };
- let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), excess_final_cltv_delta_opt);
+ let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), required_final_cltv);
let mut route_params = get_blinded_route_parameters(amt_msat, payment_secret, 1, 1_0000_0000,
nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&chan_upd_1_2],
&chanmon_cfgs[2].keys_manager);
@@ -993,11 +993,7 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
route_params.payment_params.max_path_length = 17;
let route = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck {
- let mut route = get_route(&nodes[0], &route_params).unwrap();
- // Set the final CLTV expiry too low to trigger the failure in process_pending_htlc_forwards.
- route.paths[0].hops.last_mut().map(|h| h.cltv_expiry_delta += excess_final_cltv_delta_opt.unwrap() as u32);
- route.paths[0].blinded_tail.as_mut().map(|bt| bt.excess_final_cltv_expiry_delta = excess_final_cltv_delta_opt.unwrap() as u32);
- route
+ get_route(&nodes[0], &route_params).unwrap()
} else if check == ReceiveCheckFail::PaymentConstraints {
// Create a blinded path where the receiver's encrypted payload has an htlc_minimum_msat that is
// violated by `amt_msat`, and stick it in the route_params without changing the corresponding
@@ -1115,7 +1111,6 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
check_added_monitors(&nodes[2], 1);
},
ReceiveCheckFail::ProcessPendingHTLCsCheck => {
- assert_eq!(payment_event_1_2.msgs[0].cltv_expiry, nodes[0].best_block_info().1 + 1 + excess_final_cltv_delta_opt.unwrap() as u32 + TEST_FINAL_CLTV);
nodes[2].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]);
check_added_monitors(&nodes[2], 0);
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true);
diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs
index def4a18..5111f69 100644
--- a/lightning/src/ln/onion_payment.rs
+++ b/lightning/src/ln/onion_payment.rs
@@ -438,7 +438,7 @@ pub(super) fn create_recv_pending_htlc_info(
payment_data,
payment_preimage,
payment_metadata,
- incoming_cltv_expiry: onion_cltv_expiry,
+ incoming_cltv_expiry: cltv_expiry,
custom_tlvs,
requires_blinded_error,
has_recipient_created_payment_secret,
@@ -450,7 +450,7 @@ pub(super) fn create_recv_pending_htlc_info(
payment_data: data,
payment_metadata,
payment_context,
- incoming_cltv_expiry: onion_cltv_expiry,
+ incoming_cltv_expiry: cltv_expiry,
phantom_shared_secret,
trampoline_shared_secret,
custom_tlvs,
@@ -842,7 +842,7 @@ mod tests {
PendingHTLCRouting::ReceiveKeysend { payment_preimage, payment_data, incoming_cltv_expiry, .. } => {
assert_eq!(payment_preimage, preimage);
assert_eq!(peeled2.outgoing_amt_msat, recipient_amount);
- assert_eq!(incoming_cltv_expiry, peeled2.outgoing_cltv_value);
+ assert_eq!(incoming_cltv_expiry, msg.cltv_expiry);
let msgs::FinalOnionHopData{total_msat, payment_secret} = payment_data.unwrap();
assert_eq!(total_msat, total_amt_msat);
assert_eq!(payment_secret, pay_secret);
Why this scored 44/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.