From 86ffea1c39b10e8d6b210eee80bb26d0dc0f985a Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 15 Feb 2025 19:25:34 +0000 Subject: [PATCH 1/4] Remove extra sum of tx fee dust on the counterparty tx dust exposure Previously, `get_pending_htlc_stats` did not account for the inbound htlc because `can_accept_incoming_htlc` was called before the htlc was irrevocably committed. But after commit d8d9dc7, `can_accept_incoming_htlc` is called only when the htlc is irrevocably committed, hence `get_pending_htlc_stats` does account for the inbound htlc. Nonetheless, in the case of a non-dust htlc, our calculation of the counterparty tx dust exposure still assumed that `get_pending_htlc_stats` did not account for the inbound htlc, causing us to add the dust exposure due to that inbound htlc twice. This commit removes this extra sum. --- lightning/src/ln/channel.rs | 36 ++++++++++------------------ lightning/src/ln/functional_tests.rs | 14 +++++------ 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f3abb77a849..585fa8f9a38 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7336,6 +7336,8 @@ impl FundedChannel where }) } + /// When this function is called, the HTLC is already irrevocably committed to the channel; + /// this function determines whether to fail the HTLC, or forward / claim it. pub fn can_accept_incoming_htlc( &self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator, logger: L ) -> Result<(), (&'static str, u16)> @@ -7350,33 +7352,19 @@ impl FundedChannel where let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); let htlc_stats = self.context.get_pending_htlc_stats(None, dust_exposure_limiting_feerate); let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - let (htlc_timeout_dust_limit, htlc_success_dust_limit) = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - (0, 0) + let on_counterparty_tx_dust_htlc_exposure_msat = htlc_stats.on_counterparty_tx_dust_exposure_msat; + if on_counterparty_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat { + // Note that the total dust exposure includes both the dust HTLCs and the excess mining fees of the counterparty commitment transaction + log_info!(logger, "Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx", + on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); + return Err(("Exceeded our total dust exposure limit on counterparty commitment tx", 0x1000|7)) + } + let htlc_success_dust_limit = if self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + 0 } else { let dust_buffer_feerate = self.context.get_dust_buffer_feerate(None) as u64; - (dust_buffer_feerate * htlc_timeout_tx_weight(self.context.get_channel_type()) / 1000, - dust_buffer_feerate * htlc_success_tx_weight(self.context.get_channel_type()) / 1000) + dust_buffer_feerate * htlc_success_tx_weight(self.context.get_channel_type()) / 1000 }; - let exposure_dust_limit_timeout_sats = htlc_timeout_dust_limit + self.context.counterparty_dust_limit_satoshis; - if msg.amount_msat / 1000 < exposure_dust_limit_timeout_sats { - let on_counterparty_tx_dust_htlc_exposure_msat = htlc_stats.on_counterparty_tx_dust_exposure_msat; - if on_counterparty_tx_dust_htlc_exposure_msat > max_dust_htlc_exposure_msat { - log_info!(logger, "Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", - on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); - return Err(("Exceeded our dust exposure limit on counterparty commitment tx", 0x1000|7)) - } - } else { - let htlc_dust_exposure_msat = - per_outbound_htlc_counterparty_commit_tx_fee_msat(self.context.feerate_per_kw, &self.context.channel_type); - let counterparty_tx_dust_exposure = - htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(htlc_dust_exposure_msat); - if counterparty_tx_dust_exposure > max_dust_htlc_exposure_msat { - log_info!(logger, "Cannot accept value that would put our exposure to tx fee dust at {} over the limit {} on counterparty commitment tx", - counterparty_tx_dust_exposure, max_dust_htlc_exposure_msat); - return Err(("Exceeded our tx fee dust exposure limit on counterparty commitment tx", 0x1000|7)) - } - } - let exposure_dust_limit_success_sats = htlc_success_dust_limit + self.context.holder_dust_limit_satoshis; if msg.amount_msat / 1000 < exposure_dust_limit_success_sats { let on_holder_tx_dust_htlc_exposure_msat = htlc_stats.on_holder_tx_dust_exposure_msat; diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4fb530520d8..9fd093f2503 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -10378,13 +10378,13 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e } else { 0 }; let initial_feerate = if apply_excess_fee { 253 * 2 } else { 253 }; let expected_dust_buffer_feerate = initial_feerate + 2530; - let mut commitment_tx_cost = commit_tx_fee_msat(initial_feerate - 253, nondust_htlc_count_in_limit, &ChannelTypeFeatures::empty()); - commitment_tx_cost += + let mut commitment_tx_cost_msat = commit_tx_fee_msat(initial_feerate - 253, nondust_htlc_count_in_limit, &ChannelTypeFeatures::empty()); + commitment_tx_cost_msat += if on_holder_tx { htlc_success_tx_weight(&ChannelTypeFeatures::empty()) } else { htlc_timeout_tx_weight(&ChannelTypeFeatures::empty()) - } * (initial_feerate as u64 - 253) / 1000 * nondust_htlc_count_in_limit; + } * (initial_feerate as u64 - 253) * nondust_htlc_count_in_limit; { let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap(); *feerate_lock = initial_feerate; @@ -10393,8 +10393,8 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e // Default test fee estimator rate is 253 sat/kw, so we set the multiplier to 5_000_000 / 253 // to get roughly the same initial value as the default setting when this test was // originally written. - MaxDustHTLCExposure::FeeRateMultiplier((5_000_000 + commitment_tx_cost) / 253) - } else { MaxDustHTLCExposure::FixedLimitMsat(5_000_000 + commitment_tx_cost) }; + MaxDustHTLCExposure::FeeRateMultiplier((5_000_000 + commitment_tx_cost_msat) / 253) + } else { MaxDustHTLCExposure::FixedLimitMsat(5_000_000 + commitment_tx_cost_msat) }; let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), None]); let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); @@ -10538,8 +10538,8 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e } else { // Outbound dust balance: 5200 sats nodes[0].logger.assert_log("lightning::ln::channel", - format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", - dust_htlc_on_counterparty_tx_msat * dust_htlc_on_counterparty_tx + commitment_tx_cost + 4, + format!("Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx", + dust_htlc_on_counterparty_tx_msat * dust_htlc_on_counterparty_tx + commitment_tx_cost_msat + 4, max_dust_htlc_exposure_msat), 1); } } else if exposure_breach_event == ExposureEvent::AtUpdateFeeOutbound { From 177e51d25a2d96732c107a988175542738795457 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 15 Feb 2025 19:27:07 +0000 Subject: [PATCH 2/4] For the candidate outbound htlc, sum weights, then sum fees Previously, we calculated the fee of the commitment transaction with n htlcs, and the fee due to the candidate htlc, rounded the two fees to the lower satoshi, and then summed the fees. This is not equal to how fees of commitment transactions are calculated, which is to add up the total weight of the (n+1) htlc commitment transaction, convert to fee, then round to the lower satoshi. This commit corrects this delta by running the full fee calculation twice, once for the n htlc, and once for the (n+1) htlc counterparty commitment transactions. --- lightning/src/ln/chan_utils.rs | 17 ++++++++-------- lightning/src/ln/channel.rs | 37 ++++++++++++++-------------------- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index ae76308f0ce..a471bd05c7d 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -196,15 +196,16 @@ pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_t / 1000 } -pub(crate) fn per_outbound_htlc_counterparty_commit_tx_fee_msat(feerate_per_kw: u32, channel_type_features: &ChannelTypeFeatures) -> u64 { - // Note that we need to divide before multiplying to round properly, - // since the lowest denomination of bitcoin on-chain is the satoshi. - let commitment_tx_fee = COMMITMENT_TX_WEIGHT_PER_HTLC * feerate_per_kw as u64 / 1000 * 1000; - if channel_type_features.supports_anchors_zero_fee_htlc_tx() { - commitment_tx_fee + htlc_success_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 +pub(crate) fn commit_and_htlc_tx_fees_sat(feerate_per_kw: u32, num_accepted_htlcs: usize, num_offered_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 { + let num_htlcs = num_accepted_htlcs + num_offered_htlcs; + let commit_tx_fees_sat = commit_tx_fee_sat(feerate_per_kw, num_htlcs, channel_type_features); + let htlc_tx_fees_sat = if !channel_type_features.supports_anchors_zero_fee_htlc_tx() { + num_accepted_htlcs as u64 * htlc_success_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 + + num_offered_htlcs as u64 * htlc_timeout_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 } else { - commitment_tx_fee - } + 0 + }; + commit_tx_fees_sat + htlc_tx_fees_sat } // Various functions for key derivation and transaction creation for use within channels. Primarily diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 585fa8f9a38..3f69d4bd39f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -46,7 +46,7 @@ use crate::ln::chan_utils::{ HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, - ClosingTransaction, commit_tx_fee_sat, per_outbound_htlc_counterparty_commit_tx_fee_msat, + ClosingTransaction, commit_tx_fee_sat, }; use crate::ln::chan_utils; use crate::ln::onion_utils::HTLCFailReason; @@ -834,6 +834,10 @@ struct HTLCStats { pending_inbound_htlcs_value_msat: u64, pending_outbound_htlcs_value_msat: u64, on_counterparty_tx_dust_exposure_msat: u64, + // If the counterparty sets a feerate on the channel in excess of our dust_exposure_limiting_feerate, + // this will be set to the dust exposure that would result from us adding an additional nondust outbound + // htlc on the counterparty's commitment transaction. + extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat: Option, on_holder_tx_dust_exposure_msat: u64, outbound_holding_cell_msat: u64, on_holder_tx_outbound_holding_cell_htlcs_count: u32, // dust HTLCs *non*-included @@ -3705,20 +3709,13 @@ impl ChannelContext where SP::Target: SignerProvider { .or(self.pending_update_fee.map(|(fee, _)| fee)) .unwrap_or(self.feerate_per_kw) .checked_sub(dust_exposure_limiting_feerate); - if let Some(excess_feerate) = excess_feerate_opt { - let on_counterparty_tx_nondust_htlcs = - on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs; - on_counterparty_tx_dust_exposure_msat += - commit_tx_fee_sat(excess_feerate, on_counterparty_tx_nondust_htlcs, &self.channel_type) * 1000; - if !self.channel_type.supports_anchors_zero_fee_htlc_tx() { - on_counterparty_tx_dust_exposure_msat += - on_counterparty_tx_accepted_nondust_htlcs as u64 * htlc_success_tx_weight(&self.channel_type) - * excess_feerate as u64 / 1000; - on_counterparty_tx_dust_exposure_msat += - on_counterparty_tx_offered_nondust_htlcs as u64 * htlc_timeout_tx_weight(&self.channel_type) - * excess_feerate as u64 / 1000; - } - } + let extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat = excess_feerate_opt.map(|excess_feerate| { + let extra_htlc_dust_exposure = on_counterparty_tx_dust_exposure_msat + + chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs + 1, on_counterparty_tx_offered_nondust_htlcs, &self.channel_type) * 1000; + on_counterparty_tx_dust_exposure_msat + += chan_utils::commit_and_htlc_tx_fees_sat(excess_feerate, on_counterparty_tx_accepted_nondust_htlcs, on_counterparty_tx_offered_nondust_htlcs, &self.channel_type) * 1000; + extra_htlc_dust_exposure + }); HTLCStats { pending_inbound_htlcs: self.pending_inbound_htlcs.len(), @@ -3726,6 +3723,7 @@ impl ChannelContext where SP::Target: SignerProvider { pending_inbound_htlcs_value_msat, pending_outbound_htlcs_value_msat, on_counterparty_tx_dust_exposure_msat, + extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat, on_holder_tx_dust_exposure_msat, outbound_holding_cell_msat, on_holder_tx_outbound_holding_cell_htlcs_count, @@ -3930,13 +3928,8 @@ impl ChannelContext where SP::Target: SignerProvider { context.holder_dust_limit_satoshis + dust_buffer_feerate * htlc_timeout_tx_weight(context.get_channel_type()) / 1000) }; - let excess_feerate_opt = self.feerate_per_kw.checked_sub(dust_exposure_limiting_feerate); - if let Some(excess_feerate) = excess_feerate_opt { - let htlc_dust_exposure_msat = - per_outbound_htlc_counterparty_commit_tx_fee_msat(excess_feerate, &context.channel_type); - let nondust_htlc_counterparty_tx_dust_exposure = - htlc_stats.on_counterparty_tx_dust_exposure_msat.saturating_add(htlc_dust_exposure_msat); - if nondust_htlc_counterparty_tx_dust_exposure > max_dust_htlc_exposure_msat { + if let Some(extra_htlc_dust_exposure) = htlc_stats.extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat { + if extra_htlc_dust_exposure > max_dust_htlc_exposure_msat { // If adding an extra HTLC would put us over the dust limit in total fees, we cannot // send any non-dust HTLCs. available_capacity_msat = cmp::min(available_capacity_msat, htlc_success_dust_limit * 1000); From 8ed5a2345ef97eb3dcbae2d5578aace8ec1bfc31 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 15 Feb 2025 18:36:56 +0000 Subject: [PATCH 3/4] Assert dust exposure exhaustion in the excess fees are dust test The payments in this test previously failed for reasons other than exhausting the dust exposure limit with excess fees. Upon payment failures, we now check the logs to assert failures due to dust exposure exhaustion. --- lightning/src/ln/functional_tests.rs | 92 ++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 9fd093f2503..a0a33faf74b 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -10596,9 +10596,20 @@ fn test_max_dust_htlc_exposure() { } #[test] -fn test_nondust_htlc_fees_are_dust() { - // Test that the transaction fees paid in nondust HTLCs count towards our dust limit +fn test_nondust_htlc_excess_fees_are_dust() { + // Test that the excess transaction fees paid in nondust HTLCs count towards our dust limit + const DEFAULT_FEERATE: u32 = 253; + const HIGH_FEERATE: u32 = 275; + const EXCESS_FEERATE: u32 = HIGH_FEERATE - DEFAULT_FEERATE; let chanmon_cfgs = create_chanmon_cfgs(3); + { + // Set the feerate of the channel funder above the `dust_exposure_limiting_feerate` of + // the fundee. This delta means that the fundee will add the mining fees of the commitment and + // htlc transactions in excess of its `dust_exposure_limiting_feerate` to its total dust htlc + // exposure. + let mut feerate_lock = chanmon_cfgs[1].fee_estimator.sat_per_kw.lock().unwrap(); + *feerate_lock = HIGH_FEERATE; + } let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); let mut config = test_default_channel_config(); @@ -10606,18 +10617,16 @@ fn test_nondust_htlc_fees_are_dust() { config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FeeRateMultiplier(10_000); // Make sure the HTLC limits don't get in the way - config.channel_handshake_limits.min_max_accepted_htlcs = 400; - config.channel_handshake_config.our_max_accepted_htlcs = 400; + config.channel_handshake_limits.min_max_accepted_htlcs = chan_utils::MAX_HTLCS; + config.channel_handshake_config.our_max_accepted_htlcs = chan_utils::MAX_HTLCS; config.channel_handshake_config.our_htlc_minimum_msat = 1; + config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100; let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(config), Some(config), Some(config)]); let nodes = create_network(3, &node_cfgs, &node_chanmgrs); - // Create a channel from 1 -> 0 but immediately push all of the funds towards 0 - let chan_id_1 = create_announced_chan_between_nodes(&nodes, 1, 0).2; - while nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat > 0 { - send_payment(&nodes[1], &[&nodes[0]], nodes[1].node.list_channels()[0].next_outbound_htlc_limit_msat); - } + // Leave enough on the funder side to let it pay the mining fees for a commit tx with tons of htlcs + let chan_id_1 = create_announced_chan_between_nodes_with_value(&nodes, 1, 0, 1_000_000, 750_000_000).2; // First get the channel one HTLC_VALUE HTLC away from the dust limit by sending dust HTLCs // repeatedly until we run out of space. @@ -10637,16 +10646,24 @@ fn test_nondust_htlc_fees_are_dust() { assert_ne!(nodes[0].node.list_channels()[0].next_outbound_htlc_minimum_msat, 0, "Make sure we are able to send once we clear one HTLC"); + // Skip the router complaint when node 0 will attempt to pay node 1 + let (route_0_1, payment_hash_0_1, _, payment_secret_0_1) = get_route_and_payment_hash!(nodes[0], nodes[1], dust_limit * 2); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + // At this point we have somewhere between dust_limit and dust_limit * 2 left in our dust // exposure limit, and we want to max that out using non-dust HTLCs. let commitment_tx_per_htlc_cost = - htlc_success_tx_weight(&ChannelTypeFeatures::empty()) * 253; + htlc_success_tx_weight(&ChannelTypeFeatures::empty()) * EXCESS_FEERATE as u64; let max_htlcs_remaining = dust_limit * 2 / commitment_tx_per_htlc_cost; - assert!(max_htlcs_remaining < 30, + assert!(max_htlcs_remaining < chan_utils::MAX_HTLCS.into(), "We should be able to fill our dust limit without too many HTLCs"); for i in 0..max_htlcs_remaining + 1 { assert_ne!(i, max_htlcs_remaining); - if nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat < dust_limit { + if nodes[0].node.list_channels()[0].next_outbound_htlc_limit_msat <= dust_limit { // We found our limit, and it was less than max_htlcs_remaining! // At this point we can only send dust HTLCs as any non-dust HTLCs will overuse our // remaining dust exposure. @@ -10655,6 +10672,57 @@ fn test_nondust_htlc_fees_are_dust() { route_payment(&nodes[0], &[&nodes[1]], dust_limit * 2); } + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + + // Send an additional non-dust htlc from 1 to 0, and check the complaint + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], dust_limit * 2); + nodes[1].node.send_payment_with_route(route, payment_hash, + RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap(); + check_added_monitors!(nodes[1], 1); + let mut events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let payment_event = SendEvent::from_event(events.remove(0)); + nodes[0].node.handle_update_add_htlc(nodes[1].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[0], nodes[1], payment_event.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[0]); + expect_htlc_handling_failed_destinations!(nodes[0].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]); + nodes[0].logger.assert_log("lightning::ln::channel", + format!("Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx", + 2535000, 2530000), 1); + check_added_monitors!(nodes[0], 1); + + // Clear the failed htlc + let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id()); + assert!(updates.update_add_htlcs.is_empty()); + assert!(updates.update_fulfill_htlcs.is_empty()); + assert_eq!(updates.update_fail_htlcs.len(), 1); + assert!(updates.update_fail_malformed_htlcs.is_empty()); + assert!(updates.update_fee.is_none()); + nodes[1].node.handle_update_fail_htlc(nodes[0].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[1], nodes[0], updates.commitment_signed, false); + expect_payment_failed!(nodes[1], payment_hash, false); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + + // Send an additional non-dust htlc from 0 to 1 using the pre-calculated route above, and check the immediate complaint + unwrap_send_err!(nodes[0], nodes[0].node.send_payment_with_route(route_0_1, payment_hash_0_1, + RecipientOnionFields::secret_only(payment_secret_0_1), PaymentId(payment_hash_0_1.0) + ), true, APIError::ChannelUnavailable { .. }, {}); + nodes[0].logger.assert_log("lightning::ln::outbound_payment", + format!("Failed to send along path due to error: Channel unavailable: Cannot send more than our next-HTLC maximum - {} msat", 2325000), 1); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + // At this point non-dust HTLCs are no longer accepted from node 0 -> 1, we also check that // such HTLCs can't be routed over the same channel either. create_announced_chan_between_nodes(&nodes, 2, 0); From f931db55729527b0291b45798d91cf1d5fd78dee Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Sat, 15 Feb 2025 04:29:48 +0000 Subject: [PATCH 4/4] Test the accounting of dust exposure due to excess fees This test checks to a 1msat precision the accounting of dust exposure due to excess fees on counterparty commmitment and htlc transactions, for both inbound and outbound htlcs. --- lightning/src/ln/functional_tests.rs | 200 ++++++++++++++++++++++++++- 1 file changed, 199 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index a0a33faf74b..ff0646bfa6c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -24,7 +24,7 @@ use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentSecret, PaymentHash}; use crate::ln::channel::{get_holder_selected_channel_reserve_satoshis, Channel, InboundV1Channel, OutboundV1Channel, COINBASE_MATURITY, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT}; use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; -use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError}; +use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError, MIN_CHAN_DUST_LIMIT_SATOSHIS}; use crate::ln::{chan_utils, onion_utils}; use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment}; use crate::routing::gossip::{NetworkGraph, NetworkUpdate}; @@ -10750,6 +10750,204 @@ fn test_nondust_htlc_excess_fees_are_dust() { expect_payment_failed_conditions(&nodes[2], payment_hash, false, PaymentFailedConditions::new()); } +fn do_test_nondust_htlc_fees_dust_exposure_delta(features: ChannelTypeFeatures) { + // Tests the increase in htlc dust exposure due to the excess mining fees of a single non-dust + // HTLC on the counterparty commitment transaction, for both incoming and outgoing htlcs. + // + // Brings the dust exposure up to the base dust exposure using dust htlcs. + // Sets the max dust exposure to 1msat below the expected dust exposure given an additional non-dust htlc. + // Checks a failed payment for a non-dust htlc. + // Sets the max dust exposure equal to the expected dust exposure given an additional non-dust htlc. + // Checks a successful payment for a non-dust htlc. + // + // Runs this sequence for both directions. + + let chanmon_cfgs = create_chanmon_cfgs(2); + + const DEFAULT_FEERATE: u64 = 253; + const HIGH_FEERATE: u64 = 275; + const EXCESS_FEERATE: u64 = HIGH_FEERATE - DEFAULT_FEERATE; + + const DUST_HTLC_COUNT: usize = 4; + // Set dust htlcs to a satoshi value plus a non-zero msat amount to assert that + // the dust accounting rounds transaction fees to the lower satoshi, but does not round dust htlc values. + const DUST_HTLC_MSAT: u64 = 125_123; + const BASE_DUST_EXPOSURE_MSAT: u64 = DUST_HTLC_COUNT as u64 * DUST_HTLC_MSAT; + + const NON_DUST_HTLC_MSAT: u64 = 4_000_000; + + { + // Set the feerate of the channel funder above the `dust_exposure_limiting_feerate` of + // the fundee. This delta means that the fundee will add the mining fees of the commitment and + // htlc transactions in excess of its `dust_exposure_limiting_feerate` to its total dust htlc + // exposure. + let mut feerate_lock = chanmon_cfgs[0].fee_estimator.sat_per_kw.lock().unwrap(); + *feerate_lock = HIGH_FEERATE as u32; + } + + // Set `expected_dust_exposure_msat` to match the calculation in `FundedChannel::can_accept_incoming_htlc` + // only_static_remote_key: 500_492 + 22 * (724 + 172) / 1000 * 1000 + 22 * 663 / 1000 * 1000 = 533_492 + // anchors_zero_htlc_fee: 500_492 + 22 * (1_124 + 172) / 1000 * 1000 = 528_492 + let mut expected_dust_exposure_msat = BASE_DUST_EXPOSURE_MSAT + EXCESS_FEERATE * (commitment_tx_base_weight(&features) + COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 * 1000; + if features == ChannelTypeFeatures::only_static_remote_key() { + expected_dust_exposure_msat += EXCESS_FEERATE * htlc_timeout_tx_weight(&features) / 1000 * 1000; + assert_eq!(expected_dust_exposure_msat, 533_492); + } else { + assert_eq!(expected_dust_exposure_msat, 528_492); + } + + let mut default_config = test_default_channel_config(); + if features == ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() { + default_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + // in addition to the one above, this setting is also needed to create an anchor channel + default_config.manually_accept_inbound_channels = true; + } + + // Set node 1's max dust htlc exposure to 1msat below `expected_dust_exposure_msat` + let mut fixed_limit_config = default_config.clone(); + fixed_limit_config.channel_config.max_dust_htlc_exposure = MaxDustHTLCExposure::FixedLimitMsat(expected_dust_exposure_msat - 1); + + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(default_config), Some(fixed_limit_config)]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + let chan_id = create_chan_between_nodes_with_value(&nodes[0], &nodes[1], 100_000, 50_000_000).3; + + let node_1_dust_buffer_feerate = { + let per_peer_state = nodes[1].node.per_peer_state.read().unwrap(); + let chan_lock = per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap(); + let chan = chan_lock.channel_by_id.get(&chan_id).unwrap(); + chan.context().get_dust_buffer_feerate(None) as u64 + }; + + // Skip the router complaint when node 1 will attempt to pay node 0 + let (route_1_0, payment_hash_1_0, _, payment_secret_1_0) = get_route_and_payment_hash!(nodes[1], nodes[0], NON_DUST_HTLC_MSAT); + + // Bring node 1's dust htlc exposure up to `BASE_DUST_EXPOSURE_MSAT` + for _ in 0..DUST_HTLC_COUNT { + route_payment(&nodes[0], &[&nodes[1]], DUST_HTLC_MSAT); + } + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + assert_eq!(nodes[0].node.list_channels()[0].pending_outbound_htlcs.len(), DUST_HTLC_COUNT); + assert_eq!(nodes[1].node.list_channels()[0].pending_inbound_htlcs.len(), DUST_HTLC_COUNT); + + // Send an additional non-dust htlc from 0 to 1, and check the complaint + let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], NON_DUST_HTLC_MSAT); + nodes[0].node.send_payment_with_route(route, payment_hash, + RecipientOnionFields::secret_only(payment_secret), 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 payment_event = SendEvent::from_event(events.remove(0)); + nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &payment_event.msgs[0]); + commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false); + expect_pending_htlcs_forwardable!(nodes[1]); + expect_htlc_handling_failed_destinations!(nodes[1].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]); + nodes[1].logger.assert_log("lightning::ln::channel", + format!("Cannot accept value that would put our total dust exposure at {} over the limit {} on counterparty commitment tx", + expected_dust_exposure_msat, expected_dust_exposure_msat - 1), 1); + check_added_monitors!(nodes[1], 1); + + // Clear the failed htlc + let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + assert!(updates.update_add_htlcs.is_empty()); + assert!(updates.update_fulfill_htlcs.is_empty()); + assert_eq!(updates.update_fail_htlcs.len(), 1); + assert!(updates.update_fail_malformed_htlcs.is_empty()); + assert!(updates.update_fee.is_none()); + nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false); + expect_payment_failed!(nodes[0], payment_hash, false); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + assert_eq!(nodes[0].node.list_channels()[0].pending_outbound_htlcs.len(), DUST_HTLC_COUNT); + assert_eq!(nodes[1].node.list_channels()[0].pending_inbound_htlcs.len(), DUST_HTLC_COUNT); + + // Set node 1's max dust htlc exposure equal to the `expected_dust_exposure_msat` + nodes[1].node.update_partial_channel_config(&nodes[0].node.get_our_node_id(), &[chan_id], &ChannelConfigUpdate { + max_dust_htlc_exposure_msat: Some(MaxDustHTLCExposure::FixedLimitMsat(expected_dust_exposure_msat)), + ..ChannelConfigUpdate::default() + }).unwrap(); + + // Check a successful payment + send_payment(&nodes[0], &[&nodes[1]], NON_DUST_HTLC_MSAT); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + assert_eq!(nodes[0].node.list_channels()[0].pending_outbound_htlcs.len(), DUST_HTLC_COUNT); + assert_eq!(nodes[1].node.list_channels()[0].pending_inbound_htlcs.len(), DUST_HTLC_COUNT); + + // The `expected_dust_exposure_msat` for the outbound htlc changes in the non-anchor case, as the htlc success and timeout transactions have different weights + // only_static_remote_key: 500_492 + 22 * (724 + 172) / 1000 * 1000 + 22 * 703 / 1000 * 1000 = 534_492 + if features == ChannelTypeFeatures::only_static_remote_key() { + expected_dust_exposure_msat = BASE_DUST_EXPOSURE_MSAT + EXCESS_FEERATE * (commitment_tx_base_weight(&features) + COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 * 1000 + EXCESS_FEERATE * htlc_success_tx_weight(&features) / 1000 * 1000; + assert_eq!(expected_dust_exposure_msat, 534_492); + } else { + assert_eq!(expected_dust_exposure_msat, 528_492); + } + + // Set node 1's max dust htlc exposure to 1msat below `expected_dust_exposure_msat` + nodes[1].node.update_partial_channel_config(&nodes[0].node.get_our_node_id(), &[chan_id], &ChannelConfigUpdate { + max_dust_htlc_exposure_msat: Some(MaxDustHTLCExposure::FixedLimitMsat(expected_dust_exposure_msat - 1)), + ..ChannelConfigUpdate::default() + }).unwrap(); + + // Send an additional non-dust htlc from 1 to 0 using the pre-calculated route above, and check the immediate complaint + unwrap_send_err!(nodes[1], nodes[1].node.send_payment_with_route(route_1_0, payment_hash_1_0, + RecipientOnionFields::secret_only(payment_secret_1_0), PaymentId(payment_hash_1_0.0) + ), true, APIError::ChannelUnavailable { .. }, {}); + let dust_limit = if features == ChannelTypeFeatures::only_static_remote_key() { + MIN_CHAN_DUST_LIMIT_SATOSHIS * 1000 + htlc_success_tx_weight(&features) * node_1_dust_buffer_feerate / 1000 * 1000 + } else { + MIN_CHAN_DUST_LIMIT_SATOSHIS * 1000 + }; + nodes[1].logger.assert_log("lightning::ln::outbound_payment", + format!("Failed to send along path due to error: Channel unavailable: Cannot send more than our next-HTLC maximum - {} msat", dust_limit), 1); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + assert_eq!(nodes[0].node.list_channels()[0].pending_outbound_htlcs.len(), DUST_HTLC_COUNT); + assert_eq!(nodes[1].node.list_channels()[0].pending_inbound_htlcs.len(), DUST_HTLC_COUNT); + + // Set node 1's max dust htlc exposure equal to `expected_dust_exposure_msat` + nodes[1].node.update_partial_channel_config(&nodes[0].node.get_our_node_id(), &[chan_id], &ChannelConfigUpdate { + max_dust_htlc_exposure_msat: Some(MaxDustHTLCExposure::FixedLimitMsat(expected_dust_exposure_msat)), + ..ChannelConfigUpdate::default() + }).unwrap(); + + // Check a successful payment + send_payment(&nodes[1], &[&nodes[0]], NON_DUST_HTLC_MSAT); + + assert_eq!(nodes[0].node.list_channels().len(), 1); + assert_eq!(nodes[1].node.list_channels().len(), 1); + + assert_eq!(nodes[0].node.list_channels()[0].pending_inbound_htlcs.len(), 0); + assert_eq!(nodes[1].node.list_channels()[0].pending_outbound_htlcs.len(), 0); + assert_eq!(nodes[0].node.list_channels()[0].pending_outbound_htlcs.len(), DUST_HTLC_COUNT); + assert_eq!(nodes[1].node.list_channels()[0].pending_inbound_htlcs.len(), DUST_HTLC_COUNT); +} + +#[test] +fn test_nondust_htlc_fees_dust_exposure_delta() { + do_test_nondust_htlc_fees_dust_exposure_delta(ChannelTypeFeatures::only_static_remote_key()); + do_test_nondust_htlc_fees_dust_exposure_delta(ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()); +} #[test] fn test_non_final_funding_tx() {