Skip to content

Conversation

@jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Dec 18, 2025

Rather than requiring the user to pass FundingTxInputs when initiating a splice, generate a FundingNeeded event once the channel has become quiescent. This simplifies error handling and UTXO / change address clean-up by consolidating it in SpliceFailed event handling.

Later, this event will be used for opportunistic contributions (i.e., when the counterparty wins quiescence or initiates), dual-funding, and RBF.

Based on #4261.

This is still fairly rough. It does not yet include any code for creating a FundingNegotiationContext from a FundingContribution. The former may need to a dedicated struct instead so that any data needed from ChannelManager or ChannelContext can be produced internally. Alternatively, that data could be included in FundingContribution, but it would need to be serializable.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Dec 18, 2025

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

@jkczyz jkczyz self-assigned this Dec 18, 2025
@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch from f5933e5 to 854e9ca Compare January 12, 2026 17:47
@jkczyz
Copy link
Contributor Author

jkczyz commented Jan 12, 2026

@TheBlueMatt @wpaulino Looking for some high-level feedback on the API introduced in the last commit. In summary:

  • User passes SpliceContribution -- which no longer contains any FundingTxInputs -- to ChannelManager::splice_channel
  • Upon quiescence LDK generates a FundingNeeded event which contains a FundingTemplate
  • User calls FundingTemplate::build or FundingTemplate::build_sync with a WalletSource or WalletSourceSync, respectively, to produce a FundingContribution
  • User passes FundingContribution -- which contains the FundingTxInputs -- to ChannelManager::funding_contributed
  • LDK validates that the FundingContribution can pay for inputs / outputs, causing LDK to either send splice_init or produce a SpliceFailed event.

The same mechanism can be used later for contributing inputs for counterparty-initiated splices or v2 channel opens since FundingTemplate and FundingContribution contains the context.

Test code still needs to be fixed up, and change_script generation will follow in another commit.

Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

The API design LGTM, though there's one issue with WalletSource. One thing users need to keep in mind now is that from the moment they receive FundingNeeded, they need to act quickly to ensure the counterparty doesn't disconnect due to quiescence taking too long.

fn list_confirmed_utxos(&self) -> Result<Vec<Utxo>, ()>;

