Repository navigation
Conversation
A withdrawal request's status now only describes its own transaction. Creating, executing or abandoning an RBF replacement never writes the request it replaces: the API derives WITHDRAWAL_BUMPED / _CANCELLED from a broadcast replacement, and the monitor stores Bumped / Cancelled only once a replacement confirms, or settles the request itself when it was mined instead. Bumped is terminal. - Replacements navigation, EffectiveStatus and the one-live-replacement guard (service check plus partial unique index) on WalletWithdrawalRequest - MonitorWithdrawalsJob: null-safe, per-row errors, floor of one confirmation, then ReconcileReplacementChainsAsync writes each linked row from its chain alone (fails every replacement of a mined original, a bump of a bump included; a bump of a cancellation still cancels) - GetWithdrawalsRequestStatusByReferenceIds returns the whole chain, and WithdrawalRequest gains replaced_by_request_id - Migration repairs legacy Bumped / RBF-cancelled rows with no confirmed replacement back to pending and guards the index creation - Withdrawals page gates bump / cancel on no live replacement, shows the derived status, and the revert paths are gone
markettes
added this pull request to stack #646
October 19, 2026 13:41
- One Ancestors iterator replaces the two hand-rolled upward walks; the chain is cached as a dictionary and the live rows are fetched as ids - One "live" status set: the repository and the index filter derive from WalletWithdrawalRequest.LiveStatuses; the FinalizingPSBT skip lives in the service only - SupersededBy gives the derived and the stored Bumped / Cancelled the same (status, reason), and the reason builders share one tx suffix - Repository.AddAsync returns the database's message like AddRangeAsync, so the unique-index race is recognised from it and HasLiveReplacementAsync goes away - GetRbfChainAsync walks up to the root, then down level by level, no re-fetching; the status RPC looks confirmations up in parallel - TRANSACTION_CONFIRMATION_MINIMUM_BLOCKS is clamped at its source - The Withdrawals page shows the stored status plus the replacement; the migration keys RBF-cancelled rows on the link, not on the reason text
markettes
force-pushed
the
refactor/bumped-terminal-only
branch
from
October 19, 2026 16:51
6cca56d to
e51cc24
Compare
markettes
force-pushed
the
fix/failed-rbf-original-confirmed
branch
from
October 19, 2026 17:32
c06d7b8 to
01b77a0
Compare
Contributor
Author
|
Folded into #642. |
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.
Stacked on #642.
What changes
A withdrawal request's status now only describes its own transaction. Creating, executing or abandoning an RBF replacement never writes the request it replaces; "replaced" is derived from the replacement through the existing
BumpingWalletWithdrawalRequestIdlink, andBumpedbecomes terminal: written only by the monitor once a replacement confirms.Why:
Bumpedmeant two things at once ("replacement in flight" and "superseded, done"), and that dual meaning is behind the bugs the RBF work keeps hitting. A bumped original was not polled, so its confirmation was only noticed through the replacement's row; whenever the replacement was abandoned without a revert (RPCMarkWithdrawalRequestCancelled, the UI's dismissed confirmation modal, a failed revert write) the original satBumpedforever with a transaction that could still confirm, by-id clients sawWITHDRAWAL_BUMPEDforever, and its inputs stopped counting as locked. The revert itself was the cross-row write Copilot kept flagging on #642.OnChainConfirmationPendingWITHDRAWAL_PENDING_CONFIRMATION,replaced_by_request_idsetWITHDRAWAL_BUMPED/WITHDRAWAL_CANCELLED(derived)OnChainConfirmationPendingWITHDRAWAL_PENDING_CONFIRMATIONagainBumped/Cancelled+ reason, by the monitorOnChainConfirmedOnChainConfirmed, the monitor polls it itselfWITHDRAWAL_SETTLEDFailed"Withdrawal request A was mined instead", every replacement, a cold one awaiting approval includedReplacementsnavigation,EffectiveStatus()(single derivation point for the RPC and the Withdrawals page),LiveReplacement/BroadcastReplacement. The AutoMapper self-map ignores both RBF navigations soRepository.Updatenever writes a neighbour.IX_WalletWithdrawalRequests_LiveReplacement; the loser of a race getsFAILED_PRECONDITION, notINTERNAL.ReconcileReplacementChainsAsyncgives each live linked row its final status from the database alone. Each row writes only itself, so a failed write is retried next run. Path rule: a bump of a cancellation still cancels the first original.GetWithdrawalsRequestStatusByReferenceIdsnow returns every request carrying the reference id, oldest first, each with its own status.WithdrawalRequestgainsreplaced_by_request_id(newest non-abandoned replacement, set even while awaiting approval).WITHDRAWAL_BUMPEDstays, derived while in flight, stored once settled.MarkOriginalAsReplacedAsync,RevertOriginalAsync,SettleMinedOriginalAsync, the"Cancelled by RBF replacement request"marker comparison.Migration
BumpedIsTerminalOnlyRepairs legacy rows in SQL before creating the index: a
Bumpedor RBF-cancelled request none of whose replacements (recursively) confirmed goes back toOnChainConfirmationPending; rows whose replacement confirmed are already final. Then a guard raises a readable error if any request has more than one live replacement. Before deploying to prod run:Tests
Category!=E2E),just check-migrationsclean. New coverage for the guard, the race on insert, the chain reconciliation (bump of a bump, bump of a cancellation, cancellation of a bump, cold replacement awaiting approval, finalizing rows skipped, both directions confirmed), the derived statuses over gRPC, the by-reference chain response, and the mapper.BumpOfABump…now expects B and C failed; the by-reference assertion expects the whole chain). Not run locally yet: add therun-e2e-testslabel.