From ceede42fd94d3e07bf1396d97014569bb33cdd87 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 28 May 2025 17:33:53 -0500 Subject: [PATCH 1/8] Check pending funding when validating update_add_htlc If there are any pending splices when an update_add_htlc message is received, it must be validated against each pending FundingScope. Otherwise, the HTLC could be invalid once the splice is locked. --- lightning/src/ln/channel.rs | 83 ++++++++++++++++++++++--------------- 1 file changed, 49 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c5eb9de777a..2a89e20080d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5749,34 +5749,19 @@ impl FundedChannel where Ok(self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger)) } - pub fn update_add_htlc( - &mut self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator, - ) -> Result<(), ChannelError> where F::Target: FeeEstimator { - if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() { - return Err(ChannelError::WarnAndDisconnect("Got add HTLC message while quiescent".to_owned())); - } - if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { - return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned())); - } - // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec. - if self.context.channel_state.is_remote_shutdown_sent() { - return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned())); - } - if self.context.channel_state.is_peer_disconnected() { - return Err(ChannelError::close("Peer sent update_add_htlc when we needed a channel_reestablish".to_owned())); - } - if msg.amount_msat > self.funding.get_value_satoshis() * 1000 { + fn validate_update_add_htlc( + &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, + fee_estimator: &LowerBoundedFeeEstimator, + ) -> Result<(), ChannelError> + where + F::Target: FeeEstimator, + { + if msg.amount_msat > funding.get_value_satoshis() * 1000 { return Err(ChannelError::close("Remote side tried to send more than the total value of the channel".to_owned())); } - if msg.amount_msat == 0 { - return Err(ChannelError::close("Remote side tried to send a 0-msat HTLC".to_owned())); - } - if msg.amount_msat < self.context.holder_htlc_minimum_msat { - return Err(ChannelError::close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.context.holder_htlc_minimum_msat, msg.amount_msat))); - } let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, None, dust_exposure_limiting_feerate); + let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); if htlc_stats.pending_inbound_htlcs + 1 > self.context.holder_max_accepted_htlcs as usize { return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.context.holder_max_accepted_htlcs))); } @@ -5806,9 +5791,9 @@ impl FundedChannel where } let pending_value_to_self_msat = - self.funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; + funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; let pending_remote_value_msat = - self.funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; + funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; if pending_remote_value_msat < msg.amount_msat { return Err(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned())); } @@ -5816,11 +5801,11 @@ impl FundedChannel where // Check that the remote can afford to pay for this HTLC on-chain at the current // feerate_per_kw, while maintaining their channel reserve (as required by the spec). { - let remote_commit_tx_fee_msat = if self.funding.is_outbound() { 0 } else { + let remote_commit_tx_fee_msat = if funding.is_outbound() { 0 } else { let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - self.context.next_remote_commit_tx_fee_msat(&self.funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations + self.context.next_remote_commit_tx_fee_msat(funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations }; - let anchor_outputs_value_msat = if !self.funding.is_outbound() && self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if !funding.is_outbound() && funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 @@ -5828,24 +5813,50 @@ impl FundedChannel where if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat { return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned())); }; - if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < self.funding.holder_selected_channel_reserve_satoshis * 1000 { + if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { return Err(ChannelError::close("Remote HTLC add would put them under remote reserve value".to_owned())); } } - let anchor_outputs_value_msat = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 }; - if self.funding.is_outbound() { + if funding.is_outbound() { // Check that they won't violate our local required channel reserve by adding this HTLC. let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(&self.funding, htlc_candidate, None); - if self.funding.value_to_self_msat < self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat { + let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(funding, htlc_candidate, None); + if funding.value_to_self_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat { return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); } } + + Ok(()) + } + + pub fn update_add_htlc( + &mut self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator, + ) -> Result<(), ChannelError> where F::Target: FeeEstimator { + if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() { + return Err(ChannelError::WarnAndDisconnect("Got add HTLC message while quiescent".to_owned())); + } + if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { + return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned())); + } + // If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec. + if self.context.channel_state.is_remote_shutdown_sent() { + return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned())); + } + if self.context.channel_state.is_peer_disconnected() { + return Err(ChannelError::close("Peer sent update_add_htlc when we needed a channel_reestablish".to_owned())); + } + if msg.amount_msat == 0 { + return Err(ChannelError::close("Remote side tried to send a 0-msat HTLC".to_owned())); + } + if msg.amount_msat < self.context.holder_htlc_minimum_msat { + return Err(ChannelError::close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.context.holder_htlc_minimum_msat, msg.amount_msat))); + } if self.context.next_counterparty_htlc_id != msg.htlc_id { return Err(ChannelError::close(format!("Remote skipped HTLC ID (skipped ID: {})", self.context.next_counterparty_htlc_id))); } @@ -5853,6 +5864,10 @@ impl FundedChannel where return Err(ChannelError::close("Remote provided CLTV expiry in seconds instead of block height".to_owned())); } + core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .try_for_each(|funding| self.validate_update_add_htlc(funding, msg, fee_estimator))?; + // Now update local state: self.context.next_counterparty_htlc_id += 1; self.context.pending_inbound_htlcs.push(InboundHTLCOutput { From df468751a72bf388c3994a3bf1330ea12fa06595 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 29 May 2025 14:36:06 -0500 Subject: [PATCH 2/8] Update pending funding when handling revoke_and_ack If there are any pending splices when a revoke_and_ack message is received, FundingScope::value_to_self_msat needs to be updated for each. Otherwise, the promoted FundingScope will be invalid when the splice is locked. --- lightning/src/ln/channel.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2a89e20080d..4320ce2f1cb 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6364,8 +6364,10 @@ impl FundedChannel where #[cfg(any(test, fuzzing))] { - *self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; - *self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; + for funding in core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) { + *funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None; + *funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None; + } } match &self.context.holder_signer { @@ -6515,7 +6517,10 @@ impl FundedChannel where } } } - self.funding.value_to_self_msat = (self.funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64; + + for funding in core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut()) { + funding.value_to_self_msat = (funding.value_to_self_msat as i64 + value_to_self_msat_diff) as u64; + } if let Some((feerate, update_state)) = self.context.pending_update_fee { match update_state { From 2a76bfee9727b60fcd64912865bcdad918f550c7 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 29 May 2025 14:59:57 -0500 Subject: [PATCH 3/8] Check pending funding when validating update_fee If there are any pending splices when an update_fee message is received, it must be validated against each pending FundingScope. Otherwise, it may be invalid once the splice is locked. --- lightning/src/ln/channel.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4320ce2f1cb..28faf5c5567 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7036,13 +7036,29 @@ impl FundedChannel where if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() { return Err(ChannelError::WarnAndDisconnect("Got fee update message while quiescent".to_owned())); } - FundedChannel::::check_remote_fee(self.funding.get_channel_type(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger)?; + + core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .try_for_each(|funding| FundedChannel::::check_remote_fee(funding.get_channel_type(), fee_estimator, msg.feerate_per_kw, Some(self.context.feerate_per_kw), logger))?; self.context.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced)); self.context.update_time_counter += 1; + + core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .try_for_each(|funding| self.validate_update_fee(funding, fee_estimator, msg)) + } + + fn validate_update_fee( + &self, funding: &FundingScope, fee_estimator: &LowerBoundedFeeEstimator, + msg: &msgs::UpdateFee, + ) -> Result<(), ChannelError> + where + F::Target: FeeEstimator, + { // Check that we won't be pushed over our dust exposure limit by the feerate increase. let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, None, dust_exposure_limiting_feerate); + let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)", From 131cd252102338b5eee250f296189fd6d16f0738 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 29 May 2025 15:26:03 -0500 Subject: [PATCH 4/8] Check pending funding in send_update_fee If there are any pending splices when an sending an update_fee message, the new fee rate must be validated against each pending FundingScope. Otherwise, it may be invalid once the splice is locked. --- lightning/src/ln/channel.rs | 64 ++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 28faf5c5567..6f1c84380b0 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -6737,29 +6737,10 @@ impl FundedChannel where panic!("Cannot update fee while peer is disconnected/we're awaiting a monitor update (ChannelManager should have caught this)"); } - // Before proposing a feerate update, check that we can actually afford the new fee. - let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); - let commitment_data = self.context.build_commitment_transaction( - &self.funding, self.holder_commitment_point.transaction_number(), - &self.holder_commitment_point.current_point(), true, true, logger, - ); - let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.funding.get_channel_type()) * 1000; - let holder_balance_msat = commitment_data.stats.local_balance_before_fee_anchors_msat - htlc_stats.outbound_holding_cell_msat; - if holder_balance_msat < buffer_fee_msat + commitment_data.stats.total_anchors_sat * 1000 + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { - //TODO: auto-close after a number of failures? - log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); - return None; - } - - // Note, we evaluate pending htlc "preemptive" trimmed-to-dust threshold at the proposed `feerate_per_kw`. - let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { - log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); - return None; - } - if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { - log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); + let can_send_update_fee = core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .all(|funding| self.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger)); + if !can_send_update_fee { return None; } @@ -6784,6 +6765,43 @@ impl FundedChannel where }) } + fn can_send_update_fee( + &self, funding: &FundingScope, feerate_per_kw: u32, + fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + ) -> bool + where + F::Target: FeeEstimator, + L::Target: Logger, + { + // Before proposing a feerate update, check that we can actually afford the new fee. + let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); + let htlc_stats = self.context.get_pending_htlc_stats(funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); + let commitment_data = self.context.build_commitment_transaction( + funding, self.holder_commitment_point.transaction_number(), + &self.holder_commitment_point.current_point(), true, true, logger, + ); + let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, funding.get_channel_type()) * 1000; + let holder_balance_msat = commitment_data.stats.local_balance_before_fee_anchors_msat - htlc_stats.outbound_holding_cell_msat; + if holder_balance_msat < buffer_fee_msat + commitment_data.stats.total_anchors_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { + //TODO: auto-close after a number of failures? + log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); + return false; + } + + // Note, we evaluate pending htlc "preemptive" trimmed-to-dust threshold at the proposed `feerate_per_kw`. + let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); + if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); + return false; + } + if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); + return false; + } + + return true; + } + /// Removes any uncommitted inbound HTLCs and resets the state of uncommitted outbound HTLC /// updates, to be used on peer disconnection. After this, update_*_htlc messages need to be /// resent. From b7783adef1797f0b063def42fae8f9f1217e2954 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 29 May 2025 15:49:42 -0500 Subject: [PATCH 5/8] Check pending funding in can_accept_incoming_htlc If there are any pending splices when an accepting an incoming HTLC, the HTLC needs to be validated against each pending FundingScope. Otherwise, once the splice is locked, the HTLC could have been failed when it should have been forwarded / claimed, or vice versa, under the promoted FundingScope. --- lightning/src/ln/channel.rs | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 6f1c84380b0..9336f211ed2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -8134,7 +8134,20 @@ 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(&self.funding, None, dust_exposure_limiting_feerate); + + core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .try_for_each(|funding| self.can_accept_incoming_htlc_for_funding(funding, msg, dust_exposure_limiting_feerate, &logger)) + } + + fn can_accept_incoming_htlc_for_funding( + &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, + dust_exposure_limiting_feerate: u32, logger: &L, + ) -> Result<(), LocalHTLCFailureReason> + where + L::Target: Logger, + { + let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); 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 { @@ -8143,11 +8156,11 @@ impl FundedChannel where on_counterparty_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); return Err(LocalHTLCFailureReason::DustLimitCounterparty) } - let htlc_success_dust_limit = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let htlc_success_dust_limit = if funding.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_success_tx_weight(self.funding.get_channel_type()) / 1000 + dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 }; 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 { @@ -8159,7 +8172,7 @@ impl FundedChannel where } } - let anchor_outputs_value_msat = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 } else { 0 @@ -8175,11 +8188,11 @@ impl FundedChannel where } let pending_value_to_self_msat = - self.funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; + funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; let pending_remote_value_msat = - self.funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; + funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - if !self.funding.is_outbound() { + if !funding.is_outbound() { // `Some(())` is for the fee spike buffer we keep for the remote. This deviates from // the spec because the fee spike buffer requirement doesn't exist on the receiver's // side, only on the sender's. Note that with anchor outputs we are no longer as @@ -8187,11 +8200,11 @@ impl FundedChannel where // // A `None` `HTLCCandidate` is used as in this case because we're already accounting for // the incoming HTLC as it has been fully committed by both sides. - let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(&self.funding, None, Some(())); - if !self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(funding, None, Some(())); + if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; } - if pending_remote_value_msat.saturating_sub(self.funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat { + if pending_remote_value_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat { log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.context.channel_id()); return Err(LocalHTLCFailureReason::FeeSpikeBuffer); } From 802e73a2ea617aec36ef41f507feaa90ef4d7cf8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 29 May 2025 16:12:55 -0500 Subject: [PATCH 6/8] Check pending funding in send_htlc If there are any pending splices when an sending an update_add_htlc message, the HTLC amount must be validated against each pending FundingScope. Otherwise, it may be invalid once the splice is locked. --- lightning/src/ln/channel.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9336f211ed2..35000abb405 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -9140,10 +9140,13 @@ impl FundedChannel where return Err((LocalHTLCFailureReason::ChannelNotReady, "Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned())); } - let channel_total_msat = self.funding.get_value_satoshis() * 1000; - if amount_msat > channel_total_msat { - return Err((LocalHTLCFailureReason::AmountExceedsCapacity, - format!("Cannot send amount {}, because it is more than the total value of the channel {}", amount_msat, channel_total_msat))); + + for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) { + let channel_total_msat = funding.get_value_satoshis() * 1000; + if amount_msat > channel_total_msat { + return Err((LocalHTLCFailureReason::AmountExceedsCapacity, + format!("Cannot send amount {}, because it is more than the total value of the channel {}", amount_msat, channel_total_msat))); + } } if amount_msat == 0 { From 7ffaa566c21d526f779d958ab11c6388dd4619eb Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 30 May 2025 12:07:10 -0500 Subject: [PATCH 7/8] s - drop redundant LocalHTLCFailureReason::AmountExceedsCapacity --- lightning/src/ln/channel.rs | 8 -------- lightning/src/ln/onion_utils.rs | 13 ++++--------- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 35000abb405..7727f6e58c2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -9141,14 +9141,6 @@ impl FundedChannel where "Cannot send HTLC until channel is fully established and we haven't started shutting down".to_owned())); } - for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) { - let channel_total_msat = funding.get_value_satoshis() * 1000; - if amount_msat > channel_total_msat { - return Err((LocalHTLCFailureReason::AmountExceedsCapacity, - format!("Cannot send amount {}, because it is more than the total value of the channel {}", amount_msat, channel_total_msat))); - } - } - if amount_msat == 0 { return Err((LocalHTLCFailureReason::ZeroAmount, "Cannot send 0-msat HTLC".to_owned())); } diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index b3dbd107a76..e9bc0f6a853 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -1590,8 +1590,6 @@ pub enum LocalHTLCFailureReason { /// The HTLC was failed back because its expiry height was reached and funds were timed out /// on chain. OnChainTimeout, - /// The HTLC was failed because its amount is greater than the capacity of the channel. - AmountExceedsCapacity, /// The HTLC was failed because zero amount HTLCs are not allowed. ZeroAmount, /// The HTLC was failed because its amount is less than the smallest HTLC that the channel @@ -1625,7 +1623,6 @@ impl LocalHTLCFailureReason { | Self::DustLimitCounterparty | Self::FeeSpikeBuffer | Self::ChannelNotReady - | Self::AmountExceedsCapacity | Self::ZeroAmount | Self::HTLCMinimum | Self::HTLCMaximum @@ -1780,11 +1777,10 @@ impl_writeable_tlv_based_enum!(LocalHTLCFailureReason, (71, OutgoingCLTVTooSoon) => {}, (73, ChannelClosed) => {}, (75, OnChainTimeout) => {}, - (77, AmountExceedsCapacity) => {}, - (79, ZeroAmount) => {}, - (81, HTLCMinimum) => {}, - (83, HTLCMaximum) => {}, - (85, PeerOffline) => {}, + (77, ZeroAmount) => {}, + (79, HTLCMinimum) => {}, + (81, HTLCMaximum) => {}, + (83, PeerOffline) => {}, ); impl From<&HTLCFailReason> for HTLCHandlingFailureReason { @@ -1897,7 +1893,6 @@ impl HTLCFailReason { | LocalHTLCFailureReason::DustLimitCounterparty | LocalHTLCFailureReason::FeeSpikeBuffer | LocalHTLCFailureReason::ChannelNotReady - | LocalHTLCFailureReason::AmountExceedsCapacity | LocalHTLCFailureReason::ZeroAmount | LocalHTLCFailureReason::HTLCMinimum | LocalHTLCFailureReason::HTLCMaximum From b6a92dafe2b5aac7d649eedfd8eba131ee03f0d2 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 2 Jun 2025 15:42:52 -0500 Subject: [PATCH 8/8] Move FundingScope-specific validation to ChannelContext Previous commits refactored validation checks in FundedChannel to work on a specific FundingScope. However, keeping these helpers in FundedChannel doesn't prevent them from using self.funding inadvertently instead of the passed in FundingScope. Move these helpers to ChannelContext to avoid this problem, as we done with similar helpers. --- lightning/src/ln/channel.rs | 444 ++++++++++++++++++------------------ 1 file changed, 222 insertions(+), 222 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7727f6e58c2..078fbc6138c 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3775,6 +3775,114 @@ impl ChannelContext where SP::Target: SignerProvider { ); } + fn validate_update_add_htlc( + &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, + fee_estimator: &LowerBoundedFeeEstimator, + ) -> Result<(), ChannelError> + where + F::Target: FeeEstimator, + { + if msg.amount_msat > funding.get_value_satoshis() * 1000 { + return Err(ChannelError::close("Remote side tried to send more than the total value of the channel".to_owned())); + } + + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); + if htlc_stats.pending_inbound_htlcs + 1 > self.holder_max_accepted_htlcs as usize { + return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.holder_max_accepted_htlcs))); + } + if htlc_stats.pending_inbound_htlcs_value_msat + msg.amount_msat > self.holder_max_htlc_value_in_flight_msat { + return Err(ChannelError::close(format!("Remote HTLC add would put them over our max HTLC value ({})", self.holder_max_htlc_value_in_flight_msat))); + } + + // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet + // the reserve_satoshis we told them to always have as direct payment so that they lose + // something if we punish them for broadcasting an old state). + // Note that we don't really care about having a small/no to_remote output in our local + // commitment transactions, as the purpose of the channel reserve is to ensure we can + // punish *them* if they misbehave, so we discount any outbound HTLCs which will not be + // present in the next commitment transaction we send them (at least for fulfilled ones, + // failed ones won't modify value_to_self). + // Note that we will send HTLCs which another instance of rust-lightning would think + // violate the reserve value if we do not do this (as we forget inbound HTLCs from the + // Channel state once they will not be present in the next received commitment + // transaction). + let mut removed_outbound_total_msat = 0; + for ref htlc in self.pending_outbound_htlcs.iter() { + if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state { + removed_outbound_total_msat += htlc.amount_msat; + } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { + removed_outbound_total_msat += htlc.amount_msat; + } + } + + let pending_value_to_self_msat = + funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; + let pending_remote_value_msat = + funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; + if pending_remote_value_msat < msg.amount_msat { + return Err(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned())); + } + + // Check that the remote can afford to pay for this HTLC on-chain at the current + // feerate_per_kw, while maintaining their channel reserve (as required by the spec). + { + let remote_commit_tx_fee_msat = if funding.is_outbound() { 0 } else { + let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); + self.next_remote_commit_tx_fee_msat(funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations + }; + let anchor_outputs_value_msat = if !funding.is_outbound() && funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 + } else { + 0 + }; + if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat { + return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned())); + }; + if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { + return Err(ChannelError::close("Remote HTLC add would put them under remote reserve value".to_owned())); + } + } + + let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 + } else { + 0 + }; + if funding.is_outbound() { + // Check that they won't violate our local required channel reserve by adding this HTLC. + let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); + let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(funding, htlc_candidate, None); + if funding.value_to_self_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat { + return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); + } + } + + Ok(()) + } + + fn validate_update_fee( + &self, funding: &FundingScope, fee_estimator: &LowerBoundedFeeEstimator, + msg: &msgs::UpdateFee, + ) -> Result<(), ChannelError> + where + F::Target: FeeEstimator, + { + // Check that we won't be pushed over our dust exposure limit by the feerate increase. + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); + let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); + if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)", + msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat))); + } + if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)", + msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat))); + } + Ok(()) + } + fn validate_commitment_signed( &self, funding: &FundingScope, holder_commitment_point: &HolderCommitmentPoint, msg: &msgs::CommitmentSigned, logger: &L, @@ -3885,6 +3993,116 @@ impl ChannelContext where SP::Target: SignerProvider { }) } + fn can_send_update_fee( + &self, funding: &FundingScope, holder_commitment_point: &HolderCommitmentPoint, + feerate_per_kw: u32, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + ) -> bool + where + F::Target: FeeEstimator, + L::Target: Logger, + { + // Before proposing a feerate update, check that we can actually afford the new fee. + let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(&fee_estimator); + let htlc_stats = self.get_pending_htlc_stats(funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); + let commitment_data = self.build_commitment_transaction( + funding, holder_commitment_point.transaction_number(), + &holder_commitment_point.current_point(), true, true, logger, + ); + let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, funding.get_channel_type()) * 1000; + let holder_balance_msat = commitment_data.stats.local_balance_before_fee_anchors_msat - htlc_stats.outbound_holding_cell_msat; + if holder_balance_msat < buffer_fee_msat + commitment_data.stats.total_anchors_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { + //TODO: auto-close after a number of failures? + log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); + return false; + } + + // Note, we evaluate pending htlc "preemptive" trimmed-to-dust threshold at the proposed `feerate_per_kw`. + let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); + if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); + return false; + } + if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { + log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); + return false; + } + + return true; + } + + fn can_accept_incoming_htlc( + &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, + dust_exposure_limiting_feerate: u32, logger: &L, + ) -> Result<(), LocalHTLCFailureReason> + where + L::Target: Logger, + { + let htlc_stats = self.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); + let max_dust_htlc_exposure_msat = self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); + 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(LocalHTLCFailureReason::DustLimitCounterparty) + } + let htlc_success_dust_limit = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + 0 + } else { + let dust_buffer_feerate = self.get_dust_buffer_feerate(None) as u64; + dust_buffer_feerate * htlc_success_tx_weight(funding.get_channel_type()) / 1000 + }; + let exposure_dust_limit_success_sats = htlc_success_dust_limit + self.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; + if on_holder_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 holder commitment tx", + on_holder_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); + return Err(LocalHTLCFailureReason::DustLimitHolder) + } + } + + let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 + } else { + 0 + }; + + let mut removed_outbound_total_msat = 0; + for ref htlc in self.pending_outbound_htlcs.iter() { + if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state { + removed_outbound_total_msat += htlc.amount_msat; + } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { + removed_outbound_total_msat += htlc.amount_msat; + } + } + + let pending_value_to_self_msat = + funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; + let pending_remote_value_msat = + funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; + + if !funding.is_outbound() { + // `Some(())` is for the fee spike buffer we keep for the remote. This deviates from + // the spec because the fee spike buffer requirement doesn't exist on the receiver's + // side, only on the sender's. Note that with anchor outputs we are no longer as + // sensitive to fee spikes, so we need to account for them. + // + // A `None` `HTLCCandidate` is used as in this case because we're already accounting for + // the incoming HTLC as it has been fully committed by both sides. + let mut remote_fee_cost_incl_stuck_buffer_msat = self.next_remote_commit_tx_fee_msat(funding, None, Some(())); + if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { + remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; + } + if pending_remote_value_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat { + log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.channel_id()); + return Err(LocalHTLCFailureReason::FeeSpikeBuffer); + } + } + + Ok(()) + } + /// Builds stats on a potential commitment transaction build, without actually building the /// commitment transaction. See `build_commitment_transaction` for further docs. #[inline] @@ -5749,92 +5967,6 @@ impl FundedChannel where Ok(self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger)) } - fn validate_update_add_htlc( - &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, - fee_estimator: &LowerBoundedFeeEstimator, - ) -> Result<(), ChannelError> - where - F::Target: FeeEstimator, - { - if msg.amount_msat > funding.get_value_satoshis() * 1000 { - return Err(ChannelError::close("Remote side tried to send more than the total value of the channel".to_owned())); - } - - let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); - if htlc_stats.pending_inbound_htlcs + 1 > self.context.holder_max_accepted_htlcs as usize { - return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.context.holder_max_accepted_htlcs))); - } - if htlc_stats.pending_inbound_htlcs_value_msat + msg.amount_msat > self.context.holder_max_htlc_value_in_flight_msat { - return Err(ChannelError::close(format!("Remote HTLC add would put them over our max HTLC value ({})", self.context.holder_max_htlc_value_in_flight_msat))); - } - - // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet - // the reserve_satoshis we told them to always have as direct payment so that they lose - // something if we punish them for broadcasting an old state). - // Note that we don't really care about having a small/no to_remote output in our local - // commitment transactions, as the purpose of the channel reserve is to ensure we can - // punish *them* if they misbehave, so we discount any outbound HTLCs which will not be - // present in the next commitment transaction we send them (at least for fulfilled ones, - // failed ones won't modify value_to_self). - // Note that we will send HTLCs which another instance of rust-lightning would think - // violate the reserve value if we do not do this (as we forget inbound HTLCs from the - // Channel state once they will not be present in the next received commitment - // transaction). - let mut removed_outbound_total_msat = 0; - for ref htlc in self.context.pending_outbound_htlcs.iter() { - if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; - } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; - } - } - - let pending_value_to_self_msat = - funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; - let pending_remote_value_msat = - funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - if pending_remote_value_msat < msg.amount_msat { - return Err(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned())); - } - - // Check that the remote can afford to pay for this HTLC on-chain at the current - // feerate_per_kw, while maintaining their channel reserve (as required by the spec). - { - let remote_commit_tx_fee_msat = if funding.is_outbound() { 0 } else { - let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - self.context.next_remote_commit_tx_fee_msat(funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations - }; - let anchor_outputs_value_msat = if !funding.is_outbound() && funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; - if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat { - return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned())); - }; - if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 { - return Err(ChannelError::close("Remote HTLC add would put them under remote reserve value".to_owned())); - } - } - - let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; - if funding.is_outbound() { - // Check that they won't violate our local required channel reserve by adding this HTLC. - let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered); - let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(funding, htlc_candidate, None); - if funding.value_to_self_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat { - return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned())); - } - } - - Ok(()) - } - pub fn update_add_htlc( &mut self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator, ) -> Result<(), ChannelError> where F::Target: FeeEstimator { @@ -5866,7 +5998,7 @@ impl FundedChannel where core::iter::once(&self.funding) .chain(self.pending_funding.iter()) - .try_for_each(|funding| self.validate_update_add_htlc(funding, msg, fee_estimator))?; + .try_for_each(|funding| self.context.validate_update_add_htlc(funding, msg, fee_estimator))?; // Now update local state: self.context.next_counterparty_htlc_id += 1; @@ -6739,7 +6871,7 @@ impl FundedChannel where let can_send_update_fee = core::iter::once(&self.funding) .chain(self.pending_funding.iter()) - .all(|funding| self.can_send_update_fee(funding, feerate_per_kw, fee_estimator, logger)); + .all(|funding| self.context.can_send_update_fee(funding, &self.holder_commitment_point, feerate_per_kw, fee_estimator, logger)); if !can_send_update_fee { return None; } @@ -6765,43 +6897,6 @@ impl FundedChannel where }) } - fn can_send_update_fee( - &self, funding: &FundingScope, feerate_per_kw: u32, - fee_estimator: &LowerBoundedFeeEstimator, logger: &L, - ) -> bool - where - F::Target: FeeEstimator, - L::Target: Logger, - { - // Before proposing a feerate update, check that we can actually afford the new fee. - let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(funding, Some(feerate_per_kw), dust_exposure_limiting_feerate); - let commitment_data = self.context.build_commitment_transaction( - funding, self.holder_commitment_point.transaction_number(), - &self.holder_commitment_point.current_point(), true, true, logger, - ); - let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.tx.nondust_htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, funding.get_channel_type()) * 1000; - let holder_balance_msat = commitment_data.stats.local_balance_before_fee_anchors_msat - htlc_stats.outbound_holding_cell_msat; - if holder_balance_msat < buffer_fee_msat + commitment_data.stats.total_anchors_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 { - //TODO: auto-close after a number of failures? - log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw); - return false; - } - - // Note, we evaluate pending htlc "preemptive" trimmed-to-dust threshold at the proposed `feerate_per_kw`. - let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { - log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); - return false; - } - if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { - log_debug!(logger, "Cannot afford to send new feerate at {} without infringing max dust htlc exposure", feerate_per_kw); - return false; - } - - return true; - } - /// Removes any uncommitted inbound HTLCs and resets the state of uncommitted outbound HTLC /// updates, to be used on peer disconnection. After this, update_*_htlc messages need to be /// resent. @@ -7064,29 +7159,7 @@ impl FundedChannel where core::iter::once(&self.funding) .chain(self.pending_funding.iter()) - .try_for_each(|funding| self.validate_update_fee(funding, fee_estimator, msg)) - } - - fn validate_update_fee( - &self, funding: &FundingScope, fee_estimator: &LowerBoundedFeeEstimator, - msg: &msgs::UpdateFee, - ) -> Result<(), ChannelError> - where - F::Target: FeeEstimator, - { - // Check that we won't be pushed over our dust exposure limit by the feerate increase. - let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator); - let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); - let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - if htlc_stats.on_holder_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { - return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)", - msg.feerate_per_kw, htlc_stats.on_holder_tx_dust_exposure_msat))); - } - if htlc_stats.on_counterparty_tx_dust_exposure_msat > max_dust_htlc_exposure_msat { - return Err(ChannelError::close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)", - msg.feerate_per_kw, htlc_stats.on_counterparty_tx_dust_exposure_msat))); - } - Ok(()) + .try_for_each(|funding| self.context.validate_update_fee(funding, fee_estimator, msg)) } /// Indicates that the signer may have some signatures for us, so we should retry if we're @@ -8137,80 +8210,7 @@ impl FundedChannel where core::iter::once(&self.funding) .chain(self.pending_funding.iter()) - .try_for_each(|funding| self.can_accept_incoming_htlc_for_funding(funding, msg, dust_exposure_limiting_feerate, &logger)) - } - - fn can_accept_incoming_htlc_for_funding( - &self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC, - dust_exposure_limiting_feerate: u32, logger: &L, - ) -> Result<(), LocalHTLCFailureReason> - where - L::Target: Logger, - { - let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate); - let max_dust_htlc_exposure_msat = self.context.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate); - 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(LocalHTLCFailureReason::DustLimitCounterparty) - } - let htlc_success_dust_limit = if funding.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_success_tx_weight(funding.get_channel_type()) / 1000 - }; - 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; - if on_holder_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 holder commitment tx", - on_holder_tx_dust_htlc_exposure_msat, max_dust_htlc_exposure_msat); - return Err(LocalHTLCFailureReason::DustLimitHolder) - } - } - - let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000 - } else { - 0 - }; - - let mut removed_outbound_total_msat = 0; - for ref htlc in self.context.pending_outbound_htlcs.iter() { - if let OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; - } else if let OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_)) = htlc.state { - removed_outbound_total_msat += htlc.amount_msat; - } - } - - let pending_value_to_self_msat = - funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat; - let pending_remote_value_msat = - funding.get_value_satoshis() * 1000 - pending_value_to_self_msat; - - if !funding.is_outbound() { - // `Some(())` is for the fee spike buffer we keep for the remote. This deviates from - // the spec because the fee spike buffer requirement doesn't exist on the receiver's - // side, only on the sender's. Note that with anchor outputs we are no longer as - // sensitive to fee spikes, so we need to account for them. - // - // A `None` `HTLCCandidate` is used as in this case because we're already accounting for - // the incoming HTLC as it has been fully committed by both sides. - let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(funding, None, Some(())); - if !funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() { - remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE; - } - if pending_remote_value_msat.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000).saturating_sub(anchor_outputs_value_msat) < remote_fee_cost_incl_stuck_buffer_msat { - log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", &self.context.channel_id()); - return Err(LocalHTLCFailureReason::FeeSpikeBuffer); - } - } - - Ok(()) + .try_for_each(|funding| self.context.can_accept_incoming_htlc(funding, msg, dust_exposure_limiting_feerate, &logger)) } pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 {