Skip to content

Commit 2b5571b

Browse files
ccharlyhmalik88claude
authored
refactor: remove forward selected account group (MetaMask#31761)
## **Description** The `SnapAccountService` already handle this logic, we don't need this on the client side anymore. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: N/A ## **Manual testing steps** ```gherkin Feature: Selecting an account Scenario: user selects an account Given given it has funds on this account When user select the new account Then the account IDs are forwarded to the multichain Snaps, so balances are updated ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** <!-- mms-check: type=checklist required=true --> <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I've included tests if applicable - [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Hassan Malik <hbmalik88@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent b668396 commit 2b5571b

7 files changed

Lines changed: 2 additions & 153 deletions

File tree

app/core/Engine/controllers/multichain-account-service/multichain-account-service-init.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import {
66
TRX_ACCOUNT_PROVIDER_NAME,
77
} from '@metamask/multichain-account-service';
88
import { MessengerClientInitFunction } from '../../types';
9-
import Engine from '../../Engine';
10-
import { forwardSelectedAccountGroupToSnapKeyring } from '../../../SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring';
119
import { MultichainAccountServiceInitMessenger } from '../../messengers/multichain-account-service-messenger/multichain-account-service-messenger';
1210

1311
/**
@@ -63,20 +61,5 @@ export const multichainAccountServiceInit: MessengerClientInitFunction<
6361
},
6462
});
6563

66-
// TODO: Move this logic to the SnapKeyring directly.
67-
initMessenger.subscribe(
68-
'MultichainAccountService:multichainAccountGroupUpdated',
69-
(group) => {
70-
const { AccountTreeController } = Engine.context;
71-
72-
// If the current group gets updated, then maybe there are more accounts being "selected"
73-
// now, so we have to forward them to the Snap keyring too!
74-
if (AccountTreeController.getSelectedAccountGroup() === group.id) {
75-
// eslint-disable-next-line no-void
76-
void forwardSelectedAccountGroupToSnapKeyring(group.id);
77-
}
78-
},
79-
);
80-
8164
return { controller, memStateKey: null, persistedStateKey: null };
8265
};

app/core/SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring.test.ts

Lines changed: 0 additions & 45 deletions
This file was deleted.

app/core/SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring.ts

Lines changed: 0 additions & 30 deletions
This file was deleted.

app/multichain-accounts/AccountTreeInitService/index.test.ts

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,8 @@ const mockIsMoneyAccountEnabled = jest.requireMock(
1111
const mockUpdateAccounts = jest.fn();
1212
const mockAccountTreeInit = jest.fn();
1313
const mockAccountTreeClearState = jest.fn();
14-
const mockAccountTreeGetSelectedAccountGroup = jest.fn();
1514
const mockMoneyAccountInit = jest.fn();
1615
const mockMoneyAccountClearState = jest.fn();
17-
const mockForwardSelectedAccountGroupToSnapKeyring = jest.fn();
18-
1916
jest.mock('../../core/Engine', () => ({
2017
__esModule: true,
2118
default: {
@@ -30,9 +27,6 @@ jest.mock('../../core/Engine', () => ({
3027
clearState: jest
3128
.fn()
3229
.mockImplementation(() => mockAccountTreeClearState()),
33-
getSelectedAccountGroup: jest
34-
.fn()
35-
.mockImplementation(() => mockAccountTreeGetSelectedAccountGroup()),
3630
},
3731
MoneyAccountController: {
3832
init: jest.fn().mockImplementation(() => mockMoneyAccountInit()),
@@ -47,15 +41,6 @@ jest.mock('../../core/Engine', () => ({
4741
},
4842
}));
4943

50-
jest.mock(
51-
'../../core/SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring',
52-
() => ({
53-
forwardSelectedAccountGroupToSnapKeyring: jest
54-
.fn()
55-
.mockImplementation(() => mockForwardSelectedAccountGroupToSnapKeyring()),
56-
}),
57-
);
58-
5944
describe('AccountTreeInitService', () => {
6045
let service: AccountTreeInitService;
6146

@@ -91,25 +76,6 @@ describe('AccountTreeInitService', () => {
9176

9277
expect(mockMoneyAccountInit).not.toHaveBeenCalled();
9378
});
94-
95-
it('forwards the selected account group to the Snap keyring', async () => {
96-
await service.initializeAccountTree();
97-
expect(mockForwardSelectedAccountGroupToSnapKeyring).toHaveBeenCalled();
98-
});
99-
100-
it('forwards the result of getSelectedAccountGroup to the Snap keyring', async () => {
101-
const mockGroup = { id: 'test-group' };
102-
mockAccountTreeGetSelectedAccountGroup.mockReturnValue(mockGroup);
103-
104-
await service.initializeAccountTree();
105-
106-
const { forwardSelectedAccountGroupToSnapKeyring } = jest.requireMock(
107-
'../../core/SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring',
108-
);
109-
expect(forwardSelectedAccountGroupToSnapKeyring).toHaveBeenCalledWith(
110-
mockGroup,
111-
);
112-
});
11379
});
11480

11581
describe('clearState', () => {

app/multichain-accounts/AccountTreeInitService/index.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import Engine from '../../core/Engine';
2-
import { forwardSelectedAccountGroupToSnapKeyring } from '../../core/SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring';
32
import { isMoneyAccountEnabled } from '../../lib/Money/feature-flags';
43

54
export class AccountTreeInitService {
@@ -21,11 +20,6 @@ export class AccountTreeInitService {
2120
if (isMoneyAccountEnabled(remoteFeatureFlags)) {
2221
await MoneyAccountController.init();
2322
}
24-
25-
// Forward initial selected accounts.
26-
await forwardSelectedAccountGroupToSnapKeyring(
27-
AccountTreeController.getSelectedAccountGroup(),
28-
);
2923
};
3024

3125
clearState = async (): Promise<void> => {

app/multichain-accounts/controllers/account-tree-controller/index.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@ import {
44
} from '@metamask/account-tree-controller';
55
import type { MessengerClientInitFunction } from '../../../core/Engine/types';
66
import { trace } from '../../../util/trace';
7-
import { forwardSelectedAccountGroupToSnapKeyring } from '../../../core/SnapKeyring/utils/forwardSelectedAccountGroupToSnapKeyring';
87
import { MetaMetricsEvents } from '../../../core/Analytics';
98
import { AnalyticsEventBuilder } from '../../../util/analytics/AnalyticsEventBuilder';
10-
import { AccountGroupId } from '@metamask/account-api';
119
import { AccountTreeControllerInitMessenger } from '../../messengers/account-tree-controller-messenger';
1210

1311
/**
@@ -53,15 +51,5 @@ export const accountTreeControllerInit: MessengerClientInitFunction<
5351
},
5452
});
5553

56-
// Forward selected accounts every time the selected account group changes.
57-
initMessenger.subscribe(
58-
'AccountTreeController:selectedAccountGroupChange',
59-
(groupId: AccountGroupId | '') => {
60-
// TODO: Move this logic to the SnapKeyring directly.
61-
// eslint-disable-next-line no-void
62-
void forwardSelectedAccountGroupToSnapKeyring(groupId);
63-
},
64-
);
65-
6654
return { controller };
6755
};

app/multichain-accounts/messengers/account-tree-controller-messenger/index.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
import {
2-
AccountTreeControllerMessenger,
3-
AccountTreeControllerSelectedAccountGroupChangeEvent,
4-
} from '@metamask/account-tree-controller';
1+
import { AccountTreeControllerMessenger } from '@metamask/account-tree-controller';
52
import {
63
RootExtendedMessenger,
74
RootMessenger,
@@ -60,9 +57,6 @@ export function getAccountTreeControllerMessenger(
6057
return messenger;
6158
}
6259

63-
export type AllowedInitializationEvents =
64-
AccountTreeControllerSelectedAccountGroupChangeEvent;
65-
6660
type AccountTreeControllerInitMessengerActions = AnalyticsControllerActions;
6761

6862
export type AccountTreeControllerInitMessenger = ReturnType<
@@ -82,15 +76,14 @@ export function getAccountTreeControllerInitMessenger(
8276
const messenger = new Messenger<
8377
'AccountTreeControllerInit',
8478
AccountTreeControllerInitMessengerActions,
85-
AllowedInitializationEvents,
79+
never,
8680
RootMessenger
8781
>({
8882
namespace: 'AccountTreeControllerInit',
8983
parent: rootMessenger,
9084
});
9185
rootMessenger.delegate({
9286
actions: ['AnalyticsController:trackEvent'],
93-
events: ['AccountTreeController:selectedAccountGroupChange'],
9487
messenger,
9588
});
9689
return messenger;

0 commit comments

Comments
 (0)