[Custom Transactions] Add TxBuilder::get_next_commitment_stats

lightning/src/ln/chan_utils.rs

@@ -236,7 +236,7 @@ pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_t
 }
 
 /// Returns the fees for success and timeout second stage HTLC transactions.
-pub(super) fn second_stage_tx_fees_sat(
+pub(crate) fn second_stage_tx_fees_sat(
 	channel_type: &ChannelTypeFeatures, feerate_sat_per_1000_weight: u32,
 ) -> (u64, u64) {
 	if channel_type.supports_anchors_zero_fee_htlc_tx()

lightning/src/ln/channel.rs

@@ -70,7 +70,7 @@ use crate::ln::script::{self, ShutdownScript};
 use crate::ln::types::ChannelId;
 use crate::routing::gossip::NodeId;
 use crate::sign::ecdsa::EcdsaChannelSigner;
-use crate::sign::tx_builder::{SpecTxBuilder, TxBuilder};
+use crate::sign::tx_builder::{HTLCAmountDirection, NextCommitmentStats, SpecTxBuilder, TxBuilder};
 use crate::sign::{ChannelSigner, EntropySource, NodeSigner, Recipient, SignerProvider};
 use crate::types::features::{ChannelTypeFeatures, InitFeatures};
 use crate::types::payment::{PaymentHash, PaymentPreimage};
@@ -1097,12 +1097,12 @@ pub enum AnnouncementSigsState {
 /// An enum indicating whether the local or remote side offered a given HTLC.
 enum HTLCInitiator {
 	LocalOffered,
+	#[allow(dead_code)]
 	RemoteOffered,
 }
 
 /// Current counts of various HTLCs, useful for calculating current balances available exactly.
 struct HTLCStats {
-	pending_inbound_htlcs: usize,
 	pending_outbound_htlcs: usize,
 	pending_inbound_htlcs_value_msat: u64,
 	pending_outbound_htlcs_value_msat: u64,
@@ -4104,6 +4104,163 @@ where
 		);
 	}
 
+	/// Returns a best-effort guess of the set of HTLCs that will be present
+	/// on the next local or remote commitment. We cannot be certain as the
+	/// actual set of HTLCs present on the next commitment depends on the
+	/// ordering of commitment_signed and revoke_and_ack messages.
+	///
+	/// We take the conservative approach and only assume that a HTLC will
+	/// not be in the next commitment when it is guaranteed that it won't be.
+	#[rustfmt::skip]
+	fn get_next_commitment_htlcs(
+		&self, local: bool, htlc_candidate: Option<HTLCAmountDirection>, include_counterparty_unknown_htlcs: bool,
+	) -> Vec<HTLCAmountDirection> {
+		let mut commitment_htlcs = Vec::with_capacity(
+			1 + self.pending_inbound_htlcs.len()
+				+ self.pending_outbound_htlcs.len()
+				+ self.holding_cell_htlc_updates.len(),
+		);
+		// `LocalRemoved` HTLCs will certainly not be present on any future remote
+		// commitments, but they could be in a future local commitment as the remote has
+		// not yet acknowledged the removal.
+		let pending_inbound_htlcs = self
+			.pending_inbound_htlcs
+			.iter()
+			.filter(|InboundHTLCOutput { state, .. }| match (state, local) {
+				(InboundHTLCState::RemoteAnnounced(..), _) => true,
+				(InboundHTLCState::AwaitingRemoteRevokeToAnnounce(..), _) => true,
+				(InboundHTLCState::AwaitingAnnouncedRemoteRevoke(..), _) => true,
+				(InboundHTLCState::Committed, _) => true,
+				(InboundHTLCState::LocalRemoved(..), true) => true,
+				(InboundHTLCState::LocalRemoved(..), false) => false,
+			})
+			.map(|&InboundHTLCOutput { amount_msat, .. }| HTLCAmountDirection { outbound: false, amount_msat });
+		// `RemoteRemoved` HTLCs can still be present on the next remote commitment if
+		// local produces a commitment before acknowledging the update. These HTLCs
+		// will for sure not be present on the next local commitment.
+		let pending_outbound_htlcs = self
+			.pending_outbound_htlcs
+			.iter()
+			.filter(|OutboundHTLCOutput { state, .. }| match (state, local) {
+				(OutboundHTLCState::LocalAnnounced(..), _) => include_counterparty_unknown_htlcs,
+				(OutboundHTLCState::Committed, _) => true,
+				(OutboundHTLCState::RemoteRemoved(..), true) => false,
+				(OutboundHTLCState::RemoteRemoved(..), false) => true,
+				(OutboundHTLCState::AwaitingRemoteRevokeToRemove(..), _) => false,
+				(OutboundHTLCState::AwaitingRemovedRemoteRevoke(..), _) => false,
+			})
+			.map(|&OutboundHTLCOutput { amount_msat, .. }| HTLCAmountDirection { outbound: true, amount_msat });
+
+		let holding_cell_htlcs = self.holding_cell_htlc_updates.iter().filter_map(|htlc| {
+			if let &HTLCUpdateAwaitingACK::AddHTLC { amount_msat, .. } = htlc {
+				Some(HTLCAmountDirection { outbound: true, amount_msat })
+			} else {
+				None
+			}
+		});
+
+		if include_counterparty_unknown_htlcs {
+			commitment_htlcs.extend(
+				htlc_candidate.into_iter().chain(pending_inbound_htlcs).chain(pending_outbound_htlcs).chain(holding_cell_htlcs)
+			);
+		} else {
+			commitment_htlcs.extend(
+				htlc_candidate.into_iter().chain(pending_inbound_htlcs).chain(pending_outbound_htlcs)
+			);
+		}
+
+		commitment_htlcs
+	}
+
+	/// This returns the value of `value_to_self_msat` after accounting for all the
+	/// successful inbound and outbound HTLCs that won't be present on the next
+	/// commitment.
+	///
+	/// To determine which HTLC claims to account for, we take the cases where a HTLC
+	/// will *not* be present on the next commitment from `next_commitment_htlcs`, and
+	/// check if their outcome is successful. If it is, we add the value of this claimed
+	/// HTLC to the balance of the claimer.
+	#[rustfmt::skip]
+	fn get_next_commitment_value_to_self_msat(&self, local: bool, funding: &FundingScope) -> u64 {
+		let inbound_claimed_htlc_msat: u64 =
+			self.pending_inbound_htlcs
+				.iter()
+				.filter(|InboundHTLCOutput { state, .. }| match (state, local) {
+					(InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_, _)), true) => false,
+					(InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_, _)), false) => true,
+					_ => false,
+				})
+				.map(|InboundHTLCOutput { amount_msat, .. }| amount_msat)
+				.sum();
+		let outbound_claimed_htlc_msat: u64 =
+			self.pending_outbound_htlcs
+				.iter()
+				.filter(|OutboundHTLCOutput { state, .. }| match (state, local) {
+					(OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_, _)), true) => true,
+					(OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(_, _)), false) => false,
+					(OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)), _) => true,
+					(OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)), _) => true,
+					_ => false,
+				})
+				.map(|OutboundHTLCOutput { amount_msat, .. }| amount_msat)
+				.sum();
+
+		funding
+			.value_to_self_msat
+			.saturating_sub(outbound_claimed_htlc_msat)
+			.saturating_add(inbound_claimed_htlc_msat)
+	}
+
+	fn get_next_local_commitment_stats(
+		&self, funding: &FundingScope, htlc_candidate: Option<HTLCAmountDirection>,
+		include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize,
+		feerate_per_kw: u32, dust_exposure_limiting_feerate: Option<u32>,
+	) -> NextCommitmentStats {
+		let next_commitment_htlcs = self.get_next_commitment_htlcs(
+			true,
+			htlc_candidate,
+			include_counterparty_unknown_htlcs,
+		);
+		let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(true, funding);
+		SpecTxBuilder {}.get_next_commitment_stats(
+			true,
+			funding.is_outbound(),
+			funding.get_value_satoshis(),
+			next_value_to_self_msat,
+			&next_commitment_htlcs,
+			addl_nondust_htlc_count,
+			feerate_per_kw,
+			dust_exposure_limiting_feerate,
+			self.holder_dust_limit_satoshis,
+			funding.get_channel_type(),
+		)
+	}
+
+	fn get_next_remote_commitment_stats(
+		&self, funding: &FundingScope, htlc_candidate: Option<HTLCAmountDirection>,
+		include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize,
+		feerate_per_kw: u32, dust_exposure_limiting_feerate: Option<u32>,
+	) -> NextCommitmentStats {
+		let next_commitment_htlcs = self.get_next_commitment_htlcs(
+			false,
+			htlc_candidate,
+			include_counterparty_unknown_htlcs,
+		);
+		let next_value_to_self_msat = self.get_next_commitment_value_to_self_msat(false, funding);
+		SpecTxBuilder {}.get_next_commitment_stats(
+			false,
+			funding.is_outbound(),
+			funding.get_value_satoshis(),
+			next_value_to_self_msat,
+			&next_commitment_htlcs,
+			addl_nondust_htlc_count,
+			feerate_per_kw,
+			dust_exposure_limiting_feerate,
+			self.counterparty_dust_limit_satoshis,
+			funding.get_channel_type(),
+		)
+	}
+
 	#[rustfmt::skip]
 	fn validate_update_add_htlc<F: Deref>(
 		&self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC,
@@ -4119,15 +4276,25 @@ where
 		let dust_exposure_limiting_feerate = self.get_dust_exposure_limiting_feerate(
 			&fee_estimator, funding.get_channel_type(),
 		);
-		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 {
+		// Don't include outbound update_add_htlc's in the holding cell, or those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs)
+		let include_counterparty_unknown_htlcs = false;
+		// Don't include the extra fee spike buffer HTLC in calculations
+		let fee_spike_buffer_htlc = 0;
+		let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }), include_counterparty_unknown_htlcs, fee_spike_buffer_htlc, self.feerate_per_kw, dust_exposure_limiting_feerate);
+
+		if next_remote_commitment_stats.inbound_htlcs_count > 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 {
+		if next_remote_commitment_stats.inbound_htlcs_value_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
+		let remote_balance_before_fee_msat = next_remote_commitment_stats.counterparty_balance_msat.ok_or(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).
+		//
+		// We 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
@@ -4139,50 +4306,22 @@ where
 		// 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 (local_balance_before_fee_msat, remote_balance_before_fee_msat) = {
-			let removed_outbound_total_msat: u64 = self.pending_outbound_htlcs
-				.iter()
-				.filter_map(|htlc| {
-					matches!(
-						htlc.state,
-						OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _))
-						| OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _))
-					)
-					.then_some(htlc.amount_msat)
-				})
-				.sum();
-			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;
-
-			// Subtract any non-HTLC outputs from the local and remote balances
-			SpecTxBuilder {}.subtract_non_htlc_outputs(funding.is_outbound(), funding.value_to_self_msat, pending_remote_value_msat, funding.get_channel_type())
-		};
-		if remote_balance_before_fee_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
+				next_remote_commitment_stats.commit_tx_fee_sat * 1000
 			};
-			if remote_balance_before_fee_msat.saturating_sub(msg.amount_msat) < remote_commit_tx_fee_msat {
+			if remote_balance_before_fee_msat < remote_commit_tx_fee_msat {
 				return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
 			};
-			if remote_balance_before_fee_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 {
+			if remote_balance_before_fee_msat.saturating_sub(remote_commit_tx_fee_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 next_local_commitment_stats = self.get_next_local_commitment_stats(funding, Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }), include_counterparty_unknown_htlcs, 0, self.feerate_per_kw, dust_exposure_limiting_feerate);
 		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 local_balance_before_fee_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat {
+			if next_local_commitment_stats.holder_balance_msat.unwrap_or(0) < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + next_local_commitment_stats.commit_tx_fee_sat * 1000 {
 				return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
 			}
 		}
@@ -4795,7 +4934,6 @@ where
 		});
 
 		HTLCStats {
-			pending_inbound_htlcs: self.pending_inbound_htlcs.len(),
 			pending_outbound_htlcs,
 			pending_inbound_htlcs_value_msat,
 			pending_outbound_htlcs_value_msat,