From 52f290119d8c4dea74b7d03e716e61aacd4dfe3f Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 15 Jun 2023 12:37:21 -0400 Subject: [PATCH] Set extra skimmed fee on intercepted forward Receivers need to use this value to verify incoming payments if ChannelConfig::accept_underpaying_htlcs is set. --- lightning/src/events/mod.rs | 1 + lightning/src/ln/channelmanager.rs | 18 +++++++++++++++--- pending_changelog/forward-underpaying-htlc.txt | 4 ++++ 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 pending_changelog/forward-underpaying-htlc.txt diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 76a7f884..b4b5bf83 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 7883e255..0776bd56 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, } #[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 00000000..21f4a146 --- /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 -- 2.30.2