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

Jump to bottom

Open

yashbhutwala wants to merge 1 commit into lightningdevkit:main from yashbhutwala:expose-channel-reserve-satoshis

+102 −15

yashbhutwala commented Jan 16, 2026

Summary

Add channel_reserve_satoshis: Option<u64> field to ChannelParameters struct
For V1 channels: Returns Some(value) with the explicit reserve from open_channel message
For V2 channels: Returns None since reserve is calculated from total channel value (initiator + acceptor funding), which isn't known at OpenChannelRequest time
Move channel_parameters() from CommonOpenChannelFields to individual OpenChannel and OpenChannelV2 impls to handle V1/V2 difference correctly
Add test test_open_channel_request_channel_reserve_satoshis to verify the implementation

Fixes #3909

Revives the closed PR #3910 with the approach suggested by reviewers:

Uses Option<u64> instead of trying to read from wire for V2 (which doesn't have this field)
Follows @wpaulino's suggestion to move channel_parameters() to OpenChannel/OpenChannelV2
Properly documents the V1 vs V2 behavior

Test plan

cargo check -p lightning passes
All 1191 lightning tests pass (cargo test -p lightning --lib)
New test test_open_channel_request_channel_reserve_satoshis validates V1 channels correctly populate the field
All 41 channel_open_tests pass

🤖 Generated with Claude Code

ldk-reviews-bot commented Jan 16, 2026 •

edited

Loading

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-reviews-bot requested a review from valentinewallace

January 16, 2026 16:23

yashbhutwala force-pushed the expose-channel-reserve-satoshis branch from cd475ee to 2f585af Compare

January 16, 2026 16:28

yashbhutwala mentioned this pull request

feat: expose channel_reserve_satoshis via ChannelParameters #3910

Closed

codecov bot commented Jan 16, 2026 •

edited

Loading

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.08%. Comparing base (9e91b2e) to head (48a70a8).

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4319      +/-   ##
==========================================
- Coverage   86.08%   86.08%   -0.01%     
==========================================
  Files         156      156              
  Lines      102428   102433       +5     
  Branches   102428   102433       +5     
==========================================
+ Hits        88179    88183       +4     
- Misses      11754    11757       +3     
+ Partials     2495     2493       -2

Flag	Coverage Δ
tests	`86.08% <93.75%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tankyleo self-requested a review

January 16, 2026 18:14

ldk-reviews-bot commented Jan 19, 2026

🔔 1st Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

1 similar comment

ldk-reviews-bot commented Jan 19, 2026

🔔 1st Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot commented Jan 21, 2026

🔔 2nd Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

1 similar comment

ldk-reviews-bot commented Jan 21, 2026

🔔 2nd Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Author

yashbhutwala commented Jan 21, 2026

@tnull @dunxen @TheBlueMatt please review 🙏

ldk-reviews-bot commented Jan 24, 2026

🔔 3rd Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

1 similar comment

ldk-reviews-bot commented Jan 24, 2026

🔔 3rd Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot commented Jan 26, 2026

🔔 4th Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

1 similar comment

ldk-reviews-bot commented Jan 26, 2026

🔔 4th Reminder

Hey @valentinewallace @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

valentinewallace reviewed

View reviewed changes

Contributor

valentinewallace left a comment

Basically looks good. Probably good if @tankyleo takes a look since he's working on channel reserve stuff

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

yashbhutwala force-pushed the expose-channel-reserve-satoshis branch from 2f585af to e8cabf3 Compare

January 27, 2026 18:56


          Expose channel_reserve_satoshis via ChannelParameters

48a70a8

Add `channel_reserve_satoshis: Option<u64>` to `ChannelParameters` so
users can access the counterparty's channel reserve when handling the
`OpenChannelRequest` event.

For V1 channels (`open_channel`), this returns `Some(value)` with the
explicit reserve from the message.

For V2 channels (`open_channel2`), this returns `None` because the
reserve is calculated as `max(1% of total_channel_value, dust_limit)`
per spec, where total_channel_value includes both parties' funding.
Since the acceptor's contribution is unknown at `OpenChannelRequest`
time, the final reserve cannot be determined.

Move `channel_parameters()` from `CommonOpenChannelFields` to separate
implementations on `OpenChannel` and `OpenChannelV2` to handle the
V1/V2 difference correctly.

Fixes lightningdevkit#3909

yashbhutwala force-pushed the expose-channel-reserve-satoshis branch from e8cabf3 to 48a70a8 Compare

January 27, 2026 19:00

yashbhutwala requested a review from valentinewallace

January 27, 2026 19:52

tankyleo reviewed

View reviewed changes

Contributor

tankyleo left a comment

Thanks for the PR excuse the delay on my side ! I'll ping tnull as well since he is a consumer of this API and opened the issue in the first place

lightning/src/ln/channelmanager.rs

+              	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),

Contributor

tankyleo Jan 27, 2026

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

lightning/src/ln/channel_open_tests.rs

    
              			// For V1 channels, channel_reserve_satoshis should be Some with the value from the message

              			assert_eq!(

              				params.channel_reserve_satoshis,

              				Some(expected_reserve),

Contributor

tankyleo Jan 27, 2026

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

lightning/src/ln/msgs.rs

Comment on lines +266 to +269

    
              	/// 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.

Contributor

tankyleo Jan 27, 2026

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).

lightning/src/ln/msgs.rs

    
              	/// the final reserve value cannot be determined at that point.

              	///

              	/// [`Event::OpenChannelRequest`]: crate::events::Event::OpenChannelRequest

              	pub channel_reserve_satoshis: Option<u64>,

Contributor

tankyleo Jan 27, 2026 •

edited

Loading

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 ?

tankyleo requested a review from tnull

January 27, 2026 21:07

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet