Skip to content

Commit 20d17ba

Browse files
fabiobozzoccharly
andauthored
fix(account-tree-controller): address group names consistency bug (#6601)
## Explanation **CRITICAL FIX**: Resolves a severe multi-wallet account naming bug that caused duplicate, inconsistent, and confusing account group names across different seed phrases (SRPs). ### Current Problem Users importing multiple seed phrases experienced catastrophic naming issues: - **Duplicate names**: Multiple "Account 2", "Account 3" within the same wallet - **Missing "Account 1"**: Some wallets would start at "Account 2" - **Cross-wallet confusion**: Second SRP incorrectly showing "Account 3" when it should be "Account 1" - **Inconsistent after restart**: Names would change unpredictably on app restart ### Root Cause The account group naming logic had critical flaws: 1. **Broken alphabetical sorting**: Used `Object.keys(wallet.groups).sort()` which caused conflicts and duplicates in multi-wallet scenarios 2. **Unsafe entropy access**: Could throw TypeError when accessing `group.metadata.entropy.groupIndex` without null checks 3. **Negative index bug**: Empty wallets would get `-1` index, causing invalid "Account 0" names ### Solution Implemented a robust fix with comprehensive improvements: 1. **Per-wallet sequential numbering**: Each wallet maintains independent "Account 1", "Account 2", etc. numbering with dynamic pattern detection that works with any naming convention ("Account N", "Imported Account N", etc.) 2. **Smart conflict resolution**: Universal conflict detection with `while` loop that finds next available unique name, handles user renames, and ensures unique sequential names 3. **Entropy-aware indexing**: For multichain groups, prioritizes actual `groupIndex` from entropy metadata when available with safe null checks 4. **Starting point optimization**: Begins name checking at `wallet.groups.length` to minimize iterations and improve efficiency 5. **AutoHandleConflict integration**: Added optional `autoHandleConflict` parameter to `setAccountGroupName()` for automated conflict resolution with suffix generation (`"Account 1 (2)"`, `"Account 1 (3)"`, etc.) - ready for Backup & Sync integration ## References Fixes: - [MUL-835](https://consensyssoftware.atlassian.net/browse/MUL-835) - [MUL-826](https://consensyssoftware.atlassian.net/browse/MUL-826) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes [MUL-835]: https://consensyssoftware.atlassian.net/browse/MUL-835?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Charly Chevalier <charly.chevalier@consensys.net>
1 parent 48b8a46 commit 20d17ba

File tree

4 files changed

+984
-87
lines changed

4 files changed

+984
-87
lines changed

packages/account-tree-controller/CHANGELOG.md

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add `autoHandleConflict` parameter to `setAccountGroupName` method for automatic conflict resolution with suffix generation ([#6601](https://github.com/MetaMask/core/pull/6601))
13+
14+
### Changed
15+
16+
- Computed names (inherited from previous existing accounts) is disabled temporarily ([#6601](https://github.com/MetaMask/core/pull/6601))
17+
- They do interfere with the naming mechanism, so we disable them temporarily in favor of the new per-wallet sequential naming.
18+
19+
### Fixed
20+
21+
- Fix multi-wallet account group naming inconsistencies and duplicates ([#6601](https://github.com/MetaMask/core/pull/6601))
22+
- Implement proper per-wallet sequential numbering with highest account index parsing.
23+
- Add name persistence during group initialization to ensure consistency across app restarts.
24+
1025
## [0.17.0]
1126

1227
### Changed
@@ -38,10 +53,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3853
- Bump `@metamask/utils` from `^11.4.2` to `^11.8.0` ([#6588](https://github.com/MetaMask/core/pull/6588))
3954
- Bump `@metamask/base-controller` from `^8.3.0` to `^8.4.0` ([#6632](https://github.com/MetaMask/core/pull/6632))
4055

41-
### Removed
42-
43-
- Remove use of `:getSelectedAccount` action ([#6608](https://github.com/MetaMask/core/pull/6608))
44-
4556
## [0.15.1]
4657

4758
### Fixed

0 commit comments

Comments
 (0)