Skip to content

Conversation

@smk762
Copy link
Collaborator

@smk762 smk762 commented Jul 21, 2025

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:

  • Try to create a new wallet with a name including special chars (e.g. wallet(name)). You should see an error message and have the "create" button disabled.
  • Try to import a seed phrase, with a wallet name including special chars (e.g. wallet(name)). You should see an error message and have the "import" button disabled.
  • Try to import a wallet seed file with a filename including special chars (e.g. wallet(name).txt). You should see an error message and have the "import" button disabled.
  • Try to create a wallet with only whitespace as the wallet name. It should fail validation.

Summary by CodeRabbit

  • New Features

    • Added validation for wallet names to disallow special characters and enforce length limits.
    • Introduced validation for wallet file names to ensure only allowed characters are used.
  • Bug Fixes

    • Improved enable/disable logic for wallet creation and import buttons to reflect new validation rules.
    • Trimmed whitespace from wallet names during creation and import to prevent unintended errors.
  • Documentation

    • Added new English error messages for invalid wallet names and filenames.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 21, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The 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

File(s) Change Summary
assets/translations/en.json Added two new translation keys for invalid wallet name and invalid filename error messages.
lib/blocs/wallets_repository.dart Enhanced wallet name validation to disallow special characters, trim input, and update error logic.
lib/views/wallets_manager/widgets/wallet_creation.dart Cached repository instance, added listener, trimmed name on submit, and enforced name validation.
lib/views/wallets_manager/widgets/wallet_import_by_file.dart Added file name validation, error handling for illegal characters, and updated button enable logic.
lib/views/wallets_manager/widgets/wallet_simple_import.dart Trimmed wallet name input before display and import; no other logic 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)
Loading
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
Loading

Estimated code review effort

2 (~15 minutes)

Suggested labels

bug, QA

Suggested reviewers

  • smk762

Poem

A rabbit hopped with code so neat,
Trimming names and keeping them sweet.
No special chars in wallets allowed,
File names checked, the devs are proud!
With every hop, the errors flee—
Now wallets are as clean as can be! 🐇✨


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link

github-actions bot commented Jul 21, 2025

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

@CharlVS
Copy link
Collaborator

CharlVS commented Jul 21, 2025

@smk762, what about #2792? I would like to first give it a thorough testing before you spend time it. I can close mine if yours has better functionality (lmk)

@smk762
Copy link
Collaborator Author

smk762 commented Jul 21, 2025

@smk762, what about #2792? I would like to first give it a thorough testing before you spend time it. I can close mine if yours has better functionality (lmk)

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.

@CharlVS
Copy link
Collaborator

CharlVS commented Jul 22, 2025

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 22, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

Copilot AI left a 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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between ce1956e and 48315e1.

⛔ Files ignored due to path filters (1)
  • lib/generated/codegen_loader.g.dart is 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 _onImport all 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

smk762 and others added 8 commits July 23, 2025 01:10
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): 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
@CharlVS CharlVS merged commit c33d6f9 into dev Jul 25, 2025
9 of 13 checks passed
@CharlVS CharlVS deleted the fix/illegal-import-chars branch July 25, 2025 09:17
smk762 added a commit that referenced this pull request Aug 1, 2025
* 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]>
@coderabbitai coderabbitai bot mentioned this pull request Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants