Add pending SpliceDetails to ChannelDetails#4687
Conversation
PendingFunding tracked our splice contributions in a compact list implicitly aligned to the tail of the negotiated candidates, with the in-flight negotiation round's contribution as the implicit last entry. Every consumer had to re-derive this positional relationship, which is easy to get wrong -- e.g., attributing an in-flight round's contribution to a completed counterparty-only candidate. Instead, store each candidate's contribution with the candidate itself and give the in-flight round's contribution its own field, making such misattribution unrepresentable. The contributions still form a suffix of the candidates -- once a round includes our contribution, every subsequent round carries it forward (possibly feerate-adjusted) so the splice intention is never lost -- which is now asserted when a round completes. The on-disk format is unchanged: the suffix invariant makes the compact encoding lossless, so candidate fundings and contributions are still written as parallel fields readable by LDK 0.2 and recomposed when read, with the in-flight contribution stored under a new TLV. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👋 Thanks for assigning @wpaulino as a reviewer! |
| // Data written before the in-flight round's contribution was stored separately kept it | ||
| // as the last entry while a negotiation was pending. | ||
| if negotiation_contribution.is_none() && contributions.len() > fundings.len() { | ||
| negotiation_contribution = contributions.pop(); | ||
| } | ||
| // An in-flight contribution is only meaningful while its negotiation round is alive; | ||
| // rounds not surviving serialization round trips have their events handled separately. | ||
| if funding_negotiation.is_none() { | ||
| negotiation_contribution = None; | ||
| } |
There was a problem hiding this comment.
This heuristic for recovering the in-flight contribution from LDK 0.2 data is insufficient when there are leading counterparty-only (None) candidates.
In the old format the in-flight (AwaitingSignatures) round's contribution was appended as the last entry of contributions while a negotiation was pending, but it is not in negotiated_candidates yet. Since contributions form a suffix, with K contributed prior candidates and M total prior candidates (M >= K), the old contributions length is K + 1 (the +1 being the in-flight). This heuristic only pops when K + 1 > M, i.e. only when K == M (no leading None candidates).
When M > K (at least one leading counterparty-only candidate), K + 1 <= M, so the pop is skipped and:
negotiation_contributionstaysNone(the in-flight contribution is lost), andcontrib_offset = M - (K+1)mis-assigns: one prior candidate atcontrib_offsetwrongly receives a contribution and the in-flight contribution is consumed into the candidate list.
Concrete minimal case from old 0.2 data: candidates=[None] (counterparty-only splice) followed by a we-contribute RBF round at AwaitingSignatures → old bytes are fundings=[f0], contributions=[c]. Here 1 > 1 is false, so this reads back as candidate[0].contribution = Some(c) and negotiation_contribution = None, both incorrect (should be candidate[0].contribution = None, negotiation_contribution = Some(c)). This is exactly the first-contribution-on-RBF flow exercised by the new test, and would corrupt contribution tracking / splice_funding_failed and later trip the suffix debug_assert! in splice_funding_negotiated.
The presence of an in-flight contribution in old data can't be detected by length alone here; it needs to be tied to funding_negotiation.is_some() together with the suffix structure.
There was a problem hiding this comment.
I don't think this is an issue, 0.2 doesn't support RBF so it can't have multiple candidates/contributions.
|
I've completed a thorough re-review of the current state of the PR (commit 984f4cc), tracing the redesigned serialization, the new No issues found. The two issues I flagged in my earlier pass were both eliminated by the redesign and remain resolved in this commit:
I verified:
Cross-cutting note (carried from my prior summary, not re-posted inline): the field |
|
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
| // Data written before the in-flight round's contribution was stored separately kept it | ||
| // as the last entry while a negotiation was pending. | ||
| if negotiation_contribution.is_none() && contributions.len() > fundings.len() { | ||
| negotiation_contribution = contributions.pop(); | ||
| } | ||
| // An in-flight contribution is only meaningful while its negotiation round is alive; | ||
| // rounds not surviving serialization round trips have their events handled separately. | ||
| if funding_negotiation.is_none() { | ||
| negotiation_contribution = None; | ||
| } |
There was a problem hiding this comment.
I don't think this is an issue, 0.2 doesn't support RBF so it can't have multiple candidates/contributions.
PendingFunding wrote our per-candidate contributions, the in-flight round's contribution, and the last-negotiated feerate in even (required) TLVs. LDK 0.2 predates all three but does understand a single negotiated candidate, yet any one of those even fields made it refuse the whole channel -- even a single, non-RBF splice it can otherwise operate. The feerate in particular is written for every negotiated splice, so even a non-contributory single splice was refused. Serialize the negotiated candidates so a single pending splice stays loadable by 0.2 while RBF is refused loudly: - TLV 3 carries only the first candidate's funding -- the single-splice view 0.2 reads. - The authoritative candidate list (each funding bundled with its contribution) moves to the odd TLV 11, the in-flight contribution to the odd TLV 13, and the last-negotiated feerate to the odd TLV 9. 0.2 skips all three. - An even TLV 14 is written only when there is more than one negotiation round (RBF), so 0.2 refuses exactly the splices it cannot operate while loading the rest. Putting the contributions in odd TLVs is safe: a single splice does not need them, and RBF -- where current relies on them -- always sets the even gate, so 0.2 refuses such a channel rather than skipping a contribution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A channel may have splice attempts in progress: one under negotiation with the counterparty and any negotiated transactions (the original splice and any RBF replacements) waiting on confirmations. This state was only observable through events and the broadcaster's TransactionType::InteractiveFunding, neither of which can be queried on demand. Add an optional splice_details field to ChannelDetails exposing the negotiation status and our contribution to it, the negotiated candidates (txid, post-splice channel value, our contribution, and confirmation progress), and the txids of any splice_locked messages exchanged with the counterparty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Only one negotiated candidate can confirm since they double-spend each other, so move confirmation tracking off each candidate into a single ConfirmedSpliceCandidate, identifying the confirming candidate by txid. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The splice_locked we send always refers to the candidate that confirmed: it is only sent once a candidate reaches sufficient confirmations, and is cleared if that candidate is later unconfirmed by a reorg. So SpliceDetails::sent_splice_locked_txid was redundant with the confirmed candidate's txid. Replace it with a splice_locked_sent boolean on ConfirmedSpliceCandidate. The received splice_locked stays a separate txid on SpliceDetails, as the counterparty may consider a different candidate sufficiently confirmed than we do. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jkczyz
force-pushed
the
2026-06-splice-details-fable
branch
from
30fdc02 to
3f954c5
Compare
|
🔔 1st Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
| } | ||
|
|
||
| #[test] | ||
| fn upgrade_single_splice_from_0_2() { |
There was a problem hiding this comment.
Let's expand this test to also attempt an RBF after upgrading?
There was a problem hiding this comment.
Hmm, seems like we won't be able to RBF as we can't determine if we've contributed. At least we can't without losing our prior contributions. I added a guard to prevent this, which was previously a debug_assert on the last funding feerate.
| } | ||
|
|
||
| #[test] | ||
| fn downgrade_single_splice_loads_on_0_2() { |
There was a problem hiding this comment.
Can we still RBF if we negotiate the splice on 0.3, downgrade to 0.2, and upgrade back? I assume no since we're missing the contribution.
There was a problem hiding this comment.
Yeah, same as upgrading from 0.2. See other comment.
| /// Note that a negotiation which has not yet reached | ||
| /// [`SpliceNegotiationStatus::AwaitingSignatures`] does not survive a restart, so this only | ||
| /// reflects in-memory negotiation state. | ||
| pub negotiation: Option<SpliceNegotiationDetails>, |
There was a problem hiding this comment.
Would it be possible to also surface the state after funding_contributed but before actual splice negotiation? This might be useful to tighten fuzz invariants.
Suggestion from #4699 (comment)
There was a problem hiding this comment.
Yeah, good idea. If they called funding_contributed successfully, it should show up here even if we haven't reached quiescence.
There was a problem hiding this comment.
Great, will then tighten the invariant SpliceDetails when this lands
A splice contribution committed via funding_contributed sits in the channel's quiescent action until quiescence is reached and it becomes a negotiation, so the window between funding_contributed and the negotiation starting was not reflected in ChannelDetails. Surface it as SpliceDetails::queued_contribution so that state is observable (e.g. for tightening fuzz invariants). It is reported independently of any in-progress negotiation, as the acceptor may queue its own contribution while a counterparty-initiated splice is in flight. Whether we become the initiator is not settled until quiescence, so no initiator flag is exposed. A debug assertion records that a queued splice never coexists with a negotiation we initiated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add tests exercising the 0.2/current wire boundary for pending splices: - A current node with a single pending splice (whether or not we contributed to it) is loadable by LDK 0.2. - A current node with a splice under RBF is refused by 0.2 via the even RBF-gate TLV. - A single pending splice written by 0.2 is read by current with no contribution recorded, since 0.2 never tracked one. The downgrade reload configs enable anchors so 0.2 accepts the current channel type rather than refusing it before the splice state is reached. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rsion A pending splice negotiated before an upgrade from a prior LDK version (e.g. 0.2) returns without its feerate or our contribution: 0.2 persists neither and drops the odd TLVs that carry them. splice_channel derives the RBF feerate floor from the prior splice's feerate and assumed it was always present via a debug assertion, so attempting to splice such a channel tripped that assertion. Return a clean error instead, refusing to splice a channel whose pending splice lacks the metadata to reconstruct the prior request. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jkczyz
force-pushed
the
2026-06-splice-details-fable
branch
from
22eba7d to
984f4cc
Compare
5 participants
Wallets like LDK Node need to show a channel's pending splice state (e.g., when displaying channel details), but it is currently only observable by tracking events or the broadcaster's
TransactionType::InteractiveFunding, neither of which can be queried on demand.This adds an optional
splice_detailsfield toChannelDetailsexposing that state: any splice/RBF round under negotiation (status, feerate, our contribution, txid and post-splice value once known) and any negotiated candidates awaiting confirmations, along with thesplice_lockedtxids exchanged.The first commit refactors
PendingFundingto store each round's contribution with its negotiated candidate instead of in a tail-aligned parallel list, which every consumer had to realign (and which this API initially got wrong). The on-disk format is unchanged and remains readable by LDK 0.2.