fix(swift-example-app): fail closed on persistence recovery by shumkov · Pull Request #4177 · dashpay/platform

shumkov · 2026-07-21T08:38:45Z

Issue being fixed or feature implemented

Follow-up to #3999. The persistence behavior review found two in-tree SwiftExampleApp consumer bugs: shielded Clear could wipe SwiftData after the Rust reset failed or could not run, and a retry backed by a retained local asset-lock tombstone was mislabeled as Claimed instead of Reclaimed.

What was done?

Abort the shielded host-row wipe unless the Rust coordinator reset succeeds.
Surface missing-manager and reset failures through ShieldedService.lastError.
Treat typed asset-lock code 24 as definitive evidence of a successful earlier local reclaim.
Keep consensus-message already-consumed handling conservative because it cannot identify the consumer.
Add focused classifier and shielded-clear regression tests.
Add the source-verification report for all four reviewed behavior changes.

How Has This Been Tested?

xcodebuild build-for-testing for an arm64 iOS 26.5 simulator: passed; app, unit-test bundle, and UI-test bundle compiled.
xcrun swiftc -parse for the changed Swift source and test files: passed.
git diff --check: passed.
Targeted Rust tests for the four investigated paths passed: legacy create fail-closed capabilities, nonzero changeset-begin abort, asset-lock resume tombstones, and asset-lock persistence rename.
Focused XCTest execution was attempted, but the local CoreSimulator shut down while Xcode launched the runner. The bundles compiled successfully, but runtime results were not produced in this environment.

Breaking Changes

None. This restores fail-closed consumer behavior and the intended local reclaim status semantics.

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 added ! to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

Summary by CodeRabbit

New Features
- Reclaim retries can now detect when assets were already recovered by the current wallet and mark invitations as Reclaimed.
Bug Fixes
- Shielded local-state clearing is now properly sequenced: the app asks Rust to quiesce/reset before wiping stored shielded data.
- Clear operations abort on reset/persistence failures to avoid leaving storage in an inconsistent state.
- Reclaim failure classification is improved for locally-consumed tombstone scenarios.
Tests
- Expanded reclaim and shielded clear behavior coverage.
Documentation
- Added a report detailing persistence consumer behavior changes and verification notes.

thepastaclaw · 2026-07-21T08:39:06Z

🕓 Ready for review — 54 ahead in queue (commit 352fb4a)
Queue position: 55/55

coderabbitai · 2026-07-21T08:41:51Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 68a71aad-227b-45e7-a464-1c1164e5be96

📥 Commits

Reviewing files that changed from the base of the PR and between 2324f1f and 352fb4a.

📒 Files selected for processing (4)

docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift
packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedServiceClearTests.swift
packages/swift-sdk/run_tests.sh

🚧 Files skipped from review as they are similar to previous changes (1)

docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md

📝 Walkthrough

Walkthrough

The change adds a persistence behavior report and updates Swift consumers. Reclaim classification recognizes typed local consumed-tombstone errors as reclaimed, while shielded clearing now preserves SwiftData rows when Rust reset cannot complete.

Changes

Persistence consumer behavior

Layer / File(s)	Summary
Persistence contracts and investigation report `docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md`	Documents wallet capability handling, changeset callback behavior, consumed tombstones, shielded clearing behavior, follow-up fixes, and verification results.
Consumed-tombstone reclaim classification `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ReclaimInvitationSheet.swift`, `packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ReclaimInvitationClassifierTests.swift`	Adds a `.reclaimed` outcome for typed local consumed-tombstone errors, preserves conservative consensus fallback behavior, persists the reclaimed invitation state, and expands classifier coverage.
Load-bearing shielded reset `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift`, `packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift`, `packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedServiceClearTests.swift`	Requires Rust reset success before SwiftData deletion, updates the Clear-flow documentation, and tests missing-manager, reset-failure, and reset-success paths.

CI keychain setup

Layer / File(s)	Summary
Keychain setup ordering `packages/swift-sdk/run_tests.sh`	Unlocks the dedicated CI keychain before applying its settings.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

dashpay/platform#4038: Related to shielded engine binding and rebind behavior around ShieldedService clearing.

Suggested labels: Client Only

Suggested reviewers: llbartekll, quantumexplorer, zocolini

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: making SwiftExampleApp persistence recovery fail closed.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 investigate/persistence-consumer-behavior-changes

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.

❤️ Share

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

