Skip to content

Error if the calculated reserve would be greater than the channel value #4580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
tankyleo wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Error if the calculated reserve would be greater than the channel value #4580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
767fe39
Error if the calculated v2 reserve is greater than the channel value
tankyleo Apr 8, 2026
9669465
Error if the calculated v1 reserve is greater than the channel value
tankyleo Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 101 additions & 36 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2752,16 +2752,14 @@ impl FundingScope {
) -> Result<Self, String> {
if our_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
return Err(format!(
"Channel {} cannot be spliced; our {} contribution exceeds the total bitcoin supply",
context.channel_id(),
"Our {} contribution exceeds the total bitcoin supply",
our_funding_contribution,
));
}

if their_funding_contribution.unsigned_abs() > Amount::MAX_MONEY {
return Err(format!(
"Channel {} cannot be spliced; their {} contribution exceeds the total bitcoin supply",
context.channel_id(),
"Their {} contribution exceeds the total bitcoin supply",
their_funding_contribution,
));
}
Expand Down Expand Up @@ -2821,17 +2819,20 @@ impl FundingScope {
// New reserve values are based on the new channel value and are v2-specific
let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
post_channel_value_sat,
MIN_CHAN_DUST_LIMIT_SATOSHIS,
context.holder_dust_limit_satoshis,
prev_funding
.counterparty_selected_channel_reserve_satoshis
.expect("counterparty reserve is set")
== 0,
);
)
.map_err(|()| format!("The post-splice channel value {post_channel_value_sat} is smaller than our dust limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"))?;
let their_dust_limit_satoshis = context.counterparty_dust_limit_satoshis;
let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
post_channel_value_sat,
context.counterparty_dust_limit_satoshis,
their_dust_limit_satoshis,
prev_funding.holder_selected_channel_reserve_satoshis == 0,
);
)
.map_err(|()| format!("The post-splice channel value {post_channel_value_sat} is smaller than their dust limit {their_dust_limit_satoshis}"))?;

Ok(Self {
channel_transaction_parameters: post_channel_transaction_parameters,
Expand Down Expand Up @@ -3384,6 +3385,9 @@ pub(super) struct ChannelContext<SP: SignerProvider> {
/// We use this to close if funding is never broadcasted.
pub(super) channel_creation_height: u32,

#[cfg(any(test, feature = "_test_utils"))]
pub(crate) counterparty_dust_limit_satoshis: u64,
#[cfg(not(any(test, feature = "_test_utils")))]
counterparty_dust_limit_satoshis: u64,

#[cfg(any(test, feature = "_test_utils"))]
Expand Down Expand Up @@ -6746,20 +6750,32 @@ fn get_legacy_default_holder_max_htlc_value_in_flight_msat(channel_value_satoshi
/// This is used both for outbound and inbound channels and has lower bound
/// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS`, and the `dust_limit_satoshis` of
/// the counterparty.
///
/// Returns `Err` if `channel_value_satoshis` is smaller than
/// `MIN_THEIR_CHAN_RESERVE_SATOSHIS` or the `dust_limit_satoshis` of the
/// counterparty.
pub(crate) fn get_holder_selected_channel_reserve_satoshis(
channel_value_satoshis: u64, their_dust_limit_satoshis: u64, config: &UserConfig,
is_0reserve: bool,
) -> u64 {
) -> Result<u64, ()> {
if channel_value_satoshis < MIN_THEIR_CHAN_RESERVE_SATOSHIS
|| channel_value_satoshis < their_dust_limit_satoshis
{
return Err(());
}
if is_0reserve {
return 0;
return Ok(0);
}
let counterparty_chan_reserve_prop_mil =
config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64;
// As described in the `ChannelHandshakeConfig` docs, we cap this value at 1_000_000.
let counterparty_chan_reserve_prop_mil = cmp::min(
config.channel_handshake_config.their_channel_reserve_proportional_millionths as u64,
1_000_000,
);
let calculated_reserve =
channel_value_satoshis.saturating_mul(counterparty_chan_reserve_prop_mil) / 1_000_000;
let channel_reserve_satoshis = cmp::max(calculated_reserve, MIN_THEIR_CHAN_RESERVE_SATOSHIS);
let channel_reserve_satoshis = cmp::max(channel_reserve_satoshis, their_dust_limit_satoshis);
cmp::min(channel_value_satoshis, channel_reserve_satoshis)
Ok(channel_reserve_satoshis)
}

/// This is for legacy reasons, present for forward-compatibility.
Expand All @@ -6776,19 +6792,24 @@ pub(crate) fn get_legacy_default_holder_selected_channel_reserve_satoshis(
/// Returns a minimum channel reserve value each party needs to maintain, fixed in the spec to a
/// default of 1% of the total channel value.
///
/// Guaranteed to return a value no larger than channel_value_satoshis
/// Guaranteed to return a value no larger than `channel_value_satoshis`
///
/// This is used both for outbound and inbound channels and has lower bound
/// of `dust_limit_satoshis`.
///
/// Returns `Err` if `channel_value_satoshis` is smaller than `dust_limit_satoshis`.
pub(crate) fn get_v2_channel_reserve_satoshis(
channel_value_satoshis: u64, dust_limit_satoshis: u64, is_0reserve: bool,
) -> u64 {
) -> Result<u64, ()> {
if channel_value_satoshis < dust_limit_satoshis {
return Err(());
}
if is_0reserve {
return 0;
return Ok(0);
}
// Fixed at 1% of channel value by spec.
let (q, _) = channel_value_satoshis.overflowing_div(100);
cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis))
Ok(cmp::max(q, dust_limit_satoshis))
}

/// Returns the minimum feerate for RBF attempts given a previous feerate.
Expand Down Expand Up @@ -12824,7 +12845,8 @@ where
their_funding_contribution,
counterparty_funding_pubkey,
our_new_holder_keys,
)?;
)
.map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?;

let (post_splice_holder_balance, post_splice_counterparty_balance) =
self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err(
Expand Down Expand Up @@ -14458,12 +14480,19 @@ impl<SP: SignerProvider> OutboundV1Channel<SP> {
// a dust limit higher than our selected reserve.
let their_dust_limit_satoshis = 0;
let is_0reserve = trusted_channel_features.is_some_and(|f| f.is_0reserve());
let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(
channel_value_satoshis,
their_dust_limit_satoshis,
config,
is_0reserve,
);
let holder_selected_channel_reserve_satoshis =
get_holder_selected_channel_reserve_satoshis(
channel_value_satoshis,
their_dust_limit_satoshis,
config,
is_0reserve,
)
.map_err(|()| APIError::APIMisuseError {
err: format!(
"The channel value {channel_value_satoshis} is smaller than \
{MIN_THEIR_CHAN_RESERVE_SATOSHIS}"
),
})?;
if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS && !is_0reserve {
// Protocol level safety check in place, although it should never happen because
// of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` and `MIN_CHANNEL_VALUE_SATOSHIS`
Expand Down Expand Up @@ -14855,12 +14884,20 @@ impl<SP: SignerProvider> InboundV1Channel<SP> {
let channel_type =
channel_type_from_open_channel(&msg.common_fields, our_supported_features)?;

let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(
msg.common_fields.funding_satoshis,
msg.common_fields.dust_limit_satoshis,
config,
trusted_channel_features.is_some_and(|f| f.is_0reserve()),
);
let holder_selected_channel_reserve_satoshis =
get_holder_selected_channel_reserve_satoshis(
msg.common_fields.funding_satoshis,
msg.common_fields.dust_limit_satoshis,
config,
trusted_channel_features.is_some_and(|f| f.is_0reserve()),
)
.map_err(|()| {
ChannelError::close(format!(
"The channel value {} is smaller than either their dust \
limit {}, or {MIN_THEIR_CHAN_RESERVE_SATOSHIS}",
msg.common_fields.funding_satoshis, msg.common_fields.dust_limit_satoshis,
))
})?;
let counterparty_pubkeys = ChannelPublicKeys {
funding_pubkey: msg.common_fields.funding_pubkey,
revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint),
Expand Down Expand Up @@ -15117,8 +15154,13 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
});

let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve()));

funding_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, trusted_channel_features.is_some_and(|f| f.is_0reserve())
).map_err(|()| APIError::APIMisuseError {
err: format!(
"The channel value {funding_satoshis} is smaller than their dust \
limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"
)
})?;
let funding_feerate_sat_per_1000_weight = fee_estimator.bounded_sat_per_1000_weight(funding_confirmation_target);
let funding_tx_locktime = LockTime::from_height(current_chain_height)
.map_err(|_| APIError::APIMisuseError {
Expand Down Expand Up @@ -15257,9 +15299,16 @@ impl<SP: SignerProvider> PendingV2Channel<SP> {
let channel_value_satoshis =
our_funding_contribution_sats.saturating_add(msg.common_fields.funding_satoshis);
let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some());
channel_value_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS, msg.disable_channel_reserve.is_some()
).map_err(|()| ChannelError::close(format!(
"The channel value {channel_value_satoshis} is smaller than our dust limit {MIN_CHAN_DUST_LIMIT_SATOSHIS}"
)))?;
let their_dust_limit_satoshis = msg.common_fields.dust_limit_satoshis;
let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
channel_value_satoshis, msg.common_fields.dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve()));
channel_value_satoshis, their_dust_limit_satoshis, trusted_channel_features.is_some_and(|f| f.is_0reserve())
).map_err(|()| ChannelError::close(format!(
"The channel value {channel_value_satoshis} is smaller than their dust limit {their_dust_limit_satoshis}"
)))?;

let channel_type = channel_type_from_open_channel(&msg.common_fields, our_supported_features)?;

Expand Down Expand Up @@ -17450,6 +17499,10 @@ mod tests {
// to channel value
test_self_and_counterparty_channel_reserve(10_000_000, 0.50, 0.50);
test_self_and_counterparty_channel_reserve(10_000_000, 0.60, 0.50);

// Make sure we correctly handle reserves greater than the channel value
test_self_and_counterparty_channel_reserve(100_000, 1.1, 0.30);
test_self_and_counterparty_channel_reserve(100_000, 0.30, 1.1);
}

#[rustfmt::skip]
Expand All @@ -17469,7 +17522,19 @@ mod tests {
outbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (outbound_selected_channel_reserve_perc * 1_000_000.0) as u32;
let mut chan = OutboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, outbound_node_id, &channelmanager::provided_init_features(&outbound_node_config), channel_value_satoshis, 100_000, 42, &outbound_node_config, 0, 42, None, &logger, None).unwrap();

let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * outbound_selected_channel_reserve_perc) as u64);
let outbound_capped_reserve_perc = if outbound_selected_channel_reserve_perc.lt(&1.0) {
outbound_selected_channel_reserve_perc
} else {
1.0
};

let inbound_capped_reserve_perc = if inbound_selected_channel_reserve_perc.lt(&1.0) {
inbound_selected_channel_reserve_perc
} else {
1.0
};

let expected_outbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * outbound_capped_reserve_perc) as u64);
assert_eq!(chan.funding.holder_selected_channel_reserve_satoshis, expected_outbound_selected_chan_reserve);

let chan_open_channel_msg = chan.get_open_channel(ChainHash::using_genesis_block(network), &&logger).unwrap();
Expand All @@ -17479,7 +17544,7 @@ mod tests {
if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 {
let chan_inbound_node = InboundV1Channel::<&TestKeysInterface>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, None).unwrap();

let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * inbound_selected_channel_reserve_perc) as u64);
let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.funding.get_value_satoshis() as f64 * inbound_capped_reserve_perc) as u64);

assert_eq!(chan_inbound_node.funding.holder_selected_channel_reserve_satoshis, expected_inbound_selected_chan_reserve);
assert_eq!(chan_inbound_node.funding.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve);
Expand Down
14 changes: 11 additions & 3 deletions lightning/src/ln/channel_open_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use crate::chain::{self, ChannelMonitorUpdateStatus};
use crate::events::{ClosureReason, Event, FundingInfo};
use crate::ln::channel::{
get_holder_selected_channel_reserve_satoshis, ChannelError, InboundV1Channel,
OutboundV1Channel, COINBASE_MATURITY, UNFUNDED_CHANNEL_AGE_LIMIT_TICKS,
OutboundV1Channel, COINBASE_MATURITY, MIN_THEIR_CHAN_RESERVE_SATOSHIS,
UNFUNDED_CHANNEL_AGE_LIMIT_TICKS,
};
use crate::ln::channelmanager::{
self, TrustedChannelFeatures, BREAKDOWN_TIMEOUT, MAX_UNFUNDED_CHANNEL_PEERS,
Expand Down Expand Up @@ -473,7 +474,8 @@ pub fn test_insane_channel_opens() {
// funding satoshis
let channel_value_sat = 31337; // same as funding satoshis
let channel_reserve_satoshis =
get_holder_selected_channel_reserve_satoshis(channel_value_sat, 0, &legacy_cfg, false);
get_holder_selected_channel_reserve_satoshis(channel_value_sat, 0, &legacy_cfg, false)
.unwrap();
let push_msat = (channel_value_sat - channel_reserve_satoshis) * 1000;

// Have node0 initiate a channel to node1 with aforementioned parameters
Expand Down Expand Up @@ -552,7 +554,13 @@ pub fn test_insane_channel_opens() {
},
);

insane_open_helper("Peer never wants payout outputs?", |mut msg| {
let crazy_dust_limit = channel_value_sat + 1;
let expected_error_str = format!(
"Got non-closing error: The channel value \
{channel_value_sat} is smaller than either their dust limit {crazy_dust_limit}, or \
{MIN_THEIR_CHAN_RESERVE_SATOSHIS}"
);
insane_open_helper(&expected_error_str, |mut msg| {
msg.common_fields.dust_limit_satoshis = msg.common_fields.funding_satoshis + 1;
msg
});
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,8 @@ pub fn test_inbound_outbound_capacity_is_not_zero() {
assert_eq!(channels0.len(), 1);
assert_eq!(channels1.len(), 1);

let reserve = get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false);
let reserve =
get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false).unwrap();
assert_eq!(channels0[0].inbound_capacity_msat, 95000000 - reserve * 1000);
assert_eq!(channels1[0].outbound_capacity_msat, 95000000 - reserve * 1000);

Expand Down
15 changes: 9 additions & 6 deletions lightning/src/ln/htlc_reserve_unit_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@ fn do_test_counterparty_no_reserve(send_from_initiator: bool) {
push_amt -= feerate_per_kw as u64
* (commitment_tx_base_weight(&channel_type_features) + 4 * COMMITMENT_TX_WEIGHT_PER_HTLC)
/ 1000 * 1000;
push_amt -=
get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false)
.unwrap()
* 1000;

let push = if send_from_initiator { 0 } else { push_amt };
let temp_channel_id =
Expand Down Expand Up @@ -1002,8 +1003,9 @@ pub fn test_chan_reserve_violation_outbound_htlc_inbound_chan() {
&channel_type_features,
);

push_amt -=
get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false)
.unwrap()
* 1000;

let _ = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, push_amt);

Expand Down Expand Up @@ -1048,8 +1050,9 @@ pub fn test_chan_reserve_dust_inbound_htlcs_outbound_chan() {
MIN_AFFORDABLE_HTLC_COUNT as u64,
&channel_type_features,
);
push_amt -=
get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false) * 1000;
push_amt -= get_holder_selected_channel_reserve_satoshis(100_000, 0, &default_config, false)
.unwrap()
* 1000;
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, push_amt);

let (htlc_success_tx_fee_sat, _) =
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/payment_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5043,7 +5043,7 @@ fn test_htlc_forward_considers_anchor_outputs_value() {
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, CHAN_AMT, PUSH_MSAT);

let channel_reserve_msat =
get_holder_selected_channel_reserve_satoshis(CHAN_AMT, 0, &config, false) * 1000;
get_holder_selected_channel_reserve_satoshis(CHAN_AMT, 0, &config, false).unwrap() * 1000;
let commitment_fee_msat = chan_utils::commit_tx_fee_sat(
*nodes[1].fee_estimator.sat_per_kw.lock().unwrap(),
2,
Expand Down
Loading
Loading