Skip to content

fix(auth,migration): sanitize legacy wallet name and preserve legacy flag during migration#3207

Merged
ca333 merged 6 commits intodevfrom
fix/legacy-wallet-migration-sanitize-unique
Oct 22, 2025
Merged

fix(auth,migration): sanitize legacy wallet name and preserve legacy flag during migration#3207
ca333 merged 6 commits intodevfrom
fix/legacy-wallet-migration-sanitize-unique

Conversation

@CharlVS
Copy link
Copy Markdown
Collaborator

@CharlVS CharlVS commented Oct 22, 2025

Summary

  • Preserve WalletConfig.isLegacyWallet in copy() so legacy wallets route to AuthRestoreRequested.
  • Sanitize legacy wallet names: replace non-alphanumeric (Unicode letters/digits) except "" with "".
  • Resolve collisions by appending the lowest integer suffix (e.g., name, name_1, name_2, ...).
  • Apply during legacy migration in AuthBloc._onRestore; delete legacy entry after success.

Why

Fixes a critical bug where some users can't log into wallets created with old versions and the wallet disappears.

Testing

  • Create a legacy wallet with special characters and attempt login; verify migration uses sanitized unique name and signs in.
  • Verify if a non-legacy wallet already has the sanitized name, the migrated wallet uses the lowest available _N suffix.

Notes

No API changes. Static analysis passes for changed files.


Note

Sanitizes and de-duplicates legacy wallet names during migration, preserves isLegacyWallet in WalletConfig.copy(), and updates restore flow to use sanitized names.

  • Auth/Migration:
    • Update AuthBloc._onRestore to handle legacy wallets:
      • Sanitize base name via _walletsRepository.sanitizeLegacyMigrationName() and attempt sign-in first.
      • If needed, resolve uniqueness with _walletsRepository.resolveUniqueWalletName() and use workingWallet for registration and metadata setup.
      • Delete legacy entry after successful migration.
  • Repository Utils:
    • Add sanitizeLegacyMigrationName, resolveUniqueWalletName, and sanitizeAndResolveLegacyWalletName to lib/blocs/wallets_repository.dart.
  • Model:
    • Preserve WalletConfig.isLegacyWallet in copy(); keep Wallet.copyWith API.
  • Version:
    • Bump pubspec.yaml version to 0.9.3+1.

Written by Cursor Bugbot for commit 578c7f5. This will update automatically on new commits. Configure here.

…ring migration\n\n- Keep WalletConfig.isLegacyWallet in copy() to ensure legacy flow\n- Sanitize legacy wallet names (non-alnum except _) and ensure uniqueness\n- Use sanitized, unique name in restore flow; delete legacy after success\n\nCloses: #none
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Oct 22, 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.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/legacy-wallet-migration-sanitize-unique

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

Comment @coderabbitai help to get the list of available commands and usage tips.

…ign submodules and pubspec to latest workspace state\n- No app code changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ 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".

Comment thread lib/bloc/auth_bloc/auth_bloc.dart Outdated
cursor[bot]

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 22, 2025

Visit the preview URL for this PR (updated for commit 578c7f5):

https://walletrc--pull-3207-merge-b4jyiuxc.web.app

(expires Wed, 29 Oct 2025 15:45:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc

@CharlVS CharlVS requested a review from ca333 October 22, 2025 15:40
@CharlVS CharlVS self-assigned this Oct 22, 2025
_log.info(
'Migration successful. '
'Deleting legacy wallet ${event.wallet.name}',
'Deleting legacy wallet ${workingWallet.name}',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect Wallet Name Logged During Deletion

When deleting a legacy wallet, the log message shows workingWallet.name (the sanitized name), but the deleteWallet call removes event.wallet (the original legacy wallet). This logs the incorrect wallet name for the item being deleted.

Fix in Cursor Fix in Web

@ca333 ca333 merged commit ee0b25b into dev Oct 22, 2025
10 of 16 checks passed
@smk762 smk762 mentioned this pull request Nov 23, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants