Let callers handle errors on `get_available_balances_for_scope`
What the developer wrote
Authored by Leo Nash
Let callers handle errors on `get_available_balances_for_scope`
`get_available_balances_for_scope` only errors if some party in the
channel cannot afford the HTLCs outbound from said party, and the
anchors and transaction fee if they are the funder. We do not account
for the channel reserve here, so this error should be exceedingly rare,
but could nonetheless happen due to concurrent updates on the channel's
state. The upcoming zero-reserve channel type could also make this case
more reachable.
`send_htlc` maps such an error to its own error type since it proposes
an update to the channel's state. The other callers only read the
channel's state, so it would not be a good fit to have them return an
error too. Hence, we choose to let these callers panic in debug mode,
and return saturated values in release mode.
Note that we now handle the if-we-removed-it-already-but-haven't
-fully-resolved-they-can-still-send-an-inbound-HTLC case, as
`LocalRemoved` HTLCs are considered resolved when calculating
`AvailableBalances`. We update the documentation accordingly.
What changed, and why it matters
This commit changes how a Lightning channel balance calculation reports rare error conditions. Previously the function could silently ignore an internal failure and return potentially incorrect balance numbers. Now it returns an explicit error, and callers that only read channel state either crash in debug builds or return conservative 'saturated' values in release builds. The main user-facing effect is more correct balance reporting and safer handling of an edge case where a party cannot actually afford its pending payments plus fees. It is a defensive fix, not a clear-cut exploit patch.
Review whether the saturated fallback values in release builds are safe for all downstream consumers, and consider adding tests for the new `ChannelBalanceOverdrawn` error path and the `LocalRemoved` HTLC balance case.
Security signals we found
Defensive error propagation added to balance calculation
Read-only callers panic in debug mode and saturate in release mode
send_htlc now rejects overdrawn channels explicitly
Documentation updated to describe error condition and LocalRemoved HTLC handling
Evidence from the diff
The patch converts get_available_balances and get_available_balances_for_scope from returning AvailableBalances to returning Result<AvailableBalances, ()>. The error path is taken when an internal balance/HTLC/fee check indicates a party cannot cover outbound HTLCs plus anchors and transaction fees. send_htlc maps this to a new LocalHTLCFailureReason::ChannelBalanceOverdrawn error instead of proceeding. Read-only callers (ChannelDetails construction and a route/balance query in ChannelManager) use unwrap_or_else with debug_assert!(false, ...) and saturated fallback values (zero capacities, max HTLC minimum). The commit also updates documentation and correctly treats LocalRemoved HTLCs as resolved for balance calculations.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channel_state.rslightning/src/ln/channelmanager.rsInspect captured patch +46 / −19
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 22ae13f..adb6705 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -2442,13 +2442,13 @@ 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.
+ /// Gets the available balances, see [`AvailableBalances`]'s fields for more info.
+ ///
+ /// Returns `Err` if some party cannot currently pay for the HTLCs outbound from said party, and the anchors and
+ /// transaction fee if they are the funder.
pub fn get_available_balances<F: FeeEstimator>(
&self, fee_estimator: &LowerBoundedFeeEstimator<F>,
- ) -> AvailableBalances {
+ ) -> Result<AvailableBalances, ()> {
match &self.phase {
ChannelPhase::Undefined => unreachable!(),
ChannelPhase::Funded(chan) => chan.get_available_balances(fee_estimator),
@@ -5784,7 +5784,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
#[rustfmt::skip]
fn get_available_balances_for_scope<F: FeeEstimator>(
&self, funding: &FundingScope, fee_estimator: &LowerBoundedFeeEstimator<F>,
- ) -> AvailableBalances {
+ ) -> Result<AvailableBalances, ()> {
let htlc_candidate = None;
let include_counterparty_unknown_htlcs = true;
let addl_nondust_htlc_count = 0;
@@ -5799,7 +5799,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
addl_nondust_htlc_count,
self.feerate_per_kw,
dust_exposure_limiting_feerate
- ).map(|(remote_stats, _)| remote_stats.available_balances).unwrap();
+ ).map(|(remote_stats, _)| remote_stats.available_balances)?;
#[cfg(debug_assertions)]
if balances.next_outbound_htlc_limit_msat >= balances.next_outbound_htlc_minimum_msat
@@ -5823,7 +5823,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
>= funding.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) * 1000);
}
- balances
+ Ok(balances)
}
#[rustfmt::skip]
@@ -12490,7 +12490,12 @@ where
return Err((LocalHTLCFailureReason::ZeroAmount, "Cannot send 0-msat HTLC".to_owned()));
}
- let available_balances = self.get_available_balances(fee_estimator);
+ let available_balances = self.get_available_balances(fee_estimator).map_err(|()| {
+ (
+ LocalHTLCFailureReason::ChannelBalanceOverdrawn,
+ "Channel balance overdrawn".to_owned(),
+ )
+ })?;
if amount_msat < available_balances.next_outbound_htlc_minimum_msat {
return Err((
LocalHTLCFailureReason::HTLCMinimum,
@@ -12584,22 +12589,26 @@ where
Ok(true)
}
+ /// Gets the available balances, see [`AvailableBalances`]'s fields for more info.
+ ///
+ /// Returns `Err` if some party cannot currently pay for the HTLCs outbound from said party, and the anchors and
+ /// transaction fee if they are the funder.
#[rustfmt::skip]
pub(super) fn get_available_balances<F: FeeEstimator>(
&self, fee_estimator: &LowerBoundedFeeEstimator<F>,
- ) -> AvailableBalances {
- 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 {
+ ) -> Result<AvailableBalances, ()> {
+ let init = self.context.get_available_balances_for_scope(&self.funding, fee_estimator)?;
+ self.pending_funding().iter().try_fold(
+ init,
+ |acc, funding| {
+ let e = self.context.get_available_balances_for_scope(funding, fee_estimator)?;
+ Ok(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: Logger>(&mut self, logger: &L) -> ChannelMonitorUpdate {
diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs
index c7277d1..5547bee 100644
--- a/lightning/src/ln/channel_state.rs
+++ b/lightning/src/ln/channel_state.rs
@@ -525,7 +525,16 @@ impl ChannelDetails {
) -> Self {
let context = channel.context();
let funding = channel.funding();
- let balance = channel.get_available_balances(fee_estimator);
+ let balance_result = channel.get_available_balances(fee_estimator);
+ let balance = balance_result.unwrap_or_else(|()| {
+ debug_assert!(false, "some channel balance has been overdrawn");
+ crate::ln::channel::AvailableBalances {
+ inbound_capacity_msat: 0,
+ outbound_capacity_msat: 0,
+ next_outbound_htlc_limit_msat: 0,
+ next_outbound_htlc_minimum_msat: u64::MAX,
+ }
+ });
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.
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 08cbb6f..bf2ff61 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -7708,7 +7708,16 @@ impl<
.values_mut()
.filter_map(Channel::as_funded_mut)
.filter_map(|chan| {
- let balances = chan.get_available_balances(&self.fee_estimator);
+ let balances_result = chan.get_available_balances(&self.fee_estimator);
+ let balances = balances_result.unwrap_or_else(|()| {
+ debug_assert!(false, "some channel balance has been overdrawn");
+ crate::ln::channel::AvailableBalances {
+ inbound_capacity_msat: 0,
+ outbound_capacity_msat: 0,
+ next_outbound_htlc_limit_msat: 0,
+ next_outbound_htlc_minimum_msat: u64::MAX,
+ }
+ });
let is_in_range = (balances.next_outbound_htlc_minimum_msat
..=balances.next_outbound_htlc_limit_msat)
.contains(&outgoing_amt_msat);
Why this scored 47/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.