shumkov · 2026-07-21T15:20:11Z

@coderabbitai review

coderabbitai · 2026-07-21T15:20:16Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

🧹 Nitpick comments (1)

docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix markdown heading level.

The heading level increments from h1 (# Persistence consumer behavior changes...) to h3 (### Follow-up resolution), skipping h2. Change it to h2 (##) to fix the markdownlint warning.
📝 Proposed fix
-### Follow-up resolution
+## Follow-up resolution
🤖 Prompt for 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.

In `@docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md` at line 9, Change the
“Follow-up resolution” heading from level 3 to level 2 so it follows the
document’s h1 heading without skipping h2 and satisfies markdownlint.
Source: Linters/SAST tools

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

Nitpick comments:
In `@docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md`:
- Line 9: Change the “Follow-up resolution” heading from level 3 to level 2 so
it follows the document’s h1 heading without skipping h2 and satisfies
markdownlint.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d776de10-ab3f-4d14-a887-81af3486a0a5

📥 Commits

Reviewing files that changed from the base of the PR and between 033876f and 2324f1f.

📒 Files selected for processing (6)

docs/PERSISTENCE_CONSUMER_BEHAVIOR_CHANGES.md
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Services/ShieldedService.swift
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CoreContentView.swift
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Views/DashPay/ReclaimInvitationSheet.swift
packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ReclaimInvitationClassifierTests.swift
packages/swift-sdk/SwiftExampleApp/SwiftExampleAppTests/ShieldedServiceClearTests.swift

Test would have caught this in CI: ✖ stale isBound assertion failed before the fix; ✔ all focused shielded-clear and reclaim-classifier tests pass after the fix.

Docs-only markdown hierarchy change; no behavior test is applicable.

shumkov · 2026-07-21T15:39:22Z

Addressed the markdown heading-level review comment in 4a5d3e4.

Test would have caught this in CI: ✖ set-keychain-settings on a locked temporary keychain failed; ✔ unlock-keychain before the same settings update succeeded.

shumkov · 2026-07-21T16:20:15Z

@coderabbitai review

coderabbitai · 2026-07-21T16:20:21Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Documentation-only deletion; no behavior test required.

fix(swift-example-app): fail closed on persistence recovery

2324f1f

shumkov requested review from QuantumExplorer, ZocoLini and llbartekll as code owners July 21, 2026 08:38

Base automatically changed from feat/kotlin-sdk-and-example-app to v4.1-dev July 21, 2026 10:16

github-actions Bot added this to the v4.1.0 milestone Jul 21, 2026

coderabbitai Bot reviewed Jul 21, 2026

View reviewed changes

shumkov added 2 commits July 21, 2026 22:37

fix(swift-example-app): require rebind after shielded clear

0bbc31d

Test would have caught this in CI: ✖ stale isBound assertion failed before the fix; ✔ all focused shielded-clear and reclaim-classifier tests pass after the fix.

docs: fix persistence report heading level

4a5d3e4

Docs-only markdown hierarchy change; no behavior test is applicable.

fix(ci): unlock Swift test keychain before settings

352fb4a

Test would have caught this in CI: ✖ set-keychain-settings on a locked temporary keychain failed; ✔ unlock-keychain before the same settings update succeeded.

docs: remove persistence investigation report

eeecb5c

Documentation-only deletion; no behavior test required.

shumkov merged commit 398ce26 into v4.1-dev Jul 21, 2026
6 of 8 checks passed

shumkov deleted the investigate/persistence-consumer-behavior-changes branch July 21, 2026 16:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(swift-example-app): fail closed on persistence recovery#4177

fix(swift-example-app): fail closed on persistence recovery#4177
shumkov merged 5 commits into
v4.1-devfrom
investigate/persistence-consumer-behavior-changes

shumkov commented Jul 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

thepastaclaw commented Jul 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

shumkov commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

shumkov commented Jul 21, 2026

Uh oh!

shumkov commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

shumkov commented Jul 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Summary by CodeRabbit

Uh oh!

thepastaclaw commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

shumkov commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

shumkov commented Jul 21, 2026

Uh oh!

shumkov commented Jul 21, 2026

Uh oh!

coderabbitai Bot commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

shumkov commented Jul 21, 2026 •

edited by coderabbitai Bot

Loading

thepastaclaw commented Jul 21, 2026 •

edited

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading

coderabbitai Bot commented Jul 21, 2026 •

edited

Loading