Refactor cloud backup and mobile manager ownership

[praveenperera]

No description provided.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: acf67fc1-30da-4e8d-91a3-37362b42482c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[praveenperera]

@greptile-apps

[greptile-apps]

Greptile Summary

This PR refactors cloud backup ownership and platform manager wiring. The main changes are:

• New Android Google Drive backup helpers for tokens, folder resolution, and HTTP calls.
• Rust cloud backup manager ownership and write-supervisor refactors.
• Shared reconcile-channel plumbing for manager updates.
• Progressive scanner bookkeeping extracted into shared Rust helpers.
• iOS and Android presentation/state ownership cleanup.

Confidence Score: 4/5

The cloud backup lifecycle and Drive delete paths need fixes before merging.

• A full reconcile queue can block cloud-backup actor progress during lifecycle updates.
• Multi-file Drive deletes can report failure after part of the backup record was already removed.
• Namespace deletion should reject names outside the backup namespace format before deleting Drive folders.
• The progressive scanner refactor appears behavior-preserving.

rust/src/manager/reconcile_channel.rs; android/app/src/main/java/org/bitcoinppl/cove/cloudbackup/AndroidCloudStorageAccess.kt; android/app/src/main/java/org/bitcoinppl/cove/cloudbackup/DriveFolderResolver.kt

Important Files Changed

Filename	Overview
android/app/src/main/java/org/bitcoinppl/cove/cloudbackup/AndroidCloudStorageAccess.kt	Adds Drive-backed upload, download, delete, and sync-health operations; wallet backup deletion can leave partial remote state after mid-loop failures.
android/app/src/main/java/org/bitcoinppl/cove/cloudbackup/DriveFolderResolver.kt	Adds Drive namespace and folder lookup logic; destructive namespace deletion does not enforce the namespace format used by listing.
rust/src/manager/reconcile_channel.rs	Adds shared bounded reconcile-channel plumbing; the synchronous send path can block cloud-backup actor progress.
rust/src/manager/cloud_backup_manager.rs	Refactors cloud backup manager ownership and write-client calls while preserving most write-blocking checks.
rust/crates/cove-bdk-progressive-scan/src/core.rs	Extracts scan accumulator, stop-gap, transaction status, and prevout helpers with behavior matching the prior scanner flow.
rust/crates/cove-bdk-progressive-scan/src/electrum.rs	Moves Electrum scan bookkeeping to shared helpers without changing duplicate transaction, stop-gap, or update emission behavior.
rust/crates/cove-bdk-progressive-scan/src/esplora.rs	Moves Esplora scan bookkeeping to shared helpers with equivalent batching and evicted transaction handling.

Comments Outside Diff (1)

1. android/app/src/main/java/org/bitcoinppl/cove/cloudbackup/AndroidCloudStorageAccess.kt, line 158-175 (link)

   Partial Drive Delete Persists

   When deleting a wallet backup touches multiple Drive files and a later delete fails, earlier files are already gone but the method returns a normal aggregate failure. The caller can retry against a partially removed backup record, leaving remote backup state inconsistent instead of treating the partial deletion as durable progress.

_{Reviews (1): Last reviewed commit: "Fix manual UI test flows" | Re-trigger Greptile}

[praveenperera]

@greptile-apps thanks for the review. I looked through these, and I do not think any of the three cloud-backup findings should block this refactor.

1. Bounded reconcile send

This is a theoretical failure mode, but not a realistic PR blocker. The cloud-backup reconcile queue has capacity 1000 and is drained by a dedicated listener. Lifecycle messages are emitted on state transitions, not as a high-volume event stream, so organically filling that queue would require the platform-side consumer to be wedged. If the consumer is wedged badly enough to stop draining 1000 messages, the app is already broken; changing this specific send path to async would not make cloud backup healthy. This behavior is also not introduced by this refactor: master already used a bounded flume::Sender::send for the same cloud-backup reconcile messages.

2. Partial multi-file Drive delete

The delete operation is idempotent and retryable. If deleting one location succeeds and a later location fails, the next attempt only finds the remaining files and continues cleanup. I do not see a caller that treats the aggregate error as meaning "nothing was deleted"; callers treat it as a failed delete attempt, which is compatible with best-effort remote cleanup. This behavior also existed before the refactor; the refactor moved the Drive code into helpers but did not introduce the delete semantics.

3. Namespace validation before Android delete

Production cloud-backup deletes go through Rust CloudStorageClient::delete_namespace, which validates namespace IDs before calling the platform implementation. The direct Kotlin callers are debug wipe utilities, and they source namespaces from listNamespaces, which is already filtered to valid backup namespace IDs through the Rust wrapper. Android's resolver also deletes by exact folder-name match and returns not-found if no folder matches, so a malformed string does not create a path to delete an unrelated Drive folder. Kotlin-side validation would be defense-in-depth only, and this was also pre-existing behavior rather than a refactor regression.

Bottom line: these are acknowledged as theoretical hardening notes, but they are pre-existing or low-risk behaviors and should not delay this refactor without a concrete reachable failure scenario.

[greptile-apps]

All three explanations are solid — thanks for the detailed walkthrough.

1. Bounded reconcile send — Agreed. A 1000-capacity queue drained by a dedicated consumer for low-frequency lifecycle transitions isn't a realistic saturation path, and pre-existing behavior means this refactor doesn't change the risk profile.

2. Partial Drive delete — Idempotency is the key property here. As long as callers interpret a partial failure as "retry needed" rather than "partially complete and done," the eventual-cleanup guarantee holds. Your description of the caller contract makes that clear.

3. Namespace validation — The defense-in-depth argument is valid, but given that the production path validates in Rust and the debug-only Kotlin callers source from an already-filtered list with no path-traversal surface, the practical risk is negligible. Pre-existing is the right framing.

These are fine to treat as hardening notes for a follow-up if desired, not blockers for this refactor.