Fix three funding payment record bugs - #1057
Open
jkczyz wants to merge 3 commits into
Open
Conversation
Wallet sync resolves a funding payment's id for any transaction linked to the record through its conflicting txids, and then adopted that transaction's txid and confirmation outright. A cooperative close conflicts with a pending splice in exactly that way: the splice record would report the close's txid and confirmation under its InteractiveFunding type and contribution figures and graduate as if the splice had confirmed, while the close's own record never received its confirmation. Adopt a transaction only when it is part of the payment's funding history — the record's current txid or a classified candidate. Anything else is recorded under its own txid-keyed id, which also delivers the close's confirmation to the close's own record. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A queued broadcast whose payment-record classification failed was dropped outright, on the theory that broadcasting a transaction we failed to record would leave it on-chain without a payment. For interactive funding that theory doesn't hold: the counterparty broadcasts the same transaction once the signature exchange completes, so dropping the package keeps nothing off-chain — it only guarantees the round is never recorded as a candidate on our side. The funding-status ownership gate then treats the round's confirmation as foreign to the funding record and re-keys it to a stray duplicate record, which shadows the funding record's txid lookups permanently: the splice payment stays Pending forever while an untyped duplicate holds the confirmation. Keep the package alive instead: requeue it after a short delay and retry classification until it succeeds, holding the broadcast back the whole time. Classification failures are persistence failures, so the retry is unbounded — a store that never recovers keeps the node from functioning anyway — and every failed round is logged. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bump_fee_rbf read the payment record and rejected channel-funding records before taking any lock, then took the locks and wrote the replacement. A funding classification landing in between re-types the record as channel funding, after which the bump retargets that record to the wallet-built replacement it broadcasts -- a double spend of the channel funding transaction. Hold the locks from the record read through the replacement writes so the two serialize: a record classified first is caught by the funding-kind check, and one that passes the check cannot be re-typed until the replacement is recorded. A funding round that wallet sync observes before classification leaves an untyped record that passes the funding-kind check outright; that is a stale-record problem rather than a race in this function, narrowed by the preceding classification-retry commit and by duplicate absorption later in the series. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
I've assigned @tnull as a reviewer! |
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
2 participants
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.
Three bugfixes for funding payment records (channel opens and splices), all reachable on current
main. Found while building the splice-retry work stacked on top (#930's replacement) but independent of it.Only adopt a funding payment's own transactions from wallet sync. Sync adopted the txid and confirmation of any transaction linked to a funding record through its conflicting txids. A cooperative close conflicts with a pending splice in exactly that way, so the splice record could adopt the close's confirmation and graduate as if the splice had confirmed.
Retry funding-broadcast classification instead of dropping it. A broadcast whose payment-record classification failed was dropped. For interactive funding the counterparty broadcasts the same transaction anyway, so the drop keeps nothing off-chain — it just leaves the round unrecorded, permanently stranding its confirmation on a duplicate record. Classification is now retried, with the broadcast held back, until it succeeds or the node shuts down.
Serialize on-chain RBF bumps with funding classification.
bump_fee_rbfchecked that the record isn't channel funding before taking any lock. A classification landing between the check and the writes re-types the record, and the bump then replaces a funding transaction — a double spend of channel funding. The check now happens under the locks. (A sequential variant — a sync-minted record passing the check untyped — closes later in the stack.)Each fix has a test that fails without it; the commit messages have the details.
First of three stacked PRs replacing #930's restart persistence for this release, per the discussion there; #XXXX (payment-model groundwork) and #XXXX (in-session splice retry) follow.
Developed with assistance from Claude Code.