Skip to content

Commit deb171f

Browse files
authored
refactor(keyring-internal-api)!: remove snap.{enabled,name} (#525)
Those metadata are duplicated from the `SnapController` data. We should instead be using this controller to query those, thus, making the controller the only source of truth and avoid `:stateChange` handling on the `AccountsController` too! <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Breaking change to `InternalAccount` metadata shape: code consuming `metadata.snap.name`/`metadata.snap.enabled` will fail until updated to query `SnapController:getSnap` instead. Runtime logic change is small but impacts multiple packages and downstream integrations expecting those fields. > > **Overview** > **BREAKING:** Removes `metadata.snap.name` and `metadata.snap.enabled` from `InternalAccount`, leaving only `metadata.snap.id` and updating validation accordingly. > > `SnapKeyring` now only attaches `{ id }` as snap metadata when transforming accounts, and tests/changelogs are updated to reflect the new source-of-truth being `SnapController:getSnap` for snap name/enabled state. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5a0838b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e481183 commit deb171f

6 files changed

Lines changed: 39 additions & 40 deletions

File tree

packages/keyring-internal-api/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Removed
11+
12+
- **BREAKING:** Removed `InternalAccount.metadata.snap.{enabled,name}` ([#525](https://github.com/MetaMask/accounts/pull/525))
13+
- Use `SnapController:getSnap` to get those information instead (in combination with `InternalAccount.metadata.snap.id`).
14+
1015
## [10.1.1]
1116

1217
### Changed

packages/keyring-internal-api/src/types.test.ts

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -143,45 +143,40 @@ describe('InternalAccount', () => {
143143
importTime: 1713153716,
144144
snap: {
145145
id: 'test-snap',
146-
enabled: true,
147-
name: 'Test Snap',
148146
},
149147
},
150148
};
151149

152150
expect(() => assert(account, InternalAccountStruct)).not.toThrow();
153151
});
154152

155-
it.each([['name', 'enabled', 'id']])(
156-
'should throw if snap.%s is not set',
157-
(key: string) => {
158-
const account: InternalAccount = {
159-
id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb',
160-
address: '0x000',
161-
options: {},
162-
methods: [],
163-
scopes: ['eip155:0'],
164-
type: 'eip155:eoa',
165-
metadata: {
166-
keyring: {
167-
type: 'Test Keyring',
168-
},
169-
name: 'Account 1',
170-
importTime: 1713153716,
171-
snap: {
172-
id: 'test-snap',
173-
enabled: true,
174-
name: 'Test Snap',
175-
},
153+
it('should throw if snap.id is not set', () => {
154+
// NOTE: We do not force `InternalAccount` here to make `snap.id` optional.
155+
const account = {
156+
id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb',
157+
address: '0x000',
158+
options: {},
159+
methods: [],
160+
scopes: ['eip155:0'],
161+
type: 'eip155:eoa',
162+
metadata: {
163+
keyring: {
164+
type: 'Test Keyring',
176165
},
177-
};
178-
179-
// On `InternalAccount` the `metadata.snap` is optional, hence the `?.` here.
180-
delete account.metadata.snap?.[key as keyof typeof account.metadata.snap];
166+
name: 'Account 1',
167+
importTime: 1713153716,
168+
snap: {
169+
id: 'test-snap',
170+
} as {
171+
id?: string;
172+
},
173+
},
174+
};
181175

182-
const regex = new RegExp(`At path: metadata.snap.${key}`, 'u');
176+
delete account.metadata.snap.id;
183177

184-
expect(() => assert(account, InternalAccountStruct)).toThrow(regex);
185-
},
186-
);
178+
expect(() => assert(account, InternalAccountStruct)).toThrow(
179+
'At path: metadata.snap.id -- Expected a string, but received: undefined',
180+
);
181+
});
187182
});

packages/keyring-internal-api/src/types.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
} from '@metamask/keyring-api';
2121
import { exactOptional, object } from '@metamask/keyring-utils';
2222
import type { Infer, Struct } from '@metamask/superstruct';
23-
import { boolean, string, number } from '@metamask/superstruct';
23+
import { string, number } from '@metamask/superstruct';
2424

2525
export type InternalAccountType =
2626
| EthAccountType
@@ -36,8 +36,6 @@ export const InternalAccountMetadataStruct = object({
3636
snap: exactOptional(
3737
object({
3838
id: string(),
39-
enabled: boolean(),
40-
name: string(),
4139
}),
4240
),
4341
lastSelected: exactOptional(number()),

packages/keyring-snap-bridge/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- **BREAKING:** No longer use `snapId` as constructor parameter for `SnapKeyring` (v2) ([#519](https://github.com/MetaMask/accounts/pull/519))
1313
- The `snapId` is now passed and bound to the keyring upon the first `deserialize` call, to better integrate with `KeyringController` keyrings lifecyle and keyring builders.
1414

15+
### Removed
16+
17+
- **BREAKING:** Removed `snap.{name,enabled}` from `InternalAccount` metadata produced by `SnapKeyring` ([#525](https://github.com/MetaMask/accounts/pull/525))
18+
- Use `SnapController:getSnap` to get those information instead (in combination with `InternalAccount.metadata.snap.id`).
19+
1520
## [21.0.1]
1621

1722
### Changed

packages/keyring-snap-bridge/src/SnapKeyring.test.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2380,8 +2380,6 @@ describe('SnapKeyring', () => {
23802380
it('returns the list of accounts', async () => {
23812381
const snapMetadata = {
23822382
id: snapId,
2383-
name: 'Snap Name',
2384-
enabled: true,
23852383
};
23862384
const snapObject = {
23872385
id: snapId,
@@ -2437,7 +2435,7 @@ describe('SnapKeyring', () => {
24372435
metadata: {
24382436
name: '',
24392437
importTime: 0,
2440-
snap: { id: snapId, name: 'snap-name', enabled: true },
2438+
snap: { id: snapId },
24412439
keyring: { type: 'Snap Keyring' },
24422440
},
24432441
},

packages/keyring-snap-bridge/src/SnapKeyring.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,9 +779,7 @@ export class SnapKeyring {
779779
snapId: SnapId,
780780
): InternalAccount['metadata']['snap'] | undefined {
781781
const snap = this.#getSnap(snapId);
782-
return snap
783-
? { id: snapId, name: snap.manifest.proposedName, enabled: snap.enabled }
784-
: undefined;
782+
return snap ? { id: snapId } : undefined;
785783
}
786784

787785
#transformToInternalAccount(

0 commit comments

Comments
 (0)