Allow overshooting final cltv_expiry

Final nodes previously had stricter requirements on HTLC contents
matching onion value compared to intermediate nodes. This allowed
for probing, i.e. the last intermediate node could overshoot the
value by a small amount and conclude from the acceptance or rejection
of the HTLC whether the next node was the destination. This also
applies to the msat amount, however this change was already present.

Author: alecchendev

lightning/src/ln/channelmanager.rs

@@ -2022,9 +2022,9 @@ where
 		payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result<PendingHTLCInfo, ReceiveError>
 	{
 		// final_incorrect_cltv_expiry
-		if hop_data.outgoing_cltv_value != cltv_expiry {
+		if hop_data.outgoing_cltv_value > cltv_expiry {
 			return Err(ReceiveError {
-				msg: "Upstream node set CLTV to the wrong value",
+				msg: "Upstream node set CLTV to less than outgoing CLTV",
 				err_code: 18,
 				err_data: cltv_expiry.to_be_bytes().to_vec()
 			})

lightning/src/ln/onion_route_tests.rs

@@ -552,7 +552,7 @@ fn test_onion_failure() {
 			for f in pending_forwards.iter_mut() {
 				match f {
 					&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) =>
-						forward_info.outgoing_cltv_value += 1,
+						forward_info.outgoing_cltv_value -= 1,
 					_ => {},
 				}
 			}
@@ -602,6 +602,31 @@ fn test_onion_failure() {
 	}, true, Some(23), None, None);
 }
 
+#[test]
+#[should_panic]
+fn test_overshoot_final_cltv() {
+	// Test that we can overshoot the final cltv_expiry set by the sender
+	// by making sure expecting it to fail panics
+	let chanmon_cfgs = create_chanmon_cfgs(3);
+	let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+	let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None; 3]);
+	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+	let channels = [create_announced_chan_between_nodes(&nodes, 0, 1), create_announced_chan_between_nodes(&nodes, 1, 2)];
+	let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], 40000);
+	send_payment(&nodes[0], &vec!(&nodes[1], &nodes[2])[..], 40000);
+	run_onion_failure_test("final_incorrect_cltv_expiry", 1, &nodes, &route, &payment_hash, &payment_secret, |_| {}, || {
+		for (_, pending_forwards) in nodes[1].node.forward_htlcs.lock().unwrap().iter_mut() {
+			for f in pending_forwards.iter_mut() {
+				match f {
+					&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo { ref mut forward_info, .. }) =>
+						forward_info.outgoing_cltv_value += 1,
+					_ => {},
+				}
+			}
+		}
+	}, true, Some(18), None, Some(channels[1].0.contents.short_channel_id));
+}
+
 fn do_test_onion_failure_stale_channel_update(announced_channel: bool) {
 	// Create a network of three nodes and two channels connecting them. We'll be updating the
 	// HTLC relay policy of the second channel, causing forwarding failures at the first hop.
@@ -1096,7 +1121,7 @@ fn test_phantom_final_incorrect_cltv_expiry() {
 				&mut HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
 					forward_info: PendingHTLCInfo { ref mut outgoing_cltv_value, .. }, ..
 				}) => {
-					*outgoing_cltv_value += 1;
+					*outgoing_cltv_value -= 1;
 				},
 				_ => panic!("Unexpected forward"),
 			}
@@ -1114,7 +1139,7 @@ fn test_phantom_final_incorrect_cltv_expiry() {
 	commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false);
 
 	// Ensure the payment fails with the expected error.
-	let expected_cltv: u32 = 82;
+	let expected_cltv: u32 = 80;
 	let error_data = expected_cltv.to_be_bytes().to_vec();
 	let mut fail_conditions = PaymentFailedConditions::new()
 		.blamed_scid(phantom_scid)