ln: process added trampoline htlcs with CLTV validation in tests
What changed, and why it matters
This commit changes how the Lightning node handles a special kind of forwarded payment called a 'trampoline' payment. Previously, trampoline forwards were always rejected. Now, in test builds only, the code allows them through after checking basic fee and timeout (CLTV) rules. The commit also removes one test that expected trampoline forwards to be rejected. This is a development/testing change, not a fix for an active security flaw, and production behavior still rejects these forwards.
Treat as normal development review. If trampoline forwarding is intended for production, ensure full forwarding validation, restart persistence, and interception logic are implemented before removing the #[cfg(test)] guard. No urgent security patch is indicated by this commit alone.
Security signals we found
Trampoline forwarding previously hard-rejected; now conditionally accepted in test builds with reduced validation
CLTV and fee-skim checks added for trampoline forwards, but full channel-level forwarding checks still skipped
Production code path continues to reject trampoline forwards
Restart persistence for trampoline forwards explicitly noted as unimplemented
Test-only gating via #[cfg(test)] reduces real-world exposure
Evidence from the diff
In channelmanager.rs, the can_forward_htlc_should_intercept method previously returned LocalHTLCFailureReason::InvalidTrampolineForward for any HopConnector::Trampoline. The patch keeps that rejection for non-test builds but adds a #[cfg(test)] branch that performs two validations: (1) the incoming amount is at least the outgoing amount (fee-skim check), and (2) the incoming CLTV expiry is not less than the outgoing CLTV value via check_incoming_htlc_cltv. It also moves cur_height earlier so it can be used in the trampoline branch. The removed test in blinded_payment_tests.rs verified the old UnknownNextPeer/InvalidTrampolineForward rejection path. The commit message explicitly states proper restart handling is not yet implemented and this is only enabled in tests.
Changed components
lightning/src/ln/channelmanager.rslightning/src/ln/blinded_payment_tests.rsInspect captured patch +28 / −126
diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs
index b4de379..e4538e4 100644
--- a/lightning/src/ln/blinded_payment_tests.rs
+++ b/lightning/src/ln/blinded_payment_tests.rs
@@ -2741,127 +2741,3 @@ fn do_test_trampoline_relay(blinded: bool, test_case: TrampolineTestCase) {
claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
}
}
-
-#[test]
-#[rustfmt::skip]
-fn test_trampoline_forward_rejection() {
- const TOTAL_NODE_COUNT: usize = 3;
-
- let chanmon_cfgs = create_chanmon_cfgs(TOTAL_NODE_COUNT);
- let node_cfgs = create_node_cfgs(TOTAL_NODE_COUNT, &chanmon_cfgs);
- let node_chanmgrs = create_node_chanmgrs(TOTAL_NODE_COUNT, &node_cfgs, &vec![None; TOTAL_NODE_COUNT]);
- let mut nodes = create_network(TOTAL_NODE_COUNT, &node_cfgs, &node_chanmgrs);
-
- let (_, _, chan_id_alice_bob, _) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
- let (_, _, chan_id_bob_carol, _) = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0);
-
- for i in 0..TOTAL_NODE_COUNT { // connect all nodes' blocks
- connect_blocks(&nodes[i], (TOTAL_NODE_COUNT as u32) * CHAN_CONFIRM_DEPTH + 1 - nodes[i].best_block_info().1);
- }
-
- let alice_node_id = nodes[0].node().get_our_node_id();
- let bob_node_id = nodes[1].node().get_our_node_id();
- let carol_node_id = nodes[2].node().get_our_node_id();
-
- let alice_bob_scid = nodes[0].node().list_channels().iter().find(|c| c.channel_id == chan_id_alice_bob).unwrap().short_channel_id.unwrap();
- let bob_carol_scid = nodes[1].node().list_channels().iter().find(|c| c.channel_id == chan_id_bob_carol).unwrap().short_channel_id.unwrap();
-
- let amt_msat = 1000;
- let carol_cltv_expiry_delta = 24 + 24 + 39;
- let (payment_preimage, payment_hash, _) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), None);
-
- let route = Route {
- paths: vec![Path {
- hops: vec![
- // Bob
- RouteHop {
- pubkey: bob_node_id,
- node_features: NodeFeatures::empty(),
- short_channel_id: alice_bob_scid,
- channel_features: ChannelFeatures::empty(),
- fee_msat: 1000,
- cltv_expiry_delta: 48,
- maybe_announced_channel: false,
- },
-
- // Carol
- RouteHop {
- pubkey: carol_node_id,
- node_features: NodeFeatures::empty(),
- short_channel_id: bob_carol_scid,
- channel_features: ChannelFeatures::empty(),
- fee_msat: 0,
- cltv_expiry_delta: carol_cltv_expiry_delta,
- maybe_announced_channel: false,
- }
- ],
- blinded_tail: Some(BlindedTail {
- trampoline_hops: vec![
- // Carol
- TrampolineHop {
- pubkey: carol_node_id,
- node_features: Features::empty(),
- fee_msat: amt_msat,
- cltv_expiry_delta: 24,
- },
-
- // Alice (unreachable)
- TrampolineHop {
- pubkey: alice_node_id,
- node_features: Features::empty(),
- fee_msat: amt_msat,
- cltv_expiry_delta: 24 + 39,
- },
- ],
- hops: vec![BlindedHop{
- // Fake public key
- blinded_node_id: alice_node_id,
- encrypted_payload: vec![],
- }],
- blinding_point: alice_node_id,
- excess_final_cltv_expiry_delta: 39,
- final_value_msat: amt_msat,
- })
- }],
- route_params: RouteParameters::from_payment_params_and_value(
- PaymentParameters::from_node_id(carol_node_id, carol_cltv_expiry_delta),
- amt_msat,
- ),
- };
-
- nodes[0].node.send_payment_with_route(route.clone(), payment_hash, RecipientOnionFields::spontaneous_empty(amt_msat), PaymentId(payment_hash.0)).unwrap();
-
- check_added_monitors(&nodes[0], 1);
-
- let mut events = nodes[0].node.get_and_clear_pending_msg_events();
- assert_eq!(events.len(), 1);
- let first_message_event = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events);
-
- let route: &[&Node] = &[&nodes[1], &nodes[2]];
- let args = PassAlongPathArgs::new(&nodes[0], route, amt_msat, payment_hash, first_message_event)
- .with_payment_preimage(payment_preimage)
- .without_claimable_event()
- .expect_failure(HTLCHandlingFailureType::Receive { payment_hash });
- do_pass_along_path(args);
-
- {
- let unblinded_node_updates = get_htlc_update_msgs(&nodes[2], &nodes[1].node.get_our_node_id());
- nodes[1].node.handle_update_fail_htlc(
- nodes[2].node.get_our_node_id(), &unblinded_node_updates.update_fail_htlcs[0]
- );
- do_commitment_signed_dance(&nodes[1], &nodes[2], &unblinded_node_updates.commitment_signed, true, false);
- }
- {
- let unblinded_node_updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id());
- nodes[0].node.handle_update_fail_htlc(
- nodes[1].node.get_our_node_id(), &unblinded_node_updates.update_fail_htlcs[0]
- );
- do_commitment_signed_dance(&nodes[0], &nodes[1], &unblinded_node_updates.commitment_signed, false, false);
- }
- {
- // Expect UnknownNextPeer error while we are unable to route forwarding Trampoline payments.
- let payment_failed_conditions = PaymentFailedConditions::new()
- .expected_htlc_error_data(LocalHTLCFailureReason::UnknownNextPeer, &[0; 0]);
- expect_payment_failed_conditions(&nodes[0], payment_hash, false, payment_failed_conditions);
- }
-}
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index f97c824..1cff167 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -5199,6 +5199,7 @@ impl<
fn can_forward_htlc_should_intercept(
&self, msg: &msgs::UpdateAddHTLC, prev_chan_public: bool, next_hop: &NextPacketDetails,
) -> Result<bool, LocalHTLCFailureReason> {
+ let cur_height = self.best_block.read().unwrap().height + 1;
let outgoing_scid = match next_hop.outgoing_connector {
HopConnector::ShortChannelId(scid) => scid,
HopConnector::Dummy => {
@@ -5206,8 +5207,34 @@ impl<
debug_assert!(false, "Dummy hop reached HTLC handling.");
return Err(LocalHTLCFailureReason::InvalidOnionPayload);
},
+ // We can't make forwarding checks on trampoline forwards where we don't know the
+ // outgoing channel on receipt of the incoming htlc. Our trampoline logic will check
+ // our required delta and fee later on, so here we just check that the forwarding node
+ // did not "skim" off some of the sender's intended fee/cltv.
HopConnector::Trampoline(_) => {
- return Err(LocalHTLCFailureReason::InvalidTrampolineForward);
+ // We do not yet support reloading our trampoline HTLCs on restart, so we just
+ // fail them for now (except in tests).
+ #[cfg(not(test))]
+ {
+ return Err(LocalHTLCFailureReason::InvalidTrampolineForward);
+ }
+
+ #[cfg(test)]
+ {
+ if msg.amount_msat < next_hop.outgoing_amt_msat {
+ return Err(LocalHTLCFailureReason::FeeInsufficient);
+ }
+
+ check_incoming_htlc_cltv(
+ cur_height,
+ next_hop.outgoing_cltv_value,
+ msg.cltv_expiry,
+ 0,
+ )?;
+
+ // TODO: add interception flag specifically for trampoline
+ return Ok(false);
+ }
},
};
// TODO: We do the fake SCID namespace check a bunch of times here (and indirectly via
@@ -5246,7 +5273,6 @@ impl<
},
};
- let cur_height = self.best_block.read().unwrap().height + 1;
check_incoming_htlc_cltv(
cur_height,
next_hop.outgoing_cltv_value,
Why this scored 23/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.