Skip to content

Commit d0023e3

Browse files
committed
Check negative funding_contribution_satoshis
This applies to both `splice_init` and `splice_ack` messages. From BOLT 2: ``` - If `funding_contribution_satoshis` is negative and its absolute value is greater than the sending node's current channel balance: - MUST send a `warning` and close the connection or send an `error` and fail the channel. ``` We allow the remote to be below the new reserve as long as their funding contribution is not negative; we don't care whether they were above or below the previous funding reserve.
1 parent 195a753 commit d0023e3

File tree

1 file changed

+74
-9
lines changed

1 file changed

+74
-9
lines changed

lightning/src/ln/channel.rs

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11002,6 +11002,11 @@ where
1100211002
their_funding_contribution.to_sat(),
1100311003
);
1100411004

11005+
// While we check that the remote can afford the HTLCs, anchors, and the reserve
11006+
// after creating the new `FundingScope` below, we MUST do a basic check here to
11007+
// make sure that their funding contribution doesn't completely exhaust their
11008+
// balance here because an invariant of `FundingScope` is that `value_to_self_msat`
11009+
// MUST be smaller than or equal to `channel_value_satoshis` * 1000.
1100511010
if post_channel_balance.is_none() {
1100611011
return Err(ChannelError::WarnAndDisconnect(format!(
1100711012
"Channel {} cannot be spliced out; their {} contribution exhausts their channel balance: {}",
@@ -11019,15 +11024,9 @@ where
1101911024
counterparty_funding_pubkey,
1102011025
);
1102111026

11022-
// TODO(splicing): Check that channel balance does not go below the channel reserve
11023-
11024-
// Note on channel reserve requirement pre-check: as the splice acceptor does not contribute,
11025-
// it can't go below reserve, therefore no pre-check is done here.
11026-
11027-
// TODO(splicing): Early check for reserve requirement
11028-
11029-
// TODO(splicing): Pre-check for reserve requirement
11030-
// (Note: It should also be checked later at tx_complete)
11027+
if their_funding_contribution.is_negative() {
11028+
self.validate_their_negative_funding_contribution(&splice_funding)?;
11029+
}
1103111030

1103211031
Ok(splice_funding)
1103311032
}
@@ -11193,6 +11192,72 @@ where
1119311192
)
1119411193
}
1119511194

11195+
/// Used to validate a negative `funding_contribution_satoshis` in `splice_init` and `splice_ack` messages.
11196+
#[cfg(splicing)]
11197+
fn validate_their_negative_funding_contribution(
11198+
&self, splice_funding: &FundingScope,
11199+
) -> Result<(), ChannelError> {
11200+
// We don't care about the exact value of `dust_exposure_limiting_feerate` here as
11201+
// we do not validate dust exposure below, but we want to avoid triggering a debug
11202+
// assert.
11203+
//
11204+
// TODO: clean this up here and elsewhere.
11205+
let dust_exposure_limiting_feerate =
11206+
if splice_funding.get_channel_type().supports_anchor_zero_fee_commitments() {
11207+
None
11208+
} else {
11209+
Some(self.context.feerate_per_kw)
11210+
};
11211+
// This *should* have no effect because no HTLC updates should be pending, but even if it does,
11212+
// the result may be a failed negotiation (and not a force-close), so we choose to include them.
11213+
let include_remote_unknown_htlcs = true;
11214+
let addl_nondust_htlc_count = 0;
11215+
11216+
let validate_stats = |stats: NextCommitmentStats| {
11217+
let (_, remote_balance_incl_fee_msat) = stats.get_balances_including_fee_msat();
11218+
let splice_remote_balance_msat = remote_balance_incl_fee_msat
11219+
.ok_or(ChannelError::Warn(format!("Remote balance does not cover the sum of HTLCs, anchors, and commitment transaction fee")))?;
11220+
11221+
// Check if the remote's new balance is under the specified reserve
11222+
if splice_remote_balance_msat
11223+
< splice_funding.holder_selected_channel_reserve_satoshis * 1000
11224+
{
11225+
return Err(ChannelError::Warn(format!(
11226+
"Remote balance below reserve mandated by holder: {} vs {}",
11227+
splice_remote_balance_msat,
11228+
splice_funding.holder_selected_channel_reserve_satoshis * 1000,
11229+
)));
11230+
}
11231+
Ok(())
11232+
};
11233+
11234+
// Reserve check on local commitment transaction
11235+
11236+
let splice_local_commitment_stats = self.context.get_next_local_commitment_stats(
11237+
splice_funding,
11238+
None, // htlc_candidate
11239+
include_remote_unknown_htlcs,
11240+
addl_nondust_htlc_count,
11241+
self.context.feerate_per_kw,
11242+
dust_exposure_limiting_feerate,
11243+
);
11244+
11245+
validate_stats(splice_local_commitment_stats)?;
11246+
11247+
// Reserve check on remote commitment transaction
11248+
11249+
let splice_remote_commitment_stats = self.context.get_next_remote_commitment_stats(
11250+
splice_funding,
11251+
None, // htlc_candidate
11252+
include_remote_unknown_htlcs,
11253+
addl_nondust_htlc_count,
11254+
self.context.feerate_per_kw,
11255+
dust_exposure_limiting_feerate,
11256+
);
11257+
11258+
validate_stats(splice_remote_commitment_stats)
11259+
}
11260+
1119611261
#[cfg(splicing)]
1119711262
pub fn splice_locked<NS: Deref, L: Deref>(
1119811263
&mut self, msg: &msgs::SpliceLocked, node_signer: &NS, chain_hash: ChainHash,

0 commit comments

Comments
 (0)