-
Notifications
You must be signed in to change notification settings - Fork 240
Fix/illegal import chars #2966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/illegal import chars #2966
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 changes introduce stricter validation for wallet names and wallet file names, updating error messages in the English localization file. Wallet creation and import UI components are updated to enforce these validations, trim whitespace from wallet names, and ensure error messages are displayed for invalid inputs. No structural or control flow changes outside validation logic are made. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletCreationUI
participant WalletsRepository
User->>WalletCreationUI: Enter wallet name
WalletCreationUI->>WalletsRepository: validateWalletName(name)
WalletsRepository-->>WalletCreationUI: Error or null
WalletCreationUI-->>User: Display error or enable create button
User->>WalletCreationUI: Press Create
WalletCreationUI->>WalletsRepository: Create wallet (if valid)
sequenceDiagram
participant User
participant WalletImportByFileUI
User->>WalletImportByFileUI: Select file
WalletImportByFileUI->>WalletImportByFileUI: Validate file name
alt Invalid file name
WalletImportByFileUI-->>User: Show error, disable import
else Valid file name
User->>WalletImportByFileUI: Press Import
WalletImportByFileUI->>WalletImportByFileUI: Import wallet
end
Estimated code review effort2 (~15 minutes) 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 e38f149): https://walletrc--pull-2966-merge-kn97cj9v.web.app (expires Tue, 29 Jul 2025 17:23:03 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
Ah yes.... I intended to mention that in opening this PR. Yours is more comprehensive and allows renaming, this one is a humble form validation. I think #2792 should remain open for enhancing the flow, though this PR can serve in the short term as a minimal solution for fast merge. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements validation to prevent wallet names and filenames from containing special characters, addressing issues with illegal characters in wallet imports and creation. The changes add regex-based validation that only allows alphanumeric characters, spaces, underscores, and hyphens.
- Adds special character validation for wallet names during creation and import processes
- Implements filename validation for wallet seed file imports to reject files with special characters
- Trims whitespace from wallet names to prevent space-only names and normalize input
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/blocs/wallets_repository.dart | Enhanced wallet name validation with special character checks and whitespace trimming |
| lib/views/wallets_manager/widgets/wallet_creation.dart | Integrated validation into wallet creation form and added real-time validation feedback |
| lib/views/wallets_manager/widgets/wallet_simple_import.dart | Added trimming of wallet names during import process |
| lib/views/wallets_manager/widgets/wallet_import_by_file.dart | Added filename validation to reject files with special characters |
| assets/translations/en.json | Added new error messages for invalid wallet names and filenames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/blocs/wallets_repository.dart (1)
92-122: Well-designed validation with good backward compatibility.The enhanced validation logic effectively prevents special characters while maintaining backward compatibility by using the original (untrimmed) name for duplicate checking. The regex pattern correctly identifies problematic characters, and the trimming prevents whitespace-only names.
Consider whether allowing spaces in wallet names could cause issues on certain filesystems, though this may be acceptable for wallet names that aren't directly used as filenames.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
lib/generated/codegen_loader.g.dartis excluded by!**/generated/**
📒 Files selected for processing (5)
assets/translations/en.json(1 hunks)lib/blocs/wallets_repository.dart(1 hunks)lib/views/wallets_manager/widgets/wallet_creation.dart(3 hunks)lib/views/wallets_manager/widgets/wallet_import_by_file.dart(2 hunks)lib/views/wallets_manager/widgets/wallet_simple_import.dart(2 hunks)
🧠 Learnings (4)
lib/views/wallets_manager/widgets/wallet_simple_import.dart (2)
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #2608
File: lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart:2-3
Timestamp: 2025-05-01T21:00:36.970Z
Learning: It's acceptable to use unconditional dart:io imports in the Komodo wallet codebase when the usage is guarded by !kIsWeb conditional checks that prevent the platform-specific code from being executed in web environments.
lib/blocs/wallets_repository.dart (1)
Learnt from: takenagain
PR: #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.
lib/views/wallets_manager/widgets/wallet_import_by_file.dart (5)
Learnt from: takenagain
PR: #2608
File: lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart:2-3
Timestamp: 2025-05-01T21:00:36.970Z
Learning: It's acceptable to use unconditional dart:io imports in the Komodo wallet codebase when the usage is guarded by !kIsWeb conditional checks that prevent the platform-specific code from being executed in web environments.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The excludedAssetList from app_config.dart is used in the _filterExcludedAssets method in coins_state.dart. Since coins_state.dart is a part file of coins_bloc.dart, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Learnt from: takenagain
PR: #2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.
Learnt from: takenagain
PR: #2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.
lib/views/wallets_manager/widgets/wallet_creation.dart (1)
Learnt from: takenagain
PR: #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.
🧰 Additional context used
🧠 Learnings (4)
lib/views/wallets_manager/widgets/wallet_simple_import.dart (2)
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #2608
File: lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart:2-3
Timestamp: 2025-05-01T21:00:36.970Z
Learning: It's acceptable to use unconditional dart:io imports in the Komodo wallet codebase when the usage is guarded by !kIsWeb conditional checks that prevent the platform-specific code from being executed in web environments.
lib/blocs/wallets_repository.dart (1)
Learnt from: takenagain
PR: #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.
lib/views/wallets_manager/widgets/wallet_import_by_file.dart (5)
Learnt from: takenagain
PR: #2608
File: lib/bloc/fiat/fiat_onramp_form/fiat_form_bloc.dart:2-3
Timestamp: 2025-05-01T21:00:36.970Z
Learning: It's acceptable to use unconditional dart:io imports in the Komodo wallet codebase when the usage is guarded by !kIsWeb conditional checks that prevent the platform-specific code from being executed in web environments.
Learnt from: takenagain
PR: #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.
Learnt from: takenagain
PR: #2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The excludedAssetList from app_config.dart is used in the _filterExcludedAssets method in coins_state.dart. Since coins_state.dart is a part file of coins_bloc.dart, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Learnt from: takenagain
PR: #2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.
Learnt from: takenagain
PR: #2611
File: test_units/tests/system_health/system_clock_repository_test.dart:7-8
Timestamp: 2025-05-08T19:07:13.442Z
Learning: In the Komodo Wallet project, test files are structured to define test functions that are called from a central test runner in test_units/main.dart, rather than each test file having its own main() function.
lib/views/wallets_manager/widgets/wallet_creation.dart (1)
Learnt from: takenagain
PR: #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.
🔇 Additional comments (4)
lib/views/wallets_manager/widgets/wallet_simple_import.dart (1)
100-100: LGTM: Consistent wallet name trimming implementation.The trimming of wallet names at both display (line 100) and submission (line 328) points ensures consistent handling of whitespace. This aligns well with the validation changes in other files that prevent whitespace-only wallet names.
Also applies to: 328-328
assets/translations/en.json (1)
118-119: Clear and actionable error messages added.The new error messages provide users with clear guidance on resolving wallet name and filename validation issues. The messages are appropriately concise while being actionable.
lib/views/wallets_manager/widgets/wallet_import_by_file.dart (1)
54-86: Robust filename validation implementation.The filename validation is well-structured with appropriate character restrictions for file names (allowing dots unlike wallet names). The late initialization pattern, async error message handling, and defensive programming with the early return in
_onImportall demonstrate good practices.Also applies to: 63-63, 212-215
lib/views/wallets_manager/widgets/wallet_creation.dart (1)
46-57: Excellent optimization and validation integration.The caching of the repository reference in
initState, addition of the name controller listener for UI updates, and integration of validation into the button enabling logic all demonstrate good Flutter development practices. The wallet name trimming maintains consistency with the other validation changes.Also applies to: 167-167, 189-189, 196-200
Co-authored-by: Copilot <[email protected]>
* feat(ui): auto-scroll addresses in tx pages * feat: show full transaction address * feat: show full address for own addresses * chore: roll SDK * chore: roll SDK * fix(ui): Fix address text item responsiveness * fix(ui): align transaction table columns
* remove duplicate transaltion key/value * fix source address unselected validation bug
* Adds `invalidWalletFileNameError` locale text * disallow wallet import if filename has illeagal chars * adds local message for `invalidWalletNameError` * dissallow seed import or wallet creation if wallet name has illegal chars * block creating wallet name with illegal chars * rm dupe declaration * trim wallet name before validation * Update lib/views/wallets_manager/widgets/wallet_creation.dart Co-authored-by: Copilot <[email protected]> * q pdate "seed" to "seed phrase" in login/import forms (#2972) * fix translation string name * `seed` - > `seed phrase` * removes "seed" in connect button * feat(ui): Show full pubkeys where copyable (#2955) * feat(ui): auto-scroll addresses in tx pages * feat: show full transaction address * feat: show full address for own addresses * chore: roll SDK * chore: roll SDK * fix(ui): Fix address text item responsiveness * fix(ui): align transaction table columns * feat(settings): hide log export when logged out (#2967) * bugfix: disable withdraw preview if source address not selected (#2969) * remove duplicate transaltion key/value * fix source address unselected validation bug * fix(ui): avoid autofocus conflicts (#2968) * fix(ui): remove duplicate CircularProgressIndicator on address creation (#2970) --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Charl (Nitride) <[email protected]>
Partial solution to #2956
The binary portion issue needs to be resolved in https://github.com/KomodoPlatform/komodo-defi-sdk-flutter
Also solves #2523
To Test:
wallet(name)). You should see an error message and have the "create" button disabled.wallet(name)). You should see an error message and have the "import" button disabled.wallet(name).txt). You should see an error message and have the "import" button disabled.Summary by CodeRabbit
New Features
Bug Fixes
Documentation