fix(trezor): persist password and login in HD mode#2746
fix(trezor): persist password and login in HD mode#2746CharlVS merged 3 commits intorelease/hot-fix-trezor-loginfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe Trezor wallet login process now uses secure storage to manage the wallet password. Instead of generating a new password each time, the system checks for an existing password in secure storage and only creates and stores a new one if none exists. The derivation method was updated from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TrezorInitBloc
participant SecureStorage
User->>TrezorInitBloc: Initiate Trezor Wallet Login
TrezorInitBloc->>SecureStorage: Read password with key
alt Password exists
SecureStorage-->>TrezorInitBloc: Return stored password
else No password
TrezorInitBloc->>TrezorInitBloc: Generate new password
TrezorInitBloc->>SecureStorage: Store new password with key
end
TrezorInitBloc->>TrezorInitBloc: Sign in or register with hdWallet derivation
TrezorInitBloc->>TrezorInitBloc: Add activated coins after successful login
Suggested labels
Suggested reviewers
Poem
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 (
|
|
Visit the preview URL for this PR (updated for commit f8a5ff4): https://walletrc--pull-2746-merge-87lccey6.web.app (expires Fri, 13 Jun 2025 12:10:44 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/bloc/trezor_init_bloc/trezor_init_bloc.dart(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
lib/bloc/trezor_init_bloc/trezor_init_bloc.dart (1)
Learnt from: takenagain
PR: KomodoPlatform/komodo-wallet#2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for `app_config.dart` in `coins_bloc.dart` is necessary because the part file `coins_state.dart` uses `excludedAssetList` from that package.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Desktop (windows)
🔇 Additional comments (4)
lib/bloc/trezor_init_bloc/trezor_init_bloc.dart (4)
5-5: LGTM: Secure storage import added.The addition of
flutter_secure_storageimport supports the new password persistence functionality.
27-27: LGTM: Well-defined storage key constant.Using a constant for the storage key follows good practices and prevents typos in key references.
56-56: LGTM: Proper secure storage initialization.The
FlutterSecureStorageinstance is correctly initialized with a const constructor.
314-314: Consistent derivation method update.Good consistency updating both the
signInandregisteroperations to useDerivationMethod.hdWallet.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Trezor integration by persisting the wallet password in secure storage and switching to HD wallet derivation.
- Bumps the
httpdependency from 1.3.0 to 1.4.0. - Adds
flutter_secure_storageto store and reuse the Trezor password. - Updates
_loginToTrezorWalletto read/write the password and use HD derivation options.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pubspec.yaml | Updated http version to 1.4.0 |
| lib/bloc/trezor_init_bloc/trezor_init_bloc.dart | Added secure storage import and logic for password persistence; refactored login flow with HD derivation and moved coin activation calls |
Comments suppressed due to low confidence (2)
lib/bloc/trezor_init_bloc/trezor_init_bloc.dart:324
- The logout method currently doesn’t clear the persisted password. You should delete the stored password (
_trezorPasswordKey) from secure storage on logout to avoid stale credential reuse.
Future<void> _logout() async {
lib/bloc/trezor_init_bloc/trezor_init_bloc.dart:289
- Add unit tests for the new persistent password logic: verify reading from and writing to secure storage, and ensure
_loginToTrezorWallethandles both existing and newly generated passwords correctly.
password ??= await _secureStorage.read(key: _trezorPasswordKey);
takenagain
force-pushed
the
hotfix/trezor-password-derivation
branch
from
e278e4c to
f8a5ff4
Compare
smk762
left a comment
smk762
left a comment
There was a problem hiding this comment.
- address creation functional.
- address gap (set at 3) is enforced.
- if 3rd address funded, 4th address can be created.
Confirm scanning on initial activation only shows first address, even if subsequent are funded. We can delegate this to a new issue.
DeckerSU
left a comment
DeckerSU
left a comment
There was a problem hiding this comment.
LGTM. External dependencies have been reviewed—no malicious components found. However, the scope of this review does not include the komodo-defi-sdk-flutter repo or any related packages. They will be reviewed separately, covering the range from 41b554d08ed3f42f9f784a488cedf9ab4b3b3313 to f63bebb0288db26f2a369579109e1fcc93e19b67 in the komodo-defi-sdk-flutter repository.
|
p.s. |
Trezorfor thepriv_key_policyfield in UTXO coin activationflutter_secure_storageScreen.Recording.2025-06-06.at.14.26.20.mov
@smk762 I did notice some jank with disappearing transactions when scrolling as it loads, but not sure if it's an existing issue or specific to Trezor.
Address creation appears to work fine as well with the current non-task-based method, although scanning only picks up the first address. I have to manually add to get the second DOC address
Package Dependency changes Summary
Patch files: https://drive.google.com/drive/folders/1ho4HGpIliHUBT0bnpQvMRqwU1zQ7nbc7?usp=drive_link
Summary by CodeRabbit
New Features
Refactor
Chores