From: Valentine Wallace <vwallace@protonmail.com>
Date: Thu, 15 Jun 2023 16:37:21 +0000 (-0400)
Subject: Set extra skimmed fee on intercepted forward
X-Git-Tag: v0.0.116-alpha1~6^2~8
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=52f290119d8c4dea74b7d03e716e61aacd4dfe3f;p=rust-lightning

Set extra skimmed fee on intercepted forward

Receivers need to use this value to verify incoming payments if
ChannelConfig::accept_underpaying_htlcs is set.
---

diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs
index 76a7f884a..b4b5bf838 100644
--- a/lightning/src/events/mod.rs
+++ b/lightning/src/events/mod.rs
@@ -623,6 +623,7 @@ pub enum Event {
 		inbound_amount_msat: u64,
 		/// How many msats the payer intended to route to the next node. Depending on the reason you are
 		/// intercepting this payment, you might take a fee by forwarding less than this amount.
+		/// Forwarding less than this amount may break compatibility with LDK versions prior to 0.0.116.
 		///
 		/// Note that LDK will NOT check that expected fees were factored into this value. You MUST
 		/// check that whatever fee you want has been included here or subtract it as required. Further,
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 7883e255f..0776bd56f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -131,6 +131,9 @@ pub(super) struct PendingHTLCInfo {
 	/// may overshoot this in either case)
 	pub(super) outgoing_amt_msat: u64,
 	pub(super) outgoing_cltv_value: u32,
+	/// The fee being skimmed off the top of this HTLC. If this is a forward, it'll be the fee we are
+	/// skimming. If we're receiving this HTLC, it's the fee that our counterparty skimmed.
+	pub(super) skimmed_fee_msat: Option<u64>,
 }
 
 #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
@@ -2616,6 +2619,7 @@ where
 			incoming_amt_msat: Some(amt_msat),
 			outgoing_amt_msat: hop_data.amt_to_forward,
 			outgoing_cltv_value: hop_data.outgoing_cltv_value,
+			skimmed_fee_msat: None,
 		})
 	}
 
@@ -2890,6 +2894,7 @@ where
 					incoming_amt_msat: Some(msg.amount_msat),
 					outgoing_amt_msat: next_hop_data.amt_to_forward,
 					outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
+					skimmed_fee_msat: None,
 				})
 			}
 		}
@@ -3485,13 +3490,16 @@ where
 	/// [`ChannelManager::fail_intercepted_htlc`] MUST be called in response to the event.
 	///
 	/// Note that LDK does not enforce fee requirements in `amt_to_forward_msat`, and will not stop
-	/// you from forwarding more than you received.
+	/// you from forwarding more than you received. See
+	/// [`HTLCIntercepted::expected_outbound_amount_msat`] for more on forwarding a different amount
+	/// than expected.
 	///
 	/// Errors if the event was not handled in time, in which case the HTLC was automatically failed
 	/// backwards.
 	///
 	/// [`UserConfig::accept_intercept_htlcs`]: crate::util::config::UserConfig::accept_intercept_htlcs
 	/// [`HTLCIntercepted`]: events::Event::HTLCIntercepted
+	/// [`HTLCIntercepted::expected_outbound_amount_msat`]: events::Event::HTLCIntercepted::expected_outbound_amount_msat
 	// TODO: when we move to deciding the best outbound channel at forward time, only take
 	// `next_node_id` and not `next_hop_channel_id`
 	pub fn forward_intercepted_htlc(&self, intercept_id: InterceptId, next_hop_channel_id: &[u8; 32], next_node_id: PublicKey, amt_to_forward_msat: u64) -> Result<(), APIError> {
@@ -3530,7 +3538,10 @@ where
 			},
 			_ => unreachable!() // Only `PendingHTLCRouting::Forward`s are intercepted
 		};
+		let skimmed_fee_msat =
+			payment.forward_info.outgoing_amt_msat.saturating_sub(amt_to_forward_msat);
 		let pending_htlc_info = PendingHTLCInfo {
+			skimmed_fee_msat: if skimmed_fee_msat == 0 { None } else { Some(skimmed_fee_msat) },
 			outgoing_amt_msat: amt_to_forward_msat, routing, ..payment.forward_info
 		};
 
@@ -3600,7 +3611,7 @@ where
 										prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
 										forward_info: PendingHTLCInfo {
 											routing, incoming_shared_secret, payment_hash, outgoing_amt_msat,
-											outgoing_cltv_value, incoming_amt_msat: _
+											outgoing_cltv_value, ..
 										}
 									}) => {
 										macro_rules! failure_handler {
@@ -3713,7 +3724,7 @@ where
 										prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id: _,
 										forward_info: PendingHTLCInfo {
 											incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value,
-											routing: PendingHTLCRouting::Forward { onion_packet, .. }, incoming_amt_msat: _,
+											routing: PendingHTLCRouting::Forward { onion_packet, .. }, ..
 										},
 									}) => {
 										log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
@@ -7448,6 +7459,7 @@ impl_writeable_tlv_based!(PendingHTLCInfo, {
 	(6, outgoing_amt_msat, required),
 	(8, outgoing_cltv_value, required),
 	(9, incoming_amt_msat, option),
+	(10, skimmed_fee_msat, option),
 });
 
 
diff --git a/pending_changelog/forward-underpaying-htlc.txt b/pending_changelog/forward-underpaying-htlc.txt
new file mode 100644
index 000000000..21f4a1464
--- /dev/null
+++ b/pending_changelog/forward-underpaying-htlc.txt
@@ -0,0 +1,4 @@
+## Backwards Compat
+
+* Forwarding less than the expected amount in `ChannelManager::forward_intercepted_htlc` may break
+	compatibility with versions of LDK prior to 0.0.116