Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into keep-preferences-cont…
Browse files Browse the repository at this point in the history
…roller-in-sync-with-keyrings

* origin/main:
  chore(deps): bump @metamask/eth-keyring-controller from 17.0.0 to 17.0.1 (#3805)
  fix: custody keyring name (#3803)
  chore: update dependencies for `@metamask/accounts-controller` (#3747)
  fix: quick succession of submit password causing Accounts Controller state to be cleared (#3802)
  feat: add methods to support ERC-4337 accounts (#3602)
  feat: add getAccount action to AccountsController (#1892)
  • Loading branch information
Gudahtt committed Jan 22, 2024
2 parents f8b7715 + 72424ec commit ee153a6
Show file tree
Hide file tree
Showing 9 changed files with 653 additions and 300 deletions.
2 changes: 1 addition & 1 deletion jest.config.packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ module.exports = {
coverageProvider: 'babel',

// A list of reporter names that Jest uses when writing coverage reports
coverageReporters: ['text', 'html', 'json-summary'],
coverageReporters: ['text', 'html', 'json-summary', 'lcov'],

// An object that configures minimum threshold enforcement for coverage results
// (Each package defines this separately)
Expand Down
14 changes: 7 additions & 7 deletions packages/accounts-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@
},
"dependencies": {
"@metamask/base-controller": "^4.1.0",
"@metamask/eth-snap-keyring": "^2.0.0",
"@metamask/keyring-api": "^1.1.0",
"@metamask/snaps-sdk": "^1.3.1",
"@metamask/snaps-utils": "^5.1.1",
"@metamask/eth-snap-keyring": "^2.1.1",
"@metamask/keyring-api": "^3.0.0",
"@metamask/keyring-controller": "^12.0.0",
"@metamask/snaps-sdk": "^1.3.2",
"@metamask/snaps-utils": "^5.1.2",
"@metamask/utils": "^8.3.0",
"deepmerge": "^4.2.2",
"ethereumjs-util": "^7.0.10",
Expand All @@ -44,8 +45,7 @@
},
"devDependencies": {
"@metamask/auto-changelog": "^3.4.4",
"@metamask/keyring-controller": "^12.0.0",
"@metamask/snaps-controllers": "^3.6.0",
"@metamask/snaps-controllers": "^4.0.0",
"@types/jest": "^27.4.1",
"@types/readable-stream": "^2.3.0",
"jest": "^27.5.1",
Expand All @@ -56,7 +56,7 @@
},
"peerDependencies": {
"@metamask/keyring-controller": "^12.0.0",
"@metamask/snaps-controllers": "^3.6.0"
"@metamask/snaps-controllers": "^4.0.0"
},
"engines": {
"node": ">=16.0.0"
Expand Down
70 changes: 64 additions & 6 deletions packages/accounts-controller/src/AccountsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,20 @@ const mockGetKeyringForAccount = jest.fn();
const mockGetKeyringByType = jest.fn();
const mockGetAccounts = jest.fn();

const EOA_METHODS = [
EthMethod.PersonalSign,
EthMethod.Sign,
EthMethod.SignTransaction,
EthMethod.SignTypedDataV1,
EthMethod.SignTypedDataV3,
EthMethod.SignTypedDataV4,
] as const;

const mockAccount: InternalAccount = {
id: 'mock-id',
address: '0x123',
options: {},
methods: [...Object.values(EthMethod)],
methods: [...EOA_METHODS],
type: EthAccountType.Eoa,
metadata: {
name: 'Account 1',
Expand All @@ -45,7 +54,7 @@ const mockAccount2: InternalAccount = {
id: 'mock-id2',
address: '0x1234',
options: {},
methods: [...Object.values(EthMethod)],
methods: [...EOA_METHODS],
type: EthAccountType.Eoa,
metadata: {
name: 'Account 2',
Expand All @@ -58,7 +67,7 @@ const mockAccount3: InternalAccount = {
id: 'mock-id3',
address: '0x3333',
options: {},
methods: [...Object.values(EthMethod)],
methods: [...EOA_METHODS],
type: EthAccountType.Eoa,
metadata: {
name: '',
Expand All @@ -76,7 +85,7 @@ const mockAccount4: InternalAccount = {
id: 'mock-id4',
address: '0x4444',
options: {},
methods: [...Object.values(EthMethod)],
methods: [...EOA_METHODS],
type: EthAccountType.Eoa,
metadata: {
name: 'Custom Name',
Expand Down Expand Up @@ -121,7 +130,7 @@ function createExpectedInternalAccount({
id,
address,
options: {},
methods: [...Object.values(EthMethod)],
methods: [...EOA_METHODS],
type: EthAccountType.Eoa,
metadata: {
name,
Expand Down Expand Up @@ -364,7 +373,30 @@ describe('AccountsController', () => {
afterEach(() => {
jest.clearAllMocks();
});
it('should only update if the keyring is unlocked', async () => {
it('should not update state when only keyring is unlocked without any keyrings', async () => {
const messenger = buildMessenger();
const accountsController = setupAccountsController({
initialState: {
internalAccounts: {
accounts: {},
selectedAccount: '',
},
},
messenger,
});

messenger.publish(
'KeyringController:stateChange',
{ isUnlocked: true, keyrings: [] },
[],
);

const accounts = accountsController.listAccounts();

expect(accounts).toStrictEqual([]);
});

it('should only update if the keyring is unlocked and when there are keyrings', async () => {
const messenger = buildMessenger();

const mockNewKeyringState = {
Expand Down Expand Up @@ -1841,6 +1873,7 @@ describe('AccountsController', () => {
jest.spyOn(AccountsController.prototype, 'updateAccounts');
jest.spyOn(AccountsController.prototype, 'getAccountByAddress');
jest.spyOn(AccountsController.prototype, 'getSelectedAccount');
jest.spyOn(AccountsController.prototype, 'getAccount');
});

describe('setSelectedAccount', () => {
Expand Down Expand Up @@ -1981,5 +2014,30 @@ describe('AccountsController', () => {
expect(account).toStrictEqual(mockAccount);
});
});

describe('getAccount', () => {
it('should get account by id', async () => {
const messenger = buildMessenger();

const accountsController = setupAccountsController({
initialState: {
internalAccounts: {
accounts: { [mockAccount.id]: mockAccount },
selectedAccount: mockAccount.id,
},
},
messenger,
});

const account = messenger.call(
'AccountsController:getAccount',
mockAccount.id,
);
expect(accountsController.getAccount).toHaveBeenCalledWith(
mockAccount.id,
);
expect(account).toStrictEqual(mockAccount);
});
});
});
});
43 changes: 29 additions & 14 deletions packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
import { BaseController } from '@metamask/base-controller';
import { SnapKeyring } from '@metamask/eth-snap-keyring';
import type { InternalAccount } from '@metamask/keyring-api';
import { EthAccountType } from '@metamask/keyring-api';
import { EthAccountType, EthMethod } from '@metamask/keyring-api';
import { KeyringTypes } from '@metamask/keyring-controller';
import type {
KeyringControllerState,
Expand Down Expand Up @@ -71,6 +71,12 @@ export type AccountsControllerGetAccountByAddressAction = {
type: `${typeof controllerName}:getAccountByAddress`;
handler: AccountsController['getAccountByAddress'];
};

export type AccountsControllerGetAccountAction = {
type: `${typeof controllerName}:getAccount`;
handler: AccountsController['getAccount'];
};

export type AccountsControllerActions =
| AccountsControllerGetStateAction
| AccountsControllerSetSelectedAccountAction
Expand All @@ -79,6 +85,7 @@ export type AccountsControllerActions =
| AccountsControllerUpdateAccountsAction
| AccountsControllerGetAccountByAddressAction
| AccountsControllerGetSelectedAccountAction
| AccountsControllerGetAccountAction
| KeyringControllerGetKeyringForAccountAction
| KeyringControllerGetKeyringsByTypeAction
| KeyringControllerGetAccountsAction;
Expand Down Expand Up @@ -393,12 +400,12 @@ export class AccountsController extends BaseController<
address,
options: {},
methods: [
'personal_sign',
'eth_sign',
'eth_signTransaction',
'eth_signTypedData_v1',
'eth_signTypedData_v3',
'eth_signTypedData_v4',
EthMethod.PersonalSign,
EthMethod.Sign,
EthMethod.SignTransaction,
EthMethod.SignTypedDataV1,
EthMethod.SignTypedDataV3,
EthMethod.SignTypedDataV4,
],
type: EthAccountType.Eoa,
metadata: {
Expand Down Expand Up @@ -456,12 +463,12 @@ export class AccountsController extends BaseController<
address,
options: {},
methods: [
'personal_sign',
'eth_sign',
'eth_signTransaction',
'eth_signTypedData_v1',
'eth_signTypedData_v3',
'eth_signTypedData_v4',
EthMethod.PersonalSign,
EthMethod.Sign,
EthMethod.SignTransaction,
EthMethod.SignTypedDataV1,
EthMethod.SignTypedDataV3,
EthMethod.SignTypedDataV4,
],
type: EthAccountType.Eoa,
metadata: {
Expand All @@ -487,7 +494,10 @@ export class AccountsController extends BaseController<
// check if there are any new accounts added
// TODO: change when accountAdded event is added to the keyring controller

if (keyringState.isUnlocked) {
// We check for keyrings length to be greater than 0 because the extension client may try execute
// submit password twice and clear the keyring state.
// https://github.com/MetaMask/KeyringController/blob/2d73a4deed8d013913f6ef0c9f5c0bb7c614f7d3/src/KeyringController.ts#L910
if (keyringState.isUnlocked && keyringState.keyrings.length > 0) {
const updatedNormalKeyringAddresses: AddressAndKeyringTypeObject[] = [];
const updatedSnapKeyringAddresses: AddressAndKeyringTypeObject[] = [];

Expand Down Expand Up @@ -784,5 +794,10 @@ export class AccountsController extends BaseController<
`${controllerName}:getAccountByAddress`,
this.getAccountByAddress.bind(this),
);

this.messagingSystem.registerActionHandler(
`AccountsController:getAccount`,
this.getAccount.bind(this),
);
}
}
10 changes: 9 additions & 1 deletion packages/keyring-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"dependencies": {
"@keystonehq/metamask-airgapped-keyring": "^0.13.1",
"@metamask/base-controller": "^4.1.0",
"@metamask/eth-keyring-controller": "^15.1.0",
"@metamask/eth-keyring-controller": "^17.0.1",
"@metamask/keyring-api": "^3.0.0",
"@metamask/message-manager": "^7.3.7",
"@metamask/utils": "^8.3.0",
"async-mutex": "^0.2.6",
Expand All @@ -45,6 +46,7 @@
"@ethereumjs/common": "^3.2.0",
"@ethereumjs/tx": "^4.2.0",
"@keystonehq/bc-ur-registry-eth": "^0.9.0",
"@lavamoat/allow-scripts": "^2.3.1",
"@metamask/auto-changelog": "^3.4.4",
"@metamask/eth-sig-util": "^7.0.1",
"@metamask/scure-bip39": "^2.1.1",
Expand All @@ -65,5 +67,11 @@
"publishConfig": {
"access": "public",
"registry": "https://registry.npmjs.org/"
},
"lavamoat": {
"allowScripts": {
"ethereumjs-util>ethereum-cryptography>keccak": false,
"ethereumjs-util>ethereum-cryptography>secp256k1": false
}
}
}
Loading

0 comments on commit ee153a6

Please sign in to comment.