Skip to content

Add support for the official splicing protocol#2887

Open
t-bast wants to merge 4 commits intomasterfrom
splicing-official
Open

Add support for the official splicing protocol#2887
t-bast wants to merge 4 commits intomasterfrom
splicing-official

Conversation

@t-bast
Copy link
Member

@t-bast t-bast commented Jul 23, 2024

We replace our experimental version of splice_init, splice_ack and splice_locked by their official version (see lightning/bolts#1160). If our peer is using the experimental feature bit, we convert our outgoing messages to use the experimental encoding and incoming messages to the official messages.

We also change the TLV fields added to tx_add_input, tx_signatures and splice_locked to match the spec version. We always write both the official and experimental TLV to updated nodes (because the experimental one is odd and will be ignored) but we drop the official TLV if our peer is using the experimental feature, because they won't understand the even TLV field.

We do the same thing for the commit_sig TLV. For peers who support the official splicing version, we insert the start_batch message before the batch of commit_sig messages as specified in lightning/bolts#1160.

We also support both the legacy and official reconnection logic (TLV fields in channel_reestablish). It is a bit painful because both protocols use TLV 1, but for different purposes, so we need to do it in a somewhat hacky way at the codec level.

This ensures that we're able to create splice transactions, even with non-upgraded nodes (full backwards-compatibility). It would be best to make sure Phoenix users upgrade to a version that uses the official protocol though, to allow cleaning up the codebase and guarantee that we haven't missed an edge case.

Builds on top of #3261

TODO:

  • once we're ready to merge this, we should do another round of e2e tests with LDK and/or cln

@t-bast t-bast force-pushed the splicing-official branch 2 times, most recently from 7d1c23e to 33482ec Compare August 1, 2024 14:34
@t-bast t-bast requested a review from remyers August 1, 2024 14:36
@t-bast t-bast force-pushed the splicing-official branch 2 times, most recently from fa51c31 to 2d350e5 Compare August 1, 2024 16:17
@remyers
Copy link
Contributor

remyers commented Aug 13, 2024

This is an observation that occurred to me while reviewing this PR that might be relevant for a future PR ..

There will be times when using CMD_BUMP_FUNDING_FEE instead of CMD_SPLICE would cost less on-chain and confirm faster if you want to perform an additional splice while a pending splice is unconfirmed. The total cost to create a new funding transaction with CMD_SPLICE will often be higher than making the same changes with CMD_BUMP_FUNDING_FEE at a slightly higher fee rate. If you are blocked from making a new splice because a pending splice has been fee bumped, you could also make a new fee bump to splice in/out value rather than waiting for the previously rbf'd splice to confirm.

This seems to be technically possible in the protocol because CMD_BUMP_FUNDING_FEE uses the interactive tx protocol and can change the funding contribution with the funding_output_contribution TLV. Figuring out if a splice or rbf-splice is cheaper will also need to consider the additional fees paid to bump our counter party's inputs and outputs.

Perhaps the biggest downside I can see (beside complexity) is that if your pending splice or rbf confirms before a rbf-splice, then you would need to renegotiate it as a new splice.

Am I missing something here wrt fee savings? Do you think it would be worth considering this situation now rather than later?

@t-bast
Copy link
Member Author

t-bast commented Aug 13, 2024

Perhaps the biggest downside I can see (beside complexity) is that if your pending splice or rbf confirms before a rbf-splice, then you would need to renegotiate it as a new splice.

Yes, the main reason we're not providing this yet (even though this is in theory possible) is to manage complexity. It can get really complex very quickly once you start going down that road, because if each splice "part" maps to a specific action (e.g. an on-the-fly funding), it can be a huge mess to reconcile which parts where actually done and which parts need to be replayed if a previous RBF attempt confirms.

Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

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

Looks good - the refactor for RBF makes it easier to read/understand.

I found a few nits but main issues to take a look at are related to adding logic to reject an TxInitRbf or SpliceInit when the parent splice is unconfirmed.

Copy link
Contributor

@remyers remyers left a comment

Choose a reason for hiding this comment

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

Changes to tests to disallow sequences of unconfirmed splices looks good. I only had a few minor nits/questions.

@t-bast
Copy link
Member Author

t-bast commented Oct 8, 2024

Rebased to include changes linked to liquidity ads and on-the-fly funding.

Some on-the-fly funding tests aren't passing anymore, because it's harder to make eclair mimick the behavior of a wallet that doesn't contribute at all to funding transactions: we'll need to re-work those tests, I'm not sure yet what exactly will be best.

We will probably integrate the second and third commits independently, once we're confident Phoenix users all support quiescence. This should help us integrate this bit by bit and make sure test coverage is good enough.

@t-bast
Copy link
Member Author

t-bast commented May 14, 2025

@remyers I rebased on top of #3083 and implemented the stronger batching logic for commit_sig messages. I haven't yet written the spec commit for that, but will do so tomorrow!

EDIT: the spec is now available in the last commit of lightning/bolts#1160

@t-bast t-bast force-pushed the splicing-official branch from 97de1c0 to db70aa6 Compare May 15, 2025 15:29
@t-bast t-bast force-pushed the splicing-official branch from db70aa6 to 299f14c Compare May 23, 2025 12:22
t-bast added 2 commits March 3, 2026 11:33
In lightning/bolts#1160 we add a TLV field to
`commit_sig` messages to let the receiver know to which `funding_txid`
this signature applies. This is more resilient than relying on the
order of the `commit_sig` messages in the batch. This is an odd TLV,
so we can start writing it right now without creating compatibility
issues.

We also slightly refactor existing code to make it easier to introduce
a backwards-compat layer when migrating to the official splicing. We
also increase the default number of RBF attempts allowed.
In lightning/bolts#1160, we introduce a message
to let our peer know how many `commit_sig` messages they will receive
and treat them as a batch. This replaces our previous version that did
something similar, but by adding a batch TLV in every `commit_sig`
message we send.

We currently do both: we keep inserting the experimental batch TLV, and
we start by sending a `start_batch` message (with the same information).
Since it is an odd message (127), it should be safely ignored if our
peer doesn't understand it.
@t-bast t-bast force-pushed the splicing-official branch from 0e7228e to 0964dd4 Compare March 3, 2026 10:39
We replace our experimental version of `splice_init`, `splice_ack` and
`splice_locked` by their official version. If our peer is using the
experimental feature bit, we convert our outgoing messages to use the
experimental encoding and incoming messages to the official messages.

We also change the TLV fields added to `tx_add_input`, `tx_signatures`
and `splice_locked` to match the spec version. We always write both the
official and experimental TLV to updated nodes (because the experimental
one is odd and will be ignored) but we drop the official TLV if our
peer is using the experimental feature, because it won't understand the
even TLV field.

We do the same thing for the `commit_sig` TLV. For peers who support the
official splicing version, we insert the `start_batch` message before
the batch of `commit_sig` messages.

This guarantees backwards-compatibility with peers who only support the
experimental feature.
@t-bast t-bast force-pushed the splicing-official branch from 0964dd4 to a0083a2 Compare March 4, 2026 11:00
@t-bast t-bast changed the title Use final spec values for splicing Add support for the official splicing protocol Mar 4, 2026
@t-bast t-bast force-pushed the splicing-official branch from a6b6a23 to 759ff27 Compare March 4, 2026 14:51
We introduce a `retransmit_flags` field to `my_current_funding_locked`
and `next_funding` to ask our peer to retransmit `commitment_signed` or
`announcement_signatures` if we're expecting them. With this change, we
don't need to retransmit `splice_locked` on reconnection anymore to
trigger the exchange of `announcement_signatures`. We don't need to
retransmit it to let our peer know that we've seen enough confirmations
for the splice either, since `my_current_funding_locked` implies that.

This allows us to completely remove retransmission of `splice_locked` on
reconnection, and also get rid of the `your_last_funding_locked` TLV,
which greatly simplifies the reconnection logic. We do keep them for
backwards-compatibility with existing Phoenix users though, but we'll
be able to clean it up once they have updated.

Note that this works with taproot channels since we will simply provide
nonces in `channel_reestablish` when we need our peer to send
announcement signatures (not supported yet since taproot channels are
never announced).

We rollback using the `next_commitment_number` to let our peer know that
we haven't received their `commit_sig` and instead use the retransmit
flags added to the `next_funding` TLV, unless our peer is using the
legacy splicing protocol.
@t-bast
Copy link
Member Author

t-bast commented Mar 5, 2026

Claude's review regarding backwards-compatibility with the previous protocol:

After reviewing all changes, the backwards-compatibility logic is correct. Key observations:

 1. TLV tag 1 size discrimination (ChannelTlv.scala:276-281): Works correctly -- 32 bytes = legacy, 33 bytes = official. The retransmitCommitSig flag is safely ignored for legacy (always false).
 2. ChannelReestablish accessor fallback logic (LightningMessageTypes.scala:215-225):
   - nextFundingTxId_opt: Correctly checks official tag 1 first, falls back to ExperimentalNextFundingTlv (tag 0). When tag 1 is legacy (32 bytes), it falls through to tag 0 as expected.
   - myCurrentFundingLocked_opt: Correctly checks official MyCurrentFundingLockedTlv (tag 5) first, falls back to ExperimentalMyCurrentFundingLockedTlv (tag 3).
   - yourLastFundingLocked_opt: Only populated when tag 1 is legacy (not official). Correct -- removed from official protocol.
   - retransmitAnnSigs: Only available from tag 5 (official). Legacy peers always get false, which is correct since they use splice_locked echo instead.
 3. PeerConnection message conversion (PeerConnection.scala:209-224): Correctly converts SpliceInit/Ack/Locked between experimental (37000/37002/37004) and official (80/81/77) types. TLV filtering
 correctly strips official-only TLVs (SharedInputTxId tag 0, PreviousFundingTxSig tag 0) while leaving experimental TLVs (tags 1105, 601). CommitSigBatch correctly uses StartBatch for official peers and
 inline ExperimentalBatchTlv for legacy.
 4. Channel reestablish sending (Channel.scala:2417-2560): Clean bifurcation based on useLegacySpliceProtocol. Legacy sends tag 0 + tag 1 (32 bytes) + tag 3. Official sends tag 1 (33 bytes) + tag 5. Both
 sets of TLVs are mutually exclusive and correct.
 5. Retransmission logic (Channel.scala:2606-2609, 3485-3491): Legacy uses nextLocalCommitmentNumber == remoteCommitIndex (implicit signal). Official uses retransmitInteractiveTxCommitSig (explicit flag).
 Both paths are consistent with their protocol's semantics.
 6. nextLocalCommitmentNumber computation (InteractiveTxBuilder.scala:1214-1217): Legacy rolls back to unsignedCommit.index to signal "haven't received commit_sig". Official always uses localCommitIndex +
 1 and relies on the explicit flag. Correct.
 7. resendSpliceLockedIfNeeded (Channel.scala:3542-3556): Only retransmits for legacy peers. Official peers don't need this because MyCurrentFundingLockedTlv (tag 5) already signals the locked state.
 8. resendChannelReadyIfNeeded (Channel.scala:3452-3483): Always retransmits for legacy peers. Official peers follow the standard spec condition. Correct.

 No bugs or correctness issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants