Error if the calculated v1 reserve is greater than the channel value
What changed, and why it matters
This commit tightens how Lightning Dev Kit calculates the channel reserve for older-style (v1) channels. The reserve is a portion of channel funds that must stay untouched to guarantee both parties can pay penalties if someone cheats. Previously, a misconfigured or malicious proportion could make the calculated reserve exceed the entire channel value, or a tiny channel/dust limit could produce a nonsensical reserve. Now the code rejects those cases outright and also caps the proportional reserve at 100%. The change is defensive: it prevents opening channels with impossible reserve settings rather than silently accepting them.
Treat this as a hardening fix rather than an active vulnerability. Review the preceding v2 reserve commit for consistency, run the updated test suite, and consider whether any downstream callers or bindings need to handle the new `Result` return type. No immediate incident response is indicated by the diff alone.
Security signals we found
Input validation added to channel reserve calculation
Proportional reserve capped at 100% to prevent reserve exceeding channel value
Error returned instead of silently clamping reserve to channel value
Dust limit and minimum reserve sanity checks added
Tests added for reserve greater than channel value
Evidence from the diff
The patch changes get_holder_selected_channel_reserve_satoshis in lightning/src/ln/channel.rs to return Result<u64, ()> instead of u64. It errors when channel_value_satoshis is smaller than MIN_THEIR_CHAN_RESERVE_SATOSHIS or the counterparty’s dust_limit_satoshis. It also caps their_channel_reserve_proportional_millionths at 1,000,000 (100%) and removes the previous cmp::min(channel_value_satoshis, channel_reserve_satoshis) clamp, because the function now refuses invalid inputs. Callers in OutboundV1Channel::new and InboundV1Channel::new convert the error into an APIError::APIMisuseError or a ChannelError::close, respectively. Tests are updated to expect the new error and to verify the 100% cap. The commit message notes this mirrors a v2 reserve fix made in the immediately preceding commit.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channel_open_tests.rslightning/src/ln/functional_tests.rslightning/src/ln/htlc_reserve_unit_tests.rslightning/src/ln/payment_tests.rslightning/src/ln/update_fee_tests.rsInspect captured patch +89 / −33
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index f2c5b3b..137bdd2 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -6761,20 +6761,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.
@@ -14479,12 +14491,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`
@@ -14876,12 +14895,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),
@@ -17483,6 +17510,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]
@@ -17502,7 +17533,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();
@@ -17512,7 +17555,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);
diff --git a/lightning/src/ln/channel_open_tests.rs b/lightning/src/ln/channel_open_tests.rs
index ac4a1b6..50ef072 100644
--- a/lightning/src/ln/channel_open_tests.rs
+++ b/lightning/src/ln/channel_open_tests.rs
@@ -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,
@@ -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
@@ -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
});
diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index c8ecb40..8bbb9b9 100644
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -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);
diff --git a/lightning/src/ln/htlc_reserve_unit_tests.rs b/lightning/src/ln/htlc_reserve_unit_tests.rs
index 45d3cf5..a4d92b7 100644
--- a/lightning/src/ln/htlc_reserve_unit_tests.rs
+++ b/lightning/src/ln/htlc_reserve_unit_tests.rs
@@ -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 =
@@ -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);
@@ -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, _) =
diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs
index 5b4f5f9..ccb933a 100644
--- a/lightning/src/ln/payment_tests.rs
+++ b/lightning/src/ln/payment_tests.rs
@@ -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,
diff --git a/lightning/src/ln/update_fee_tests.rs b/lightning/src/ln/update_fee_tests.rs
index b1f8257..1cb04f1 100644
--- a/lightning/src/ln/update_fee_tests.rs
+++ b/lightning/src/ln/update_fee_tests.rs
@@ -410,7 +410,7 @@ pub fn do_test_update_fee_that_funder_cannot_afford(channel_type_features: Chann
let channel_id = chan.2;
let secp_ctx = Secp256k1::new();
let bs_channel_reserve_sats =
- get_holder_selected_channel_reserve_satoshis(channel_value, 0, &cfg, false);
+ get_holder_selected_channel_reserve_satoshis(channel_value, 0, &cfg, false).unwrap();
let (anchor_outputs_value_sats, outputs_num_no_htlcs) =
if channel_type_features.supports_anchors_zero_fee_htlc_tx() {
(ANCHOR_OUTPUT_VALUE_SATOSHI * 2, 4)
@@ -886,8 +886,9 @@ pub fn test_chan_init_feerate_unaffordability() {
// During open, we don't have a "counterparty channel reserve" to check against, so that
// requirement only comes into play on the open_channel handling side.
- 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;
nodes[0].node.create_channel(node_b_id, 100_000, push_amt, 42, None, None).unwrap();
let mut open_channel_msg =
get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id);
Why this scored 59/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.