Add splice RBF support#888
Conversation
|
👋 Hi! I see this is a draft PR. |
|
@tnull Any feedback on the last few commits would be appreciated: https://github.com/lightningdevkit/ldk-node/pull/888/changes/492ec9124ae02a4048ecb5270309174b05b366d2..c770999887e1287bef8abe4f4c92ca36e9434067 |
198bfbf to
d5b6bda
Compare
When a splice is already pending, the user needs a way to replace its funding transaction at a higher feerate. This adds rbf_channel() to handle that case and guards splice_in/splice_out against being called while a pending splice exists, directing users to rbf_channel instead. Also fixes signing for RBF replacements, which requires accessing outputs spent by unconfirmed transactions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Channel-opening and splice transactions transition to Succeeded when ChannelReady fires, not after ANTI_REORG_DELAY confirmations. This matches the point at which the Lightning layer considers the channel usable: a zero-conf channel graduates as soon as its counterparty signals, and a high-conf channel waits however many confirmations the peer requires, rather than always stopping at six. For splice RBF, the payment records whichever candidate actually confirmed, with that candidate's amount and this node's share of the fee — not the fee-estimate used for weight at coin-selection time, and not the whole-tx fee for a multi-contributor splice. A channel closure whose funding or splice never confirmed discards its payment record instead of leaving it pending forever. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every on-chain payment update had to write both stores, and keeping them consistent relied on each call site doing the right thing. Nothing structural prevented a site from forgetting one of the writes or updating them inconsistently. No behavior change. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LDK calls broadcast_transactions once per signed funding or splice tx today. Record that assumption and note what would need to change if upstream starts rebroadcasting unconfirmed funding or splice transactions. No behavior change. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstream LDK replaced the prior `TransactionType::Splice` variant with a unified `TransactionType::InteractiveFunding(Vec<FundingCandidate>)` that carries the full negotiated-candidate history per broadcast, and exposed `FundingCandidate`, `ChannelFunding`, and `FundingPurpose` as named, persistable types. Drop our duplicate types and use upstream's directly. The classify path becomes stateless: each broadcast carries the full history, so there's no merge-into-existing-record branch. Anchor the payment record's `PaymentId` to the first negotiated candidate so the record stays stable across RBF replacements. Skip recording when the local node didn't contribute on a candidate; wallet sync will surface any wallet-visible activity (e.g., a counterparty's splice-out paid to our address) as a plain on-chain receive. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the BroadcasterInterface implementation wrote the payment record synchronously when LDK invoked it. With a remote KV store this could block LDK's message handling for hundreds of milliseconds per call, noticeably during force-close bursts or splice broadcasts. Persistence now happens asynchronously and must complete before the transaction is sent to the chain client. If persistence fails, the broadcast is dropped: a payment record must exist for every on-chain tx we emit, otherwise a crash could leave the tx confirmed with no matching record. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Classifying a package no longer collects the transactions into a fresh Vec; the originally-queued package is returned and the chain-source broadcast loop iterates it directly. No behavior change. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d5b6bda to
91166b9
Compare
tnull
left a comment
There was a problem hiding this comment.
Excuse the delay. Took a first look, generally makes sense, though it's kind off odd to block the broadcast queue on persistence. Hopefully we'd never get a lot of backpressure from remote persistence, for example.
I have yet to review the (rather verbose) new code for updating/persisting the pending payments in wallet.rs in more detail.
| /// # Experimental API | ||
| /// | ||
| /// This API is experimental and may change in the future. | ||
| pub fn rbf_channel( |
There was a problem hiding this comment.
nit: Maybe this could be called replace_channel_funding or bump_channel_funding_fee or similar?
Adds a public
Node::rbf_channelAPI that replaces an in-flight splice transaction with a higher-feerate version.An RBF leaves multiple candidate splice transactions in flight; only one of them will confirm on chain, and the payment record needs to reflect whichever one did — its amount and this node's share of the fee. The transition from
PendingtoSucceededshould also match when the Lightning protocol actually considers the new channel state usable — the momentChannelReadyfires — rather than afterANTI_REORG_DELAYconfirmations, which doesn't fit the zero-conf extreme on one end or higher-confirmation-depth peer configurations on the other.To support those properties, this PR:
Every broadcast from LDK now triggers a store write. For a remote KV store, running that write on LDK's thread would block message handling for hundreds of milliseconds per call, so persistence happens asynchronously while still guaranteeing that the record is committed before the transaction reaches the chain client.
Based on #878.