Skip to content

fix(account-tree-controller): no longer listen for AccountsController:selectedAccountChange #6737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(account-tree-controller): no longer listen for AccountsController:selectedAccountChange #6737

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 0 additions & 75 deletions packages/account-tree-controller/src/AccountTreeController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1656,24 +1656,6 @@ describe('AccountTreeController', () => {
expect(controller.getSelectedAccountGroup()).not.toBe('');
});

it('updates selectedAccountGroup when AccountsController selected account changes', () => {
const { controller, messenger } = setup({
accounts: [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2],
keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2],
});

controller.init();
const initialGroup = controller.getSelectedAccountGroup();

messenger.publish(
'AccountsController:selectedAccountChange',
MOCK_HD_ACCOUNT_2,
);

const newGroup = controller.getSelectedAccountGroup();
expect(newGroup).not.toBe(initialGroup);
});

it('updates AccountsController selected account (with EVM account) when selectedAccountGroup changes', () => {
const { controller, messenger } = setup({
accounts: [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2],
Expand Down Expand Up @@ -1819,28 +1801,6 @@ describe('AccountTreeController', () => {
}).toThrow('No accounts found in group: non-existent-group-id');
});

it('handles AccountsController selectedAccountChange for account not in tree gracefully', () => {
const { controller, messenger } = setup({
accounts: [MOCK_HD_ACCOUNT_1],
keyrings: [MOCK_HD_KEYRING_1],
});

controller.init();
const initialGroup = controller.getSelectedAccountGroup();

const unknownAccount: InternalAccount = {
...MOCK_HD_ACCOUNT_2,
id: 'unknown-account-id',
};

messenger.publish(
'AccountsController:selectedAccountChange',
unknownAccount,
);

expect(controller.getSelectedAccountGroup()).toBe(initialGroup);
});

it('falls back to first wallet first group when AccountsController returns EMPTY_ACCOUNT', () => {
const { controller, messenger } = setup({
accounts: [MOCK_HD_ACCOUNT_1, MOCK_HD_ACCOUNT_2],
Expand Down Expand Up @@ -3666,41 +3626,6 @@ describe('AccountTreeController', () => {
expect(selectedAccountGroupChangeListener).toHaveBeenCalledTimes(1);
});

it('emits selectedAccountGroupChange when selected account changes via AccountsController', () => {
// Use different keyring types to ensure different groups
const { controller, messenger } = setup({
accounts: [MOCK_HD_ACCOUNT_1, MOCK_SNAP_ACCOUNT_1],
keyrings: [MOCK_HD_KEYRING_1, MOCK_HD_KEYRING_2],
});

const selectedAccountGroupChangeListener = jest.fn();
messenger.subscribe(
'AccountTreeController:selectedAccountGroupChange',
selectedAccountGroupChangeListener,
);

controller.init();

const initialSelectedGroup =
controller.state.accountTree.selectedAccountGroup;

jest.clearAllMocks();

messenger.publish(
'AccountsController:selectedAccountChange',
MOCK_SNAP_ACCOUNT_1,
);

const newSelectedGroup =
controller.state.accountTree.selectedAccountGroup;

expect(selectedAccountGroupChangeListener).toHaveBeenCalledWith(
newSelectedGroup,
initialSelectedGroup,
);
expect(selectedAccountGroupChangeListener).toHaveBeenCalledTimes(1);
});

it('does NOT emit selectedAccountGroupChange when the same account group is already selected', () => {
const { controller, messenger } = setup({
accounts: [MOCK_HD_ACCOUNT_1],
Expand Down
40 changes: 0 additions & 40 deletions packages/account-tree-controller/src/AccountTreeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,6 @@ export class AccountTreeController extends BaseController<
},
);

this.messagingSystem.subscribe(
'AccountsController:selectedAccountChange',
(account) => {
this.#handleSelectedAccountChange(account);
},
);

this.messagingSystem.subscribe(
'UserStorageController:stateChange',
(userStorageControllerState) => {
Expand Down Expand Up @@ -1045,39 +1038,6 @@ export class AccountTreeController extends BaseController<
return this.#getDefaultAccountGroupId(wallets);
}

/**
* Handles selected account change from AccountsController.
* Updates selectedAccountGroup to match the selected account.
*
* @param account - The newly selected account.
*/
#handleSelectedAccountChange(account: InternalAccount): void {
const accountMapping = this.#accountIdToContext.get(account.id);
if (!accountMapping) {
// Account not in tree yet, might be during initialization
return;
}

const { groupId } = accountMapping;
const previousSelectedAccountGroup =
this.state.accountTree.selectedAccountGroup;

// Idempotent check - if the same group is already selected, do nothing
if (previousSelectedAccountGroup === groupId) {
return;
}

// Update selectedAccountGroup to match the selected account
this.update((state) => {
state.accountTree.selectedAccountGroup = groupId;
});
this.messagingSystem.publish(
`${controllerName}:selectedAccountGroupChange`,
groupId,
previousSelectedAccountGroup,
);
}

/**
* Handles multichain account wallet status change from
* the MultichainAccountService.
Expand Down
Loading