Retry user-initiated splices across restarts and disconnects

[jkczyz]

LDK abandons an in-progress splice negotiation whenever the peer disconnects (which includes stopping the node) and only durably records a splice once it reaches signing. Between calling splice_in/splice_out/bump_channel_funding_fee and that point, a restart or an ill-timed disconnect silently drops the splice with no way to recover it.

This makes those splices durable and self-healing: the intent is persisted before the contribution is handed to LDK, and a startup reconciler plus a SpliceNegotiationFailed handler resubmit it (gated on LDK's NegotiationFailureReason::is_retriable) until the splice locks or is genuinely unrecoverable. As a result SpliceNegotiationFailed is now emitted only once a splice is given up on, rather than for every failed negotiation round.

Rather than adding a dedicated store, the intent lives in the existing PendingPaymentStore under a PaymentId generated at splice time — which becomes the splice's payment id, replacing the previous first-candidate-txid derivation. That required modeling the pending record as an enum, since a not-yet-broadcast splice has no funding transaction, and therefore no PaymentDetails, yet. The change is scoped to user-initiated splices; counterparty-initiated splices and V2 opens are untouched and keep the txid-derived id.

Restart resumption is covered by two integration tests (a splice-out and an RBF fee-bump), alongside unit coverage of the retry-decision matrix; the existing splice/funding/on-chain suites continue to pass.

Based on #962.

[ldk-reviews-bot]

👋 Hi! I see this is a draft PR.
I'll wait to assign reviewers until you mark it as ready for review.
Just convert it out of draft status when you're ready for review!

[joostjager]

The PR description explains that LDK currently does not durably record a splice until signature exchange, but I don’t think it explains why ldk-node should therefore become the owner of that durability?

This seems similar to the functionality added in #882: common enough, and close enough to channel/protocol state, that it feels like it should live in LDK if we want the behavior to be durable. Persisting it in ldk-node means we now have another store tracking protocol state alongside ChannelManager/ChannelMonitor, plus reconciliation logic to infer whether LDK still has or no longer has the splice. That creates desync risk between persistence layers.

I can see ldk-node owning product policy around retries or how/when to surface failures, but the durable record of an accepted splice contribution or in-flight splice intent feels like it should be owned by LDK.

[tnull]

This will now need a (likely rather considerable) rebase now that #888 and a few related PRs landed.

[jkczyz]

Re-wrote this to use the exiting PendingPaymentStore as discussed offline. See PR description for details. @tnull Not sure what you think about making PendingPaymentDetails an enum that transitions (see 29a93ef). But high-level feedback on that design would be appreciated.

[joostjager]

I’d still be interested in the rationale for why this needs to live in ldk-node rather than ldk.

If ldk drops in-progress splice negotiation state on disconnect or restart before signing, doesn’t every splicing integration need to persist intent, reconcile on restart, retry when appropriate, and surface final failure?

[jkczyz]

I’d still be interested in the rationale for why this needs to live in ldk-node rather than ldk.

If ldk drops in-progress splice negotiation state on disconnect or restart before signing, doesn’t every splicing integration need to persist intent, reconcile on restart, retry when appropriate, and surface final failure?

Hmm... yeah it seems we didn't fully resolve this a couple meetings ago. The conversation was mostly around whether to persist in LDK Node by payment ID or channel ID.

Currently, if we have reached quiescence but haven't exchanged signatures, LDK will opportunistically persist an Event::SpliceNegotiationFailed with NegotiationFailureReason::PeerDisconnected. This allows LDK Node to retry the splice when processing the event. However, if we haven't reached quiescence, no such event is persisted nor is the corresponding QuiescentAction::Splice, which is essentially the intent.

IIUC, even if we persisted an Event::SpliceNegotiationFailed for the QuiescentAction::Splice, a user may initiated the splice using ChannelManger::splice_channel / ChannelManager::funding_contributed, but we aren't guaranteed ChannelManager has been persisted before funding_contributed returns. Thus, if a restart happens after returning but before persistence, we've lost the intent.

cc: @TheBlueMatt @wpaulino

[TheBlueMatt]

Right, currently we require the same downstream intent storage for outbound payments and splices (which are often outbound payments). One thing I've thought of is having those methods return a Future that completes when the channelmanager/monitors finish persistence and the intent is guaranteed to not be lost, which would then allow ldk-node to bubble that Future up to the caller and make the whole thing the caller's responsibility (which ultimately likely means the end user will be responsible for retrying if their app crashes when they were mid-payment). Its something we could explore for 0.4.