validate_update_add_htlc

Author: tankyleo

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::{SpecTxBuilder, TxBuilder, HTLCAmountDirection};
 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,39 @@ where
 		);
 	}
 
+	fn pending_inbound_htlcs_value_msat(&self) -> u64 {
+		self.pending_inbound_htlcs.iter().map(|htlc| htlc.amount_msat).sum()
+	}
+
+	fn next_commitment_htlcs(&self, htlc_candidate: Option<HTLCAmountDirection>) -> 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());
+		let pending_inbound_htlcs = self.pending_inbound_htlcs.iter().map(|InboundHTLCOutput { amount_msat, .. }| HTLCAmountDirection { outbound: false, amount_msat: *amount_msat });
+		use OutboundHTLCState::*;
+		let pending_outbound_htlcs = self.pending_outbound_htlcs.iter().filter_map(|OutboundHTLCOutput { ref state, amount_msat, .. }|
+			matches!(state, LocalAnnounced { .. } | Committed | RemoteRemoved { .. }).then_some(HTLCAmountDirection { outbound: true, amount_msat: *amount_msat }));
+
+		// We do not include holding cell HTLCs, we will validate them upon freeing the holding cell...
+		//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: *amount_msat }) } else { None });
+
+		commitment_htlcs.extend(htlc_candidate.into_iter().chain(pending_inbound_htlcs).chain(pending_outbound_htlcs));
+		commitment_htlcs
+	}
+
+	fn get_next_commitment_value_to_self_msat(&self, funding: &FundingScope) -> u64 {
+		let outbound_removed_htlc_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();
+		funding.value_to_self_msat.saturating_sub(outbound_removed_htlc_msat)
+	}
+
 	#[rustfmt::skip]
 	fn validate_update_add_htlc<F: Deref>(
 		&self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC,
@@ -4119,70 +4152,37 @@ 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 {
+
+		if self.pending_inbound_htlcs.len() + 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 {
+		if self.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 (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;
+		let next_commitment_htlcs = self.next_commitment_htlcs(Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }));
+		let value_to_self_msat = self.get_next_commitment_value_to_self_msat(funding);
+		let next_commitment_stats = SpecTxBuilder {}.get_builder_stats(funding.is_outbound(), funding.get_value_satoshis(), value_to_self_msat, &next_commitment_htlcs, 0, self.feerate_per_kw, dust_exposure_limiting_feerate, funding.get_channel_type(), self.holder_dust_limit_satoshis, self.counterparty_dust_limit_satoshis);
 
-			// 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()));
-		}
+		let remote_balance_before_fee_msat = next_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).
 		{
 			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_commitment_stats.counterparty_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()));
 			}
 		}
 
 		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_commitment_stats.holder_balance_msat.unwrap() < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + next_commitment_stats.holder_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 +4795,6 @@ where
 		});
 
 		HTLCStats {
-			pending_inbound_htlcs: self.pending_inbound_htlcs.len(),
 			pending_outbound_htlcs,
 			pending_inbound_htlcs_value_msat,
 			pending_outbound_htlcs_value_msat,