ln: process added trampoline htlcs with CLTV validation

We can't perform proper validation because we don't know the outgoing
channel id until we forward the HTLC, so we just perform a basic CLTV
check.

Now that we've got rejection on inbound MPP accumulation, we relax this
check to allow testing of inbound MPP trampoline processing.

Author: carlaKC

lightning/src/ln/blinded_payment_tests.rs

@@ -2715,123 +2715,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 (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: 24 + 24 + 39,
-					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: None,
-	};
-
-	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);
-	}
-}

lightning/src/ln/channelmanager.rs

@@ -5239,15 +5239,32 @@ 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 => {
 				// Dummy hops are only used for path padding and must not reach HTLC processing.
 				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);
+				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
@@ -5286,7 +5303,6 @@ impl<
 				},
 			};
 
-		let cur_height = self.best_block.read().unwrap().height + 1;
 		check_incoming_htlc_cltv(
 			cur_height,
 			next_hop.outgoing_cltv_value,