Fix cloud backup restore and recovery coverage - #886
Open
praveenperera wants to merge 57 commits into
Open
praveenperera wants to merge 57 commits into
praveenperera wants to merge 57 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: bitcoinppl/cove/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Increments Android `versionCode` from 39 to 40 and iOS `CURRENT_PROJECT_VERSION` from 116 to 117 across project targets/configurations. This advances internal build metadata for the next release while leaving the app version name unchanged (`1.4.0`).
Hot wallets with a confirmed iCloud recovery copy no longer block cloud backup enable as unverified.
Bind each cloud-only restore/delete confirmation dialog to its row so SwiftUI presentation ownership stays with the presenter.
Record why verification is required so a wallet-set change can keep the prior proof for recovery coverage, while integrity loss and unconfirmed recovery keys still block coverage.
Unverified backups were left unchanged on startup integrity downgrade, so wallet-set coverage could stay valid. Re-mark verification required so recovery coverage drops until the user verifies again.
Update `CURRENT_PROJECT_VERSION` from 117 to 118 in the Xcode project so the app and related targets use the next iOS build number consistently across configurations.
Keep the fast local-snapshot Cloud Backup detail, then finish the same refresh with a provider listing so Restore All and other provider-gated actions unlock on iOS.
After a wipe PIN unlock, land on NewWallet.Select with an empty wallet list and navigation stack. Keep failed cleanup locked, clear cached session state before unlock, show the normal launch cover during wipe, and use fixed failure copy.
A full wipe replaces the database. Restore only the completed-setup flag so wiped devices keep finished onboarding state.
Associated-domain checks must see the final HTTP 200 body. Following redirects could accept a wrong file as valid.
Cloud backup passkeys must demand verification on create and assert. Also map unexpected credential types to a typed failure instead of a generic missing-credential error.
Native passkey failures need request-mode and presentation timing context so delayed or missing anchors are diagnosable.
Sensitive enable and verification actions must wait for the current prompt to finish dismissing. Staging the action until presenter readiness avoids reopening the old prompt.
Enable now inspects namespaces once for hints and matching, with bounded concurrent wrapper reads. Restore cancels cooperatively, keeps already matched namespaces, and no longer depends on a fixed onboarding timeout. Busy copy shows discovery while existing backups are checked.
Keep each process in its own cove-test temp root so parallel nextest runs do not share ~/.data, and sweep stale dirs after an hour.
Lock ManagerCache clearWalletManager so in-flight loads cancel and the related send flow manager is cleared too.
The testflight recipe always bumps the build number first. When the bump is already committed, as with build 119, there was no recipe that archives and uploads the current number. This wraps the existing xtask upload-testflight command so the CLI path can be used as is.
Install a temporary data root before tests start, check cleanup paths, and remove the root on normal exit. Bootstrap wallet lifecycle tests before their actor close paths access storage.
Delete unsigned transactions, saved wallet selections, and KeyTeleport receive secrets with wallet cleanup. Keep mode changes atomic and update both mobile bindings for the typed cleanup stages.
Release coin control and NFC sessions with their wallet, keep send-flow updates in the correct wallet, and avoid retaining discarded mnemonic managers. Keep Android locked when switching to main mode fails.
Queue wallet exports and Cloud Backup actions until the current prompt finishes dismissing. Cancel pending actions when their host disappears.
Keep finished authorization objects alive until the main queue releases them. Include underlying error codes in diagnostics without private error text.
Copy prior TestFlight notes and assign the uploaded build to me-only, with export compliance and external beta review handled after upload. Keep accepted build numbers when distribution fails. Add Android internal-release recipes with Google Play credential checks.
Only a missing credential can start passkey discovery. Keep request failures separate so verification does not start a second prompt or report a usable passkey as missing.
Show the newer queued prompt after dismissal instead of running the old action and leaving the new prompt hidden.
Make release-ios bump the iOS build, rebuild release bindings, then upload. Point testflight and tf at that recipe so they share the same path.
Match the iOS TestFlight flow: bump, build, and upload from one command, and restore versionCode when failure happens before Google Play is invoked.
Advance the store build numbers for the next release uploads.
After an iOS reinstall the Keychain keeps wallet secrets while the database is gone. Compare those leftovers to the backup and adopt matches instead of treating the wallet id as occupied.
Keep LocalWalletConflict typed through the restore actor so an all-conflict run shows that local data was left unchanged instead of a generic cloud backup failure.
A bare platform cancelled code after Face ID does not prove the reader cancelled. State that the request did not complete, and log the discovery timing for later diagnosis.
After a coordinated delete succeeds, record process-local tombstones so stale provider snapshots cannot revive deleted paths for up to 60 seconds. Clear a file tombstone after a successful upload, and keep partial delete successes when cancellation wins.
praveenperera
force-pushed
the
cloud-backup-regression-fixes
branch
from
September 18, 2026 16:16
b866a1b to
c157895
Compare
praveenperera
marked this pull request as ready for review
September 18, 2026 16:16
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c157895d5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
set_authenticate_result now keeps a sticky default after queued results are consumed, so multi-call tests can reuse the configured authenticate response.
Provider redirects can differ from the requested path; tombstone both so deleted backups stay hidden from metadata listings.
Let restore tests cancel mid-save so keychain and persisted state stay consistent across one commit.
Cancellation between keychain writes and configured-state persist could leave a restored namespace half-activated.
Say "this device" instead of "iPhone" so the same restore conflict messages work on Android.
Replaced unconditional removal of the legacy `~/.data/test` directory with a guarded cleanup that only deletes directories positively identified as old Cove test DB storage (`cove_<alnum>.db` files only). Added focused tests covering safe deletion, unrelated/mixed contents, empty dirs, and nested dirs to prevent accidental data loss.
set_authenticate_result now only clears the queue and sets the sticky default, so tests can push one-shot failures without a duplicate queue entry of the same value.
Move cloud restore planning into its own module, replace RestoreEntry with Option writes, and drop unused cleanup warning tuples from the public restore result.
Pass keychain, passkey, namespace, state, and wallet ids as one RestoredNamespaceCommit, and share persist helpers between restore state writes.
Temp-dir test databases no longer leave ~./data/test behind, so the one-shot home cleanup and its tests are unused.
Move hot wallet metadata and keychain key suffixes into one place so backup and restore tests stop duplicating them and can assert by suffix name.
cargo test runs all tests in one process. Wipe-phase tests put the process-global coordinator in PreparingFullWipe. Persister tests that write through begin_persistence_operation then fail with CoordinatorBusy. Use in-memory storage for those persister tests. Keep persistent storage only for terminal wipe and deletion tests that already hold global_state_test_lock.
CI can take more than one second to read a small local file. The helper then tries metadata and returns SyncPending.
macOS accepted sockets inherit the listener's non-blocking mode. A read before the request bytes arrive would fail with WouldBlock.
Restore used to save the new keychain, persist configured state, then mark wallets dirty. A later write could leave the new keychain in place or wipe the previous one. Capture the prior keychain first, persist configured state and dirty wallet rows in one transaction, and restore the snapshot if that write fails.
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
1 participant
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.
Summary
Fix cloud backup regressions around restore, passkeys, iCloud metadata, and recovery coverage.
Cove treated backup coverage too loosely. Cancelled checks, wallet-set changes, and integrity downgrades could leave the UI thinking a wallet still had a confirmed cloud recovery copy. Restore also failed to handle leftover Keychain items, local Keychain conflicts, deleted iCloud paths, and passkey request errors after the system prompt.
This branch:
Also included on this branch:
Testing
Not re-run as part of opening this PR. The branch adds Rust and iOS tests for recovery coverage, passkey match and restore, presentation handoff, and TestFlight xtask composition.
Platform Coverage
Checklist