[v0.1.x-branch] Backport #1068: unroll: cancel LND broadcast of a proof tx on terminal failure - #1097
Merged
Merged
Conversation
Bump github.com/lightninglabs/lndclient to the master commit that adds WalletKit.RemoveTransaction (lightninglabs/lndclient#284), which the terminal-failure rebroadcast cleanup in this PR uses to drop an abandoned transaction from lnd's wallet. (cherry picked from commit 04c1360)
Extend TxBroadcaster with RemoveTransaction and expose it on LNDBackend as the optional chainsource.TxRemover capability, wrapping lnd's WalletKit RemoveTransaction RPC. It lets a caller abandon a transaction so lnd stops rebroadcasting it from its wallet queue (wavelength#609). (cherry picked from commit 5653aa9)
Add a TxRemover interface a ChainBackend may implement to remove a previously broadcast transaction from the wallet, plus a RemoveTxRequest routed through the chain-source actor. The handler forwards to a capable backend and treats a backend without the capability (e.g. a pure Esplora chain source) or an already-absent transaction as a no-op success, so a caller can request removal without knowing the backend (wavelength#609). (cherry picked from commit 6014477)
When an unroll job fails terminally, ask the wallet backend to drop the job's broadcast-but-unconfirmed proof (and sweep) transactions via the new chainsource RemoveTxRequest, so a full-node wallet stops perpetually rebroadcasting a transaction that can never confirm now that the exit has failed (wavelength#609). Only in-flight transactions are removed; confirmed ones are on-chain and are never rebroadcast. The removal runs once, is best-effort, and is a no-op on backends without the capability. (cherry picked from commit 1461dbb)
In this commit, we make the terminal cleanup also drop the transaction that actually failed. applyFailedEvent strips the failing txid from InFlightTxids before it stamps FailReason, so by the time removeAbandonedBroadcasts reads planner state, the tx that killed the job is no longer listed there. That tx is exactly the one lnd keeps rebroadcasting, so the cleanup was skipping its own target. We track the failed txids on the behavior rather than in planner state, which keeps the checkpoint codec untouched. It's in-memory only: the registry only restores non-terminal records, so a terminal job never re-runs the cleanup after a restart and has nothing to reload. While here, the sweep txid now gets added to the seen set before we append it. That was harmless while the sweep was the last entry, but it stops being harmless once another source follows it. We also sort the final list, since map iteration order isn't stable. The new test drives the txconfirm hard-failure path via setImmediateFailed rather than an external spend, so it fails without the fix. (cherry picked from commit 7bcc5d6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #1068
Closes #609.
Problem
After a unilateral-exit unroll job goes
FAILED, LND keeps rebroadcasting the rejecting proof tx from its own wallet queue every ~60s indefinitely — the wavelength daemon has stopped touching the job, but the transaction (a zero-fee anchor parent that can never relay alone) stays in LND's rebroadcast set. Nothing tells LND to drop it.Fix
Give the unroll actor a way to abandon its broadcast transactions on terminal failure:
go.mod— bumplndclientto the master commit addingWalletKit.RemoveTransaction.chainbackends—TxBroadcastergainsRemoveTransaction;LNDBackendexposes it as the optionalchainsource.TxRemovercapability, wrapping lnd'sWalletKit.RemoveTransactionRPC.chainsource— aTxRemoveroptional-capability interface + aRemoveTxRequestrouted through the chain-source actor. The handler forwards to a capable backend and treats a backend without the capability (a pure Esplora chain source) or an already-absent tx as a no-op success — so callers request removal uniformly without knowing the backend, and noChainBackendimplementer/fake had to change.unroll— on terminalPhaseFailed, the actor removes its in-flight (broadcast-but-unconfirmed) proof/sweep txids viaRemoveTxRequest. Confirmed txs are left alone (on-chain, never rebroadcast). The removal runs once, is best-effort, and detaches cancellation like the terminal registry handoff.Backend scope: the LND backend is fully covered.
lwwallet/btcwbackendsit on btcwallet, which also rebroadcasts — but since they don't implementTxRemover, they no-op today; a btcwallet removal path is a scoped follow-up.Testing
chainsource:TestChainSourceActorRemoveTx— forwards to a capable backend, treats an ignorable "not found" error as success, and no-ops on a backend without the capability.unroll:TestTerminalFailureRemovesAbandonedBroadcasts— a terminal failure removes the in-flight proof tx.chainsource+chainbackends+unrollsuites,go build,gofmt, andlint-changed-localare all green.🤖 Generated with Claude Code