Skip to content

feat: expose channel_reserve_satoshis via ChannelParameters #3910

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.

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
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11442,6 +11442,7 @@ where
commitment_feerate_sat_per_1000_weight: self.context.feerate_per_kw as u32,
to_self_delay: self.funding.get_holder_selected_contest_delay(),
max_accepted_htlcs: self.context.holder_max_accepted_htlcs,
channel_reserve_satoshis: self.funding.holder_selected_channel_reserve_satoshis,
funding_pubkey: keys.funding_pubkey,
revocation_basepoint: keys.revocation_basepoint.to_public_key(),
payment_basepoint: keys.payment_point,
Expand Down Expand Up @@ -11978,6 +11979,7 @@ where
commitment_feerate_sat_per_1000_weight: self.context.feerate_per_kw,
to_self_delay: self.funding.get_holder_selected_contest_delay(),
max_accepted_htlcs: self.context.holder_max_accepted_htlcs,
channel_reserve_satoshis: self.funding.holder_selected_channel_reserve_satoshis,
funding_pubkey: keys.funding_pubkey,
revocation_basepoint: keys.revocation_basepoint.to_public_key(),
payment_basepoint: keys.payment_point,
Expand Down
15 changes: 10 additions & 5 deletions lightning/src/ln/channel_acceptance_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,16 @@ fn do_test_manual_inbound_accept_with_override(

nodes[2].node.handle_open_channel(node_a, &open_channel_msg);
let events = nodes[2].node.get_and_clear_pending_events();
match events[0] {
Event::OpenChannelRequest { temporary_channel_id, .. } => nodes[2]
.node
.accept_inbound_channel(&temporary_channel_id, &node_a, 23, config_overrides)
.unwrap(),
match &events[0] {
Event::OpenChannelRequest { temporary_channel_id, params, .. } => {
// Test that channel_reserve_satoshis is properly exposed in Event::OpenChannelRequest
assert_eq!(params.channel_reserve_satoshis, open_channel_msg.channel_reserve_satoshis);

nodes[2]
.node
.accept_inbound_channel(temporary_channel_id, &node_a, 23, config_overrides)
.unwrap()
},
_ => panic!("Unexpected event"),
}
get_event_msg!(nodes[2], MessageSendEvent::SendAcceptChannel, node_a)
Expand Down
10 changes: 10 additions & 0 deletions lightning/src/ln/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ pub struct CommonOpenChannelFields {
pub to_self_delay: u16,
/// The maximum number of inbound HTLCs towards channel initiator
pub max_accepted_htlcs: u16,
/// The minimum value unencumbered by HTLCs for the counterparty to keep in the channel
pub channel_reserve_satoshis: u64,
/// The channel initiator's key controlling the funding transaction
pub funding_pubkey: PublicKey,
/// Used to derive a revocation key for transactions broadcast by counterparty
Expand Down Expand Up @@ -259,6 +261,7 @@ impl CommonOpenChannelFields {
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,
channel_reserve_satoshis: self.channel_reserve_satoshis,
}
}
}
Expand All @@ -282,6 +285,8 @@ 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
pub channel_reserve_satoshis: u64,
}

/// An [`open_channel`] message to be sent to or received from a peer.
Expand Down Expand Up @@ -3016,6 +3021,7 @@ impl LengthReadable for OpenChannel {
commitment_feerate_sat_per_1000_weight,
to_self_delay,
max_accepted_htlcs,
channel_reserve_satoshis,
funding_pubkey,
revocation_basepoint,
payment_basepoint,
Expand Down Expand Up @@ -3072,6 +3078,7 @@ impl LengthReadable for OpenChannelV2 {
let dust_limit_satoshis: u64 = Readable::read(r)?;
let max_htlc_value_in_flight_msat: u64 = Readable::read(r)?;
let htlc_minimum_msat: u64 = Readable::read(r)?;
let channel_reserve_satoshis: u64 = Readable::read(r)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't do this, as OpenChannelV2 doesn't have that field, see https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#the-open_channel2-message

It's also not a 'common' field per se as per spec:

Note that open_channel's channel_reserve_satoshi has been omitted. Instead, the channel reserve is fixed at 1% of the total channel balance (open_channel2.funding_satoshis + accept_channel2.funding_satoshis) rounded down to the nearest whole satoshi or the dust_limit_satoshis, whichever is greater.

Copy link
Contributor

Choose a reason for hiding this comment

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

The best bet would be to have a new top-level field in OpenChannelRequest for channel_reserve_satoshis and then set that in channelmanager like:

channel_reserve_satoshis: match msg {
    OpenChannelMessageRef::V1(msg) => msg.channel_reserve_satoshis,
    OpenChannelMessageRef::V2(msg) => get_v2_channel_reserve_satoshis(
        channel_value_satoshis, msg.common_fields.dust_limit_satoshis
    );
},

The get_v2_channel_reserve_satoshis exists in the channel module. You'd need to make that pub(super).

This would be a breaking change and also would need docs and shouldn't be backported.

Copy link
Contributor

@tnull tnull Jul 4, 2025

Choose a reason for hiding this comment

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

The best bet would be to have a new top-level field in OpenChannelRequest

Hmm, while that would be pretty easy, I'm not super happy about exposing such a detail at the top level API, especially if we have sub-structs in-place. can't we reuse ChannelParameters still, but apply above rules for v2 channels? Or, maybe it would be the right time to refactor OpenChannelRequest (or ChannelParameters) to discern between V1 and V2 channels?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah you could reuse ChannelParameters and just have the field there, update the doc comment for it as it currently mentions being a subset of the common fields. It will just need to be initialised as 0 in CommonOpenChannelFields::channel_parameters and then set like above when creating the OpenChannelRequest event. I'm not sure an Option is worth the effort as it'll always end up being Some() when accessed by the user.

Refactoring is fine, but we'd want to still have a single OpenChannelRequest event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think still at the current state putting the field in ChannelParameters would be a violation of the spec ? Given that if we include it in ChannelParameters it would still be under CommonOpenChannelFields and hence in both OpenChannel v1 and v2 even if we put a dummy value like 0 in the beginning . I think we might be looking at a refactor here if we want to use ChannelParameters .

let to_self_delay: u16 = Readable::read(r)?;
let max_accepted_htlcs: u16 = Readable::read(r)?;
let locktime: u32 = Readable::read(r)?;
Expand Down Expand Up @@ -3103,6 +3110,7 @@ impl LengthReadable for OpenChannelV2 {
commitment_feerate_sat_per_1000_weight,
to_self_delay,
max_accepted_htlcs,
channel_reserve_satoshis,
funding_pubkey,
revocation_basepoint,
payment_basepoint,
Expand Down Expand Up @@ -4708,6 +4716,7 @@ mod tests {
commitment_feerate_sat_per_1000_weight: 821716,
to_self_delay: 49340,
max_accepted_htlcs: 49340,
channel_reserve_satoshis: 8665828695742877976,
funding_pubkey: pubkey_1,
revocation_basepoint: pubkey_2,
payment_basepoint: pubkey_3,
Expand Down Expand Up @@ -4816,6 +4825,7 @@ mod tests {
htlc_minimum_msat: 2316138423780173,
to_self_delay: 49340,
max_accepted_htlcs: 49340,
channel_reserve_satoshis: 8665828695742877976,
funding_pubkey: pubkey_1,
revocation_basepoint: pubkey_2,
payment_basepoint: pubkey_3,
Expand Down
Loading