Add a new set of pending command results for splice and rbf #3115
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
There was a problem hiding this comment.
Pull Request Overview
This PR refines the channel funding workflow by introducing pending outcomes for splice and RBF funding commands, ensuring that success is only returned after tx_signatures are exchanged. Key changes include:
- Switching test fixtures to use wallets that return confirmed UTXO inputs and adding new tests to cover pending, success, and failure funding results.
- Updating on-chain wallet funding methods to support changeless funding and the tracking of fee excess.
- Extending the interactive transaction builder, funder, and channel state logic to handle and propagate pending command responses for splice and RBF attempts.
Reviewed Changes
Copilot reviewed 20 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingSignedStateSpec.scala | Updated wallet usage for dual-funded channel tests |
| eclair-core/src/test/scala/fr/acinq/eclair/channel/states/b/WaitForDualFundingCreatedStateSpec.scala | Updated wallet usage for dual-funded channel tests |
| eclair-core/src/test/scala/fr/acinq/eclair/channel/states/a/WaitForFundingInternalDualFundedChannelStateSpec.scala | New test cases for internal dual-funding channel state |
| eclair-core/src/test/scala/fr/acinq/eclair/channel/states/ChannelStateTestsHelperMethods.scala | Added changeless funding flag support in test helper methods |
| eclair-core/src/test/scala/fr/acinq/eclair/channel/InteractiveTxBuilderSpec.scala | Updated interactive TX builder tests to pass new fundingContributions_opt and liquidity purchase flag |
| eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala | Added excess fee/excess handling for changeless funding and introduced a new wallet variant |
| eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala | Extended funding rate configuration with an invalid funding amount check |
| eclair-core/src/main/scala/fr/acinq/eclair/wire/internal/channel/version4/ChannelCodecs4.scala | Revised splice status codecs to align with new funding sessions |
| eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala | Introduced a placeholder public key for fee estimation |
| eclair-core/src/main/scala/fr/acinq/eclair/payment/send/TrampolinePaymentLifecycle.scala | Added handling for CommandPending responses |
| eclair-core/src/main/scala/fr/acinq/eclair/payment/relay/OnTheFlyFunding.scala | Included pending response handling for HTLC relay funding |
| eclair-core/src/main/scala/fr/acinq/eclair/io/PeerReadyNotifier.scala | Updated peer readiness logic to account for dual-funded channel internal state |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxFunder.scala | Enhanced funding logic to include fee adjustments and pending funding contributions |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala | Modified the interactive TX builder to accept an optional funding contributions parameter and pass liquidity purchase flag |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala | Added rollback functionality for open attempts upon pending funding outcomes |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ChannelOpenDualFunded.scala | Revised dual-funded channel opening flow to integrate pending funding contributions in splice and RBF responses |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala | Adjusted splice status handling with pending API responses and updated commitment flow |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala | Introduced a new exception for RBF attempts with liquidity purchase exceeding funding amount |
| eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala | Updated channel data models to include funding contributions and new dual funding states |
Comments suppressed due to low confidence (4)
eclair-core/src/test/scala/fr/acinq/eclair/blockchain/DummyOnChainWallet.scala:198
- Consider adding a detailed comment explaining the rationale behind adding an excess of exactly 1,000 sats when the output amount equals 100,000 sats, to improve future maintainability.
val excess = if (amountOut - currentAmountIn == 100_000.sat) 1_000.sat else 0.sat
eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala:1511
- [nitpick] Review the pending response flow for splice commands; consider using RES_SPLICE_PENDING in scenarios where the funding transaction remains incomplete due to pending signature exchanges.
cmd_opt.foreach(cmd => cmd.replyTo ! RES_SPLICE(fundingTxIndex = signingSession.fundingTxIndex, signingSession.fundingTx.txId, signingSession.fundingParams.fundingAmount, signingSession.localCommit.fold(_.spec, _.spec).toLocal))
eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala:57
- Ensure that new pending state behaviors for splice and RBF commands are comprehensively covered in tests, particularly focusing on cases with partial funding signature exchanges and disconnection scenarios.
fundingRates = LiquidityAds.FundingRate(100_000 sat, 10_000_000 sat, 500, 100, 100 sat, 1000 sat) ::
eclair-core/src/main/scala/fr/acinq/eclair/channel/fund/InteractiveTxBuilder.scala:440
- Ensure that the new fundingContributions_opt parameter is consistently handled across the interactive transaction builder logic and that tests verify both pending and completed funding scenarios.
actor.start(fundingContributions_opt)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR builds on PR #3111 to address issue #3093. Instead of returning success when interactive tx building completes, only return success after sending our
tx_signatures. If we disconnect while waiting for signatures, return a pending result to the user for the splice or rbf command to indicate we've not yet received all of our counter party's signatures.TODO: