Skip to content

Commit

Permalink
fix: use listMultichainAccounts and set lastSelected for initial …
Browse files Browse the repository at this point in the history
…account (#4494)

## Explanation

This PR fixes the following:

1. Emits `selectedAccountChange` for initial account
2. Updates `lastSelected` for initial account
3. Uses `listMultichainAccounts` instead of `listAccounts` for
multichain methods

## References

<!--
Are there any issues that this pull request is tied to? Are there other
links that reviewers should consult to understand these changes better?

For example:

* Fixes #12345
* Related to #67890
-->

Fixes MetaMask/accounts-planning#507

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/accounts-controller`

- **FIXED**: Emits `selectedAccountChange` and updates last selected for
initial account.
- **FIXED**: Uses `listMultichainAccounts` instead of `listAccounts` for
multichain methods

## Checklist

- [x] 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 highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Charly Chevalier <[email protected]>
  • Loading branch information
montelaidev and ccharly authored Jul 3, 2024
1 parent 1755534 commit cb11300
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 55 deletions.
108 changes: 83 additions & 25 deletions packages/accounts-controller/src/AccountsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,45 @@ describe('AccountsController', () => {
});

describe('onKeyringStateChange', () => {
it('uses listMultichainAccounts', async () => {
const messenger = buildMessenger();
mockUUID
.mockReturnValueOnce('mock-id') // call to check if its a new account
.mockReturnValueOnce('mock-id2'); // call to add account

const { accountsController } = setupAccountsController({
initialState: {
internalAccounts: {
accounts: {},
selectedAccount: '',
},
},
messenger,
});

const listMultichainAccountsSpy = jest.spyOn(
accountsController,
'listMultichainAccounts',
);

const mockNewKeyringState = {
isUnlocked: true,
keyrings: [
{
type: KeyringTypes.hd,
accounts: [mockAccount.address],
},
],
};

messenger.publish(
'KeyringController:stateChange',
mockNewKeyringState,
[],
);

expect(listMultichainAccountsSpy).toHaveBeenCalled();
});
it('not update state when only keyring is unlocked without any keyrings', async () => {
const messenger = buildMessenger();
const { accountsController } = setupAccountsController({
Expand All @@ -463,7 +502,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([]);
});
Expand Down Expand Up @@ -496,7 +535,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([]);
});
Expand Down Expand Up @@ -537,7 +576,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
mockAccount,
Expand Down Expand Up @@ -595,7 +634,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
mockAccount,
Expand Down Expand Up @@ -661,7 +700,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
mockAccount,
Expand Down Expand Up @@ -709,7 +748,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
mockAccount,
Expand Down Expand Up @@ -774,7 +813,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts.map(setLastSelectedAsAny)).toStrictEqual([
mockAccount,
Expand Down Expand Up @@ -872,7 +911,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
mockAccount,
Expand Down Expand Up @@ -915,7 +954,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([setLastSelectedAsAny(mockAccount2)]);
expect(accountsController.getSelectedAccount()).toStrictEqual(
Expand Down Expand Up @@ -968,7 +1007,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
setLastSelectedAsAny(mockAccount),
Expand Down Expand Up @@ -1031,7 +1070,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
setLastSelectedAsAny(mockAccount),
Expand All @@ -1043,6 +1082,7 @@ describe('AccountsController', () => {
});

it('delete the account and select the account with the most recent lastSelected', async () => {
const currentTime = Date.now();
const messenger = buildMessenger();
mockUUID.mockReturnValueOnce('mock-id').mockReturnValueOnce('mock-id2');

Expand Down Expand Up @@ -1097,14 +1137,22 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();

expect(accounts).toStrictEqual([
mockAccountWithoutLastSelected,
{
...mockAccountWithoutLastSelected,
metadata: {
...mockAccountWithoutLastSelected.metadata,
lastSelected: expect.any(Number),
},
},
mockAccount2WithoutLastSelected,
]);
expect(accountsController.getSelectedAccount()).toStrictEqual(
mockAccountWithoutLastSelected,

const selectedAccount = accountsController.getSelectedAccount();
expect(selectedAccount.metadata.lastSelected).toBeGreaterThanOrEqual(
currentTime,
);
});
});
Expand Down Expand Up @@ -1155,7 +1203,7 @@ describe('AccountsController', () => {
);

const selectedAccount = accountsController.getSelectedAccount();
const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();
const expectedAccount = setLastSelectedAsAny(mockReinitialisedAccount);

expect(selectedAccount).toStrictEqual(expectedAccount);
Expand Down Expand Up @@ -1340,7 +1388,9 @@ describe('AccountsController', () => {

await accountsController.updateAccounts();

expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
expect(accountsController.listMultichainAccounts()).toStrictEqual(
expectedAccounts,
);
});

it('update accounts with Snap accounts when snap keyring is defined and has accounts', async () => {
Expand Down Expand Up @@ -1395,7 +1445,7 @@ describe('AccountsController', () => {
await accountsController.updateAccounts();

expect(
accountsController.listAccounts().map(setLastSelectedAsAny),
accountsController.listMultichainAccounts().map(setLastSelectedAsAny),
).toStrictEqual(expectedAccounts);
});

Expand Down Expand Up @@ -1425,7 +1475,9 @@ describe('AccountsController', () => {

await accountsController.updateAccounts();

expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
expect(accountsController.listMultichainAccounts()).toStrictEqual(
expectedAccounts,
);
});

it('set the account with the correct index', async () => {
Expand Down Expand Up @@ -1474,7 +1526,9 @@ describe('AccountsController', () => {

await accountsController.updateAccounts();

expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
expect(accountsController.listMultichainAccounts()).toStrictEqual(
expectedAccounts,
);
});

it('filter Snap accounts from normalAccounts', async () => {
Expand Down Expand Up @@ -1529,7 +1583,9 @@ describe('AccountsController', () => {

await accountsController.updateAccounts();

expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
expect(accountsController.listMultichainAccounts()).toStrictEqual(
expectedAccounts,
);
});

it('filter Snap accounts from normalAccounts even if the snap account is listed before normal accounts', async () => {
Expand Down Expand Up @@ -1585,7 +1641,9 @@ describe('AccountsController', () => {

await accountsController.updateAccounts();

expect(accountsController.listAccounts()).toStrictEqual(expectedAccounts);
expect(accountsController.listMultichainAccounts()).toStrictEqual(
expectedAccounts,
);
});

it.each([
Expand Down Expand Up @@ -1641,7 +1699,7 @@ describe('AccountsController', () => {
await accountsController.updateAccounts();

expect(
accountsController.listAccounts().map(setLastSelectedAsAny),
accountsController.listMultichainAccounts().map(setLastSelectedAsAny),
).toStrictEqual(expectedAccounts);
});

Expand Down Expand Up @@ -2253,7 +2311,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();
expect(accounts).toStrictEqual([
mockAccount,
setLastSelectedAsAny(mockSimpleKeyring1),
Expand Down Expand Up @@ -2310,7 +2368,7 @@ describe('AccountsController', () => {
[],
);

const accounts = accountsController.listAccounts();
const accounts = accountsController.listMultichainAccounts();
expect(accounts).toStrictEqual([
mockAccount,
setLastSelectedAsAny(mockSimpleKeyring2),
Expand Down
Loading

0 comments on commit cb11300

Please sign in to comment.