← Watch feed
Make `TxBuilder::get_next_commitment_stats` fallible
Public commit record
63/100 · Adequate
What the developer wrote
Authored by Leo Nash
Make `TxBuilder::get_next_commitment_stats` fallible
Anytime we ask `TxBuilder` for stats on a commitment transaction,
`TxBuilder` can now return an error to indicate that a balance not
including the commitment transaction fee has been overdrawn. We then
map this error to the appropriate action depending on where in the
life cycle of the channel the error occurred.
We now do not require that `channel_value_satoshis * 1000` is greater
than or equal to `value_to_holder_msat`; we previously would panic
in this case.
✓ Specific, descriptive subject✓ Provides detailed explanatory context
What changed, and why it matters
This commit removes a panic (crash) condition in the Lightning Dev Kit's channel transaction builder. Previously, if a channel's accounting reached an unexpected state where the holder's balance exceeded the total channel value, the code would panic and crash the node. Now it returns a proper error, allowing the node to close the channel gracefully instead of crashing. This is a defensive fix that prevents a potential denial-of-service where a malicious or buggy counterparty could crash your Lightning node by driving channel state into an overdrawn condition.
Recommended action
Review whether any other call sites still unwrap or expect results from `get_next_commitment_stats` or `get_holder_counterparty_balances_incl_fee_msat`. The test/fuzzing paths use `.expect()` with explanatory messages, which is acceptable for test-only builds but should not be present in production code paths. Consider adding regression tests that exercise the overdrawn balance paths to ensure graceful handling.
Security signals we found
01
Replaces panic/expect with fallible Result propagation
02
Adds graceful channel closure instead of node crash on balance overdraw
03
Adds new HTLC failure reason for overdrawn channel balance
04
Changes balance fields from Option<u64> to u64 with checked arithmetic
05
Prevents potential denial-of-service via crafted channel state
Evidence from the diff
The commit makes TxBuilder::get_next_commitment_stats return Result<NextCommitmentStats, ()> instead of panicking when balance calculations overflow or overdraw. Key changes: (1) value_to_counterparty_msat, HTLC subtractions, and anchor output subtractions now use checked_sub(...).ok_or(()) instead of expect/checked_sub returning Option; (2) NextCommitmentStats balance fields change from Option<u64> to u64; (3) callers in channel.rs propagate the error as ChannelError::close, log and return false for fee updates, or fail HTLCs with new LocalHTLCFailureReason::ChannelBalanceOverdrawn; (4) onion_utils.rs adds serialization and failure-reason handling for the new overdrawn variant. The previous panic was reachable when channel_value_satoshis * 1000 < value_to_holder_msat.
Changed components
lightning/src/sign/tx_builder.rslightning/src/ln/channel.rslightning/src/ln/onion_utils.rsInspect captured patch +213 / −175
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 08ca1d3..53da7b4 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4478,7 +4478,7 @@ where
&self, funding: &FundingScope, htlc_candidate: Option<HTLCAmountDirection>,
include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize,
feerate_per_kw: u32, dust_exposure_limiting_feerate: Option<u32>,
- ) -> NextCommitmentStats {
+ ) -> Result<NextCommitmentStats, ()> {
let next_commitment_htlcs = self.get_next_commitment_htlcs(
true,
htlc_candidate,
@@ -4497,7 +4497,7 @@ where
dust_exposure_limiting_feerate,
self.holder_dust_limit_satoshis,
funding.get_channel_type(),
- );
+ )?;
#[cfg(any(test, fuzzing))]
{
@@ -4508,18 +4508,20 @@ where
predicted_fee_sat: ret.commit_tx_fee_sat,
};
} else {
- let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats(
- true,
- funding.is_outbound(),
- funding.get_value_satoshis(),
- next_value_to_self_msat,
- &next_commitment_htlcs,
- 0,
- feerate_per_kw,
- dust_exposure_limiting_feerate,
- self.holder_dust_limit_satoshis,
- funding.get_channel_type(),
- );
+ let predicted_stats = SpecTxBuilder {}
+ .get_next_commitment_stats(
+ true,
+ funding.is_outbound(),
+ funding.get_value_satoshis(),
+ next_value_to_self_msat,
+ &next_commitment_htlcs,
+ 0,
+ feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ self.holder_dust_limit_satoshis,
+ funding.get_channel_type(),
+ )
+ .expect("Balance after HTLCs and anchors exhausted on local commitment");
*funding.next_local_fee.lock().unwrap() = PredictedNextFee {
predicted_feerate: feerate_per_kw,
predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count,
@@ -4528,14 +4530,14 @@ where
}
}
- ret
+ Ok(ret)
}
fn get_next_remote_commitment_stats(
&self, funding: &FundingScope, htlc_candidate: Option<HTLCAmountDirection>,
include_counterparty_unknown_htlcs: bool, addl_nondust_htlc_count: usize,
feerate_per_kw: u32, dust_exposure_limiting_feerate: Option<u32>,
- ) -> NextCommitmentStats {
+ ) -> Result<NextCommitmentStats, ()> {
let next_commitment_htlcs = self.get_next_commitment_htlcs(
false,
htlc_candidate,
@@ -4554,7 +4556,7 @@ where
dust_exposure_limiting_feerate,
self.counterparty_dust_limit_satoshis,
funding.get_channel_type(),
- );
+ )?;
#[cfg(any(test, fuzzing))]
{
@@ -4565,18 +4567,20 @@ where
predicted_fee_sat: ret.commit_tx_fee_sat,
};
} else {
- let predicted_stats = SpecTxBuilder {}.get_next_commitment_stats(
- false,
- funding.is_outbound(),
- funding.get_value_satoshis(),
- next_value_to_self_msat,
- &next_commitment_htlcs,
- 0,
- feerate_per_kw,
- dust_exposure_limiting_feerate,
- self.counterparty_dust_limit_satoshis,
- funding.get_channel_type(),
- );
+ let predicted_stats = SpecTxBuilder {}
+ .get_next_commitment_stats(
+ false,
+ funding.is_outbound(),
+ funding.get_value_satoshis(),
+ next_value_to_self_msat,
+ &next_commitment_htlcs,
+ 0,
+ feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ self.counterparty_dust_limit_satoshis,
+ funding.get_channel_type(),
+ )
+ .expect("Balance after HTLCs and anchors exhausted on remote commitment");
*funding.next_remote_fee.lock().unwrap() = PredictedNextFee {
predicted_feerate: feerate_per_kw,
predicted_nondust_htlc_count: predicted_stats.nondust_htlc_count,
@@ -4585,7 +4589,7 @@ where
}
}
- ret
+ Ok(ret)
}
fn validate_update_add_htlc<F: Deref>(
@@ -4608,14 +4612,18 @@ where
let include_counterparty_unknown_htlcs = false;
// Don't include the extra fee spike buffer HTLC in calculations
let fee_spike_buffer_htlc = 0;
- let next_remote_commitment_stats = self.get_next_remote_commitment_stats(
- funding,
- Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }),
- include_counterparty_unknown_htlcs,
- fee_spike_buffer_htlc,
- self.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
+ let next_remote_commitment_stats = self
+ .get_next_remote_commitment_stats(
+ funding,
+ Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }),
+ include_counterparty_unknown_htlcs,
+ fee_spike_buffer_htlc,
+ self.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| {
+ ChannelError::close(String::from("Remote HTLC add would overdraw remaining funds"))
+ })?;
if next_remote_commitment_stats.inbound_htlcs_count
> self.holder_max_accepted_htlcs as usize
@@ -4634,11 +4642,6 @@ where
)));
}
- let remote_balance_before_fee_msat =
- next_remote_commitment_stats.counterparty_balance_before_fee_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).
//
@@ -4660,12 +4663,16 @@ where
} else {
next_remote_commitment_stats.commit_tx_fee_sat * 1000
};
- if remote_balance_before_fee_msat < remote_commit_tx_fee_msat {
+ if next_remote_commitment_stats.counterparty_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(remote_commit_tx_fee_msat)
+ if next_remote_commitment_stats
+ .counterparty_balance_before_fee_msat
+ .saturating_sub(remote_commit_tx_fee_msat)
< funding.holder_selected_channel_reserve_satoshis * 1000
{
return Err(ChannelError::close(
@@ -4675,20 +4682,22 @@ where
}
if funding.is_outbound() {
- let next_local_commitment_stats = self.get_next_local_commitment_stats(
- funding,
- Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }),
- include_counterparty_unknown_htlcs,
- fee_spike_buffer_htlc,
- self.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
- let holder_balance_msat =
- next_local_commitment_stats.holder_balance_before_fee_msat.expect(
- "Adding an inbound HTLC should never exhaust the holder's balance before fees",
- );
+ let next_local_commitment_stats = self
+ .get_next_local_commitment_stats(
+ funding,
+ Some(HTLCAmountDirection { outbound: false, amount_msat: msg.amount_msat }),
+ include_counterparty_unknown_htlcs,
+ fee_spike_buffer_htlc,
+ self.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| {
+ ChannelError::close(String::from(
+ "Balance after HTLCs and anchors exhausted on local commitment",
+ ))
+ })?;
// Check that they won't violate our local required channel reserve by adding this HTLC.
- if holder_balance_msat
+ if next_local_commitment_stats.holder_balance_before_fee_msat
< funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000
+ next_local_commitment_stats.commit_tx_fee_sat * 1000
{
@@ -4714,22 +4723,34 @@ where
// Do not include outbound update_add_htlc's in the holding cell, or those which haven't yet been ACK'ed
// by the counterparty (ie. LocalAnnounced HTLCs)
let include_counterparty_unknown_htlcs = false;
- let next_local_commitment_stats = self.get_next_local_commitment_stats(
- funding,
- None,
- include_counterparty_unknown_htlcs,
- 0,
- msg.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
- let next_remote_commitment_stats = self.get_next_remote_commitment_stats(
- funding,
- None,
- include_counterparty_unknown_htlcs,
- 0,
- msg.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
+ let next_local_commitment_stats = self
+ .get_next_local_commitment_stats(
+ funding,
+ None,
+ include_counterparty_unknown_htlcs,
+ 0,
+ msg.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| {
+ ChannelError::close(String::from(
+ "Balance after HTLCs and anchors exhausted on local commitment",
+ ))
+ })?;
+ let next_remote_commitment_stats = self
+ .get_next_remote_commitment_stats(
+ funding,
+ None,
+ include_counterparty_unknown_htlcs,
+ 0,
+ msg.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| {
+ ChannelError::close(String::from(
+ "Balance after HTLCs and anchors exhausted on remote commitment",
+ ))
+ })?;
let max_dust_htlc_exposure_msat =
self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
@@ -4926,20 +4947,25 @@ where
// 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(
+ let next_remote_commitment_stats = if let Ok(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,
- );
- let holder_balance_msat = next_remote_commitment_stats
- .holder_balance_before_fee_msat
- .expect("The holder's balance before fees should never underflow.");
+ ) {
+ stats
+ } else {
+ log_debug!(
+ logger,
+ "Cannot afford to send new feerate due to balance after HTLCs and anchors exhausted on remote commitment",
+ );
+ return false;
+ };
// 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
+ if next_remote_commitment_stats.holder_balance_before_fee_msat
< next_remote_commitment_stats.commit_tx_fee_sat * 1000
+ funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000
{
@@ -4961,14 +4987,22 @@ where
return false;
}
- let next_local_commitment_stats = self.get_next_local_commitment_stats(
+ let next_local_commitment_stats = if let Ok(stats) = self.get_next_local_commitment_stats(
funding,
None,
include_counterparty_unknown_htlcs,
CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize,
feerate_per_kw,
dust_exposure_limiting_feerate,
- );
+ ) {
+ stats
+ } else {
+ log_debug!(
+ logger,
+ "Cannot afford to send new feerate due to balance after HTLCs and anchors exhausted on local commitment",
+ );
+ return false;
+ };
if next_local_commitment_stats.dust_exposure_msat > max_dust_htlc_exposure_msat {
log_debug!(
logger,
@@ -4997,22 +5031,32 @@ where
let include_counterparty_unknown_htlcs = false;
// A `None` `HTLCCandidate` is used as in this case because we're already accounting for
// the incoming HTLC as it has been fully committed by both sides.
- let next_local_commitment_stats = self.get_next_local_commitment_stats(
- funding,
- None,
- include_counterparty_unknown_htlcs,
- fee_spike_buffer_htlc,
- self.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
- let next_remote_commitment_stats = self.get_next_remote_commitment_stats(
- funding,
- None,
- include_counterparty_unknown_htlcs,
- fee_spike_buffer_htlc,
- self.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
+ let next_local_commitment_stats = self
+ .get_next_local_commitment_stats(
+ funding,
+ None,
+ include_counterparty_unknown_htlcs,
+ fee_spike_buffer_htlc,
+ self.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| {
+ log_trace!(logger, "Attempting to fail HTLC due to balance after HTLCs and anchors exhausted on local commitment");
+ LocalHTLCFailureReason::ChannelBalanceOverdrawn
+ })?;
+ let next_remote_commitment_stats = self
+ .get_next_remote_commitment_stats(
+ funding,
+ None,
+ include_counterparty_unknown_htlcs,
+ fee_spike_buffer_htlc,
+ self.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| {
+ log_trace!(logger, "Attempting to fail HTLC due to balance after HTLCs and anchors exhausted on remote commitment");
+ LocalHTLCFailureReason::ChannelBalanceOverdrawn
+ })?;
let max_dust_htlc_exposure_msat =
self.get_max_dust_htlc_exposure_msat(dust_exposure_limiting_feerate);
@@ -5046,12 +5090,8 @@ where
remote_fee_incl_fee_spike_buffer_htlc_msat *=
FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
}
- // We unwrap here; if the HTLC exhausts the counterparty's balance, we should have rejected it
- // at `update_add_htlc`, here the HTLC is already irrevocably committed to the channel.
- let remote_balance_before_fee_msat = next_remote_commitment_stats
+ if next_remote_commitment_stats
.counterparty_balance_before_fee_msat
- .expect("The counterparty's balance before fees should never underflow");
- if remote_balance_before_fee_msat
.saturating_sub(funding.holder_selected_channel_reserve_satoshis * 1000)
< remote_fee_incl_fee_spike_buffer_htlc_msat
{
@@ -11950,43 +11990,44 @@ where
// nondust HTLC on the channel.
let addl_nondust_htlc_count = 1;
- let local_commitment_stats = self.context.get_next_local_commitment_stats(
- funding,
- None, // htlc_candidate
- include_counterparty_unknown_htlcs,
- addl_nondust_htlc_count,
- self.context.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
+ let local_commitment_stats = self
+ .context
+ .get_next_local_commitment_stats(
+ funding,
+ None, // htlc_candidate
+ include_counterparty_unknown_htlcs,
+ addl_nondust_htlc_count,
+ self.context.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| "Balance after HTLCs and anchors exhausted on local commitment")?;
let (holder_balance_on_local_msat, counterparty_balance_on_local_msat) =
- local_commitment_stats.get_holder_counterparty_balances_incl_fee_msat();
+ local_commitment_stats
+ .get_holder_counterparty_balances_incl_fee_msat()
+ .map_err(|()| "Channel funder cannot afford the fee on local commitment")?;
- let remote_commitment_stats = self.context.get_next_remote_commitment_stats(
- funding,
- None, // htlc_candidate
- include_counterparty_unknown_htlcs,
- addl_nondust_htlc_count,
- self.context.feerate_per_kw,
- dust_exposure_limiting_feerate,
- );
+ let remote_commitment_stats = self
+ .context
+ .get_next_remote_commitment_stats(
+ funding,
+ None, // htlc_candidate
+ include_counterparty_unknown_htlcs,
+ addl_nondust_htlc_count,
+ self.context.feerate_per_kw,
+ dust_exposure_limiting_feerate,
+ )
+ .map_err(|()| "Balance after HTLCs and anchors exhausted on remote commitment")?;
let (holder_balance_on_remote_msat, counterparty_balance_on_remote_msat) =
- remote_commitment_stats.get_holder_counterparty_balances_incl_fee_msat();
+ remote_commitment_stats
+ .get_holder_counterparty_balances_incl_fee_msat()
+ .map_err(|()| "Channel funder cannot afford the fee on remote commitment")?;
let holder_balance_floor = Amount::from_sat(
- cmp::min(
- holder_balance_on_local_msat
- .ok_or("holder balance exhausted on local commitment")?,
- holder_balance_on_remote_msat
- .ok_or("holder balance exhausted on remote commitment")?,
- ) / 1000,
+ cmp::min(holder_balance_on_local_msat, holder_balance_on_remote_msat) / 1000,
);
let counterparty_balance_floor = Amount::from_sat(
- cmp::min(
- counterparty_balance_on_local_msat
- .ok_or("counterparty balance exhausted on local commitment")?,
- counterparty_balance_on_remote_msat
- .ok_or("counterparty balance exhausted on remote commitment")?,
- ) / 1000,
+ cmp::min(counterparty_balance_on_local_msat, counterparty_balance_on_remote_msat)
+ / 1000,
);
Ok((holder_balance_floor, counterparty_balance_floor))
diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs
index 962bb9a..6bba2b5 100644
--- a/lightning/src/ln/onion_utils.rs
+++ b/lightning/src/ln/onion_utils.rs
@@ -1678,6 +1678,8 @@ pub enum LocalHTLCFailureReason {
HTLCMaximum,
/// The HTLC was failed because our remote peer is offline.
PeerOffline,
+ /// The HTLC was failed because the channel balance was overdrawn.
+ ChannelBalanceOverdrawn,
}
impl LocalHTLCFailureReason {
@@ -1697,7 +1699,8 @@ impl LocalHTLCFailureReason {
| Self::ZeroAmount
| Self::HTLCMinimum
| Self::HTLCMaximum
- | Self::PeerOffline => UPDATE | 7,
+ | Self::PeerOffline
+ | Self::ChannelBalanceOverdrawn => UPDATE | 7,
Self::PermanentChannelFailure | Self::ChannelClosed | Self::OnChainTimeout => PERM | 8,
Self::RequiredChannelFeature => PERM | 9,
Self::UnknownNextPeer
@@ -1876,7 +1879,8 @@ ser_failure_reasons!(
(38, ZeroAmount),
(39, HTLCMinimum),
(40, HTLCMaximum),
- (41, PeerOffline)
+ (41, PeerOffline),
+ (42, ChannelBalanceOverdrawn)
);
impl From<&HTLCFailReason> for HTLCHandlingFailureReason {
@@ -1992,7 +1996,8 @@ impl HTLCFailReason {
| LocalHTLCFailureReason::ZeroAmount
| LocalHTLCFailureReason::HTLCMinimum
| LocalHTLCFailureReason::HTLCMaximum
- | LocalHTLCFailureReason::PeerOffline => {
+ | LocalHTLCFailureReason::PeerOffline
+ | LocalHTLCFailureReason::ChannelBalanceOverdrawn => {
debug_assert_eq!(
data.len() - 2,
u16::from_be_bytes(data[0..2].try_into().unwrap()) as usize
diff --git a/lightning/src/sign/tx_builder.rs b/lightning/src/sign/tx_builder.rs
index cb5415c..a4bcdff 100644
--- a/lightning/src/sign/tx_builder.rs
+++ b/lightning/src/sign/tx_builder.rs
@@ -38,8 +38,8 @@ pub(crate) struct NextCommitmentStats {
pub is_outbound_from_holder: bool,
pub inbound_htlcs_count: usize,
pub inbound_htlcs_value_msat: u64,
- pub holder_balance_before_fee_msat: Option<u64>,
- pub counterparty_balance_before_fee_msat: Option<u64>,
+ pub holder_balance_before_fee_msat: u64,
+ pub counterparty_balance_before_fee_msat: u64,
pub nondust_htlc_count: usize,
pub commit_tx_fee_sat: u64,
pub dust_exposure_msat: u64,
@@ -50,23 +50,21 @@ pub(crate) struct NextCommitmentStats {
}
impl NextCommitmentStats {
- pub(crate) fn get_holder_counterparty_balances_incl_fee_msat(
- &self,
- ) -> (Option<u64>, Option<u64>) {
+ pub(crate) fn get_holder_counterparty_balances_incl_fee_msat(&self) -> Result<(u64, u64), ()> {
if self.is_outbound_from_holder {
- (
- self.holder_balance_before_fee_msat.and_then(|balance_msat| {
- balance_msat.checked_sub(self.commit_tx_fee_sat * 1000)
- }),
+ Ok((
+ self.holder_balance_before_fee_msat
+ .checked_sub(self.commit_tx_fee_sat * 1000)
+ .ok_or(())?,
self.counterparty_balance_before_fee_msat,
- )
+ ))
} else {
- (
+ Ok((
self.holder_balance_before_fee_msat,
- self.counterparty_balance_before_fee_msat.and_then(|balance_msat| {
- balance_msat.checked_sub(self.commit_tx_fee_sat * 1000)
- }),
- )
+ self.counterparty_balance_before_fee_msat
+ .checked_sub(self.commit_tx_fee_sat * 1000)
+ .ok_or(())?,
+ ))
}
}
}
@@ -133,9 +131,9 @@ fn excess_fees_on_counterparty_tx_dust_exposure_msat(
}
fn subtract_addl_outputs(
- is_outbound_from_holder: bool, value_to_self_after_htlcs_msat: Option<u64>,
- value_to_remote_after_htlcs_msat: Option<u64>, channel_type: &ChannelTypeFeatures,
-) -> (Option<u64>, Option<u64>) {
+ is_outbound_from_holder: bool, value_to_self_after_htlcs_msat: u64,
+ value_to_remote_after_htlcs_msat: u64, channel_type: &ChannelTypeFeatures,
+) -> Result<(u64, u64), ()> {
let total_anchors_sat = if channel_type.supports_anchors_zero_fee_htlc_tx() {
ANCHOR_OUTPUT_VALUE_SATOSHI * 2
} else {
@@ -150,17 +148,15 @@ fn subtract_addl_outputs(
// cover the total anchor sum.
if is_outbound_from_holder {
- (
- value_to_self_after_htlcs_msat
- .and_then(|balance_msat| balance_msat.checked_sub(total_anchors_sat * 1000)),
+ Ok((
+ value_to_self_after_htlcs_msat.checked_sub(total_anchors_sat * 1000).ok_or(())?,
value_to_remote_after_htlcs_msat,
- )
+ ))
} else {
- (
+ Ok((
value_to_self_after_htlcs_msat,
- value_to_remote_after_htlcs_msat
- .and_then(|balance_msat| balance_msat.checked_sub(total_anchors_sat * 1000)),
- )
+ value_to_remote_after_htlcs_msat.checked_sub(total_anchors_sat * 1000).ok_or(())?,
+ ))
}
}
@@ -181,7 +177,7 @@ pub(crate) trait TxBuilder {
addl_nondust_htlc_count: usize, feerate_per_kw: u32,
dust_exposure_limiting_feerate: Option<u32>, broadcaster_dust_limit_satoshis: u64,
channel_type: &ChannelTypeFeatures,
- ) -> NextCommitmentStats;
+ ) -> Result<NextCommitmentStats, ()>;
fn commit_tx_fee_sat(
&self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures,
) -> u64;
@@ -208,7 +204,7 @@ impl TxBuilder for SpecTxBuilder {
addl_nondust_htlc_count: usize, feerate_per_kw: u32,
dust_exposure_limiting_feerate: Option<u32>, broadcaster_dust_limit_satoshis: u64,
channel_type: &ChannelTypeFeatures,
- ) -> NextCommitmentStats {
+ ) -> Result<NextCommitmentStats, ()> {
let excess_feerate_opt =
feerate_per_kw.checked_sub(dust_exposure_limiting_feerate.unwrap_or(0));
// Dust exposure is only decoupled from feerate for zero fee commitment channels.
@@ -225,9 +221,8 @@ impl TxBuilder for SpecTxBuilder {
next_commitment_htlcs.iter().filter(|htlc| !htlc.outbound).count();
// Calculate balances after htlcs
- let value_to_counterparty_msat = (channel_value_satoshis * 1000)
- .checked_sub(value_to_holder_msat)
- .expect("value_to_holder_msat outgrew the value of the channel!");
+ let value_to_counterparty_msat =
+ (channel_value_satoshis * 1000).checked_sub(value_to_holder_msat).ok_or(())?;
let outbound_htlcs_value_msat: u64 = next_commitment_htlcs
.iter()
.filter_map(|htlc| htlc.outbound.then_some(htlc.amount_msat))
@@ -236,13 +231,10 @@ impl TxBuilder for SpecTxBuilder {
.iter()
.filter_map(|htlc| (!htlc.outbound).then_some(htlc.amount_msat))
.sum();
- // Note there is no guarantee that the subtractions of the HTLC amounts don't
- // overflow, so we do not panic. Instead, we return `None` to signal an overflow
- // to channel, and let channel take the appropriate action.
let value_to_holder_after_htlcs_msat =
- value_to_holder_msat.checked_sub(outbound_htlcs_value_msat);
+ value_to_holder_msat.checked_sub(outbound_htlcs_value_msat).ok_or(())?;
let value_to_counterparty_after_htlcs_msat =
- value_to_counterparty_msat.checked_sub(inbound_htlcs_value_msat);
+ value_to_counterparty_msat.checked_sub(inbound_htlcs_value_msat).ok_or(())?;
// Subtract the anchors from the channel funder
let (holder_balance_before_fee_msat, counterparty_balance_before_fee_msat) =
@@ -251,7 +243,7 @@ impl TxBuilder for SpecTxBuilder {
value_to_holder_after_htlcs_msat,
value_to_counterparty_after_htlcs_msat,
channel_type,
- );
+ )?;
// Increment the feerate by a buffer to calculate dust exposure
let dust_buffer_feerate = get_dust_buffer_feerate(feerate_per_kw);
@@ -300,7 +292,7 @@ impl TxBuilder for SpecTxBuilder {
(dust_exposure_msat, None)
};
- NextCommitmentStats {
+ Ok(NextCommitmentStats {
is_outbound_from_holder,
inbound_htlcs_count,
inbound_htlcs_value_msat,
@@ -310,7 +302,7 @@ impl TxBuilder for SpecTxBuilder {
commit_tx_fee_sat,
dust_exposure_msat,
extra_nondust_htlc_on_counterparty_tx_dust_exposure_msat,
- }
+ })
}
fn commit_tx_fee_sat(
&self, feerate_per_kw: u32, nondust_htlc_count: usize, channel_type: &ChannelTypeFeatures,
Risk score
Our methodology →Why this scored 67/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.