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

👋 Thanks for assigning @TheBlueMatt 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-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

Review Summary

New Issue

lightning/src/ln/funding.rs:1199 — self.funding_inputs.take() in build_from_prior_contribution destroys the requested value_added, causing the coin selection fallback in AsyncFundingBuilder::build / SyncFundingBuilder::build to request 0 sats instead of the user's amount. This is a regression from the pre-PR code where self.value_added was a Copy field that survived the amend attempt.

Previously Flagged Issues (still open)

lightning/src/ln/funding.rs:903 — Backwards-compat: old persisted coin-selected contributions have input_mode == None, routing them into the manual/no-inputs branch of compute_feerate_adjustment.
lightning/src/ln/funding.rs:1318 — Same backwards-compat pattern in the builder initialization: old coin-selected priors bypass the guard preventing manual inputs from being mixed onto a coin-selected prior.

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

Codecov Report

❌ Patch coverage is 93.91979% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (b64efcd) to head (6e16493).
⚠️ Report is 23 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/funding.rs	94.98%	22 Missing and 16 partials ⚠️
lightning/src/ln/channel.rs	43.75%	8 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4575      +/-   ##
==========================================
- Coverage   86.22%   86.16%   -0.07%     
==========================================
  Files         159      157       -2     
  Lines      109170   109318     +148     
  Branches   109170   109318     +148     
==========================================
+ Hits        94136    94191      +55     
- Misses      12424    12498      +74     
- Partials     2610     2629      +19

Flag	Coverage Δ
tests	`86.16% <93.91%> (-0.07%)`	⬇️

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.

TheBlueMatt

All LGTM. One question.

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.

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.

TheBlueMatt

thanks

elnosh

Not that familiar with splicing-related code in the codebase but looking at this for learning purposes. After much staring, changes make sense although agree with @jkczyz comment on making an enum since those fields (value_added and manually_selected_inputs) are mutually exclusive in the ways inputs can be provided.

elnosh · 2026-04-30T22:25:07Z

+/// used if the request cannot be satisfied by reusing a prior contribution, by using only
+/// manually selected inputs, or by building a pure splice-out directly.


I'm not sure I follow this comment. If we have a builder with either sync or async CoinSelectionSource then manually added inputs are not allowed.

Manually selected inputs are still allowed as long as the added_value is 0.

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.

While this is already enforced when we get to the interactive negotiation phase, we choose to fail early anyway.

ldk-claude-review-bot · 2026-05-06T19:34:01Z

 		return contribution
 			.amend_without_coin_selection(
-				funding_inputs,
+				self.funding_inputs.take(),


Bug: self.funding_inputs.take() destroys the requested value_added, causing the coin selection fallback to request 0 sats.

When amend_without_coin_selection returns None (prior's inputs can't satisfy the amended request), this method returns Err(MissingCoinSelectionSource). At that point self.funding_inputs is None because of .take(). The AsyncFundingBuilder::build / SyncFundingBuilder::build fallback then reads:

let value_added = inner.funding_inputs.as_ref().map_or(Amount::ZERO, FundingInputs::value_added); let (must_spend, must_pay_to) = inner.prepare_coin_selection_request(value_added)?;

Since funding_inputs is None, value_added is 0, and coin selection runs asking for 0 sats added instead of the user's requested amount.

Scenario: User has a prior coin-selected contribution adding 50k sats with a 100k input. They call add_value(100k) to increase to 150k total. request_matches_prior is false (values differ). amend_without_coin_selection fails because the 100k input can't cover 150k. The fallback runs coin selection with value_added = 0 instead of 150k.

This is a regression — the pre-PR code used self.value_added (a Copy field) which was never consumed.

Fix: use .clone() instead of .take() so funding_inputs is preserved for the fallback path:

Suggested change

self.funding_inputs.take(),

self.funding_inputs.clone(),

Hmmm.. could we get around this by taking self instead of &mut self in build_from_prior_contribution and try_build_without_coin_selection?

No because we first call build_without_coin_selection (which calls try_build_without_coin_selection) in AsyncFundingBuilder::build so we need to keep the state around for when we fall back to coin selection.

Ah right. Is this an actual bug then?

It is yeah, don't see a nice way of fixing this aside from just restoring the value_added if we fall back to coin selection.

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

Comment thread lightning/src/ln/funding.rs Outdated

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

wpaulino requested a review from TheBlueMatt April 29, 2026 16:58

TheBlueMatt previously approved these changes Apr 29, 2026

View reviewed changes

jkczyz reviewed Apr 30, 2026

View reviewed changes

Comment thread lightning/src/ln/funding.rs Outdated

Comment thread lightning/src/ln/funding.rs Outdated

Comment thread lightning/src/ln/funding.rs Outdated

Comment thread lightning/src/ln/funding.rs Outdated

elnosh reviewed Apr 30, 2026

View reviewed changes

wpaulino dismissed TheBlueMatt’s stale review via 17c864b May 5, 2026 23:36

wpaulino force-pushed the funding-contribution-builder-manual-inputs branch 2 times, most recently from 17c864b to c3da7fe Compare May 5, 2026 23:43

wpaulino added 3 commits May 6, 2026 12:27

Include spliceable balance in every FundingTemplate

cc0f81b

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.

Detect duplicate inputs and outputs upon building FundingBuilder

6e16493

While this is already enforced when we get to the interactive negotiation phase, we choose to fail early anyway.

wpaulino force-pushed the funding-contribution-builder-manual-inputs branch from c3da7fe to 6e16493 Compare May 6, 2026 19:27

ldk-claude-review-bot reviewed May 6, 2026

View reviewed changes

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

		/// used if the request cannot be satisfied by reusing a prior contribution, by using only
		/// manually selected inputs, or by building a pure splice-out directly.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

New Issue

Previously Flagged Issues (still open)

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!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ldk-reviews-bot commented Apr 27, 2026

Uh oh!

ldk-reviews-bot commented Apr 29, 2026

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elnosh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 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