///
fn select_confirmed_utxos(
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding this here now requires implementers to satisfy this method when, in the context of anchor channels, WalletSource is only intended to be used such that we perform coin selection on behalf of the user. Ideally, we also give users the option between choosing WalletSource/CoinSelectionSource when funding channels.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right, I guess I'm a bit confused why we can't use select_confirmed_utxos as-is? Indeed the claim_id is annoying, but we can make that either an enum across a ClaimId and some unit value describing a splice or just make it an Option. Aside from that it seems to be basically what we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this here now requires implementers to satisfy this method when, in the context of anchor channels, WalletSource is only intended to be used such that we perform coin selection on behalf of the user. Ideally, we also give users the option between choosing WalletSource/CoinSelectionSource when funding channels.

Hmm... I see. Would a separate trait be desirable? Also, see my reply to @TheBlueMatt below.

Right, I guess I'm a bit confused why we can't use select_confirmed_utxos as-is? Indeed the claim_id is annoying, but we can make that either an enum across a ClaimId and some unit value describing a splice or just make it an Option. Aside from that it seems to be basically what we want.

The return value also isn't compatible. It contains Utxos but we also need the previous tx and sequence number as part of each FundingTxInput. Though its constructor will give a default sequence number.

We could change CoinSelection to use FundingTxInput instead of Utxo, but that would be odd for use with the anchor context.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Honestly that seems fine to me? We expect ~all of our users to want to use splicing, which implies they need to support the "return coin selection with full transactions" interface. So what if anchors throw away some of that data?

If we feel strongly about it we can add a new trait method that does return the full transactions and provide a default implementation for the current method so that those that really want to avoid always fetching the transaction data can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I don't have a strong opinion, but note that Wallet's implementation of CoinSelectionSource::select_confirmed_utxos delegates to WalletSource::list_confirmed_utxos. So it might be expensive to use that abstraction. @wpaulino WDYT?

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 was thinking of keeping CoinSelectionSource the same (with the full transaction data in the response, or with the default-impl-method described above) but changing WalletSource so that we don't have to fetch all previous-transactions at the start.

Yeah, Wallet wraps a WalletSource and implements CoinSelectionSource by listing all the UTXOs and selecting from them. So WalletSource's interface would remain unchanged while CoinSelection would use FundingTxInput instead of Utxo. Which I guess means FundingTemplate::build should actually take a CoinSelectionSource.

Seems reasonable to require a sequence number in the response for that as well, even for anchors?

Hmm... in WalletSource::list_confirmed_utxos by adding a field to Utxto (and removing it from FundingTxInput)? Or by having the CoinSelectionSource implementation fill it in on FundingTxInput?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So WalletSource's interface would remain unchanged

Wouldn't we need a WalletSource::get_previous_transaction_for_utxo method to fetch the full tx data for the UTXOs we selected?

Hmm... in WalletSource::list_confirmed_utxos by adding a field to Utxto (and removing it from FundingTxInput)? Or by having the CoinSelectionSource implementation fill it in on FundingTxInput?

ISTM we should replace Utxo with FundingTxInput since FundingTxInput has strictly more fields (it contains a Utxo!) and we'd move to returning FundingTxInput from the trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't we need a WalletSource::get_previous_transaction_for_utxo method to fetch the full tx data for the UTXOs we selected?

Right, we need another method for that.

ISTM we should replace Utxo with FundingTxInput since FundingTxInput has strictly more fields (it contains a Utxo!) and we'd move to returning FundingTxInput from the trait.

The question is more what should be setting Sequence? Either:

(1) Move it to Utxo and have WalletSource::list_confirmed_utxos set it since it returns Vec<Utxo>.
(2) Have CoinSelectionSource::select_confirmed_utxos set it since CoinSelection would now contain Vec<FundingTxInput>

We just can't replace Utxo with FundingTxInput in WalletSource::list_confirmed_utxos since we don't want to return the previous tx there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(1) Move it to Utxo and have WalletSource::list_confirmed_utxos set it since it returns Vec.

Presumably this. No reason to want it to not be possible in WalletSource.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done as discussed here and offline. I'm in the middle of updating the tests, but I've pushed an update for now.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Aside from the above which-interface question I think the API is good.

@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch from 854e9ca to 6d78c3f Compare January 14, 2026 17:03
@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! 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
Copy link

🔔 1st Reminder

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

@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch from 6d78c3f to 94b1aa9 Compare January 15, 2026 17:02
@jkczyz jkczyz mentioned this pull request Jan 16, 2026
@ldk-reviews-bot
Copy link

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! 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
Copy link

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! 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
Copy link

🔔 3rd Reminder

Hey @TheBlueMatt @wpaulino! 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
Copy link

🔔 3rd Reminder

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

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.
@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch from 94b1aa9 to c3f3453 Compare January 20, 2026 19:16
Comment on lines +190 to +168
// FIXME: Should claim_id be an Option?
let claim_id = ClaimId([0; 32]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding the CoinSelectionSource API, do we want to make claim_id an Option?

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO yes

Comment on lines 883 to 884
Amount::from_sat(383)
Amount::from_sat(385)
} else {
Amount::from_sat(384)
Amount::from_sat(386)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems select_confirmed_utxos_internal might be off on the change calculation because it's using the weight of the change output to compute additional fees instead of re-computing the total fees using the total weight when including a change output.

@jkczyz jkczyz marked this pull request as ready for review January 20, 2026 19:25
@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch from c3f3453 to 3253a99 Compare January 20, 2026 23:57
@ldk-reviews-bot
Copy link

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! 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
Copy link

🔔 4th Reminder

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

@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch 2 times, most recently from 6b92b77 to f2d9fa5 Compare January 21, 2026 05:59
Rather than requiring the user to pass FundingTxInputs when initiating a
splice, generate a FundingNeeded event once the channel has become
quiescent. This simplifies error handling and UTXO / change address
clean-up by consolidating it in SpliceFailed event handling.

Later, this event will be used for opportunistic contributions (i.e.,
when the counterparty wins quiescence or initiates), dual-funding, and
RBF.
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.
@jkczyz jkczyz force-pushed the 2025-12-new-splice-api branch from f2d9fa5 to fcccbac Compare January 21, 2026 18:22
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

❌ Patch coverage is 76.35830% with 161 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.39%. Comparing base (8eb9e70) to head (fcccbac).
⚠️ Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 59.23% 61 Missing and 3 partials ⚠️
lightning/src/ln/splicing_tests.rs 0.00% 33 Missing ⚠️
lightning/src/ln/funding.rs 89.74% 17 Missing and 15 partials ⚠️
lightning/src/ln/channelmanager.rs 71.60% 23 Missing ⚠️
lightning/src/events/bump_transaction/mod.rs 90.47% 3 Missing and 1 partial ⚠️
lightning/src/ln/functional_test_utils.rs 40.00% 3 Missing ⚠️
lightning/src/events/mod.rs 87.50% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4290      +/-   ##
==========================================
- Coverage   86.61%   86.39%   -0.23%     
==========================================
  Files         158      158              
  Lines      103067   103469     +402     
  Branches   103067   103469     +402     
==========================================
+ Hits        89275    89394     +119     
- Misses      11366    11654     +288     
+ Partials     2426     2421       -5     
Flag Coverage Δ
fuzzing 37.02% <42.32%> (-0.05%) ⬇️
tests 85.70% <80.09%> (-0.21%) ⬇️

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.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

API looks good. some misc comments i noted while skimming it

}

/// An unspent transaction output with at least one confirmation.
pub type ConfirmedUtxo = FundingTxInput;
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets not rename types with aliases. If we are making a type more general lets rename the type itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, though it seems like neither funding nor bump_transaction are suitable places for this or the CoinSelection / Wallet related traits and types now. Should we move these into a separate module under util or elsewhere?

/// Indicates that funding is needed for a channel splice or a dual-funded channel open.
///
/// The client should build a [`FundingContribution`] from the provided [`FundingTemplate`] and
/// pass it to [`ChannelManager::funding_contributed`].
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs a call-out to what to do if you actually don't want to splice anymore (ie on failure)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also maybe a note that the channel is hung waiting on our response, so we need to respond quickly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should they simply not handle the event? We'll be in quiescence and timeout after DISCONNECT_PEER_AWAITING_RESPONSE_TICKS. Though it seems this now inadvertently (but maybe expectedly) now applies to us sending splice_init. So maybe some renaming is in order?

Or should we expose ChannelManager::exit_quiescence?

Comment on lines +190 to +168
// FIXME: Should claim_id be an Option?
let claim_id = ClaimId([0; 32]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO yes


/// Creates a `FundingContribution` from the template by using `wallet` to perform coin
/// selection with the given fee rate.
pub fn build_sync<W: Deref>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also have a method for building with a provided set of inputs rather than going through the trait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could... or probably a CoinSelection so that a change output can be given.

Copy link
Collaborator

Choose a reason for hiding this comment

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

spurious diff?

if self.context.channel_state.is_quiescent() {
return Err(APIError::APIMisuseError {
err: format!(
"Channel {} cannot be spliced as it is already quiescent",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want to support queuing up the splice to do afterwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm... this was to handle the time after we've generated FundingNeeded when we no longer have a quiescent_action but are still quiescent waiting on the user to call funding_contributed. But then we can't differentiate this from counterparty-initiated quiescence. Maybe we need to make a placeholder QuiescentAction for when we are waiting on the user to respond? Something like AwaitingFundingContribution, which could also be checked when funding_contributed is called.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding error handling, since the FundingContribution is taken by value we need to generate a SpliceFailed event to allow the user to unlock the UTXOs. However, some failure may be result of misuse (e.g., wrong channel / counterparty, unexpected funding) or bad timing (e.g., peer already disconnected timing out quiescence).

In those cases, it doesn't make sense to generate SpliceFailed. Maybe we need to use DiscardFunding for some of these cases? We'd need another FundingInfo variant, though.

Otherwise, we'd need to return the UTXOs back to the caller, which we wanted to avoid.

Thoughts?

) -> Result<msgs::SpliceInit, SpliceFundingFailed>
where
L::Target: Logger,
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

presumably we need to check that we're quiescent and its our turn to talk?

impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,
(0, DoNothing) => {},
{1, Splice} => (),
{1, LegacySplice} => (),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs docs on when we switched so we know when to remove it

@jkczyz
Copy link
Contributor Author

jkczyz commented Jan 22, 2026

Given that the new API is to be reused for RBF, I wrote up a proposal to consider with this PR. @TheBlueMatt @wpaulino Looking for feedback / alternative ideas before continuing that work.

Observations

  • DiscardFunding contains the funding outpoint. So I assume users will need to look up the transaction to unlock any UTXOs and reclaim script pubkeys?
  • For RBF, I see we have FundingScope::funding_transaction, which is an Option. I thought we wanted to clear this, but I don't see where we are doing that.
  • Either of those require the user to know which outputs are their own and isn't change in order to recreate the SpliceContribution.
  • We produce a FundingTemplate from a SpliceContribution. The user may add more value / outputs before constructing a FundingContribution.
  • When implementing the acceptor case, there will not be a  SpliceContribution -- just an empty FundingTemplate to be modified if needed before producing a FundingContribution. Similarly for dual funding.
  • FundingContribution can't differentiate user selected UTXOs from change.

Proposal

  • Store change output separately in FundingContribution instead of losing the information to outputs.
  • Store the shared input in FundingContribution to allow recreating the FundingTemplate
  • Recreate the FundingTemplate internally from the FundingContribution (before it is consumed into a FundingNegotiationContext) and keep track of it until it can be included alongside the negotiated FundingScopes
  • Alternatively, re-create FundingTemplate somehow from the InteractiveTxConstructor / InteractiveTxSigningSession data. Note, however, that which output, if any, is change will have been already lost.
  • Expose these FundingTemplates in ChannelDetails for informational purposes.
  • User initiated RBF doesn't require passing a SpliceContribution.
  • Instead another FundingNeeded event is generated -- once the channel is quiescent -- containing the last used FundingTemplate
  • User uses any of the previously used FundingTemplates or the one given in the event (modifying as they see fit) to call ChannelManager::funding_contributed.

Questions

  • How should we expose the FeeRate used in each negotiation? We need to make sure the RBF uses a greater fee rate, so at very least we need to keep track of it. User also needs to know to pass a higher fee rate when creating the new FundingContribution.
  • Do we need to also expose the inputs used in each negotiation? They aren't needed for RBF, so would only be informational.

@TheBlueMatt
Copy link
Collaborator

DiscardFunding contains the funding outpoint. So I assume users will need to look up the transaction to unlock any UTXOs and reclaim script pubkeys?

Hmm? It has a full-transaction field, I assume that should be used in splicing?

For RBF, I see we have FundingScope::funding_transaction, which is an Option. I thought we wanted to clear this, but I don't see where we are doing that.

Not sure what this is referring to maybe that was something you and @wpaulino discussed?

As for the proposal. Indeed, I agree we definitely want to retain change info so that devs have it handy.

Expose these FundingTemplates in ChannelDetails for informational purposes.

I'm confused why we want to expose the templates that don't have the full list of inputs, vs exposing the FundingContribution which is basically strictly more info?

User initiated RBF doesn't require passing a SpliceContribution.

Don't we use this to send the peer the intended-to-be-added amount in the splice_init? I'm kinda unclear on why that's there anyway tbh, can we just always set it to zero and remove value_added from SpliceContribution (I mean not that we should, its kinda nice for the user to be able to pass that info but still).

IIUC that the point of the value is really so that the dev can declare their intended changes to the channel up-front and get them back when we're asking them to fund the transaction. That way they're not storing them and always going to fetch them. If that's the case, shouldn't we try for something similar here? Have some way to specify the fee you intend to use (maybe in SpliceContribution for a fresh splice as well as well?) as well as any additional outputs or maybe even removed outputs?

Instead another FundingNeeded event is generated -- once the channel is quiescent -- containing the last used FundingTemplate

Again shouldn't this be a FundingContribution?

@jkczyz
Copy link
Contributor Author

jkczyz commented Jan 22, 2026

Hmm? It has a full-transaction field, I assume that should be used in splicing?

AFAICT, we don't have the Transaction in the ChannelMonitor, so we use FundingInfo::OutPoint in DiscardFunding.

Not sure what this is referring to maybe that was something you and @wpaulino discussed?

As for the proposal. Indeed, I agree we definitely want to retain change info so that devs have it handy.

I have a vague recollection that we didn't want to keep around the funding transaction in the FundingScope once it has enough confirmations. Maybe I'm misremembering?

Expose these FundingTemplates in ChannelDetails for informational purposes.

I'm confused why we want to expose the templates that don't have the full list of inputs, vs exposing the FundingContribution which is basically strictly more info?

Partly because we don't need the FundingTxInputs as the user will need to re-run coin selection. It's possible the same UTXOs could be used after bumping the fee rate and adjusting the change. But it may not be enough to cover mining fees.

Also, storing FundingTxInputs is more expensive because it contains prevtx, which may also be duplicated if inputs come from the same transaction. That said, while we would persist these as part of PendingFunding, they would at least be ephemeral.

User initiated RBF doesn't require passing a SpliceContribution.

Don't we use this to send the peer the intended-to-be-added amount in the splice_init? I'm kinda unclear on why that's there anyway tbh, can we just always set it to zero and remove value_added from SpliceContribution (I mean not that we should, its kinda nice for the user to be able to pass that info but still).

We don't send splice_init (or in this case tx_init_rbf) until we've become quiescent. It's not until we have become quiescent do we generate a FundingNeeded event. Having the amount and outputs in FundingTemplate is purely so the user can call splice_channel and not need to remember why they called it when handling FundingNeeded. It's not until that event is handled do we send any messages to the counterparty.

IIUC that the point of the value is really so that the dev can declare their intended changes to the channel up-front and get them back when we're asking them to fund the transaction. That way they're not storing them and always going to fetch them. If that's the case, shouldn't we try for something similar here? Have some way to specify the fee you intend to use (maybe in SpliceContribution for a fresh splice as well as well?) as well as any additional outputs or maybe even removed outputs?

Right, for RBF, the question is what should the user provide upfront vs when handling FundedNeeded? Do we expect them to want to increase / decrease the amount or add / remove inputs then? Or is it fine for them to do it when handling FundedNeeded. This PR doesn't current allow the the later but it can be trivially added by exposing an API on FundingTemplate. This is because when generating the event we are quiescent but haven't yet sent splice_init / tx_init_rbf.

For the fee rate, it is not supplied until handling FundedNeeded in this PR. We could have the user give it upfront. I think we at least need to know what fee rate was used during coin selection. Whether it passed at that time by the user or taken from the FundingTemplate probably doesn't matter.

Again shouldn't this be a FundingContribution?

Not in FundingNeeded since the user may need to re-run coin selection. Using FundingTemplate allows them to do this and adjust the amount and outputs.

I think I'm fine with storing the FundingContributions if that is what we want to expose to the user. But we should have a conversion back to FundingTemplate so that the funding flow is the same. We can't really adjust a FundingContribution for them without re-running coin selection.

@jkczyz jkczyz requested a review from TheBlueMatt January 22, 2026 22:28
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.

4 participants