Skip to content

Refactor CoinSelection-related types to wallet_utils module#4382

Draft
jkczyz wants to merge 10 commits intolightningdevkit:mainfrom
jkczyz:2026-01-wallet-utils
Draft

Refactor CoinSelection-related types to wallet_utils module#4382
jkczyz wants to merge 10 commits intolightningdevkit:mainfrom
jkczyz:2026-01-wallet-utils

Conversation

@jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Feb 4, 2026

CoinSelection is used for both anchor bumping and splicing. Move all related types to a dedicated module now that they aren't strictly used for anchor bumping.

Based on #4290.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 4, 2026

👋 Hi! This PR is now in draft status.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

@jkczyz jkczyz requested a review from TheBlueMatt February 4, 2026 15:35
@jkczyz jkczyz marked this pull request as draft February 4, 2026 16:01
@jkczyz
Copy link
Contributor Author

jkczyz commented Feb 4, 2026

Converting to draft given the dependent PR is still under review.

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 75.72634% with 259 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.91%. Comparing base (3497b59) to head (4048cd8).

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 56.18% 83 Missing and 2 partials ⚠️
lightning/src/util/wallet_utils.rs 80.00% 65 Missing and 7 partials ⚠️
lightning/src/ln/funding.rs 83.47% 41 Missing and 18 partials ⚠️
lightning/src/ln/channelmanager.rs 52.00% 36 Missing ⚠️
lightning/src/events/bump_transaction/mod.rs 92.30% 0 Missing and 3 partials ⚠️
lightning/src/ln/functional_test_utils.rs 50.00% 3 Missing ⚠️
lightning/src/util/ser.rs 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4382      +/-   ##
==========================================
- Coverage   86.09%   85.91%   -0.19%     
==========================================
  Files         156      157       +1     
  Lines      103623   103969     +346     
  Branches   103623   103969     +346     
==========================================
+ Hits        89211    89320     +109     
- Misses      11895    12123     +228     
- Partials     2517     2526       +9     
Flag Coverage Δ
tests 85.91% <75.72%> (-0.19%) ⬇️

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.

@jkczyz jkczyz force-pushed the 2026-01-wallet-utils branch from 0276889 to ced9273 Compare February 5, 2026 15:46
@jkczyz jkczyz self-assigned this Feb 5, 2026
@jkczyz jkczyz force-pushed the 2026-01-wallet-utils branch 4 times, most recently from 6daa738 to b19030b Compare February 12, 2026 16:01
jkczyz and others added 10 commits February 13, 2026 15:07
A forthcoming commit will change CoinSelection to include FundingTxInput
instead of Utxo, though the former will probably be renamed. This is so
CoinSelectionSource can be used when funding a splice. Further updating
WalletSource to use FundingTxInput is not desirable, however, as it
would result in looking up each confirmed UTXOs previous transaction
even if it is not selected. See Wallet's implementation of
CoinSelectionSource, which delegates to WalletSource for listing all
confirmed UTXOs.

This commit moves FundingTxInput::sequence to Utxo, and thus the
responsibility for setting it to WalletSource implementations. Doing so
will allow Wallet's CoinSelectionSource implementation to delegate
looking up previous transactions to WalletSource without having to
explicitly set the sequence on any FundingTxInput.
In order to reuse CoinSelectionSource for splicing, the previous
transaction of each UTXO is needed. Update CoinSelection to use
FundingTxInput (renamed to ConfirmedUtxo) so that it is available.

This requires adding a method to WalletSource to look up a previous
transaction for a UTXO. Otherwise, Wallet's implementation of
CoinSelectionSource would need WalletSource to include the previous
transactions when listing confirmed UTXOs to select from. But this would
be inefficient since only some UTXOs are selected.
CoinSelectionSource is used for anchor bumping where a ClaimId is passed
in to avoid double spending other claims. To re-use this trait for
funding a splice, the ClaimId must be optional. And, if None, then any
locked UTXOs may be considered ineligible by an implementation.
Previously, splice_channel required callers to manually construct
funding inputs and pass them directly, making coin selection the
caller's responsibility. This made the API difficult to use and
prevented reuse of the existing CoinSelectionSource trait.

Introduce a two-phase API: splice_channel now returns a FundingTemplate
that callers use to build a FundingContribution via wallet-backed
splice methods (e.g., splice_in_sync, splice_out_sync), which handle
coin selection automatically. The completed contribution is then passed
to a new funding_contributed method to begin quiescence and negotiation.

This also renames SpliceContribution to FundingContribution and moves
fee estimation and input validation into the funding module, co-located
with the types they operate on.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Now that CoinSelection is used to fund a splice funding transaction, use
that for determining of a change output should be used. Previously, the
initiator could either provide a change script upfront or let LDK
generate one using SignerProvider::get_destination_script.

Since older versions may have serialized a SpliceInstruction without a
change script while waiting on quiescence, LDK must still generate a
change output in this case.
Instead of logging both inside propose_quiescence and at the call site,
only log inside it. This simplifies the return type.
Wallet-related types were tightly coupled to bump_transaction, making
them less accessible for other use cases like channel funding and
splicing. Extract these utilities to a dedicated module for improved
code organization and reusability across the codebase.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Synchronous wallet utilities were coupled to bump_transaction::sync,
limiting their reusability for other features like channel funding and
splicing which need synchronous wallet operations. Consolidate all
wallet utilities in a single module for consistency and improved code
organization.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
FundingTxInput was originally designed for channel funding but is now
used more broadly for coin selection and splicing. The name
ConfirmedUtxo better reflects its general-purpose nature as a confirmed
UTXO with previous transaction data. Make ConfirmedUtxo the real struct
in wallet_utils and alias FundingTxInput to it for backward
compatibility.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@jkczyz jkczyz force-pushed the 2026-01-wallet-utils branch from b19030b to 4048cd8 Compare February 14, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants