fix(onboarding): wallet name prompt + drop simplified backup (Telegram 3, 4)#274
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds three wallet management enhancements: a naming step in onboarding before wallet creation, optional parent wallet preselection in the add-wallet navigation flow, and safety checks to prevent deletion of the device's only wallet or auto-switch before deleting an active wallet. ChangesWallet Management Enhancements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/com/rjnr/pocketnode/ui/screens/onboarding/OnboardingViewModel.kt (1)
69-85:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard against duplicate wallet-creation requests.
Line 69 can be triggered multiple times while a previous request is in flight, which can create duplicate wallets. Add an early loading guard in
createNewWallet.Suggested fix
fun createNewWallet(name: String = "My Wallet") { + if (_uiState.value.isLoading) return if (!canCreateV2BoundKey()) { _uiState.update { it.copy(🤖 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 `@android/app/src/main/java/com/rjnr/pocketnode/ui/screens/onboarding/OnboardingViewModel.kt` around lines 69 - 85, The createNewWallet function can be invoked while a previous creation is in flight; add an early guard that checks the current _uiState.value.isLoading and returns immediately if true to prevent duplicate requests, then set isLoading synchronously before starting the coroutine (update _uiState to isLoading = true prior to viewModelScope.launch) and wrap the walletRepository.createWallet call in a try/finally inside the launched coroutine to ensure _uiState.isLoading is cleared after success or error; reference createNewWallet, _uiState, isLoading, viewModelScope.launch, canCreateV2BoundKey, and walletRepository.createWallet when making these changes.
🤖 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
`@android/app/src/main/java/com/rjnr/pocketnode/ui/screens/wallet/WalletSettingsViewModel.kt`:
- Around line 170-209: The delete flow currently returns early in
requestDelete() for active wallets, making the auto-switch + delete branch in
WalletSettingsViewModel unreachable; change requestDelete() so it does not
return immediately when the wallet is active but instead invokes the existing
auto-switch logic (the block that computes candidates, checks for empty
candidates and updates _uiState with vm_error_delete_last_wallet, or calls
walletRepository.switchActiveWallet(next.walletId)) before continuing to show
the delete confirmation and perform the deletion; preserve the last-wallet guard
that sets showDeleteConfirm=false and the error UiMessage and ensure subsequent
deletion code runs after a successful switch.
---
Outside diff comments:
In
`@android/app/src/main/java/com/rjnr/pocketnode/ui/screens/onboarding/OnboardingViewModel.kt`:
- Around line 69-85: The createNewWallet function can be invoked while a
previous creation is in flight; add an early guard that checks the current
_uiState.value.isLoading and returns immediately if true to prevent duplicate
requests, then set isLoading synchronously before starting the coroutine (update
_uiState to isLoading = true prior to viewModelScope.launch) and wrap the
walletRepository.createWallet call in a try/finally inside the launched
coroutine to ensure _uiState.isLoading is cleared after success or error;
reference createNewWallet, _uiState, isLoading, viewModelScope.launch,
canCreateV2BoundKey, and walletRepository.createWallet when making these
changes.
🪄 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: c98c8d9b-8891-4d15-a66c-8886a0be44b8
📒 Files selected for processing (8)
android/app/src/main/java/com/rjnr/pocketnode/ui/navigation/NavGraph.ktandroid/app/src/main/java/com/rjnr/pocketnode/ui/screens/onboarding/OnboardingScreen.ktandroid/app/src/main/java/com/rjnr/pocketnode/ui/screens/onboarding/OnboardingViewModel.ktandroid/app/src/main/java/com/rjnr/pocketnode/ui/screens/wallet/AddWalletScreen.ktandroid/app/src/main/java/com/rjnr/pocketnode/ui/screens/wallet/AddWalletViewModel.ktandroid/app/src/main/java/com/rjnr/pocketnode/ui/screens/wallet/WalletManagerScreen.ktandroid/app/src/main/java/com/rjnr/pocketnode/ui/screens/wallet/WalletSettingsViewModel.ktandroid/app/src/main/res/values/strings.xml
![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: 86130fa0ea
ℹ️ 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".
| // accounts of other parents. | ||
| val next = candidates.firstOrNull { it.parentWalletId == null } | ||
| ?: candidates.first() | ||
| walletRepository.switchActiveWallet(next.walletId) |
There was a problem hiding this comment.
When confirmDelete() auto-switches away from the wallet being deleted, it only calls walletRepository.switchActiveWallet(...) and never triggers GatewayRepository.onActiveWalletChanged(...). In this codebase, other switch paths (e.g., HomeViewModel and WalletManagerViewModel) call both, because the gateway keeps its own active-wallet state for registration/sync. If this auto-switch path runs, the app can continue syncing/serving state for the old wallet even after it is deleted.
Useful? React with 👍 / 👎.
RaheemJnr
force-pushed
the
fix/onboarding-consistency
branch
from
86130fa to
e0882af
Compare
…ortcut (Telegram 3, 4)
Two consistency complaints from the Telegram report converging on
wallet creation.
Bug 3 (MED): the onboarding "Create New Wallet" path hardcoded the
wallet name to "My Wallet". Adding a second wallet from inside the
app went through AddWalletScreen which prompts for a name. Now
both surfaces prompt.
- `OnboardingViewModel.createNewWallet` accepts a name (default
"My Wallet" for callers that haven't been migrated).
- `OnboardingScreen` shows an AlertDialog with a name TextField
before kicking off creation. Cancelling closes the dialog and
leaves onboarding state untouched.
Bug 4 (MED): adding a new parent wallet from inside the app
skipped the mnemonic-verify step that first-wallet creation has
always required. The cause was a `simplified = true` shortcut on
the AddWallet → "New Wallet" → MnemonicBackup nav, which made the
backup screen render a single display-only page instead of the
3-step display → verify → success flow.
- `NavGraph.onNewMnemonicWalletCreated` now passes
`simplified = false`. Both wallet-creation entry points run
the verify step.
The `simplified` flag itself is kept on `MnemonicBackup` because
the screen still uses it for the "post-import" path that has no
user-supplied mnemonic to verify (the user just confirmed it on
import). No other call sites pass `simplified = true`.
Builds clean. Unit tests pass.
Refs Telegram bug report (Radoslav, 2026-05-21)
RaheemJnr
force-pushed
the
fix/onboarding-consistency
branch
from
e0882af to
fc95717
Compare
1 participant
Summary
Two consistency complaints from the Telegram report.
What's fixed
Bug 3 — name prompt parity
`OnboardingScreen` now shows an AlertDialog asking for the wallet name before tapping into the wallet generation flow. Default value is "My Wallet" so a user who just hits Create gets the same behavior as before. `OnboardingViewModel.createNewWallet` accepts the name with a fallback to "My Wallet" if the field is blank.
Adds the same name surface that AddWalletScreen already presents for subsequent wallets.
Bug 4 — backup verify parity
`NavGraph.onNewMnemonicWalletCreated` (the AddWallet → "New Wallet" path) used `Screen.MnemonicBackup.createRoute(simplified = true)` which routed to a single-screen display-only flow. The flag now passes `simplified = false` so the user goes through the same 3-step display → verify → success flow as first-wallet onboarding.
The `simplified` flag itself is kept on `MnemonicBackup` because the screen still uses it for the post-import path that doesn't need a verify step (the user confirmed the phrase on import). No other call sites pass `simplified = true`.
Test plan
Refs Telegram bug report (Radoslav, 2026-05-21)
Summary by CodeRabbit
New Features
Improvements