Skip to content

Commit

Permalink
feat!: add scopes field to KeyringAccount (#5066)
Browse files Browse the repository at this point in the history
## Explanation

Testing the new `scopes` field for `KeyringAccount`

## References

- MetaMask/accounts#101
- MetaMask/metamask-extension#29195

## Changelog

### `@metamask/accounts-controller`

- **CHANGED**: Bump `keyring-api` to `^13.0.0`
- **CHANGED**: Bump `keyring-internal-api` to `^1.1.0`
- **CHANGED**: **BREAKING:** Add new `scopes` field to `InternalAccount`
- This new field wil automatically defaults to `eip155` (CAIP-2 chain ID
namespace for EVM) for newly created EOA accounts (both Snap and
"normal" accounts).

### `@metamask/assets-controllers`

- **CHANGED**: Bump `keyring-internal-api` to `^1.1.0`

### `@metamask/chain-controller`

- **CHANGED**: Bump `keyring-internal-api` to `^1.1.0`

### `@metamask/keyring-controller`

- **CHANGED**: Bump `keyring-api` to `^13.0.0`
- **CHANGED**: Bump `keyring-internal-api` to `^1.1.0`
- **CHANGED**: Await for `generateRandomMnemonic` keyring calls
- Not all keyrings uses an `async` implementation for this method, but
`await`ing should cover both (synchronous and asynchronous) cases.
- This change is required since the bump of `@metamask/utils@^11.0.1`
that changed the `generateRandomMnemonic` function signature for the
`EthKeyring` interface (coming from `@metamask/keyring-internal-api`.

### `@metamask/profile-sync-controller`

- **CHANGED**: Bump `keyring-api` to `^13.0.0`
- **CHANGED**: Bump `keyring-internal-api` to `^1.1.0`

## 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 highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
  • Loading branch information
ccharly authored and PatrykLucka committed Jan 13, 2025
1 parent 3fcd635 commit 0964413
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 70 deletions.
4 changes: 2 additions & 2 deletions packages/accounts-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
"@ethereumjs/util": "^8.1.0",
"@metamask/base-controller": "^7.1.0",
"@metamask/eth-snap-keyring": "^7.0.0",
"@metamask/keyring-api": "^12.0.0",
"@metamask/keyring-internal-api": "^1.0.0",
"@metamask/keyring-api": "^13.0.0",
"@metamask/keyring-internal-api": "^1.1.0",
"@metamask/snaps-sdk": "^6.7.0",
"@metamask/snaps-utils": "^8.3.0",
"@metamask/utils": "^11.0.1",
Expand Down
38 changes: 26 additions & 12 deletions packages/accounts-controller/src/AccountsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {
EthAccountType,
BtcMethod,
EthMethod,
EthScopes,
BtcScopes,
} from '@metamask/keyring-api';
import { KeyringTypes } from '@metamask/keyring-controller';
import type {
Expand Down Expand Up @@ -66,6 +68,7 @@ const mockAccount: InternalAccount = {
options: {},
methods: [...ETH_EOA_METHODS],
type: EthAccountType.Eoa,
scopes: [EthScopes.Namespace],
metadata: {
name: 'Account 1',
keyring: { type: KeyringTypes.hd },
Expand All @@ -81,6 +84,7 @@ const mockAccount2: InternalAccount = {
options: {},
methods: [...ETH_EOA_METHODS],
type: EthAccountType.Eoa,
scopes: [EthScopes.Namespace],
metadata: {
name: 'Account 2',
keyring: { type: KeyringTypes.hd },
Expand All @@ -95,6 +99,7 @@ const mockAccount3: InternalAccount = {
options: {},
methods: [...ETH_EOA_METHODS],
type: EthAccountType.Eoa,
scopes: [EthScopes.Namespace],
metadata: {
name: '',
keyring: { type: KeyringTypes.snap },
Expand All @@ -114,6 +119,7 @@ const mockAccount4: InternalAccount = {
options: {},
methods: [...ETH_EOA_METHODS],
type: EthAccountType.Eoa,
scopes: [EthScopes.Namespace],
metadata: {
name: 'Custom Name',
keyring: { type: KeyringTypes.snap },
Expand Down Expand Up @@ -191,24 +197,32 @@ function createExpectedInternalAccount({
lastSelected?: number;
nameLastUpdatedAt?: number;
}): InternalAccount {
const accountTypeToMethods = {
[`${EthAccountType.Eoa}`]: [...Object.values(ETH_EOA_METHODS)],
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
[`${EthAccountType.Erc4337}`]: [...Object.values(ETH_ERC_4337_METHODS)],
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
[`${BtcAccountType.P2wpkh}`]: [...Object.values(BtcMethod)],
const accountTypeToInfo: Record<
string,
{ methods: string[]; scopes: string[] }
> = {
[`${EthAccountType.Eoa}`]: {
methods: [...Object.values(ETH_EOA_METHODS)],
scopes: [EthScopes.Namespace],
},
[`${EthAccountType.Erc4337}`]: {
methods: [...Object.values(ETH_ERC_4337_METHODS)],
scopes: [EthScopes.Mainnet], // Assuming we are using mainnet for those Smart Accounts
},
[`${BtcAccountType.P2wpkh}`]: {
methods: [...Object.values(BtcMethod)],
scopes: [BtcScopes.Mainnet],
},
};

const methods =
accountTypeToMethods[type as keyof typeof accountTypeToMethods];
const { methods, scopes } = accountTypeToInfo[type];

const account = {
const account: InternalAccount = {
id,
address,
options: {},
methods,
scopes,
type,
metadata: {
name,
Expand All @@ -217,7 +231,7 @@ function createExpectedInternalAccount({
lastSelected: lastSelected || expect.any(Number),
...(nameLastUpdatedAt && { nameLastUpdatedAt }),
},
} as InternalAccount;
};

if (snapId) {
account.metadata.snap = {
Expand Down
6 changes: 5 additions & 1 deletion packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { SnapKeyring } from '@metamask/eth-snap-keyring';
import {
EthAccountType,
EthMethod,
EthScopes,
isEvmAccountType,
} from '@metamask/keyring-api';
import { KeyringTypes } from '@metamask/keyring-controller';
Expand Down Expand Up @@ -203,6 +204,7 @@ export const EMPTY_ACCOUNT = {
options: {},
methods: [],
type: EthAccountType.Eoa,
scopes: [EthScopes.Namespace],
metadata: {
name: '',
keyring: {
Expand Down Expand Up @@ -456,7 +458,7 @@ export class AccountsController extends BaseController<
};
// Do not remove this comment - This error is flaky: Comment out or restore the `ts-expect-error` directive below as needed.
// See: https://github.com/MetaMask/utils/issues/168
// @ts-expect-error Known issue - `Json` causes recursive error in immer `Draft`/`WritableDraft` types
// // @ts-expect-error Known issue - `Json` causes recursive error in immer `Draft`/`WritableDraft` types
currentState.internalAccounts.accounts[accountId] = internalAccount;

if (metadata.name) {
Expand Down Expand Up @@ -582,6 +584,7 @@ export class AccountsController extends BaseController<
EthMethod.SignTypedDataV3,
EthMethod.SignTypedDataV4,
],
scopes: [EthScopes.Namespace],
type: EthAccountType.Eoa,
metadata: {
name: '',
Expand Down Expand Up @@ -656,6 +659,7 @@ export class AccountsController extends BaseController<
EthMethod.SignTypedDataV3,
EthMethod.SignTypedDataV4,
],
scopes: [EthScopes.Namespace],
type: EthAccountType.Eoa,
metadata: {
name: this.#populateExistingMetadata(id, 'name') ?? '',
Expand Down
2 changes: 1 addition & 1 deletion packages/assets-controllers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"@metamask/auto-changelog": "^3.4.4",
"@metamask/ethjs-provider-http": "^0.3.0",
"@metamask/keyring-controller": "^19.0.2",
"@metamask/keyring-internal-api": "^1.0.0",
"@metamask/keyring-internal-api": "^1.1.0",
"@metamask/network-controller": "^22.1.1",
"@metamask/preferences-controller": "^15.0.1",
"@metamask/providers": "^18.1.1",
Expand Down
2 changes: 1 addition & 1 deletion packages/chain-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"dependencies": {
"@metamask/base-controller": "^7.1.0",
"@metamask/chain-api": "^0.1.0",
"@metamask/keyring-internal-api": "^1.0.0",
"@metamask/keyring-internal-api": "^1.1.0",
"@metamask/keyring-utils": "^1.0.0",
"@metamask/snaps-controllers": "^9.10.0",
"@metamask/snaps-sdk": "^6.7.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/keyring-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
"@metamask/eth-hd-keyring": "^7.0.4",
"@metamask/eth-sig-util": "^8.0.0",
"@metamask/eth-simple-keyring": "^6.0.5",
"@metamask/keyring-api": "^12.0.0",
"@metamask/keyring-internal-api": "^1.0.0",
"@metamask/keyring-api": "^13.0.0",
"@metamask/keyring-internal-api": "^1.1.0",
"@metamask/message-manager": "^11.0.3",
"@metamask/utils": "^11.0.1",
"async-mutex": "^0.5.0",
Expand Down
8 changes: 7 additions & 1 deletion packages/keyring-controller/src/KeyringController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2238,7 +2238,13 @@ export class KeyringController extends BaseController<
);
}

keyring.generateRandomMnemonic();
// NOTE: Not all keyrings implement this method in a asynchronous-way. Using `await` for
// non-thenable will still be valid (despite not being really useful). It allows us to cover both
// cases and allow retro-compatibility too.
// FIXME: For some reason, it seems that eslint is complaining about this call being non-thenable
// even though it is... For now, we just disable it:
// eslint-disable-next-line @typescript-eslint/await-thenable
await keyring.generateRandomMnemonic();
await keyring.addAccounts(1);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/profile-sync-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
},
"dependencies": {
"@metamask/base-controller": "^7.1.0",
"@metamask/keyring-api": "^12.0.0",
"@metamask/keyring-api": "^13.0.0",
"@metamask/keyring-controller": "^19.0.2",
"@metamask/network-controller": "^22.1.1",
"@metamask/snaps-sdk": "^6.7.0",
Expand All @@ -117,7 +117,7 @@
"@lavamoat/preinstall-always-fail": "^2.1.0",
"@metamask/accounts-controller": "^20.0.2",
"@metamask/auto-changelog": "^3.4.4",
"@metamask/keyring-internal-api": "^1.0.0",
"@metamask/keyring-internal-api": "^1.1.0",
"@metamask/providers": "^18.1.1",
"@metamask/snaps-controllers": "^9.10.0",
"@types/jest": "^27.4.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@ const INTERNAL_ACCOUNT_MOCK: InternalAccount = {
id: '58def058-d35f-49a1-a7ab-e2580565f6f5',
address: ACCOUNT_MOCK,
type: 'eip155:eoa',
scopes: ['eip155'],
options: {},
methods: [],
metadata: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ const createMockInternalAccount = ({
options: {},
methods: [],
type: 'eip155:eoa',
scopes: ['eip155'],
metadata: {
name,
keyring: { type: 'HD Key Tree' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const CONTROLLER_ARGS_MOCK: ConstructorParameters<
type: 'eip155:eoa' as const,
options: {},
methods: [],
scopes: ['eip155'],
metadata: {
name: 'Account 1',
keyring: { type: 'HD Key Tree' },
Expand Down
Loading

0 comments on commit 0964413

Please sign in to comment.