Skip to content

Update to select funding inputs first before sending open_channel2, splice_init and tx_init_rbf #3111

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.

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

remyers
Copy link
Contributor

@remyers remyers commented Jun 17, 2025

This PR replaces #2924 to find funding inputs first for all interactive funding requests: dual funding, splices and rbfs. The main reasons for this change is to retain excess funding from changeless inputs, but it also allows nodes to fail early if funding is not available.

When changeless inputs are used to fund a dual funded channel or spliced into an existing channel, our channel balance will be credited with any excess that is added to the channel over what the user requests. Previously that value would have gone to fees up to the dust limit which increases as fee rates increase. Before this PR when a changeless solution was used for funding, excess value over what was requested was treated as waste and used as extra fees.

This PR also changes the RBF behavior of nodes providing funding from a liquidity ad. A node will not add inputs in response to an RBF request when it added funding in response to a liquidity ad. This change protects a liquidity provider from locking a potentially unlimited number of inputs for a dual-funded channel or splice it did not initiate. Instead of adding inputs, only the change output amount will used to increase the feerate. The change amount may be insufficient to reach the newly requested RBF feerate and that's ok; the channel/splice initiator can lower their liquidity request in the RBF to allocate more of the liquidity provider's input amount to change.

We introduce a new optional funding contributions parameter to InteractiveTxBuilder. When set, we will start by processing the passed in funding inputs/outputs instead of using InteractiveTxFunder to fund the transaction. The call to InteractiveTxFunder is now made before sending open_channel2, splice_init, tx_init_rbf or tx_ack_rbf and the results are then passed to InteractiveTxBuilder.

remyers added 3 commits June 17, 2025 08:37
If a changeless set of inputs is found, excess funding (if any) will be added to the proposed `fundingAmount`/`fundingContribution` before sending `open_channel2`/`splice_init` respectively.

InteractiveTxFunder sets `excess_opt` with the input value in excess of what is needed to achieve the requested target feerate.

We assume our peer requires confirmed inputs. In the future we could add a heuristic for this, but it's safer to assume they want confirmed inputs.
Select funding inputs before sending splice_ack when adding liquidity

Added the `SpliceStatus.SpliceInitiated` state for when `SpliceInit` is received with a valid liquidity request, but the wallet has yet returned funding inputs.
Reuse previous inputs when sending tx_rbf_ack if adding liquidity and adjust change amount to increase fees or modify the funding amount.

Reject RBF attempts that increase  liquidity requests, but allow valid lower liquidity requests.

If we request liquidity in our RBF, our peer will not add additional inputs and may not be able to fully fund the feerate increase from their change output; that's ok.

TODO: Check that all failure cases end quiescence, send tx_abort and/or unlocked inputs as needed.

TODO: Refactor InteractiveTxBuilder to not call InteractiveTxFunder; fundingContributions should always be set now.
@remyers
Copy link
Contributor Author

remyers commented Jun 17, 2025

@t-bast This is ready for a first round of review now that I've added RBF support. Some areas that may require additional review or work:

  • Do all failure cases end quiescence, send tx_abort and/or unlocked inputs as needed? A funding request can fail or be aborted in new ways now and this needs more review and probably more tests to make sure inputs are rolled back and/or quiescence is ended properly.
  • The InteractiveTxBuilder should no longer call InteractiveTxFunder. Should its fundingContributions_opt parameter be changed from an Option to always require a FundingContributions?
  • How do these changes impact Phoenix; in particular related to underfunded tx_ack_rbf after a liquidity request?
  • Some tests may still be failing in the checkFeerate test function of NormalSpliceStateSpec. This is related to the calculation of the non-initiator/remote node fees. I can't figure out why the feerate is sometimes slightly under the requested feerate. It shouldn't matter because the non-initiator's effective feerate is higher after removing the duplicated shared overhead (42 weight units), but it bothers me. fixed by e447e93

When checking feerates, variation in signature sizes can make our remoteFundingTx feerate go below the target feerate. This doesn't matter because it will always have a larger feerate after removing the duplicate common elements.
@remyers remyers marked this pull request as ready for review June 17, 2025 15:46
@t-bast
Copy link
Member

t-bast commented Jun 18, 2025

How do these changes impact Phoenix; in particular related to underfunded tx_ack_rbf after a liquidity request?

This won't have any impact on Phoenix, since 0-conf wallets don't use RBF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants