Improve prediction of commitment stats in can_send_send_update_fee

`ChannelContext::get_pending_htlc_stats` predicts that the set of HTLCs
on the next commitment will be all the HTLCs in
`ChannelContext.pending_inbound_htlcs`, and
`ChannelContext.pending_outbound_htlcs`, as well as all the outbound
HTLC adds in the holding cell.

This is an overestimate:

* Outbound HTLC removals which have been ACK'ed by the counterparty will
  certainly not be present in any *next* commitment, even though they
  remain in `pending_outbound_htlcs` (I refer to states
  `AwaitingRemoteRevokeToRemove` and `AwaitingRemovedRemoteRevoke`).

* Outbound HTLCs in the `RemoteRemoved` state, will not be present in
  the next *local* commitment.

* Inbound HTLCs in the `LocalRemoved` state will not be present in the
  next *remote* commitment.

`ChannelContext::build_commitment_stats(funding, true, true, ..)` makes
these errors when predicting the HTLC count on the remote commitment:

* Inbound HTLCs in the state `RemoteAnnounced` are not included, but
  they will be in the next remote commitment transaction if the local
  ACK's the addition before producing the next remote commitment.

* Inbound HTLCs in the state `AwaitingRemoteRevokeToAnnounce` are not
  included, even though the local has ACK'ed the addition.

* Outbound HTLCs in the state `AwaitingRemoteRevokeToRemove` are
  counted, even though the local party has ACK'ed the removal.

This commit replaces these functions in favor of the newly added
`ChannelContext::get_next_{local, remote}_commitment_stats` methods,
and fixes the issues described above.

This commit also includes outbound HTLC additions in the holding cell in
the fees on the next remote commitment transaction; they were
previously not included.

Author: tankyleo

lightning/src/ln/channel.rs

@@ -1112,8 +1112,6 @@ struct HTLCStats {
 	// htlc on the counterparty's commitment transaction.
 	extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat: Option<u64>,
 	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
 }
 
 /// A struct gathering data on a commitment, either local or remote.
@@ -4463,23 +4461,24 @@ 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, Some(feerate_per_kw), dust_exposure_limiting_feerate);
-		let stats = self.build_commitment_stats(funding, true, true, Some(feerate_per_kw), Some(htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize));
-		let holder_balance_msat = stats.local_balance_before_fee_msat - htlc_stats.outbound_holding_cell_msat;
+		// Include outbound update_add_htlc's in the holding cell, and those which haven't yet been ACK'ed by the counterparty (ie. LocalAnnounced HTLCs)
+		let include_counterparty_unknown_htlcs = true;
+		let next_remote_commitment_stats = self.get_next_remote_commitment_stats(funding, None, include_counterparty_unknown_htlcs, CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, feerate_per_kw, dust_exposure_limiting_feerate);
 		// Note that `stats.commit_tx_fee_sat` accounts for any HTLCs that transition from non-dust to dust under a higher feerate (in the case where HTLC-transactions pay endogenous fees).
-		if holder_balance_msat < stats.commit_tx_fee_sat * 1000 + funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
+		if next_remote_commitment_stats.holder_balance_msat.unwrap_or(0) < next_remote_commitment_stats.commit_tx_fee_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 {
+		if next_remote_commitment_stats.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 {
+
+		if next_remote_commitment_stats.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;
 		}
@@ -4874,8 +4873,6 @@ where
 		}
 
 		let mut pending_outbound_htlcs_value_msat = 0;
-		let mut outbound_holding_cell_msat = 0;
-		let mut on_holder_tx_outbound_holding_cell_htlcs_count = 0;
 		let mut pending_outbound_htlcs = self.pending_outbound_htlcs.len();
 		{
 			let counterparty_dust_limit_success_sat = htlc_success_tx_fee_sat + context.counterparty_dust_limit_satoshis;
@@ -4896,16 +4893,13 @@ where
 				if let &HTLCUpdateAwaitingACK::AddHTLC { ref amount_msat, .. } = update {
 					pending_outbound_htlcs += 1;
 					pending_outbound_htlcs_value_msat += amount_msat;
-					outbound_holding_cell_msat += amount_msat;
 					if *amount_msat / 1000 < counterparty_dust_limit_success_sat {
 						on_counterparty_tx_dust_exposure_msat += amount_msat;
 					} else {
 						on_counterparty_tx_accepted_nondust_htlcs += 1;
 					}
 					if *amount_msat / 1000 < holder_dust_limit_timeout_sat {
 						on_holder_tx_dust_exposure_msat += amount_msat;
-					} else {
-						on_holder_tx_outbound_holding_cell_htlcs_count += 1;
 					}
 				}
 			}
@@ -4943,8 +4937,6 @@ where
 			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,
 		}
 	}