Skip to content

Conversation

hmalik88
Copy link
Contributor

@hmalik88 hmalik88 commented Sep 22, 2025

Explanation

Currently, PK/Hardware imports were being started at Account 2 as opposed to Account 1. When we apply group metadata we already have that group in state, the logic in #applyGroupMetadata doesn't account for the fact that the group we're applying metadata shouldn't be counted for in the following line: https://github.com/MetaMask/core/blob/main/packages/account-tree-controller/src/AccountTreeController.ts#L461. Due to the way we apply group metadata (after we've added the groups into state), we need to take the min between proposedNameIndex and highestAccountNameIndex NOT the max. This change in logic accounts for group metadata being applied in the scenario when the controller is being rehydrated through init and when a new pk/hardware account is added in an active session (and handled through accountAdded).

Before

Screen.Recording.2025-09-22.at.12.32.55.PM.mov

After

after-pk-naming-fix.mov

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@hmalik88
Copy link
Contributor Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/account-tree-controller": "1.0.0-preview-5a701133",
  "@metamask-previews/accounts-controller": "33.1.0-preview-5a701133",
  "@metamask-previews/address-book-controller": "6.1.1-preview-5a701133",
  "@metamask-previews/announcement-controller": "7.0.3-preview-5a701133",
  "@metamask-previews/app-metadata-controller": "1.0.0-preview-5a701133",
  "@metamask-previews/approval-controller": "7.1.3-preview-5a701133",
  "@metamask-previews/assets-controllers": "76.0.0-preview-5a701133",
  "@metamask-previews/base-controller": "8.4.0-preview-5a701133",
  "@metamask-previews/bridge-controller": "44.0.0-preview-5a701133",
  "@metamask-previews/bridge-status-controller": "44.0.0-preview-5a701133",
  "@metamask-previews/build-utils": "3.0.3-preview-5a701133",
  "@metamask-previews/chain-agnostic-permission": "1.1.1-preview-5a701133",
  "@metamask-previews/composable-controller": "11.0.0-preview-5a701133",
  "@metamask-previews/controller-utils": "11.14.0-preview-5a701133",
  "@metamask-previews/delegation-controller": "0.7.0-preview-5a701133",
  "@metamask-previews/earn-controller": "8.0.0-preview-5a701133",
  "@metamask-previews/eip-5792-middleware": "1.2.0-preview-5a701133",
  "@metamask-previews/eip1193-permission-middleware": "1.0.0-preview-5a701133",
  "@metamask-previews/ens-controller": "17.0.1-preview-5a701133",
  "@metamask-previews/error-reporting-service": "2.0.0-preview-5a701133",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-5a701133",
  "@metamask-previews/foundryup": "1.0.1-preview-5a701133",
  "@metamask-previews/gas-fee-controller": "24.0.0-preview-5a701133",
  "@metamask-previews/gator-permissions-controller": "0.2.0-preview-5a701133",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-5a701133",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-5a701133",
  "@metamask-previews/keyring-controller": "23.1.0-preview-5a701133",
  "@metamask-previews/logging-controller": "6.0.4-preview-5a701133",
  "@metamask-previews/message-manager": "12.0.2-preview-5a701133",
  "@metamask-previews/messenger": "0.3.0-preview-5a701133",
  "@metamask-previews/multichain-account-service": "1.0.0-preview-5a701133",
  "@metamask-previews/multichain-api-middleware": "1.0.0-preview-5a701133",
  "@metamask-previews/multichain-network-controller": "1.0.0-preview-5a701133",
  "@metamask-previews/multichain-transactions-controller": "5.0.0-preview-5a701133",
  "@metamask-previews/name-controller": "8.0.3-preview-5a701133",
  "@metamask-previews/network-controller": "24.1.0-preview-5a701133",
  "@metamask-previews/network-enablement-controller": "2.0.0-preview-5a701133",
  "@metamask-previews/notification-services-controller": "18.1.0-preview-5a701133",
  "@metamask-previews/permission-controller": "11.0.6-preview-5a701133",
  "@metamask-previews/permission-log-controller": "4.0.0-preview-5a701133",
  "@metamask-previews/phishing-controller": "13.1.0-preview-5a701133",
  "@metamask-previews/polling-controller": "14.0.0-preview-5a701133",
  "@metamask-previews/preferences-controller": "19.0.0-preview-5a701133",
  "@metamask-previews/profile-sync-controller": "25.0.0-preview-5a701133",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-5a701133",
  "@metamask-previews/remote-feature-flag-controller": "1.7.0-preview-5a701133",
  "@metamask-previews/sample-controllers": "1.0.0-preview-5a701133",
  "@metamask-previews/seedless-onboarding-controller": "4.0.0-preview-5a701133",
  "@metamask-previews/selected-network-controller": "24.0.0-preview-5a701133",
  "@metamask-previews/shield-controller": "0.1.2-preview-5a701133",
  "@metamask-previews/signature-controller": "33.0.0-preview-5a701133",
  "@metamask-previews/subscription-controller": "0.1.0-preview-5a701133",
  "@metamask-previews/token-search-discovery-controller": "3.3.0-preview-5a701133",
  "@metamask-previews/transaction-controller": "60.4.0-preview-5a701133",
  "@metamask-previews/user-operation-controller": "39.0.0-preview-5a701133"
}

@hmalik88 hmalik88 marked this pull request as ready for review September 22, 2025 18:50
@hmalik88 hmalik88 requested review from a team as code owners September 22, 2025 18:50
@hmalik88 hmalik88 enabled auto-merge (squash) September 23, 2025 09:09
@hmalik88 hmalik88 merged commit 680b813 into main Sep 23, 2025
239 checks passed
@hmalik88 hmalik88 deleted the hm/fix-account-naming-for-pk-imports branch September 23, 2025 09:12
proposedNameIndex = Object.keys(wallet.groups).length - 1;
}

// Use the higher of the two: highest parsed index or computed index
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Use the lowest of the two: highest parsed index or computed index

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.

3 participants