fix: gate sheets and rotate receive address on restore

[ovitrif]

Roadmap feature: Backup & Restore

Description

This PR:

• Prevents timed sheets popping up while wallet restores.
• restores former functionality to rotate receive address on wallet restore, needed for e2e tests to pass

Preview

n/a

QA Notes

Validation: Failing E2E tests should pass.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

[Copilot]

Pull Request Overview

This PR implements two key improvements to the wallet restore functionality: preventing UI disruptions during restore operations and ensuring fresh receive addresses after restoration.

• Gates timed promotional sheets from appearing while wallet restore is in progress
• Converts the isRestoring flag to a reactive StateFlow for better state management
• Ensures onchain receive addresses are rotated after wallet restoration by clearing them from backup data

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt	Added BackupRepo dependency and early return check to prevent timed sheets from displaying during wallet restore
app/src/main/java/to/bitkit/repositories/BackupRepo.kt	Converted isRestoring boolean to StateFlow for reactive state management, updated all references, and cleared onchain address from backup data to force address rotation on restore

[claude]

Code Review

Summary

This PR makes two targeted fixes to prevent issues during wallet restoration:

1. Converts isRestoring from a simple boolean to a StateFlow<Boolean> to enable observation across components
2. Gates timed sheets from appearing during restore operations
3. Forces onchain address rotation on restore (needed for E2E tests)

Positive Aspects ✅

Good architectural improvement:

• Converting isRestoring to StateFlow is the right pattern for Kotlin coroutines and allows reactive observation
• Properly uses MutableStateFlow internally with public read-only StateFlow exposure - excellent encapsulation
• Uses asStateFlow() to prevent external mutation

Consistent implementation:

• All 10 occurrences of the boolean check are updated consistently
• The update { } pattern is used correctly for state updates

Thread safety:

• StateFlow provides thread-safe state management out of the box

Issues & Concerns 🔍

1. Potential Race Condition (Medium Priority)

Location: BackupRepo.kt:420

The onchain address is cleared by setting it to an empty string:

val caches = parsed.cache.copy(onchainAddress = "")

However, there's a potential timing issue:

• If isRestoring is set to false (line 463) before the address rotation completes
• Other components observing isRestoring might trigger backups prematurely
• The backup listeners check isRestoring.value but there's no guarantee the restore process is fully complete

Recommendation: Consider whether the address rotation should happen before or after the finally block sets isRestoring = false. The current code restores metadata first (which includes the address clear), but other restore operations happen after. Ensure all state-modifying operations complete before isRestoring becomes false.

2. Missing Comment/Documentation (Low Priority)

Location: BackupRepo.kt:420

The comment says "Force onchain address rotation" but it's not immediately clear WHY this is needed.

Recommendation: Add a brief explanation:

// Force onchain address rotation by clearing cached address
// This ensures E2E tests receive a fresh address on restore
val caches = parsed.cache.copy(onchainAddress = "")

3. Test Coverage Observation

I don't see any new tests added for:

• The StateFlow behavior of isRestoring
• The timed sheet gating during restore
• The address rotation on restore

Recommendation: Consider adding unit tests to verify:

// Example test cases:
// 1. Verify timed sheets are blocked when isRestoring = true
// 2. Verify backup listeners don't trigger when isRestoring = true  
// 3. Verify onchain address is empty after restore

Performance Considerations ⚡

Positive:

• StateFlow is more efficient than manually notifying observers
• No performance regressions introduced
• The distinctUntilChanged() operators already in place will prevent unnecessary emissions

Note: The .value property access on StateFlow is synchronous and cheap - no concerns there.

Security Concerns 🔒

No security issues identified. The changes don't introduce any new attack vectors. The address rotation is intentional and documented in the PR description as needed for testing.

Code Quality & Best Practices 📋

Strengths:

• Follows Kotlin conventions and coroutines best practices
• Proper use of immutability with StateFlow
• Consistent naming (_isRestoring for private mutable, isRestoring for public)

Minor suggestion:
The PR description mentions "prevents timed sheets popping up while wallet restores" - this is well implemented in AppViewModel.kt:1583, but consider if there are other places that should also check isRestoring (e.g., other background operations that might interfere).

Recommendations

Priority: Medium

1. Review the timing of when isRestoring is set to false - ensure all restore side effects complete first
2. Verify no other background operations need similar gating

Priority: Low

1. Add inline comment explaining why address rotation is needed
2. Consider adding unit tests for the new StateFlow behavior
3. Consider if BackupRepo.isRestoring should be documented in code comments as a public API

Verdict

LGTM with minor suggestions ✅

The core implementation is solid and follows best practices. The conversion to StateFlow is a good architectural improvement. The main concern is ensuring the restore flag is managed correctly with respect to completion of all restore operations. The suggested improvements are mostly about documentation and test coverage rather than functional issues.

---

Review generated with Claude Code