Define a method for determining the max HTLCs rather than a const

Once we support zero-fee commitment transactions, we will no longer
have a constant number of maximum HTLCs in-flight per channel but
rather it will depend on the channel type.

Here we prepare for this by removing the `MAX_HTLCS` constant and
replacing it with a function.

Author: TheBlueMatt

lightning/src/ln/chan_utils.rs

@@ -52,8 +52,14 @@ use super::channel_keys::{DelayedPaymentBasepoint, DelayedPaymentKey, HtlcKey, H
 #[allow(unused_imports)]
 use crate::prelude::*;
 
-/// Maximum number of one-way in-flight HTLC (protocol-level value).
-pub const MAX_HTLCS: u16 = 483;
+/// Maximum number of in-flight HTLCs in each direction allowed by the lightning protocol.
+///
+/// 483 for non-zero-fee-commitment channels and 114 for zero-fee-commitment channels.
+///
+/// Actual maximums can be set equal to or below this value by each channel participant.
+pub fn max_htlcs(_channel_type: &ChannelTypeFeatures) -> u16 {
+	483
+}
 /// The weight of a BIP141 witnessScript for a BOLT3's "offered HTLC output" on a commitment transaction, non-anchor variant.
 pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133;
 /// The weight of a BIP141 witnessScript for a BOLT3's "offered HTLC output" on a commitment transaction, anchor variant.

lightning/src/ln/channel.rs

@@ -43,7 +43,7 @@ use crate::ln::chan_utils::{
 	CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight,
 	htlc_timeout_tx_weight, ChannelPublicKeys, CommitmentTransaction,
 	HolderCommitmentTransaction, ChannelTransactionParameters,
-	CounterpartyChannelTransactionParameters, MAX_HTLCS,
+	CounterpartyChannelTransactionParameters, max_htlcs,
 	get_commitment_transaction_number_obscure_factor,
 	ClosingTransaction, commit_tx_fee_sat,
 };
@@ -2457,8 +2457,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		if open_channel_fields.max_accepted_htlcs < 1 {
 			return Err(ChannelError::close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
 		}
-		if open_channel_fields.max_accepted_htlcs > MAX_HTLCS {
-			return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", open_channel_fields.max_accepted_htlcs, MAX_HTLCS)));
+		if open_channel_fields.max_accepted_htlcs > max_htlcs(&channel_type) {
+			return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", open_channel_fields.max_accepted_htlcs, max_htlcs(&channel_type))));
 		}
 
 		// Now check against optional parameters as set by config...
@@ -2686,7 +2686,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			counterparty_htlc_minimum_msat: open_channel_fields.htlc_minimum_msat,
 			holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat },
 			counterparty_max_accepted_htlcs: open_channel_fields.max_accepted_htlcs,
-			holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS),
+			holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)),
 			minimum_depth,
 
 			counterparty_forwarding_info: None,
@@ -2923,7 +2923,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			counterparty_htlc_minimum_msat: 0,
 			holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat },
 			counterparty_max_accepted_htlcs: 0,
-			holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS),
+			holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)),
 			minimum_depth: None, // Filled in in accept_channel
 
 			counterparty_forwarding_info: None,
@@ -3183,6 +3183,22 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		if !matches!(self.channel_state, ChannelState::NegotiatingFunding(flags) if flags == NegotiatingFundingFlags::OUR_INIT_SENT) {
 			return Err(ChannelError::close("Got an accept_channel message at a strange time".to_owned()));
 		}
+
+		if let Some(ty) = &common_fields.channel_type {
+			if *ty != self.channel_type {
+				return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned()));
+			}
+		} else if their_features.supports_channel_type() {
+			// Assume they've accepted the channel type as they said they understand it.
+		} else {
+			let channel_type = ChannelTypeFeatures::from_init(&their_features);
+			if channel_type != ChannelTypeFeatures::only_static_remote_key() {
+				return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
+			}
+			self.channel_type = channel_type.clone();
+			funding.channel_transaction_parameters.channel_type_features = channel_type;
+		}
+
 		if common_fields.dust_limit_satoshis > 21000000 * 100000000 {
 			return Err(ChannelError::close(format!("Peer never wants payout outputs? dust_limit_satoshis was {}", common_fields.dust_limit_satoshis)));
 		}
@@ -3207,8 +3223,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		if common_fields.max_accepted_htlcs < 1 {
 			return Err(ChannelError::close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
 		}
-		if common_fields.max_accepted_htlcs > MAX_HTLCS {
-			return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", common_fields.max_accepted_htlcs, MAX_HTLCS)));
+
+		let channel_type = &funding.channel_transaction_parameters.channel_type_features;
+		if common_fields.max_accepted_htlcs > max_htlcs(channel_type) {
+			return Err(ChannelError::close(format!("max_accepted_htlcs was {}. It must not be larger than {}", common_fields.max_accepted_htlcs, max_htlcs(channel_type))));
 		}
 
 		// Now check against optional parameters as set by config...
@@ -3234,21 +3252,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			return Err(ChannelError::close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", peer_limits.max_minimum_depth, common_fields.minimum_depth)));
 		}
 
-		if let Some(ty) = &common_fields.channel_type {
-			if *ty != self.channel_type {
-				return Err(ChannelError::close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned()));
-			}
-		} else if their_features.supports_channel_type() {
-			// Assume they've accepted the channel type as they said they understand it.
-		} else {
-			let channel_type = ChannelTypeFeatures::from_init(&their_features);
-			if channel_type != ChannelTypeFeatures::only_static_remote_key() {
-				return Err(ChannelError::close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
-			}
-			self.channel_type = channel_type.clone();
-			funding.channel_transaction_parameters.channel_type_features = channel_type;
-		}
-
 		let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
 			match &common_fields.shutdown_scriptpubkey {
 				&Some(ref script) => {

lightning/src/ln/functional_tests.rs

@@ -10609,8 +10609,9 @@ pub fn test_nondust_htlc_excess_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 = chan_utils::MAX_HTLCS;
-	config.channel_handshake_config.our_max_accepted_htlcs = chan_utils::MAX_HTLCS;
+	let chan_ty = ChannelTypeFeatures::only_static_remote_key();
+	config.channel_handshake_limits.min_max_accepted_htlcs = chan_utils::max_htlcs(&chan_ty);
+	config.channel_handshake_config.our_max_accepted_htlcs = chan_utils::max_htlcs(&chan_ty);
 	config.channel_handshake_config.our_htlc_minimum_msat = 1;
 	config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = 100;
 
@@ -10651,7 +10652,7 @@ pub fn test_nondust_htlc_excess_fees_are_dust() {
 	let commitment_tx_per_htlc_cost =
 		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 < chan_utils::MAX_HTLCS.into(),
+	assert!(max_htlcs_remaining < chan_utils::max_htlcs(&chan_ty).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);

lightning/src/util/anchor_channel_reserves.rs

@@ -25,10 +25,11 @@ use crate::chain::chainmonitor::ChainMonitor;
 use crate::chain::chainmonitor::Persist;
 use crate::chain::Filter;
 use crate::events::bump_transaction::Utxo;
-use crate::ln::chan_utils::MAX_HTLCS;
+use crate::ln::chan_utils::max_htlcs;
 use crate::ln::channelmanager::AChannelManager;
 use crate::prelude::new_hash_set;
 use crate::sign::ecdsa::EcdsaChannelSigner;
+use crate::types::features::ChannelTypeFeatures;
 use crate::util::logger::Logger;
 use bitcoin::constants::WITNESS_SCALE_FACTOR;
 use bitcoin::Amount;
@@ -178,7 +179,8 @@ impl Default for AnchorChannelReserveContext {
 fn get_reserve_per_channel_with_input(
 	context: &AnchorChannelReserveContext, initial_input_weight: Weight,
 ) -> Amount {
-	let expected_accepted_htlcs = min(context.expected_accepted_htlcs, MAX_HTLCS) as u64;
+	let max_max_htlcs = max_htlcs(&ChannelTypeFeatures::only_static_remote_key());
+	let expected_accepted_htlcs = min(context.expected_accepted_htlcs, max_max_htlcs) as u64;
 	let weight = Weight::from_wu(
 		COMMITMENT_TRANSACTION_BASE_WEIGHT +
 		// Reserves are calculated in terms of accepted HTLCs, as their timeout defines the urgency of

lightning/src/util/config.rs

@@ -193,8 +193,10 @@ pub struct ChannelHandshakeConfig {
 	///
 	/// Default value: `50`
 	///
-	/// Maximum value: `483` (Any values larger will be treated as `483`. This is the BOLT #2 spec
-	/// limit on `max_accepted_htlcs`.)
+	/// Maximum value: depends on channel type, see docs on [`max_htlcs`] (any values over the
+	///                maximum will be silently reduced to the maximum).
+	///
+	/// [`max_htlcs`]: crate::ln::chan_utils::max_htlcs
 	pub our_max_accepted_htlcs: u16,
 }