Skip to content

Splicing robustness improvements - #3417

Merged
pm47 merged 12 commits into
masterfrom
splice-unpublishable-funding-tx
Oct 27, 2026
Merged

pm47 merged 12 commits into
masterfrom
splice-unpublishable-funding-tx

Conversation

@pm47

@pm47 pm47 commented Oct 27, 2026

Copy link
Copy Markdown
Member

This PR makes splicing more robust when splice transactions don't confirm:

  • add require-confirmed-inputs-for-splicing (enabled by default): splices only spend confirmed transactions, and we limit the number of inputs and outputs our peer can add
  • when force-closing, use a commitment whose funding transaction is known by our bitcoind, and re-evaluate it when blocks are found or splice transactions are published
  • when CPFP-ing our commit tx, take its unconfirmed funding transaction into account
  • claim the outputs of a previous remote commitment as soon as it confirms
  • warn against using zero_fee_commitments when splicing with untrusted peers

Note that require-confirmed-inputs-for-dual-funding doesn't apply to splices anymore.

Reported-by: @erickcestari

pm47 and others added 10 commits October 22, 2026 11:29
We add a `require-confirmed-inputs-for-splicing` setting (not used yet)
and tests showing that we should:
 - require our peer's splice inputs to be confirmed
 - not splice on top of an unconfirmed splice transaction
 - limit the number of inputs and outputs our peer adds to a splice

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When `require-confirmed-inputs-for-splicing` is set (the default), we
now require our peer to only use confirmed inputs in splices, and we
don't initiate a splice on top of an unconfirmed splice transaction (our
peer already cannot). We also limit the number of inputs and outputs our
peer can add to a splice transaction to 10.

Note that `require-confirmed-inputs-for-dual-funding` doesn't apply to
splices anymore, only to channel creation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
We use the default value of `require-confirmed-inputs-for-splicing` in
tests. Our test wallet now considers the inputs it uses to fund
transactions confirmed (this can be disabled to test unconfirmed inputs),
and shares them with other test wallets so that our peer can verify them.

Tests that splice on top of unconfirmed splices now explicitly disable
it: those scenarios can still happen with zero-conf, so we keep testing
them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
We add tests for the commitment we use when force-closing while splice
transactions are unconfirmed. Our test wallet now considers the
transactions it has published to be in its mempool.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When force-closing while splice transactions are unconfirmed (without
zero-conf), we now use the most recent commitment whose funding tx is
known by our bitcoind (in its mempool or confirmed), or the most recent
confirmed commitment otherwise. We re-evaluate it at every block and
when a splice transaction spends a funding output, and force-close again
using a different commitment if needed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When our commit tx spends an unconfirmed funding transaction, our anchor
tx should also pay for that funding transaction to reach our target
feerate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Miners evaluate our anchor tx with all of its unconfirmed ancestors.
When our commit tx spends an unconfirmed funding transaction (with
zero-conf or splices), we now include the fees and weight of its
unconfirmed ancestors when computing the fees our anchor tx must pay.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With zero_fee_commitments, our commit tx is a v3 transaction, which
cannot spend an unconfirmed v2 transaction such as a splice transaction
(TRUC rules), so we cannot CPFP it. We document that this feature should
not be used when splicing with untrusted peers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When our peer publishes its commit tx for a previous commitment, we
should claim its outputs as soon as it confirms.

Existing tests that confirm a previous commit tx now use a height that
is deep enough to directly test the min-depth handling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When our peer publishes its commit tx for a previous commitment, we
waited for min-depth before claiming its outputs. We now claim them as
soon as it confirms, without forgetting our other commitments, which we
only reset once it is deeply confirmed. We don't do it for revoked
commit txs, whose outputs are delayed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pm47
pm47 requested a review from sstone October 27, 2026 10:35
@sstone

sstone commented Oct 27, 2026

Copy link
Copy Markdown
Member

LGTM, just one fix for when we force-close while a splice transaction is unconfirmed and gets evicted from the mempool, see #3419

* Add test for published stale local commit tx

When we switch to a different commitment while force-closing (because
the funding tx of our commit tx left our mempool), the commit tx we
previously published may still confirm, since miners may still have it
along with its parent splice transaction. We must then claim its outputs
instead of treating it as a remote commit tx.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* Handle our stale local commit tx in CLOSING

When force-closing, we may switch to a different commitment if the
funding tx of our commit tx isn't known by our bitcoind anymore. The
commit tx we previously published may still confirm though: we now
recognize it as one of our local commit txs and force-close again with
its commitment, instead of handling it as a remote commit tx, which
would lead to ERR_INFORMATION_LEAK or to a bogus revoked commit.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: claude <noreply@anthropic.com>
@pm47
pm47 merged commit c07cc74 into master Oct 27, 2026
1 check passed
@pm47
pm47 deleted the splice-unpublishable-funding-tx branch October 27, 2026 15:44
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