Skip to content

Commit c6e88bd

Browse files
rekmarksclaudeFrederikBolding
committed
fix(wallet): Fix builds and most lint issues (#8420)
Builds and tests pass. All lint issues are fixed except a handful that are deferred pending future changes. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Moderate risk due to new controller dependencies and changes to initialization wiring/typing that could affect runtime messaging and controller lifecycle. Test behavior now depends on external `INFURA_PROJECT_KEY` and mocked timers, which may introduce CI/environment sensitivity. > > **Overview** > **Stabilizes wallet builds/tests by wiring in missing controllers and config.** The wallet package now depends on additional controllers (accounts/approval/connectivity/network/remote feature flags/transaction) and updates TS project references accordingly. > > **Improves runtime/test ergonomics.** Jest loads a local `.env` (with `.env.example` added and `.env` gitignored), `Wallet` exposes stronger typed `messenger`/`state` and adds `destroy()` to clean up controller instances; tests are updated to require `INFURA_PROJECT_KEY`, use fake timers, and properly teardown the wallet. > > **Tightens initialization typing and controller wiring.** Adds `initialization/defaults.ts` for inferred `DefaultInstances`/`DefaultActions`/`DefaultEvents`, introduces `bindMessengerAction` to preserve action typings, and updates controller initializers (notably `TransactionController` and `RemoteFeatureFlagController`) to pass required options and bind messenger actions safely. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit a652933. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Frederik Bolding <frederik.bolding@gmail.com>
1 parent 98b94ca commit c6e88bd

20 files changed

+371
-120
lines changed

.gitignore

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,9 @@ scripts/coverage
3737
packages/*/*.tsbuildinfo
3838

3939
# AI
40-
.sisyphus/
40+
.sisyphus/
41+
42+
# Wallet
43+
.claude/
44+
.env
45+
!.env.example

packages/wallet/.env.example

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
INFURA_PROJECT_KEY=
2+

packages/wallet/jest.config.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ module.exports = merge(baseConfig, {
1414
// The display name when running multiple projects
1515
displayName,
1616

17+
// Load dotenv before tests
18+
setupFiles: [path.resolve(__dirname, 'test/setup.ts')],
19+
1720
// An object that configures minimum threshold enforcement for coverage results
1821
coverageThreshold: {
1922
global: {

packages/wallet/package.json

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,27 @@
4949
"test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch"
5050
},
5151
"dependencies": {
52+
"@metamask/accounts-controller": "^37.2.0",
53+
"@metamask/approval-controller": "^9.0.1",
5254
"@metamask/browser-passworder": "^6.0.0",
55+
"@metamask/connectivity-controller": "^0.2.0",
5356
"@metamask/controller-utils": "^11.20.0",
5457
"@metamask/keyring-controller": "^25.2.0",
5558
"@metamask/messenger": "^1.1.1",
59+
"@metamask/network-controller": "^30.0.1",
60+
"@metamask/remote-feature-flag-controller": "^4.2.0",
5661
"@metamask/scure-bip39": "^2.1.1",
57-
"@metamask/utils": "^11.11.0"
62+
"@metamask/transaction-controller": "^64.0.0",
63+
"@metamask/utils": "^11.9.0"
5864
},
5965
"devDependencies": {
6066
"@metamask/auto-changelog": "^3.4.4",
6167
"@ts-bridge/cli": "^0.6.4",
6268
"@types/jest": "^29.5.14",
6369
"deepmerge": "^4.2.2",
70+
"dotenv": "^16.4.7",
6471
"jest": "^29.7.0",
72+
"nock": "^13.3.1",
6573
"ts-jest": "^29.2.5",
6674
"tsx": "^4.20.5",
6775
"typedoc": "^0.25.13",

packages/wallet/src/Wallet.test.ts

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
import {
2+
ClientConfigApiService,
3+
ClientType,
4+
DistributionType,
5+
EnvironmentType,
6+
} from '@metamask/remote-feature-flag-controller';
17
import { enableNetConnect } from 'nock';
28

39
import { importSecretRecoveryPhrase, sendTransaction } from './utilities';
@@ -7,12 +13,27 @@ const TEST_PHRASE =
713
'test test test test test test test test test test test ball';
814
const TEST_PASSWORD = 'testpass';
915

10-
async function setupWallet() {
16+
async function setupWallet(): Promise<Wallet> {
17+
if (!process.env.INFURA_PROJECT_KEY) {
18+
throw new Error(
19+
'INFURA_PROJECT_KEY is not set. Copy .env.example to .env and fill in your key.',
20+
);
21+
}
22+
1123
const wallet = new Wallet({
1224
options: {
13-
infuraProjectId: 'infura-project-id',
25+
infuraProjectId: process.env.INFURA_PROJECT_KEY,
1426
clientVersion: '1.0.0',
15-
showApprovalRequest: () => undefined,
27+
showApprovalRequest: (): undefined => undefined,
28+
clientConfigApiService: new ClientConfigApiService({
29+
fetch: globalThis.fetch,
30+
config: {
31+
client: ClientType.Extension,
32+
distribution: DistributionType.Main,
33+
environment: EnvironmentType.Production,
34+
},
35+
}),
36+
getMetaMetricsId: (): string => 'fake-metrics-id',
1637
},
1738
});
1839

@@ -22,8 +43,21 @@ async function setupWallet() {
2243
}
2344

2445
describe('Wallet', () => {
46+
let wallet: Wallet;
47+
48+
beforeEach(() => {
49+
jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'] });
50+
});
51+
52+
afterEach(async () => {
53+
await wallet?.destroy();
54+
enableNetConnect();
55+
jest.useRealTimers();
56+
});
57+
2558
it('can unlock and populate accounts', async () => {
26-
const { messenger } = await setupWallet();
59+
wallet = await setupWallet();
60+
const { messenger } = wallet;
2761

2862
expect(
2963
messenger
@@ -35,7 +69,7 @@ describe('Wallet', () => {
3569
it('signs transactions', async () => {
3670
enableNetConnect();
3771

38-
const wallet = await setupWallet();
72+
wallet = await setupWallet();
3973

4074
const addresses = wallet.messenger
4175
.call('AccountsController:listAccounts')
@@ -47,7 +81,8 @@ describe('Wallet', () => {
4781
{ networkClientId: 'sepolia' },
4882
);
4983

50-
const hash = await result;
84+
// Advance timers by an arbitrary value to trigger downstream timer logic.
85+
const hash = await jest.advanceTimersByTimeAsync(60_000).then(() => result);
5186

5287
expect(hash).toStrictEqual(expect.any(String));
5388
expect(transactionMeta).toStrictEqual(
@@ -61,10 +96,11 @@ describe('Wallet', () => {
6196
}),
6297
}),
6398
);
64-
});
99+
}, 10_000);
65100

66101
it('exposes state', async () => {
67-
const { state } = await setupWallet();
102+
wallet = await setupWallet();
103+
const { state } = wallet;
68104

69105
expect(state.KeyringController).toStrictEqual({
70106
isUnlocked: true,

packages/wallet/src/Wallet.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,26 @@
11
import { Messenger } from '@metamask/messenger';
22
import type { Json } from '@metamask/utils';
33

4+
import type {
5+
DefaultActions,
6+
DefaultEvents,
7+
DefaultInstances,
8+
DefaultState,
9+
RootMessenger,
10+
} from './initialization';
411
import { initialize } from './initialization';
5-
import { RootMessenger, WalletOptions } from './types';
12+
import type { WalletOptions } from './types';
613

714
export type WalletConstructorArgs = {
815
state?: Record<string, Json>;
916
options: WalletOptions;
1017
};
1118

1219
export class Wallet {
13-
public messenger: RootMessenger;
20+
// TODO: Expand types when passing additionalConfigurations.
21+
public readonly messenger: RootMessenger<DefaultActions, DefaultEvents>;
1422

15-
readonly #instances;
23+
readonly #instances: DefaultInstances;
1624

1725
constructor({ state = {}, options }: WalletConstructorArgs) {
1826
this.messenger = new Messenger({
@@ -22,13 +30,26 @@ export class Wallet {
2230
this.#instances = initialize({ state, messenger: this.messenger, options });
2331
}
2432

25-
get state(): Record<string, unknown> {
33+
get state(): DefaultState {
2634
return Object.entries(this.#instances).reduce<Record<string, unknown>>(
27-
(accumulator, [key, instance]) => {
28-
accumulator[key] = instance.state ?? null;
29-
return accumulator;
35+
(totalState, [name, instance]) => {
36+
totalState[name] = instance.state ?? null;
37+
return totalState;
3038
},
3139
{},
40+
) as DefaultState;
41+
}
42+
43+
async destroy(): Promise<void> {
44+
await Promise.all(
45+
Object.values(this.#instances).map((instance) => {
46+
// @ts-expect-error Accessing protected property.
47+
if (typeof instance.destroy === 'function') {
48+
// @ts-expect-error Accessing protected property.
49+
return instance.destroy();
50+
}
51+
return undefined;
52+
}),
3253
);
3354
}
3455
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import type {
2+
ActionConstraint,
3+
EventConstraint,
4+
Messenger,
5+
MessengerActions,
6+
MessengerEvents,
7+
} from '@metamask/messenger';
8+
9+
import * as defaultConfigurations from './instances';
10+
import type { InitializationConfiguration, InstanceState } from './types';
11+
12+
export { defaultConfigurations };
13+
14+
type ExtractInstance<Config> =
15+
Config extends InitializationConfiguration<infer Instance, infer _>
16+
? Instance
17+
: never;
18+
19+
type ExtractInstanceMessenger<Config> =
20+
Config extends InitializationConfiguration<infer _, infer InferredMessenger>
21+
? InferredMessenger
22+
: never;
23+
24+
type ExtractName<Config> =
25+
ExtractInstance<Config> extends { name: infer Name extends string }
26+
? Name
27+
: never;
28+
29+
type Configs = typeof defaultConfigurations;
30+
31+
type AllMessengers = ExtractInstanceMessenger<Configs[keyof Configs]>;
32+
33+
export type DefaultInstances = {
34+
[Key in keyof Configs as ExtractName<Configs[Key]>]: ExtractInstance<
35+
Configs[Key]
36+
>;
37+
};
38+
39+
export type DefaultActions = MessengerActions<AllMessengers>;
40+
41+
export type DefaultEvents = MessengerEvents<AllMessengers>;
42+
43+
export type RootMessenger<
44+
AllowedActions extends ActionConstraint = ActionConstraint,
45+
AllowedEvents extends EventConstraint = EventConstraint,
46+
> = Messenger<'Root', AllowedActions, AllowedEvents>;
47+
48+
export type DefaultState = {
49+
[Key in keyof DefaultInstances]: InstanceState<DefaultInstances[Key]>;
50+
};
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,8 @@
1+
export type {
2+
DefaultActions,
3+
DefaultEvents,
4+
DefaultInstances,
5+
DefaultState,
6+
RootMessenger,
7+
} from './defaults';
18
export { initialize } from './initialization';

packages/wallet/src/initialization/initialization.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { Json } from '@metamask/utils';
22

3-
import * as defaultConfigurations from './instances';
3+
import type { DefaultInstances } from './defaults';
4+
import { defaultConfigurations, RootMessenger } from './defaults';
45
import { InitializationConfiguration } from './types';
5-
import { RootMessenger, WalletOptions } from '../types';
6+
import { WalletOptions } from '../types';
67

78
export type InitializeArgs = {
89
state: Record<string, Json>;
@@ -19,7 +20,7 @@ export function initialize({
1920
messenger,
2021
initializationConfigurations = [],
2122
options,
22-
}: InitializeArgs) {
23+
}: InitializeArgs): DefaultInstances {
2324
const overriddenConfiguration = initializationConfigurations.map(
2425
(config) => config.name,
2526
);
@@ -30,7 +31,7 @@ export function initialize({
3031
),
3132
);
3233

33-
const instances = {};
34+
const instances: Record<string, unknown> = {};
3435

3536
for (const config of configurationEntries) {
3637
const { name } = config;
@@ -45,8 +46,8 @@ export function initialize({
4546
options,
4647
});
4748

48-
instances[name] = instance;
49+
instances[name] = instance as Record<string, unknown>;
4950
}
5051

51-
return instances;
52+
return instances as DefaultInstances;
5253
}

packages/wallet/src/initialization/instances/keyring-controller.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
import type {
2+
DetailedEncryptionResult,
3+
EncryptionKey,
4+
KeyDerivationOptions,
5+
} from '@metamask/browser-passworder';
16
import {
27
encrypt,
38
encryptWithDetail,
@@ -10,9 +15,8 @@ import {
1015
importKey,
1116
exportKey,
1217
generateSalt,
13-
EncryptionKey,
14-
KeyDerivationOptions,
1518
} from '@metamask/browser-passworder';
19+
import type { Encryptor } from '@metamask/keyring-controller';
1620
import {
1721
KeyringController,
1822
KeyringControllerMessenger,
@@ -35,7 +39,7 @@ const encryptFactory =
3539
data: unknown,
3640
key?: EncryptionKey | CryptoKey,
3741
salt?: string,
38-
) =>
42+
): Promise<string> =>
3943
encrypt(password, data, key, salt, {
4044
algorithm: 'PBKDF2',
4145
params: {
@@ -52,7 +56,11 @@ const encryptFactory =
5256
*/
5357
const encryptWithDetailFactory =
5458
(iterations: number) =>
55-
async (password: string, object: unknown, salt?: string) =>
59+
async (
60+
password: string,
61+
object: unknown,
62+
salt?: string,
63+
): Promise<DetailedEncryptionResult> =>
5664
encryptWithDetail(password, object, salt, {
5765
algorithm: 'PBKDF2',
5866
params: {
@@ -77,7 +85,7 @@ const keyFromPasswordFactory =
7785
salt: string,
7886
exportable?: boolean,
7987
opts?: KeyDerivationOptions,
80-
) =>
88+
): Promise<EncryptionKey> =>
8189
keyFromPassword(
8290
password,
8391
salt,
@@ -97,13 +105,15 @@ const keyFromPasswordFactory =
97105
* @param iterations - The number of iterations to use for the PBKDF2 algorithm.
98106
* @returns A function that checks if the vault was encrypted with the given number of iterations.
99107
*/
100-
const isVaultUpdatedFactory = (iterations: number) => (vault: string) =>
101-
isVaultUpdated(vault, {
102-
algorithm: 'PBKDF2',
103-
params: {
104-
iterations,
105-
},
106-
});
108+
const isVaultUpdatedFactory =
109+
(iterations: number) =>
110+
(vault: string): boolean =>
111+
isVaultUpdated(vault, {
112+
algorithm: 'PBKDF2',
113+
params: {
114+
iterations,
115+
},
116+
});
107117

108118
/**
109119
* A factory function that returns an encryptor with the given number of iterations.
@@ -114,7 +124,7 @@ const isVaultUpdatedFactory = (iterations: number) => (vault: string) =>
114124
* @param iterations - The number of iterations to use for the PBKDF2 algorithm.
115125
* @returns An encryptor set with the given number of iterations.
116126
*/
117-
const encryptorFactory = (iterations: number) => ({
127+
const encryptorFactory = (iterations: number): Encryptor => ({
118128
encrypt: encryptFactory(iterations),
119129
encryptWithKey,
120130
encryptWithDetail: encryptWithDetailFactory(iterations),

0 commit comments

Comments
 (0)