Skip to content
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.

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

feat: add scopes field to KeyringAccount #29195

Draft
wants to merge 47 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
f6d5942
refactor: use new accounts packages
ccharly Sep 12, 2024
cbd9f36
chore: lavamoat:auto
ccharly Dec 3, 2024
bf4405f
chore: yarn dedupe
ccharly Dec 3, 2024
de54690
chore: update resolutions
ccharly Dec 5, 2024
356eab7
Merge branch 'main' into refactor/use-new-accounts-packages
ccharly Dec 5, 2024
0584a61
chore: update resolutions
ccharly Dec 11, 2024
0fb1493
Merge branch 'main' into refactor/use-new-accounts-packages
ccharly Dec 11, 2024
a2b7d0b
chore: dedupe + lavamoat:auto
ccharly Dec 11, 2024
a39c3df
chore: update deps
ccharly Dec 11, 2024
6dce397
chore: lint
ccharly Dec 11, 2024
238f522
chore: update deps
ccharly Dec 11, 2024
ab65f1f
chore: remove preview builds
ccharly Dec 11, 2024
7cf8701
Merge branch 'main' into refactor/use-new-accounts-packages
ccharly Dec 12, 2024
dfa88e1
chore: dedupe
ccharly Dec 12, 2024
9584e9e
Merge branch 'main' into refactor/use-new-accounts-packages
ccharly Dec 12, 2024
dc10d94
Merge branch 'main' into refactor/use-new-accounts-packages
ccharly Dec 12, 2024
5ea8323
chore: lavamoat:auto
ccharly Dec 12, 2024
45784bb
chore: dedupe
ccharly Dec 12, 2024
50035f3
Merge branch 'main' into refactor/use-new-accounts-packages
ccharly Dec 12, 2024
384c9e9
Merge branch 'main' into refactor/use-new-accounts-packages
HowardBraham Dec 12, 2024
281c5bf
ci: add flags.circleci.timeoutMinutes
HowardBraham Dec 12, 2024
5a28bd5
feat: use new scopes from KeyringAccount
ccharly Dec 13, 2024
98e9e2d
fix: add resolution for keyring-snap-client
ccharly Dec 13, 2024
f013c1c
fix: fix tests (missing scopes)
ccharly Dec 13, 2024
e0e5664
feat: add migration to inject account.scopes
ccharly Dec 13, 2024
00e8194
chore: force migration 135 (hack, needs to be removed)
ccharly Dec 13, 2024
370f06e
chore: add local bitcoin snap (hack, needs to be removed)
ccharly Dec 13, 2024
6070c38
chore: update preview deps
ccharly Jan 6, 2025
409801b
Merge branch 'main' into feat/keyring-account-scopes
ccharly Jan 6, 2025
670e89a
chore: update resolutions
ccharly Jan 6, 2025
59e5bde
chore: lint + dedupe
ccharly Jan 7, 2025
8a91bc6
chore: lavamost:auto
ccharly Jan 7, 2025
57e5b2d
Update LavaMoat policies
metamaskbot Jan 7, 2025
840eef9
fix: missing scopes in test + force type in migration 105
ccharly Jan 7, 2025
1cd78ab
chore: rename account.scopes migration (135 -> 136)
ccharly Jan 7, 2025
c3e5137
Merge branch 'main' into feat/keyring-account-scopes
ccharly Jan 7, 2025
c628911
fix: disable forced migration
ccharly Jan 7, 2025
7b62692
test(user-ops): use [email protected]
ccharly Jan 8, 2025
74aba6a
Revert "chore: add local bitcoin snap (hack, needs to be removed)"
ccharly Jan 8, 2025
4a19134
test(btc): use [email protected]
ccharly Jan 8, 2025
393f6e8
test(btc): update privacy snapshot + mock sat protection service
ccharly Jan 9, 2025
b3d4ae8
test(btc): fix BTC balance mocking when Sat Protection is enabled
ccharly Jan 9, 2025
117e74a
chore: update resolutions
ccharly Jan 9, 2025
77e5f17
test(sol): use [email protected]
ccharly Jan 9, 2025
6918983
chore: lint
ccharly Jan 9, 2025
49a676b
Merge branch 'main' into feat/keyring-account-scopes
ccharly Jan 9, 2025
400d2d3
chore: update privacy-snapshot.json for solana support
ccharly Jan 9, 2025
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
7 changes: 7 additions & 0 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,13 @@ export async function loadStateFromPersistence() {
versionedData =
(await localStore.get()) || migrator.generateInitialState(firstTimeState);

// Force migration 136 to inject scopes on existing accounts:
// versionedData.meta.version = 135;
// console.log(
// 'HACK -- Forcing version to force account.scopes migration, using: ',
// versionedData.meta.version,
// );

// check if somehow state is empty
// this should never happen but new error reporting suggests that it has
// for a small number of users
Expand Down
1 change: 1 addition & 0 deletions app/scripts/controllers/alert-controller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ describe('AlertController', () => {
address: '0x1234567',
options: {},
methods: [],
scopes: ['eip155'],
type: 'eip155:eoa',
metadata: {
name: '',
Expand Down
1 change: 1 addition & 0 deletions app/scripts/lib/snap-keyring/snap-keyring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const mockAccount = {
id: '3afa663e-0600-4d93-868a-61c2e553013b',
address,
methods: [],
scopes: ['eip155'],
options: {},
};
const mockInternalAccount = {
Expand Down
8 changes: 1 addition & 7 deletions app/scripts/migrations/105.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { v4 as uuid } from 'uuid';
import { sha256FromString } from 'ethereumjs-util';
import { InternalAccount } from '@metamask/keyring-internal-api';
import { ETH_EOA_METHODS } from '../../../shared/constants/eth-methods';
import { migrate } from './105';
import type { Identity, InternalAccount } from './105';

const MOCK_ADDRESS = '0x0';
const MOCK_ADDRESS_2 = '0x1';
Expand All @@ -22,12 +22,6 @@ function addressToUUID(address: string): string {
});
}

type Identity = {
name: string;
address: string;
lastSelected?: number;
};

type Identities = {
[key: string]: Identity;
};
Expand Down
20 changes: 18 additions & 2 deletions app/scripts/migrations/105.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,37 @@
import { EthAccountType } from '@metamask/keyring-api';
import { InternalAccount } from '@metamask/keyring-internal-api';
import { sha256FromString } from 'ethereumjs-util';
import { v4 as uuid } from 'uuid';
import { cloneDeep } from 'lodash';
import { Json } from '@metamask/utils';
import { ETH_EOA_METHODS } from '../../../shared/constants/eth-methods';

type VersionedData = {
meta: { version: number };
data: Record<string, unknown>;
};

type Identity = {
export type Identity = {
name: string;
address: string;
lastSelected?: number;
};

export type InternalAccount = {
type: string;
id: string;
options: Record<string, Json>;
metadata: {
name: string;
importTime: number;
keyring: {
type: string;
};
lastSelected?: number;
};
address: string;
methods: string[];
};

export const version = 105;

/**
Expand Down
177 changes: 177 additions & 0 deletions app/scripts/migrations/136.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
import { AccountsControllerState } from '@metamask/accounts-controller';
import { cloneDeep } from 'lodash';
import { createMockInternalAccount } from '../../../test/jest/mocks';
import { migrate, version } from './136';

const sentryCaptureExceptionMock = jest.fn();

global.sentry = {
captureException: sentryCaptureExceptionMock,
};

const oldVersion = 134;

const mockInternalAccount = createMockInternalAccount();
const mockAccountsControllerState: AccountsControllerState = {
internalAccounts: {
accounts: {
[mockInternalAccount.id]: mockInternalAccount,
},
selectedAccount: mockInternalAccount.id,
},
};

describe(`migration #${version}`, () => {
afterEach(() => jest.resetAllMocks());

it('updates the version metadata', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
AccountsController: mockAccountsControllerState,
},
};

const newStorage = await migrate(cloneDeep(oldStorage));
expect(newStorage.meta).toStrictEqual({ version });
});

it('does nothing if all accounts have scopes already', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
AccountsController: mockAccountsControllerState,
},
};

const newStorage = await migrate(cloneDeep(oldStorage));
expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it('does nothing if AccountsController state is missing', async () => {
const oldStorage = {
meta: { version: oldVersion },
data: {
OtherController: {},
},
};

const newStorage = await migrate(cloneDeep(oldStorage));
expect(newStorage.data).toStrictEqual(oldStorage.data);
});

it.only('adds a scopes if it is missing on an account', async () => {
const { scopes: _, ...mockInternalAccountWithoutScopes } =
mockInternalAccount;
const oldStorage = {
meta: { version: oldVersion },
data: {
AccountsController: {
internalAccounts: {
accounts: {
[mockInternalAccountWithoutScopes.id]:
mockInternalAccountWithoutScopes,
},
selectedAccount: mockInternalAccountWithoutScopes.id,
},
},
},
};

const newStorage = await migrate(cloneDeep(oldStorage));
expect(newStorage.data).toStrictEqual({
// This mock state already has the expected scope
AccountsController: mockAccountsControllerState,
});
});

// @ts-expect-error 'each' function missing from type definitions, but it does exist
it.each([
{
label: 'AccountsController type',
message: `Migration ${version}: Invalid AccountsController state of type 'string'`,
state: { AccountsController: 'invalid' },
},
{
label: 'Missing internalAccounts',
message: `Migration ${version}: Invalid AccountsController state, missing internalAccounts`,
state: { AccountsController: {} },
},
{
label: 'Invalid internalAccounts',
message: `Migration ${version}: Invalid AccountsController internalAccounts state of type 'string'`,
state: { AccountsController: { internalAccounts: 'invalid' } },
},
{
label: 'Missing accounts',
message: `Migration ${version}: Invalid AccountsController internalAccounts state, missing accounts`,
state: {
AccountsController: {
internalAccounts: {},
},
},
},
{
label: 'Invalid accounts',
message: `Migration ${version}: Invalid AccountsController internalAccounts.accounts state of type 'string'`,
state: {
AccountsController: {
internalAccounts: { accounts: 'invalid' },
},
},
},
{
label: 'Invalid accounts entry',
message: `Migration ${version}: Invalid AccountsController's account (object) type, is 'string'`,
state: {
AccountsController: {
internalAccounts: {
accounts: { [mockInternalAccount.id]: 'invalid' },
},
},
},
},
{
label: 'Missing type in accounts entry',
message: `Migration ${version}: Invalid AccountsController's account missing 'type' property`,
state: {
AccountsController: {
internalAccounts: {
accounts: { [mockInternalAccount.id]: {} },
},
},
},
},
{
label: 'Invalid type for accounts entry',
message: `Migration ${version}: Invalid AccountsController's account.type type, is 'object'`,
state: {
AccountsController: {
internalAccounts: {
accounts: { [mockInternalAccount.id]: { type: {} } },
},
},
},
},
])(
'captures error when state is invalid due to: $label',
async ({
message,
state,
}: {
message: string;
state: Record<string, unknown>;
}) => {
const oldStorage = {
meta: { version: oldVersion },
data: state,
};

const newStorage = await migrate(cloneDeep(oldStorage));
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
new Error(message),
);
expect(newStorage.data).toStrictEqual(oldStorage.data);
},
);
});
Loading
Loading