Support manually selecting inputs consuming their entire value by wpaulino · Pull Request #4575 · lightningdevkit/rust-lightning

wpaulino · 2026-04-23T17:08:48Z

This commit introduces an alternative way of splicing in funds without coin selection by requiring the full UTXO to be provided. Each UTXO's entire value (minus fees) is allocated towards the channel, which provides unified balance wallets a more intuitive API when splicing funds into the channel, as they don't particularly care about maintaining a portion of their balance onchain.

To simplify the implementation, we require that contributions are not allowed to mix coin-selected inputs with manually-selected ones. Users will need to start a fresh contribution if they want to change the funding input mode.

ldk-reviews-bot · 2026-04-23T17:08:52Z

🎉 This PR is now ready for review!
Please choose at least one reviewer by assigning them on the right bar.
If no reviewers are assigned within 10 minutes, I'll automatically assign one.
Once the first reviewer has submitted a review, a second will be assigned if required.

ldk-claude-review-bot · 2026-04-23T17:16:47Z

 		);

-		if !self.inputs.is_empty() {
+		if !self.inputs.is_empty() && self.input_mode == Some(FundingInputMode::CoinSelected) {


Bug: Backwards compatibility issue with persisted FundingContribution objects.

FundingContribution is persisted in PendingFunding.contributions (channel.rs:2917). When deserializing contributions created before this PR, input_mode will be None (it's a new option TLV field). Old coin-selected contributions with inputs will have input_mode == None, causing this condition to be false even though they should take the coin-selected branch.

This causes two problems for old persisted coin-selected contributions:

Wrong fee buffer calculation: Uses holder_balance + net_value_without_fee instead of estimated_fee + change_value, potentially allowing or rejecting feerate adjustments incorrectly.

Change output silently dropped: compute_feerate_adjustment returns None for change, and at_feerate sets change_output = None, losing the change value.

This is reachable via for_acceptor_at_feerate / for_initiator_at_feerate called on contributions loaded from pending_splice.contributions (channel.rs lines 12504, 12944, 13127, 13145).

Fix: use self.input_mode != Some(FundingInputMode::Manual) instead of self.input_mode == Some(FundingInputMode::CoinSelected) to preserve old behavior for contributions where input_mode is None:

Suggested change

if !self.inputs.is_empty() && self.input_mode == Some(FundingInputMode::CoinSelected) {

if !self.inputs.is_empty() && self.input_mode != Some(FundingInputMode::Manual) {

There is no backwards compatibility concern because the serialized object has not been included in a release yet.

ldk-claude-review-bot · 2026-04-23T17:17:33Z

+		if let Some(PriorContribution { contribution: prior_contribution, .. }) =
+			self.prior_contribution.as_ref()
+		{
+			if prior_contribution.input_mode == Some(FundingInputMode::CoinSelected)


Minor: Same backwards-compat pattern as line 880. Old persisted coin-selected contributions will have input_mode == None, so this guard won't fire for them. In practice this is mostly mitigated by the check at line 1309 (value_added > 0 && manually_selected_inputs non-empty), but it could miss edge cases where the old prior had value_added() == 0 (inputs exactly covered outputs + fees).

Consider using != Some(FundingInputMode::Manual) combined with a non-empty inputs check:

Suggested change

if prior_contribution.input_mode == Some(FundingInputMode::CoinSelected)

if prior_contribution.input_mode != Some(FundingInputMode::Manual)

&& !prior_contribution.inputs.is_empty()

ldk-claude-review-bot · 2026-04-23T17:18:44Z

I've completed a thorough review of the entire PR diff. The two backwards-compatibility issues flagged in the prior review are still present and are the only material bugs. No new issues were found.

Review Summary

Previously Flagged Issues (still open)

These are the only issues in this PR and were already flagged in the prior review:

lightning/src/ln/funding.rs:866 — Old persisted FundingContribution objects have input_mode == None, causing == Some(CoinSelected) to route them into the wrong branch of compute_feerate_adjustment. Should use != Some(FundingInputMode::Manual).
lightning/src/ln/funding.rs:1307 — Same backwards-compat pattern for the guard preventing manual inputs on a coin-selected prior. Old priors with input_mode == None bypass this check.

New Issues

No new issues found. The rest of the implementation is correct:

The ManuallySelected flow through amend_without_coin_selection correctly replaces inputs, drops change output, and adjusts feerate.
The fee buffer calculation for manual inputs in compute_feerate_adjustment (lines 913-934) correctly uses net_value_without_fee + holder_balance as the available buffer.
splice_in_inputs correctly appends to prior manual inputs via FundingBuilder::new copying prior inputs + add_inputs extending.
Validation properly rejects mixing manual inputs with coin-selected value (line 1300) and manual inputs on coin-selected priors (line 1306-1313).
ManuallySelectedInputsInsufficient errors do not fall through to coin selection in AsyncFundingBuilder::build / SyncFundingBuilder::build.
The spliceable_balance check in try_build_without_coin_selection (lines 1234-1243) correctly validates that the channel balance can cover any net-negative contribution.
The input_mode TLV option field (tag 15) is properly serialized and handles None for new contributions without inputs.

codecov · 2026-04-23T18:30:17Z

Codecov Report

❌ Patch coverage is 96.37931% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.20%. Comparing base (2313bd5) to head (228cae8).
⚠️ Report is 16 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/funding.rs	96.28%	12 Missing and 9 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4575      +/-   ##
==========================================
+ Coverage   87.08%   87.20%   +0.11%     
==========================================
  Files         161      161              
  Lines      109255   109662     +407     
  Branches   109255   109662     +407     
==========================================
+ Hits        95147    95626     +479     
+ Misses      11627    11557      -70     
+ Partials     2481     2479       -2

Flag	Coverage Δ
fuzzing-fake-hashes	`31.14% <12.10%> (+0.16%)`	⬆️
fuzzing-real-hashes	`22.89% <0.00%> (+0.23%)`	⬆️
tests	`86.26% <96.03%> (+0.09%)`	⬆️

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.

Alkamal01 · 2026-04-23T18:41:13Z

The root problem seems to be relying on == Some(CoinSelected) for persisted data. For legacy contributions, None effectively behaved like coin selection when inputs were present, so switching to != Some(Manual) (with an inputs non-empty check where needed) would preserve the old behavior.

One additional edge case in request_matches_prior: (Some(CoinSelected), None) currently falls into _ => false, so an RBF with the same value on a legacy prior won't be treated as a match and skips the feerate-adjustment path. Not strictly a correctness issue, but it becomes inconsistent if the other branches get fixed.

So overall, it'd be good to make the handling of None consistent across all three paths.

TheBlueMatt

All LGTM. One question.

TheBlueMatt · 2026-04-24T00:34:59Z

 				self.feerate,
 			);
+			if !inputs.is_empty() {
+				total_input_value


Should this not include the channel's spliceable balance and outputs as well?

This is for a fresh contribution where the spliceable balance is not available (either because they discarded the previous contribution or there wasn't one at all). The balance will get checked once ChannelManager::funding_contributed is called.

Its weird that the spliceable-balance is in priorcontribution and not in the FundingTemplate...

ldk-reviews-bot · 2026-04-27T00:00:09Z

🔔 1st Reminder

Hey @jkczyz! 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.

This commit introduces an alternative way of splicing in funds without coin selection by requiring the full UTXO to be provided. Each UTXO's entire value (minus fees) is allocated towards the channel, which provides unified balance wallets a more intuitive API when splicing funds into the channel, as they don't particularly care about maintaining a portion of their balance onchain. To simplify the implementation, we require that contributions are not allowed to mix coin-selected inputs with manually-selected ones. Users will need to start a fresh contribution if they want to change the funding input mode.

There's no reason not to do so, and it allows us to fail earlier when the user's net contribution exceeds their spliceable balance.

ldk-reviews-bot · 2026-04-29T00:01:00Z

🔔 2nd Reminder

Hey @jkczyz! 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.

wpaulino added this to the 0.3 milestone Apr 23, 2026

wpaulino requested review from TheBlueMatt and jkczyz April 23, 2026 17:08

wpaulino self-assigned this Apr 23, 2026

wpaulino marked this pull request as ready for review April 23, 2026 17:08

ldk-claude-review-bot reviewed Apr 23, 2026

View reviewed changes

wpaulino force-pushed the funding-contribution-builder-manual-inputs branch from 12e5994 to 412ec3d Compare April 23, 2026 17:19

TheBlueMatt reviewed Apr 24, 2026

View reviewed changes

wpaulino added 2 commits April 28, 2026 14:43

Include spliceable balance in every FundingTemplate

228cae8

There's no reason not to do so, and it allows us to fail earlier when the user's net contribution exceeds their spliceable balance.

wpaulino force-pushed the funding-contribution-builder-manual-inputs branch from 412ec3d to 228cae8 Compare April 28, 2026 22:00

	if !self.inputs.is_empty() && self.input_mode == Some(FundingInputMode::CoinSelected) {
	if !self.inputs.is_empty() && self.input_mode != Some(FundingInputMode::Manual) {

	if prior_contribution.input_mode == Some(FundingInputMode::CoinSelected)
	if prior_contribution.input_mode != Some(FundingInputMode::Manual)
	&& !prior_contribution.inputs.is_empty()

Conversation

wpaulino commented Apr 23, 2026

Uh oh!

ldk-reviews-bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-claude-review-bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

wpaulino Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-claude-review-bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Summary

Previously Flagged Issues (still open)

New Issues

Uh oh!

codecov Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Alkamal01 commented Apr 23, 2026

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

wpaulino Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Apr 27, 2026

Uh oh!

ldk-reviews-bot commented Apr 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ldk-reviews-bot commented Apr 23, 2026 •

edited

Loading

ldk-claude-review-bot commented Apr 23, 2026 •

edited

Loading

codecov Bot commented Apr 23, 2026 •

edited

Loading