fix(auth): trezor auth flow and wallet main state updates#2890
Conversation
WalkthroughThe changes update authentication and wallet UI logic. They introduce an abstract method for listening to authentication state changes in the Trezor authentication mixin, refine error handling formatting, and improve tab controller management in the wallet main view. Mixins and class inheritance are updated for clarity and correctness. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletMain
participant AuthBloc
participant TrezorAuthMixin
User->>WalletMain: Interacts with wallet UI
WalletMain->>AuthBloc: Listens for Auth State
AuthBloc->>TrezorAuthMixin: Calls _listenToAuthStateChanges()
TrezorAuthMixin-->>AuthBloc: Abstract method implemented in AuthBloc
AuthBloc-->>WalletMain: Emits Auth State changes
WalletMain->>WalletMain: Calls _updateTabController(authenticated)
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (3)lib/views/wallet/wallet_page/wallet_main/wallet_main.dart (6)lib/bloc/auth_bloc/auth_bloc.dart (6)lib/bloc/auth_bloc/trezor_auth_mixin.dart (1)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🔇 Additional comments (18)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Visit the preview URL for this PR (updated for commit a0565e9): https://walletrc--pull-2890-merge-vm2xtza9.web.app (expires Wed, 16 Jul 2025 13:19:30 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
takenagain
requested review from
CharlVS and
Copilot
and removed request for
Copilot
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the Trezor authentication flow by ensuring UI state updates correctly after login and adjusts the TabController when auth state changes. It also refactors Trezor mixin error handling and scopes the initial auth listener to non-Trezor wallets.
- Add post-frame callback for safe
setStateon tab changes and introduce_updateTabController - Refactor Trezor auth mixin to use
const, multi-line error emits, and declare abstract listener hook - Modify auth bloc to only auto-listen for non-Trezor wallets on startup
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/views/wallet/wallet_page/wallet_main/wallet_main.dart | Wraps setState in addPostFrameCallback, replaces inline length checks with _updateTabController helper |
| lib/bloc/auth_bloc/trezor_auth_mixin.dart | Adds abstract _listenToAuthStateChanges(), converts AuthOptions to const, reformats error emits |
| lib/bloc/auth_bloc/auth_bloc.dart | Imports PrivateKeyPolicy higher, changes startup listener to skip Trezor wallets |
Comments suppressed due to low confidence (1)
lib/views/wallet/wallet_page/wallet_main/wallet_main.dart:77
- [nitpick] Consider guarding against disposing an uninitialized
_tabController(e.g., check for null ormounted) before callingdispose()to avoid potential runtime errors.
void _updateTabController(bool authenticated) {
| on<AuthTrezorCancelled>(_onTrezorCancel); | ||
| } | ||
|
|
||
| /// Abstract method overriden in [AuthBloc] to start listening |
There was a problem hiding this comment.
Spelling: "overriden" should be corrected to "overridden" in the doc comment.
| /// Abstract method overriden in [AuthBloc] to start listening | |
| /// Abstract method overridden in [AuthBloc] to start listening |
| authState.error ?? 'Trezor authentication failed', | ||
| type: AuthExceptionType.generalAuthError, | ||
| )); | ||
| return AuthBlocState.error( |
There was a problem hiding this comment.
[nitpick] The error emission code is duplicated across multiple branches; consider extracting a helper method to construct and emit error states to reduce repetition.
| final isTrezorWallet = currentUser?.walletId.authOptions.privKeyPolicy == | ||
| const PrivateKeyPolicy.trezor(); | ||
|
|
||
| if (currentUser != null && !isTrezorWallet) { |
There was a problem hiding this comment.
This condition prevents emitting loggedIn for Trezor wallets on startup. If Trezor authentication should also trigger loggedIn, this logic may skip necessary state updates.
| if (currentUser != null && !isTrezorWallet) { | |
| if (currentUser != null) { |
Fixes the Trezor auth login bug by
setStateinaddPostFrameCallbackto fix the issue with BlocConsumer not calling the builder function for auth state updates (successful login).Before
Screen.Recording.2025-07-09.at.14.34.48.mov
After
after.mov
Summary by CodeRabbit
Bug Fixes
Refactor