feat: add KeyringController:accountAdded event + use keyring events in AccountsController

[ccharly]

Explanation

Moving the add/remove account/address logic in the KeyringController. This way, we don't need to run a complex logic in the AccountsController everytime a new account gets added.

Part of this logic is now at the KeyringController-level yes, but the logic itself seems simpler, and hopefully, is faster too!

References

N/A

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
• I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

---

Note

Adds KeyringController:accountAdded/accountRemoved events and updates AccountsController to use these events instead of KeyringController:stateChange, with corresponding tests and changelog updates.

• Keyring Controller:
  • Add and emit KeyringController:accountAdded and KeyringController:accountRemoved events (via overridden update, persistAllKeyrings, and withKeyring).
  • Adjust internals (e.g., make #updateVault async) and add comprehensive event tests.
• Accounts Controller:
  • Subscribe to KeyringController:account{Added,Removed}; implement #handleOnKeyringAccountAdded/#handleOnKeyringAccountRemoved to update internal accounts and publish AccountsController:account{Added,Removed}.
  • Remove reliance on KeyringController:stateChange; update event wiring and tests (including order guarantees for :accountAdded before :selectedAccountChange).
• Changelogs:
  • Document new events and breaking change: AccountsController now requires KeyringController:account{Added,Removed} instead of :stateChange.

^{Written by Cursor Bugbot for commit 5009b0a. This will update automatically on new commits. Configure here.}

[ccharly]

Kept it as a Map even for oldAccounts for simplicity. We're not sending the KeyringObject alongside :accountRemoved, maybe we should?

But then, should the removed account be part of keyring.accounts still? What about other removed accounts? Should we remove them all before sending the events?

For now, I'd just keep the original signature for :accountRemoved.

[cursor]

Bug: Case-sensitive address comparison may fail to find accounts

The #handleOnKeyringAccountRemoved method uses strict equality (===) to compare addresses, while the existing getAccountByAddress method uses case-insensitive comparison with .toLowerCase() on both sides. Ethereum addresses can exist in different case formats (checksummed vs lowercase), so if there's any inconsistency in how addresses are stored versus how they're received from the KeyringController:accountRemoved event, the account lookup will fail silently and the account won't be removed from the state.

 

[cursor]

Bug: Event ordering inconsistency when removing selected account

The #handleOnKeyringAccountRemoved method publishes accountRemoved event after calling #update, while the analogous #handleOnKeyringAccountAdded method publishes accountAdded inside the beforeAutoSelectAccount callback of #update. This asymmetry causes an event ordering bug: when the removed account is the currently selected one, #update triggers auto-selection and publishes selectedAccountChange before accountRemoved gets published. This reverses the expected event order. The PR includes a test ("fires :accountAdded before :selectedAccountChange") that validates the correct ordering for added accounts, but the same ordering guarantee isn't maintained for removed accounts. Moving messenger.publish('AccountsController:accountRemoved', ...) into a beforeAutoSelectAccount callback would fix this inconsistency.

 

[ccharly]

That's true for the asymmetry, but IMO this makes more sense to call :selectedAccountChange before :accountRemoved:

• We change the account cause it will get removed
• We remove the account

This way, we don't have any "dangling" account being wrongly selected (from a consumer point of view)

[cursor]

Bug: Address comparison uses strict equality instead of case-insensitive

The #handleOnKeyringAccountRemoved method uses strict equality (===) to compare addresses, but the codebase convention (seen in getAccountByAddress and utils.ts) uses case-insensitive comparison with .toLowerCase(). Ethereum addresses can be checksummed (mixed case) or lowercase, so if the KeyringController emits an address with different casing than what's stored in AccountsController, the account won't be found and won't be removed, leaving orphaned accounts in state.

 

[cursor]

Bug: Wallet lock removes accounts and loses custom names

The old #handleOnKeyringStateChange method had a guard if (!isUnlocked || keyrings.length === 0) { return; } that prevented account state changes when the wallet was locked. The new event-based handlers lack this guard. When setLocked() is called, it sets state.keyrings = [], which triggers accountRemoved events for all accounts via the update override. The AccountsController processes these events and removes all accounts. On unlock, accounts are re-added with auto-generated names via getNextAvailableAccountName, causing loss of any custom account names the user had set.

Additional Locations (1)

• packages/keyring-controller/src/KeyringController.ts#L2347-L2351