Skip to content

feat(wallet): remove a never-accepted transaction from the tx detail sheet - #954

Merged
QuantumExplorer merged 2 commits into
developfrom
feat/remove-unconfirmed-tx
Aug 10, 2026
Merged

feat(wallet): remove a never-accepted transaction from the tx detail sheet#954
QuantumExplorer merged 2 commits into
developfrom
feat/remove-unconfirmed-tx

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Aug 10, 2026

Copy link
Copy Markdown
Member

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:

  1. Refuse if local state says the transaction is confirmed.
  2. Ask the network's Insight explorer. A known transaction (mempool or mined) refuses the removal with "Transaction is on the blockchain"; an unreachable explorer also refuses — the button's claim is checked, never guessed.
  3. Delete the PersistentTransaction row (outputs + pending-input placeholders cascade), flip the input TXOs it spent back to unspent (the denormalized isSpent column doesn't follow the relationship nullify on its own), delete the PersistentAssetLock bookmark that drove the launch-time rebroadcasts, and the app-side tax-category metadata.
  4. Full runtime reload (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, and spent_outpoints is 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.
  5. Arm spvRescanFilters back 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, abandonTransaction only covers pre-broadcast handles, and untrack_asset_lock (the exact primitive wanted) is pub(crate) with a status == Built guard. TODO(sdk-remove-tx) marks the seam for a first-class SDK call once one exists upstream.

Pending-label fix: new isPendingIdentityRegistration narrows the "Pending — tap to finish" + Create-Username routing to funding types 0/3 (exactly the ones AssetLockRecoveryService documents 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 dashpay arm64 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:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added an option to remove eligible unconfirmed asset-lock transactions that were not accepted on-chain.
    • Added confirmation, progress, success, and failure feedback during transaction removal.
    • Wallet state and transaction data refresh automatically after successful removal.
  • Improvements

    • Pending identity registration transactions now display clearer status messaging.
    • Tapping a pending identity registration opens the DashPay username setup flow.
    • Pending identity and platform funding transactions now use consistent “Pending” labels.

QuantumExplorer and others added 2 commits August 10, 2026 19:08
…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>
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4bf9250d-f735-48a8-b3e5-4555613a1308

📥 Commits

Reviewing files that changed from the base of the PR and between db2f011 and 9778361.

📒 Files selected for processing (7)
  • DashWallet.xcodeproj/project.pbxproj
  • DashWallet/Sources/Infrastructure/SwiftDashSDK/UnconfirmedTransactionRemover.swift
  • DashWallet/Sources/Models/Transactions/Model/Transaction.swift
  • DashWallet/Sources/UI/Home/Views/HomeView.swift
  • DashWallet/Sources/UI/Tx/Details/Model/TxDetailModel.swift
  • DashWallet/Sources/UI/Tx/Details/TxDetailViewController.swift
  • DashWallet/en.lproj/Localizable.strings

📝 Walkthrough

Walkthrough

Adds 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.

Changes

Transaction recovery

Layer / File(s) Summary
Verified transaction removal
DashWallet/Sources/Infrastructure/SwiftDashSDK/UnconfirmedTransactionRemover.swift, DashWallet.xcodeproj/project.pbxproj
Adds explorer verification, local transaction cleanup, wallet reload, compact-filter recovery, cache refresh, and target registration.
Transaction-detail removal action
DashWallet/Sources/UI/Tx/Details/Model/TxDetailModel.swift, DashWallet/Sources/UI/Tx/Details/TxDetailViewController.swift, DashWallet/en.lproj/Localizable.strings
Adds eligibility checks, confirmation, progress and result handling, destructive styling, and localized messages.
Pending identity-registration routing
DashWallet/Sources/Models/Transactions/Model/Transaction.swift, DashWallet/Sources/UI/Home/Views/HomeView.swift
Identifies pending registration funding and routes it to the DashPay username flow.

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
Loading

Possibly related PRs

Suggested reviewers: jeanpierreroma

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/remove-unconfirmed-tx

Comment @coderabbitai help to get the list of available commands.

@QuantumExplorer
QuantumExplorer merged commit 7e6b393 into develop Aug 10, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant