Simplify force-close transaction signing and replaceable publishers#3106
Merged
Simplify force-close transaction signing and replaceable publishers#3106
Conversation
b93d85d to
262144f
Compare
We always have the preimage available when we create claim-htlc-success transactions, so we can directly include it instead of the payment hash now that we don't store those transactions on-disk anymore.
There are two stages of HTLC transactions: we first generate unsigned transactions to create our signatures for the remote commitment when sending our `commit_sig`. Then we receive our peer's signatures for the HTLC transactions for our local commitment, which we simply store. We only need them when we want to force-close, at which point we enrich HTLC transactions with their witness data (preimage for incoming HTLCs and remote signatures). We make this explicit in our transactions hierarchy, which lets us simplify the publisher actors, get rid of the `redeemable` functions in `ErrorHandlers` and clean up `Helpers.scala`.
Now that we don't need to support encoding anchor txs in our channel data, we can separate the anchor tx spending our local commitment from the anchor tx spending a remote commitment. This is a trivial refactoring that will allow us to go further and enrich transaction with the corresponding commitment keys in the next commit.
Now that we never store `ForceCloseTransaction` instances in our channel data, we can enrich those types with runtime information such as the keys necessary to sign those transactions. We also include the commitment format, which is necessary to compute the redeem info and otherwise had to be provided as argument everywhere. The codecs changes are safe since we only use the input info and htlc_id when reading old data.
We refactor the replaceable tx publisher to remove the wrapper classes for replaceable txs: we instead introduce a `WalletInputs` class and directly provide signing helpers on the `ForceCloseTransaction` instances. We avoid modifying the internal transaction of a `ForceCloseTransaction` instance, which was confusing: we instead return a `Transaction` from the signing functions without modifying the "base" force-close tx. This simplifies the `ReplaceableTxFunder` and paves the way for using it for all transaction types that can be RBF-ed (main transaction, htlc delayed transactions, penalty transactions, etc).
262144f to
30b4c1b
Compare
pm47
previously approved these changes
Jun 18, 2025
sstone
reviewed
Jun 18, 2025
eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala
Show resolved
Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala
Show resolved
Hide resolved
sstone
reviewed
Jun 18, 2025
eclair-core/src/main/scala/fr/acinq/eclair/channel/publish/ReplaceableTxFunder.scala
Show resolved
Hide resolved
sstone
approved these changes
Jun 18, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Now that
ForceCloseTransactioninstances aren't stored in our channel data anymore (we only need the input they're spending and thehtlc_idin the HTLC case), we can enrich those classes with runtime information such as commitment keys and commitment format. This lets us directly provide an genericdef sign(): Transactionfunction and helpers for RBF (by simply lowering the output amount).This finally lets us get rid of the
ReplaceableTxwrapper classes introduced for replaceable transaction publishing. We're able to simplify theReplaceableTxFunderand make it generic enough to support all types of force-close transactions in the future, if we want to provide automatic RBF of claim-main transactions, htlc-delayed transactions or penalty transactions.This PR should be reviewed commit-by-commit, as each commit is independent and should be easier to review individually.
🎉 🎉 🎉 This finally ends the transactions refactoring work, it should now be easier to introduce more commitment formats (taproot, v3 txs, etc). 🎉 🎉 🎉