feat(swift-sdk): compact-filter rescan button in Core Sync Status#4103
Conversation
Adds a Rescan button to SwiftExampleApp's Core Sync Status card that arms an SPV compact-filter rescan (via the spvRescanFilters API from #4099) for every wallet on the active network, with a height-choice dialog (last 1k/10k blocks, everything, or a custom height) and a status caption reporting the outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesSPV rescan flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CoreContentView
participant walletManager
User->>CoreContentView: select rescan range
CoreContentView->>walletManager: spvRescanFilters for each active wallet
walletManager-->>CoreContentView: rescan result or error
CoreContentView-->>User: display rescan status
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Review complete (commit f08071d) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift`:
- Around line 232-246: Update the custom-height “Rescan” action in the
showCustomHeightAlert handler to set rescanStatus to an appropriate
invalid-input message when UInt32 parsing fails, including empty values and
values above UInt32.max. Preserve the existing armRescan(fromHeight:) path for
valid input and continue clearing customHeightText.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 842c261d-5719-4a84-9f4a-56007e81c7d2
📒 Files selected for processing (1)
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The latest delta fixes the invalid-custom-height path by disabling Rescan until customHeightValue parses, with no new latest-delta findings. The two prior rescan-correctness blockers remain unchanged, and the test-coverage nitpick remains. Prior reconciliation: prior-1 STILL VALID, prior-2 STILL VALID, prior-3 FIXED, prior-4 STILL VALID.
Carried-forward prior findings: 3 (2 blocking, 1 nitpick). New findings in the latest delta: none. Fixed prior findings: 1 (prior-3).
Source: reviewers gpt-5.6-sol (Sol reviewer) and claude-sonnet-5 (Sonnet reviewer); verifier claude-sonnet-5. Orchestrator openai/gpt-5.6-sol (orchestration-only).
🔴 2 blocking | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift`:
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift:908-919: [Carried forward — STILL VALID] Relative rescan presets collapse to a full rescan once SPV is stopped
`armRescan(lastBlocks:)` computes its 'last N blocks' target relative to `rescanReferenceTip`, which reads `walletManager.spvProgress.{filters,filterHeaders,headers}?.currentHeight`. Once SPV is stopped, `SpvRuntime::stop()` drops the client, `sync_progress()` returns `None`, and the FFI layer publishes an empty progress snapshot back through Swift — every sub-progress field becomes absent/0. `rescanReferenceTip` then falls through all three `?? 0` fallbacks to 0, so both 'Last 1,000 blocks' and 'Last 10,000 blocks' silently call `armRescan(fromHeight: 0)`, behaving identically to 'Everything (from height 0)' with no indication to the user that their targeted partial rescan just became a full rescan.
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift:932-952: [Carried forward — STILL VALID] Rescan reports wallets as armed without verifying an actual rewind occurred
The per-wallet loop in `armRescan(fromHeight:)` treats every non-throwing `spvRescanFilters` call as 'armed', without comparing `fromHeight` against that wallet's current checkpoint. `spv_rescan_filters_blocking` unconditionally overwrites `synced_height` and returns success even when `fromHeight` is at or above the checkpoint, so no rewind actually occurs despite the reported success — and a later height applied to an already-behind wallet can skip pending scan work. Use `walletManager.coreWalletState(for:)` to gate the call on a strict rewind.
| private var rescanReferenceTip: UInt32 { | ||
| let filters = walletManager.spvProgress.filters?.currentHeight ?? 0 | ||
| if filters > 0 { return filters } | ||
| let filterHeaders = walletManager.spvProgress.filterHeaders?.currentHeight ?? 0 | ||
| if filterHeaders > 0 { return filterHeaders } | ||
| return walletManager.spvProgress.headers?.currentHeight ?? 0 | ||
| } | ||
|
|
||
| private func armRescan(lastBlocks: UInt32) { | ||
| let tip = rescanReferenceTip | ||
| armRescan(fromHeight: tip > lastBlocks ? tip - lastBlocks : 0) | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: [Carried forward — STILL VALID] Relative rescan presets collapse to a full rescan once SPV is stopped
armRescan(lastBlocks:) computes its 'last N blocks' target relative to rescanReferenceTip, which reads walletManager.spvProgress.{filters,filterHeaders,headers}?.currentHeight. Once SPV is stopped, SpvRuntime::stop() drops the client, sync_progress() returns None, and the FFI layer publishes an empty progress snapshot back through Swift — every sub-progress field becomes absent/0. rescanReferenceTip then falls through all three ?? 0 fallbacks to 0, so both 'Last 1,000 blocks' and 'Last 10,000 blocks' silently call armRescan(fromHeight: 0), behaving identically to 'Everything (from height 0)' with no indication to the user that their targeted partial rescan just became a full rescan.
| private var rescanReferenceTip: UInt32 { | |
| let filters = walletManager.spvProgress.filters?.currentHeight ?? 0 | |
| if filters > 0 { return filters } | |
| let filterHeaders = walletManager.spvProgress.filterHeaders?.currentHeight ?? 0 | |
| if filterHeaders > 0 { return filterHeaders } | |
| return walletManager.spvProgress.headers?.currentHeight ?? 0 | |
| } | |
| private func armRescan(lastBlocks: UInt32) { | |
| let tip = rescanReferenceTip | |
| armRescan(fromHeight: tip > lastBlocks ? tip - lastBlocks : 0) | |
| } | |
| private func armRescan(lastBlocks: UInt32) { | |
| let tip = rescanReferenceTip | |
| guard tip > 0 else { | |
| print("⚠️ Start SPV before using a relative rescan preset, or choose a specific height.") | |
| return | |
| } | |
| armRescan(fromHeight: tip > lastBlocks ? tip - lastBlocks : 0) | |
| } |
source: ['codex-general', 'sonnet5-general']
| private func armRescan(fromHeight: UInt32) { | ||
| let ids = walletIdsOnNetwork | ||
| guard !ids.isEmpty else { return } | ||
|
|
||
| var failures: [String] = [] | ||
| for walletId in ids { | ||
| do { | ||
| try walletManager.spvRescanFilters(walletId: walletId, fromHeight: fromHeight) | ||
| } catch { | ||
| failures.append("\(rescanShortId(walletId)): \(error.localizedDescription)") | ||
| } | ||
| } | ||
|
|
||
| let armed = ids.count - failures.count | ||
| let applied = isSpvRunning ? "" : " (applies on next SPV start)" | ||
| print("🔁 Rescan armed from height \(fromHeight.formatted()) " | ||
| + "for \(armed) wallet\(armed == 1 ? "" : "s")\(applied)") | ||
| if !failures.isEmpty { | ||
| print("❌ Rescan failed for: \(failures.joined(separator: ", "))") | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: [Carried forward — STILL VALID] Rescan reports wallets as armed without verifying an actual rewind occurred
The per-wallet loop in armRescan(fromHeight:) treats every non-throwing spvRescanFilters call as 'armed', without comparing fromHeight against that wallet's current checkpoint. spv_rescan_filters_blocking unconditionally overwrites synced_height and returns success even when fromHeight is at or above the checkpoint, so no rewind actually occurs despite the reported success — and a later height applied to an already-behind wallet can skip pending scan work. Use walletManager.coreWalletState(for:) to gate the call on a strict rewind.
source: ['codex-general', 'sonnet5-general']
| @@ -185,6 +203,34 @@ var body: some View { | |||
| } | |||
| } | |||
| .padding(.vertical, 4) | |||
| .confirmationDialog( | |||
| "Rescan Compact Filters", | |||
| isPresented: $showRescanDialog, | |||
| titleVisibility: .visible | |||
| ) { | |||
| Button("Last 1,000 blocks") { armRescan(lastBlocks: 1_000) } | |||
| Button("Last 10,000 blocks") { armRescan(lastBlocks: 10_000) } | |||
| Button("Everything (from height 0)") { armRescan(fromHeight: 0) } | |||
| Button("Custom height…") { showCustomHeightAlert = true } | |||
| Button("Cancel", role: .cancel) {} | |||
| } message: { | |||
| Text("Rewind the filter scan to re-download and re-match " | |||
| + "compact filters for every wallet on this network.") | |||
| } | |||
| .alert("Rescan From Height", isPresented: $showCustomHeightAlert) { | |||
| TextField("Block height", text: $customHeightText) | |||
| .keyboardType(.numberPad) | |||
| Button("Rescan") { | |||
| if let height = customHeightValue { | |||
| armRescan(fromHeight: height) | |||
| } | |||
| customHeightText = "" | |||
| } | |||
| .disabled(customHeightValue == nil) | |||
| Button("Cancel", role: .cancel) { customHeightText = "" } | |||
| } message: { | |||
| Text("Enter the core block height to rescan compact filters from.") | |||
| } | |||
There was a problem hiding this comment.
💬 Nitpick: [Carried forward — STILL VALID] Rescan control still has no UI or unit test coverage
The cumulative PR adds dialog, parsing, preset resolution, per-wallet iteration, and failure aggregation, all without UI or unit test coverage. Targeted unit tests on rescanReferenceTip/armRescan would have caught the two blocking defects above directly.
source: ['codex-general', 'sonnet5-general']
…nce executor + mnemonic error codes Three lifecycle blockers from review at 97cd577: - Use-after-free window on teardown: scope.cancel() is cooperative, so a coroutine already inside a blocking JNI call (the DashPay contact-crypto drain, a mid-call poll) kept running while close() freed the boxed PersistenceCallbacks context via nativeDestroy — a late persister.store() then dereferenced freed memory through a dropped GlobalRef. close() now joins the manager scope before nativeDestroy so every in-flight native call has returned first. - Executor leak per network switch: the handler's owned non-daemon "dash-persistence" single-thread executor was created inline as a default arg and never shut down. The handler now tracks ownership (injected dispatchers are never closed), implements AutoCloseable, and the manager closes it after nativeDestroy quiesces callbacks. - Mnemonic failures lost their code: FFIError::clean() resets code to Success, and mnemonic.rs read error.code AFTER clean() — every failure surfaced as 0/InternalError. Capture the code first (an invalid word count keeps InvalidParameter). Review suggestions in the same round: - Surface the exact DashPay send fee: the JNI now returns a 40-byte packed txid[32] || fee(u64 LE) and Dashpay.sendPayment returns SendPaymentResult(txid, feeDuffs) — the Swift (txid, feeDuffs) shape. (The prior comment claiming the fee was readable from the persisted payment entry was wrong — DashpayPaymentEntity has no fee column.) - PARITY.md: CoreContentView row downgraded to partial — the Swift-only compact-filter Rescan button + height picker (#4103 over the #4099 rescan FFI) is documented as a deferred Android gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
#4099 exposed SPV compact-filter rescan (
spvRescanFilters(walletId:fromHeight:), a wallet synced-height rewind), but SwiftExampleApp had no way to trigger it. The only way to recheck filters was the Core sync Clear button, which throws away headers and all sync state. This adds the cheap middle option: recheck filters without nuking anything.What was done?
All in
SwiftExampleApp/Core/Views/CoreContentView.swift(app-layer orchestration only; no SDK/FFI changes):coreSync.rescanFiltersButton), disabled when the active network has no wallets.confirmationDialogwith height choices — last 1,000 blocks, last 10,000 blocks, everything (height 0), or a custom height via a numeric-input alert. "Last N" counts back from the filter-scan tip, falling back to filter-headers then headers height.armRescan(fromHeight:)callswalletManager.spvRescanFiltersfor every wallet on the active network, collecting per-wallet failures instead of aborting on the first.How Has This Been Tested?
build_ios.sh --target sim(warnings-as-errors) passes.Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit