Consider funding scopes in get_available_balances

A FundedChannel may have more than one pending FundingScope during
splicing, one for the splice attempt and one or more for any RBF
attempts. When calling get_available_balances, consider all funding
scopes and take the minimum by next_outbound_htlc_limit_msat. This is
used both informationally and to determine which channel to use to
forward an HTLC.

The choice of next_outbound_htlc_limit_msat is somewhat arbitrary but
matches the field used when determining which channel used to forward an
HTLC. Any field should do since each field should be adjusted by the
same amount relative to another FundingScope given the nature of the
fields (i.e., inbound/outbound capacity, min/max HTLC limit).

Using the minimum was chosen since an order for an HTLC to be sent over
the channel, it must be possible for each funding scope -- both the
confirmed one and any pending scopes, one of which may eventually
confirm.

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1550,6 +1550,25 @@ impl<SP: Deref> Channel<SP> where
 			}
 		}
 	}
+
+	/// Get the available balances, see [`AvailableBalances`]'s fields for more info.
+	/// Doesn't bother handling the
+	/// if-we-removed-it-already-but-haven't-fully-resolved-they-can-still-send-an-inbound-HTLC
+	/// corner case properly.
+	pub fn get_available_balances<F: Deref>(
+		&self, fee_estimator: &LowerBoundedFeeEstimator<F>,
+	) -> AvailableBalances
+	where
+		F::Target: FeeEstimator,
+	{
+		match &self.phase {
+			ChannelPhase::Undefined => unreachable!(),
+			ChannelPhase::Funded(chan) => chan.get_available_balances(fee_estimator),
+			ChannelPhase::UnfundedOutboundV1(chan) => chan.context.get_available_balances_for_scope(&chan.funding, fee_estimator),
+			ChannelPhase::UnfundedInboundV1(chan) => chan.context.get_available_balances_for_scope(&chan.funding, fee_estimator),
+			ChannelPhase::UnfundedV2(chan) => chan.context.get_available_balances_for_scope(&chan.funding, fee_estimator),
+		}
+	}
 }
 
 impl<SP: Deref> From<OutboundV1Channel<SP>> for Channel<SP>
@@ -4123,11 +4142,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		outbound_details
 	}
 
-	/// Get the available balances, see [`AvailableBalances`]'s fields for more info.
-	/// Doesn't bother handling the
-	/// if-we-removed-it-already-but-haven't-fully-resolved-they-can-still-send-an-inbound-HTLC
-	/// corner case properly.
-	pub fn get_available_balances<F: Deref>(
+	fn get_available_balances_for_scope<F: Deref>(
 		&self, funding: &FundingScope, fee_estimator: &LowerBoundedFeeEstimator<F>,
 	) -> AvailableBalances
 	where
@@ -8707,7 +8722,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			return Err(ChannelError::Ignore("Cannot send 0-msat HTLC".to_owned()));
 		}
 
-		let available_balances = self.context.get_available_balances(&self.funding, fee_estimator);
+		let available_balances = self.get_available_balances(fee_estimator);
 		if amount_msat < available_balances.next_outbound_htlc_minimum_msat {
 			return Err(ChannelError::Ignore(format!("Cannot send less than our next-HTLC minimum - {} msat",
 				available_balances.next_outbound_htlc_minimum_msat)));
@@ -8779,6 +8794,26 @@ impl<SP: Deref> FundedChannel<SP> where
 		Ok(Some(res))
 	}
 
+	pub(super) fn get_available_balances<F: Deref>(
+		&self, fee_estimator: &LowerBoundedFeeEstimator<F>,
+	) -> AvailableBalances
+	where
+		F::Target: FeeEstimator,
+	{
+		core::iter::once(&self.funding)
+			.chain(self.pending_funding.iter())
+			.map(|funding| self.context.get_available_balances_for_scope(funding, fee_estimator))
+			.reduce(|acc, e| {
+				AvailableBalances {
+					inbound_capacity_msat: acc.inbound_capacity_msat.min(e.inbound_capacity_msat),
+					outbound_capacity_msat: acc.outbound_capacity_msat.min(e.outbound_capacity_msat),
+					next_outbound_htlc_limit_msat: acc.next_outbound_htlc_limit_msat.min(e.next_outbound_htlc_limit_msat),
+					next_outbound_htlc_minimum_msat: acc.next_outbound_htlc_minimum_msat.max(e.next_outbound_htlc_minimum_msat),
+				}
+			})
+			.expect("At least one FundingScope is always provided")
+	}
+
 	fn build_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> ChannelMonitorUpdate where L::Target: Logger {
 		log_trace!(logger, "Updating HTLC state for a newly-sent commitment_signed...");
 		// We can upgrade the status of some HTLCs that are waiting on a commitment, even if we

lightning/src/ln/channel_state.rs

@@ -15,7 +15,7 @@ use bitcoin::secp256k1::PublicKey;
 
 use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator};
 use crate::chain::transaction::OutPoint;
-use crate::ln::channel::{ChannelContext, FundingScope};
+use crate::ln::channel::Channel;
 use crate::ln::types::ChannelId;
 use crate::sign::SignerProvider;
 use crate::types::features::{ChannelTypeFeatures, InitFeatures};
@@ -475,15 +475,17 @@ impl ChannelDetails {
 		self.short_channel_id.or(self.outbound_scid_alias)
 	}
 
-	pub(super) fn from_channel_context<SP: Deref, F: Deref>(
-		context: &ChannelContext<SP>, funding: &FundingScope, best_block_height: u32,
-		latest_features: InitFeatures, fee_estimator: &LowerBoundedFeeEstimator<F>,
+	pub(super) fn from_channel<SP: Deref, F: Deref>(
+		channel: &Channel<SP>, best_block_height: u32, latest_features: InitFeatures,
+		fee_estimator: &LowerBoundedFeeEstimator<F>,
 	) -> Self
 	where
 		SP::Target: SignerProvider,
 		F::Target: FeeEstimator,
 	{
-		let balance = context.get_available_balances(funding, fee_estimator);
+		let context = channel.context();
+		let funding = channel.funding();
+		let balance = channel.get_available_balances(fee_estimator);
 		let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
 			funding.get_holder_counterparty_selected_channel_reserve_satoshis();
 		#[allow(deprecated)] // TODO: Remove once balance_msat is removed.

lightning/src/ln/channelmanager.rs

@@ -3726,7 +3726,7 @@ where
 		Ok(temporary_channel_id)
 	}
 
-	fn list_funded_channels_with_filter<Fn: FnMut(&(&ChannelId, &FundedChannel<SP>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
+	fn list_funded_channels_with_filter<Fn: FnMut(&(&ChannelId, &Channel<SP>)) -> bool + Copy>(&self, f: Fn) -> Vec<ChannelDetails> {
 		// Allocate our best estimate of the number of channels we have in the `res`
 		// Vec. Sadly the `short_to_chan_info` map doesn't cover channels without
 		// a scid or a scid alias. Therefore reallocations may still occur, but is
@@ -3741,11 +3741,13 @@ where
 				let peer_state = &mut *peer_state_lock;
 				res.extend(peer_state.channel_by_id.iter()
 					// Only `Channels` in the `Channel::Funded` phase can be considered funded.
-					.filter_map(|(chan_id, chan)| chan.as_funded().map(|chan| (chan_id, chan)))
+					.filter(|(_, chan)| chan.is_funded())
 					.filter(f)
 					.map(|(_channel_id, channel)| {
-						ChannelDetails::from_channel_context(&channel.context, &channel.funding, best_block_height,
-							peer_state.latest_features.clone(), &self.fee_estimator)
+						ChannelDetails::from_channel(
+							channel, best_block_height, peer_state.latest_features.clone(),
+							&self.fee_estimator,
+						)
 					})
 				);
 			}
@@ -3768,9 +3770,11 @@ where
 			for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
 				let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 				let peer_state = &mut *peer_state_lock;
-				for (context, funding) in peer_state.channel_by_id.iter().map(|(_, chan)| (chan.context(), chan.funding())) {
-					let details = ChannelDetails::from_channel_context(context, funding, best_block_height,
-						peer_state.latest_features.clone(), &self.fee_estimator);
+				for (_, channel) in peer_state.channel_by_id.iter() {
+					let details = ChannelDetails::from_channel(
+						channel, best_block_height, peer_state.latest_features.clone(),
+						&self.fee_estimator,
+					);
 					res.push(details);
 				}
 			}
@@ -3788,7 +3792,7 @@ where
 		// Note we use is_live here instead of usable which leads to somewhat confused
 		// internal/external nomenclature, but that's ok cause that's probably what the user
 		// really wanted anyway.
-		self.list_funded_channels_with_filter(|&(_, ref channel)| channel.context.is_live())
+		self.list_funded_channels_with_filter(|&(_, ref channel)| channel.context().is_live())
 	}
 
 	/// Gets the list of channels we have with a given counterparty, in random order.
@@ -3800,13 +3804,15 @@ where
 			let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 			let peer_state = &mut *peer_state_lock;
 			let features = &peer_state.latest_features;
-			let context_to_details = |(context, funding)| {
-				ChannelDetails::from_channel_context(context, funding, best_block_height, features.clone(), &self.fee_estimator)
+			let channel_to_details = |channel| {
+				ChannelDetails::from_channel(
+					channel, best_block_height, features.clone(), &self.fee_estimator,
+				)
 			};
 			return peer_state.channel_by_id
 				.iter()
-				.map(|(_, chan)| (chan.context(), chan.funding()))
-				.map(context_to_details)
+				.map(|(_, chan)| (chan))
+				.map(channel_to_details)
 				.collect();
 		}
 		vec![]
@@ -6087,7 +6093,7 @@ where
 								let maybe_optimal_channel = peer_state.channel_by_id.values_mut()
 									.filter_map(Channel::as_funded_mut)
 									.filter_map(|chan| {
-										let balances = chan.context.get_available_balances(&chan.funding, &self.fee_estimator);
+										let balances = chan.get_available_balances(&self.fee_estimator);
 										if outgoing_amt_msat <= balances.next_outbound_htlc_limit_msat &&
 											outgoing_amt_msat >= balances.next_outbound_htlc_minimum_msat &&
 											chan.context.is_usable() {