Skip to content

Commit b617ba9

Browse files
authored
feat(keyring-snap-bridge): add SnapKeyringV1Adapter (#557)
A more specialized adapter for the `SnapKeyring` v2 instances. For recall, each v2 instances are being wrapped in a v1 equivalent adapter since the `KeyringController` requires it. We now have a more specific one mostly to address a different implementation of `removeAccount` (it always had been `async` even in the legacy Snap keyring) AND [the `KeyringController` already `await` for this call](https://github.com/MetaMask/core/blob/5e36b5f8f733177d120815d0c2f8819007a159da/packages/keyring-controller/src/KeyringController.ts#L1353-L1360). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, focused adapter with tests; account removal behavior is intentional compatibility with KeyringController, not new crypto or auth logic. > > **Overview** > Introduces **`SnapKeyringV1Adapter`**, a thin wrapper around v2 **`SnapKeyring`** so it can be used where **`KeyringController`** still expects the legacy v1 keyring surface. > > Most v1 behavior (including EVM signing) comes from **`EthKeyringV1Adapter`** in `@metamask/keyring-sdk`. This PR adds Snap-specific **`removeAccount(address)`**: resolve the account by address, then **`deleteAccount`** by id—matching the legacy Snap keyring’s async removal flow that the controller already awaits. > > The adapter is **exported** from `@metamask/keyring-snap-bridge/v2`, documented in the package changelog, and covered by unit tests for generic v1 adapter behavior and account removal errors. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7c29631. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 10ce1e6 commit b617ba9

4 files changed

Lines changed: 216 additions & 0 deletions

File tree

packages/keyring-snap-bridge/CHANGELOG.md

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

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add `SnapKeyringV1Adapter` to adapt Snap v2 keyrings to legacy v1 keyring operations ([#557](https://github.com/MetaMask/accounts/pull/557))
13+
- This adapter mostly exposes EVM signing operations through `EthKeyringV1Adapter`.
14+
- This adapter also implements `removeAccount` the way it was implemented by the legacy Snap keyring (compatible with the `KeyringController` account removal flow).
15+
1016
### Changed
1117

1218
- Normalize `KeyringAccount`'s address with `:accountCreated` and `setAccounts` ([#556](https://github.com/MetaMask/accounts/pull/556))
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
import type { KeyringAccount } from '@metamask/keyring-api';
2+
import { EthAccountType, EthScope } from '@metamask/keyring-api';
3+
import { KeyringType } from '@metamask/keyring-api/v2';
4+
import { KeyringV1Adapter } from '@metamask/keyring-sdk/v2';
5+
import type { AccountId } from '@metamask/keyring-utils';
6+
import type { SnapId } from '@metamask/snaps-sdk';
7+
8+
import type { SnapKeyringMessenger } from '../SnapKeyringMessenger';
9+
import { SnapKeyring } from './SnapKeyring';
10+
import type { SnapKeyringCallbacks, SnapKeyringState } from './SnapKeyring';
11+
import { SnapKeyringV1Adapter } from './SnapKeyringV1Adapter';
12+
13+
const ACCOUNT_ID = 'f2b88e0e-82a4-4e93-8c60-4fe59c6892d7';
14+
const ACCOUNT_ADDRESS = '0xdeadbeef00000000000000000000000000000000';
15+
const OTHER_ACCOUNT_ID = '978b6a3d-ce0f-4fc9-9746-70ea61fa714f';
16+
const OTHER_ACCOUNT_ADDRESS = '0xcafecafe00000000000000000000000000000000';
17+
const SNAP_ID = 'local:snap.mock' as SnapId;
18+
19+
type SetupOptions = {
20+
accounts?: KeyringAccount[];
21+
};
22+
23+
function buildAccount({
24+
id = ACCOUNT_ID,
25+
address = ACCOUNT_ADDRESS,
26+
}: {
27+
id?: AccountId;
28+
address?: string;
29+
} = {}): KeyringAccount {
30+
return {
31+
id,
32+
type: EthAccountType.Eoa,
33+
address,
34+
scopes: [EthScope.Eoa],
35+
options: {},
36+
methods: [],
37+
};
38+
}
39+
40+
type SetupResult = {
41+
adapter: SnapKeyringV1Adapter;
42+
inner: SnapKeyring;
43+
mocks: {
44+
deleteAccount: jest.SpyInstance<
45+
ReturnType<SnapKeyring['deleteAccount']>,
46+
Parameters<SnapKeyring['deleteAccount']>
47+
>;
48+
deserialize: jest.SpyInstance<
49+
ReturnType<SnapKeyring['deserialize']>,
50+
Parameters<SnapKeyring['deserialize']>
51+
>;
52+
lookupByAddress: jest.SpyInstance<
53+
ReturnType<SnapKeyring['lookupByAddress']>,
54+
Parameters<SnapKeyring['lookupByAddress']>
55+
>;
56+
};
57+
};
58+
59+
async function setup({
60+
accounts = [buildAccount()],
61+
}: SetupOptions = {}): Promise<SetupResult> {
62+
const state: SnapKeyringState = {
63+
snapId: SNAP_ID,
64+
accounts: Object.fromEntries(
65+
accounts.map((account) => [account.id, account]),
66+
),
67+
};
68+
69+
const inner = new SnapKeyring({
70+
callbacks: buildCallbacks(),
71+
messenger: buildMessenger(),
72+
});
73+
await inner.deserialize(state);
74+
75+
const deleteAccount = jest
76+
.spyOn(inner, 'deleteAccount')
77+
.mockResolvedValue(undefined);
78+
const deserialize = jest.spyOn(inner, 'deserialize');
79+
const lookupByAddress = jest.spyOn(inner, 'lookupByAddress');
80+
81+
return {
82+
adapter: new SnapKeyringV1Adapter(inner),
83+
inner,
84+
mocks: {
85+
deleteAccount,
86+
deserialize,
87+
lookupByAddress,
88+
},
89+
};
90+
}
91+
92+
function buildCallbacks(): SnapKeyringCallbacks {
93+
return {
94+
addAccount: jest
95+
.fn<
96+
ReturnType<SnapKeyringCallbacks['addAccount']>,
97+
Parameters<SnapKeyringCallbacks['addAccount']>
98+
>()
99+
.mockResolvedValue(undefined),
100+
removeAccount: jest
101+
.fn<
102+
ReturnType<SnapKeyringCallbacks['removeAccount']>,
103+
Parameters<SnapKeyringCallbacks['removeAccount']>
104+
>()
105+
.mockResolvedValue(undefined),
106+
saveState: jest
107+
.fn<
108+
ReturnType<SnapKeyringCallbacks['saveState']>,
109+
Parameters<SnapKeyringCallbacks['saveState']>
110+
>()
111+
.mockResolvedValue(undefined),
112+
redirectUser: jest
113+
.fn<
114+
ReturnType<SnapKeyringCallbacks['redirectUser']>,
115+
Parameters<SnapKeyringCallbacks['redirectUser']>
116+
>()
117+
.mockResolvedValue(undefined),
118+
assertAccountCanBeUsed: jest
119+
.fn<
120+
ReturnType<SnapKeyringCallbacks['assertAccountCanBeUsed']>,
121+
Parameters<SnapKeyringCallbacks['assertAccountCanBeUsed']>
122+
>()
123+
.mockResolvedValue(undefined),
124+
};
125+
}
126+
127+
function buildMessenger(): SnapKeyringMessenger {
128+
return {
129+
call: jest.fn(),
130+
publish: jest.fn(),
131+
} as unknown as SnapKeyringMessenger;
132+
}
133+
134+
describe('SnapKeyringV1Adapter', () => {
135+
it('inherits generic v1 adapter behavior', async () => {
136+
const account = buildAccount();
137+
const otherAccount = buildAccount({
138+
id: OTHER_ACCOUNT_ID,
139+
address: OTHER_ACCOUNT_ADDRESS,
140+
});
141+
const { adapter, inner, mocks } = await setup({
142+
accounts: [account, otherAccount],
143+
});
144+
const state: SnapKeyringState = {
145+
snapId: SNAP_ID,
146+
accounts: {},
147+
};
148+
149+
expect(adapter).toBeInstanceOf(KeyringV1Adapter);
150+
expect(adapter.type).toBe(KeyringType.Snap);
151+
expect(adapter.unwrap()).toBe(inner);
152+
expect(await adapter.getAccounts()).toStrictEqual([
153+
ACCOUNT_ADDRESS,
154+
OTHER_ACCOUNT_ADDRESS,
155+
]);
156+
expect(await adapter.serialize()).toStrictEqual({
157+
snapId: SNAP_ID,
158+
accounts: {
159+
[ACCOUNT_ID]: account,
160+
[OTHER_ACCOUNT_ID]: otherAccount,
161+
},
162+
});
163+
164+
await adapter.deserialize(state);
165+
166+
expect(mocks.deserialize).toHaveBeenCalledWith(state);
167+
});
168+
169+
it('removes an account by resolving its address and deleting its account ID', async () => {
170+
const { adapter, mocks } = await setup();
171+
172+
await adapter.removeAccount(ACCOUNT_ADDRESS);
173+
174+
expect(mocks.lookupByAddress).toHaveBeenCalledWith(ACCOUNT_ADDRESS);
175+
expect(mocks.deleteAccount).toHaveBeenCalledWith(ACCOUNT_ID);
176+
});
177+
178+
it('throws if no account matches the address', async () => {
179+
const { adapter, mocks } = await setup({ accounts: [] });
180+
181+
await expect(adapter.removeAccount(ACCOUNT_ADDRESS)).rejects.toThrow(
182+
`Account '${ACCOUNT_ADDRESS}' not found`,
183+
);
184+
expect(mocks.deleteAccount).not.toHaveBeenCalled();
185+
});
186+
});
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { EthKeyringV1Adapter } from '@metamask/keyring-sdk/v2';
2+
3+
import type { SnapKeyring } from './SnapKeyring';
4+
5+
/**
6+
* Adapts a v2 Snap keyring to the legacy v1 keyring API.
7+
*/
8+
export class SnapKeyringV1Adapter extends EthKeyringV1Adapter<SnapKeyring> {
9+
/**
10+
* Remove an account matching the given address.
11+
*
12+
* @param address - Address of the account to remove.
13+
*/
14+
async removeAccount(address: string): Promise<void> {
15+
const account = this.inner.lookupByAddress(address);
16+
17+
if (!account) {
18+
throw new Error(`Account '${address}' not found`);
19+
}
20+
21+
await this.inner.deleteAccount(account.id);
22+
}
23+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
export * from './SnapKeyring';
2+
export * from './SnapKeyringV1Adapter';

0 commit comments

Comments
 (0)