feat(wallet): remove a never-accepted transaction from the tx detail sheet - #954
Merged
Conversation
…sheet A stuck asset lock the network keeps rejecting (rebroadcast ends "no acceptance signal", tx on no explorer) had no way out: the tx row sat in history forever, its inputs stayed locked, and the launch-time resume kept rebroadcasting it. New "Remove if not on Blockchain" row under Rebroadcast — shown only for locks that never reached IS/CL (a locked tx is proven on-chain and never removable). The flow, per the confirmation dialog's contract: 1. Refuse if local state says the tx is confirmed. 2. Ask the network's Insight explorer; a known tx (mempool or mined) refuses the removal, and an unreachable explorer refuses too — "if not on Blockchain" is checked, never guessed. 3. Delete the PersistentTransaction row (outputs + pending inputs cascade), flip its spent input TXOs back to unspent, drop the PersistentAssetLock bookmark (the launch-resume trigger), and the app-side tx metadata. 4. Full runtime reload — Rust rebuilds tx set, UTXOs and spent_outpoints from the SwiftData rows, so the wallet has never heard of the tx and the coins are spendable again. 5. Arm spvRescanFilters back past the tx's first-seen time: if the explorer was wrong, the compact-filter re-match restores the tx. There is no removal API at any FFI layer (key-wallet has no rollback path at all; untrack_asset_lock is pub(crate) + Built-only), hence the persistence-layer route — TODO(sdk-remove-tx) tracks replacing it with a first-class SDK call. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The home row gave every pending identity-funding lock the registration recovery treatment — "Pending — tap to finish" with the tap routed into the Create Username flow. For a top-up both halves are wrong: there is nothing to finish there, and the tap never opened the transaction. New isPendingIdentityRegistration narrows that routing to funding types 0/3 (the ones AssetLockRecoveryService documents as registration-flow recovery); a pending top-up now reads plain "Pending" and opens the detail sheet, where Rebroadcast and Remove if not on Blockchain live. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds verified removal for eligible unconfirmed transactions, including persistence cleanup and wallet recovery. It also adds transaction-detail controls and updates pending identity-registration transactions to open the DashPay username flow. ChangesTransaction recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TxDetailViewController
participant UnconfirmedTransactionRemover
participant DashExplorer
participant PersistenceStore
participant WalletRuntime
TxDetailViewController->>UnconfirmedTransactionRemover: Request transaction removal
UnconfirmedTransactionRemover->>DashExplorer: Verify transaction absence
DashExplorer-->>UnconfirmedTransactionRemover: Return verification result
UnconfirmedTransactionRemover->>PersistenceStore: Remove local transaction records
UnconfirmedTransactionRemover->>WalletRuntime: Reload wallet and refresh lookup state
UnconfirmedTransactionRemover-->>TxDetailViewController: Return removal status
Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This was referenced Aug 10, 2026
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.
Issue being fixed or feature implemented
A stuck asset lock the network keeps rejecting had no way out. Concrete case from testnet QA: an identity top-up whose every Rebroadcast reached 2/3 peers and ended "outcome uncertain: no acceptance signal within 60s", absent from the testnet Insight explorer in both txid byte orders. The row sat in history forever ("Broadcasting"), the coins it tried to spend stayed locked, and the launch-time asset-lock resume kept rebroadcasting it on every start.
Also fixes the adjacent mislabel this QA surfaced: a pending identity top-up row read "Pending — tap to finish" and the tap opened the Create Username flow — the registration-recovery treatment applied to all identity funding types 0–3.
What was done
"Remove if not on Blockchain" (red, under Rebroadcast; only for locks that never reached IS/CL — a locked transaction is proven on-chain and never removable). The confirmation dialog states the exact contract, and the flow implements it in order:
PersistentTransactionrow (outputs + pending-input placeholders cascade), flip the input TXOs it spent back to unspent (the denormalizedisSpentcolumn doesn't follow the relationship nullify on its own), delete thePersistentAssetLockbookmark that drove the launch-time rebroadcasts, and the app-side tax-category metadata.rearmPlatformSync— the same serialized stop → load → start a network switch runs). The Rust side rehydrates its entire core state from the SwiftData rows on load, andspent_outpointsis rebuilt from the transaction set — so after reload the wallet has never heard of the transaction, in all places: history, UTXO set, balance, and dash-spv's mempool tracker.spvRescanFiltersback past the transaction's first-seen time (age-scaled window, clamped ~30h…~5w, +1 day margin): if the explorer was wrong, the compact-filter re-match rediscovers the transaction on-chain and state self-heals.Why the persistence-layer route: there is no transaction-removal API at any reachable FFI layer — verified against the pinned revs: key-wallet has no reorg/rollback/remove path at all,
abandonTransactiononly covers pre-broadcast handles, anduntrack_asset_lock(the exact primitive wanted) ispub(crate)with astatus == Builtguard.TODO(sdk-remove-tx)marks the seam for a first-class SDK call once one exists upstream.Pending-label fix: new
isPendingIdentityRegistrationnarrows the "Pending — tap to finish" + Create-Username routing to funding types 0/3 (exactly the onesAssetLockRecoveryServicedocuments as registration-flow recovery). A pending top-up now reads plain "Pending" and opens the detail sheet, where Rebroadcast and the new Remove action actually live.How Has This Been Tested?
Clean
dashpayarm64 simulator build; installed on the testnet QA simulator with the real stuck top-up present. In-app verification of the end-to-end removal (explorer check → delete → reload → rescan) is in progress on that simulator and results will be posted on this PR. (Unit-test target pre-existing broken.)Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements