Skip to content

Expose channel_reserve_satoshis via ChannelParameters #4319

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

Open
yashbhutwala wants to merge 1 commit into
base: main
Choose a base branch
from
+102 −15
Open

Expose channel_reserve_satoshis via ChannelParameters #4319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 69 additions & 0 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 @@ -1750,6 +1750,75 @@ pub fn test_invalid_funding_tx() {
mine_transaction(&nodes[1], &spend_tx);
}

#[xtest(feature = "_externalize_tests")]
pub fn test_open_channel_request_channel_reserve_satoshis() {
// Test that the `channel_reserve_satoshis` field is correctly populated in the
// `OpenChannelRequest` event's `params` field for V1 channels.
let mut manually_accept_conf = UserConfig::default();
manually_accept_conf.manually_accept_inbound_channels = true;

let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs =
create_node_chanmgrs(2, &node_cfgs, &[None, Some(manually_accept_conf.clone())]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let node_a_id = nodes[0].node.get_our_node_id();
let node_b_id = nodes[1].node.get_our_node_id();

// Create channel with 100,000 sats
nodes[0]
.node
.create_channel(node_b_id, 100_000, 10_001, 42, None, Some(manually_accept_conf))
.unwrap();
let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, node_b_id);

// The channel_reserve_satoshis in the open_channel message is set by the opener
let expected_reserve = open_channel_msg.channel_reserve_satoshis;

nodes[1].node.handle_open_channel(node_a_id, &open_channel_msg);

// Verify the OpenChannelRequest event contains the correct channel_reserve_satoshis
let events = nodes[1].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
match &events[0] {
Event::OpenChannelRequest { temporary_channel_id, params, .. } => {
// For V1 channels, channel_reserve_satoshis should be Some with the value from the message
assert_eq!(
params.channel_reserve_satoshis,
Some(expected_reserve),
Copy link
Contributor

@tankyleo tankyleo Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described elsewhere, let's also add coverage for the V2 case

"channel_reserve_satoshis in OpenChannelRequest params should match the open_channel message"
);

// Verify other params fields are also correctly populated
assert_eq!(
params.dust_limit_satoshis,
open_channel_msg.common_fields.dust_limit_satoshis
);
assert_eq!(
params.max_htlc_value_in_flight_msat,
open_channel_msg.common_fields.max_htlc_value_in_flight_msat
);
assert_eq!(params.htlc_minimum_msat, open_channel_msg.common_fields.htlc_minimum_msat);
assert_eq!(params.to_self_delay, open_channel_msg.common_fields.to_self_delay);
assert_eq!(
params.max_accepted_htlcs,
open_channel_msg.common_fields.max_accepted_htlcs
);

// Accept the channel to clean up
nodes[1]
.node
.accept_inbound_channel(temporary_channel_id, &node_a_id, 0, None)
.unwrap();
},
_ => panic!("Expected OpenChannelRequest event"),
}

// Clear the SendAcceptChannel message event generated by accepting the channel
nodes[1].node.get_and_clear_pending_msg_events();
}

#[xtest(feature = "_externalize_tests")]
pub fn test_coinbase_funding_tx() {
// Miners are able to fund channels directly from coinbase transactions, however
Expand Down
21 changes: 20 additions & 1 deletion lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1682,6 +1682,25 @@ pub(super) enum OpenChannelMessageRef<'a> {
V2(&'a msgs::OpenChannelV2),
}

impl<'a> OpenChannelMessageRef<'a> {
pub(super) fn channel_parameters(&self) -> msgs::ChannelParameters {
let (common_fields, channel_reserve_satoshis) = match self {
Self::V1(msg) => (&msg.common_fields, Some(msg.channel_reserve_satoshis)),
Self::V2(msg) => (&msg.common_fields, None),
Copy link
Contributor

@tankyleo tankyleo Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test suite still passes if I set this to Some(0), let's add coverage for the V2 case.

};
msgs::ChannelParameters {
dust_limit_satoshis: common_fields.dust_limit_satoshis,
max_htlc_value_in_flight_msat: common_fields.max_htlc_value_in_flight_msat,
htlc_minimum_msat: common_fields.htlc_minimum_msat,
commitment_feerate_sat_per_1000_weight: common_fields
.commitment_feerate_sat_per_1000_weight,
to_self_delay: common_fields.to_self_delay,
max_accepted_htlcs: common_fields.max_accepted_htlcs,
channel_reserve_satoshis,
}
}
}

/// A not-yet-accepted inbound (from counterparty) channel. Once
/// accepted, the parameters will be used to construct a channel.
pub(super) struct InboundChannelRequest {
Expand Down Expand Up @@ -10716,7 +10735,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
},
channel_type,
is_announced,
params: common_fields.channel_parameters(),
params: msg.channel_parameters(),
}, None));
peer_state.inbound_channel_request_by_id.insert(channel_id, InboundChannelRequest {
open_channel_msg: match msg {
Expand Down
27 changes: 13 additions & 14 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,20 +244,6 @@ pub struct CommonOpenChannelFields {
pub channel_type: Option<ChannelTypeFeatures>,
}

impl CommonOpenChannelFields {
/// The [`ChannelParameters`] for this channel.
pub fn channel_parameters(&self) -> ChannelParameters {
ChannelParameters {
dust_limit_satoshis: self.dust_limit_satoshis,
max_htlc_value_in_flight_msat: self.max_htlc_value_in_flight_msat,
htlc_minimum_msat: self.htlc_minimum_msat,
commitment_feerate_sat_per_1000_weight: self.commitment_feerate_sat_per_1000_weight,
to_self_delay: self.to_self_delay,
max_accepted_htlcs: self.max_accepted_htlcs,
}
}
}

/// A subset of [`CommonOpenChannelFields`], containing various parameters which are set by the
/// channel initiator and which are not part of the channel funding transaction.
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
Expand All @@ -277,6 +263,19 @@ pub struct ChannelParameters {
pub to_self_delay: u16,
/// The maximum number of pending HTLCs towards the channel initiator.
pub max_accepted_htlcs: u16,
/// The minimum value unencumbered by HTLCs for the counterparty to keep in the channel.
///
/// For V1 channels (`open_channel`), this is the explicit `channel_reserve_satoshis` value
/// from the counterparty.
Comment on lines +266 to +269
Copy link
Contributor

@tankyleo tankyleo Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described above, this is a subset of CommonOpenChannelFields, and the channel_reserve_satoshis of the OpenChannel message actually sets the minimum value that the non-channel-initiator must keep in the channel, and not the counterparty (as long as we define the counterparty here to be the sender of the OpenChannel message).

///
/// For V2 channels (`open_channel2`), this is `None` at the time of [`Event::OpenChannelRequest`]
/// because the channel reserve is calculated as `max(1% of total_channel_value, dust_limit_satoshis)`
/// per the spec, where `total_channel_value` includes both the initiator's and acceptor's funding
/// contributions. Since the acceptor's contribution is not yet known when the event is generated,
/// the final reserve value cannot be determined at that point.
///
/// [`Event::OpenChannelRequest`]: crate::events::Event::OpenChannelRequest
pub channel_reserve_satoshis: Option<u64>,
Copy link
Contributor

@tankyleo tankyleo Jan 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question for @tnull : OpenChannelV2 drops both the channel_reserve_satoshis, and push_msat fields. For the push_msats field, we currently handle this transition in the OpenChannelRequest event with an InboundChannelFunds enum, with PushMsat(u64) as the V1 variant, and DualFunded as the V2 variant.

We also note for InboundChannelFunds that it "allows the differentiation between a request for a dual-funded and non-dual-funded channel."

Seems to me now we can also differentiate V1 vs V2 based on the channel_reserve_satohis field.

To keep things consistent, would it be worth moving push_msats to ChannelParameters too, and set it to None in the V2 case, like we are doing here for channel_reserve_satoshis ?

}

/// An [`open_channel`] message to be sent to or received from a peer.
Expand Down
Loading