Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,9 @@ scripts/coverage
packages/*/*.tsbuildinfo

# AI
.sisyphus/
.sisyphus/

# Wallet
.claude/
.env
!.env.example
2 changes: 2 additions & 0 deletions packages/wallet/.env.example
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
INFURA_PROJECT_KEY=

3 changes: 3 additions & 0 deletions packages/wallet/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ module.exports = merge(baseConfig, {
// The display name when running multiple projects
displayName,

// Load dotenv before tests
setupFiles: [path.resolve(__dirname, 'test/setup.ts')],

// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
Expand Down
8 changes: 8 additions & 0 deletions packages/wallet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,27 @@
"test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch"
},
"dependencies": {
"@metamask/accounts-controller": "^37.2.0",
"@metamask/approval-controller": "^9.0.1",
"@metamask/browser-passworder": "^6.0.0",
"@metamask/connectivity-controller": "^0.2.0",
"@metamask/controller-utils": "^11.20.0",
"@metamask/keyring-controller": "^25.2.0",
"@metamask/messenger": "^1.1.1",
"@metamask/network-controller": "^30.0.1",
"@metamask/remote-feature-flag-controller": "^4.2.0",
"@metamask/scure-bip39": "^2.1.1",
"@metamask/transaction-controller": "^64.0.0",
"@metamask/utils": "^11.11.0"
},
"devDependencies": {
"@metamask/auto-changelog": "^3.4.4",
"@ts-bridge/cli": "^0.6.4",
"@types/jest": "^29.5.14",
"deepmerge": "^4.2.2",
"dotenv": "^16.4.7",
"jest": "^29.7.0",
"nock": "^13.3.1",
"ts-jest": "^29.2.5",
"tsx": "^4.20.5",
"typedoc": "^0.25.13",
Expand Down
54 changes: 45 additions & 9 deletions packages/wallet/src/Wallet.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { enableNetConnect } from 'nock';
import {
ClientConfigApiService,
ClientType,
DistributionType,
EnvironmentType,
} from '@metamask/remote-feature-flag-controller';
import { cleanAll, enableNetConnect } from 'nock';

import { importSecretRecoveryPhrase, sendTransaction } from './utilities';
import { Wallet } from './Wallet';
Expand All @@ -7,12 +13,27 @@ const TEST_PHRASE =
'test test test test test test test test test test test ball';
const TEST_PASSWORD = 'testpass';

async function setupWallet() {
async function setupWallet(): Promise<Wallet> {
if (!process.env.INFURA_PROJECT_KEY) {
throw new Error(
'INFURA_PROJECT_KEY is not set. Copy .env.example to .env and fill in your key.',
);
}

const wallet = new Wallet({
options: {
infuraProjectId: 'infura-project-id',
infuraProjectId: process.env.INFURA_PROJECT_KEY,
clientVersion: '1.0.0',
showApprovalRequest: () => undefined,
showApprovalRequest: (): undefined => undefined,
clientConfigApiService: new ClientConfigApiService({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we need to pass this entire thing in? Maybe just add client, distribution, environment to WalletOptions and we can construct ClientConfigApiService based on that?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think merging the constructor params of ClientConfigApiService into the Wallet constructor params would be more trouble than it's worth. If we ultimately decide that we don't care about all of the options of the former, we can always fold it in later.

fetch: globalThis.fetch,
config: {
client: ClientType.Extension,
distribution: DistributionType.Main,
environment: EnvironmentType.Production,
},
}),
getMetaMetricsId: (): string => 'fake-metrics-id',
},
});

Expand All @@ -22,8 +43,22 @@ async function setupWallet() {
}

describe('Wallet', () => {
let wallet: Wallet;

beforeEach(() => {
jest.useFakeTimers({ doNotFake: ['nextTick', 'queueMicrotask'] });
});

afterEach(async () => {
await wallet?.destroy();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Any concern with just doing wallet.destroy() in each test case for now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As opposed to in afterEach? Why?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally find that more readable but it also provides flexibility in the way that we can pass different arguments to setupWallet in each test case.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer keeping cleanup in the hooks because customizing it between different test cases has a tendency to cause pernicious bugs in the test suite, IME.

cleanAll();
Comment thread
rekmarks marked this conversation as resolved.
Outdated
enableNetConnect();
jest.useRealTimers();
});

it('can unlock and populate accounts', async () => {
const { messenger } = await setupWallet();
wallet = await setupWallet();
const { messenger } = wallet;

expect(
messenger
Expand All @@ -35,7 +70,7 @@ describe('Wallet', () => {
it('signs transactions', async () => {
enableNetConnect();

const wallet = await setupWallet();
wallet = await setupWallet();

const addresses = wallet.messenger
.call('AccountsController:listAccounts')
Expand All @@ -47,7 +82,7 @@ describe('Wallet', () => {
{ networkClientId: 'sepolia' },
);

const hash = await result;
const hash = await jest.advanceTimersByTimeAsync(60_000).then(() => result);
Comment thread
FrederikBolding marked this conversation as resolved.

expect(hash).toStrictEqual(expect.any(String));
expect(transactionMeta).toStrictEqual(
Expand All @@ -61,10 +96,11 @@ describe('Wallet', () => {
}),
}),
);
});
}, 30_000);

it('exposes state', async () => {
const { state } = await setupWallet();
wallet = await setupWallet();
const { state } = wallet;

expect(state.KeyringController).toStrictEqual({
isUnlocked: true,
Expand Down
19 changes: 15 additions & 4 deletions packages/wallet/src/Wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export type WalletConstructorArgs = {
export class Wallet {
public messenger: RootMessenger;

readonly #instances;
readonly #instances: Record<string, Record<string, unknown>>;

constructor({ state = {}, options }: WalletConstructorArgs) {
this.messenger = new Messenger({
Expand All @@ -24,11 +24,22 @@ export class Wallet {

get state(): Record<string, unknown> {
return Object.entries(this.#instances).reduce<Record<string, unknown>>(
(accumulator, [key, instance]) => {
accumulator[key] = instance.state ?? null;
return accumulator;
(totalState, [name, instance]) => {
totalState[name] = instance.state ?? null;
return totalState;
},
{},
);
}

async destroy(): Promise<void> {
await Promise.all(
Object.values(this.#instances).map((instance) => {
if (typeof instance.destroy === 'function') {
return instance.destroy();
}
return undefined;
}),
);
}
}
6 changes: 3 additions & 3 deletions packages/wallet/src/initialization/initialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function initialize({
messenger,
initializationConfigurations = [],
options,
}: InitializeArgs) {
}: InitializeArgs): Record<string, Record<string, unknown>> {
Comment thread
rekmarks marked this conversation as resolved.
Outdated
const overriddenConfiguration = initializationConfigurations.map(
(config) => config.name,
);
Expand All @@ -30,7 +30,7 @@ export function initialize({
),
);

const instances = {};
const instances: Record<string, Record<string, unknown>> = {};

for (const config of configurationEntries) {
const { name } = config;
Expand All @@ -45,7 +45,7 @@ export function initialize({
options,
});

instances[name] = instance;
instances[name] = instance as Record<string, unknown>;
}

return instances;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import type {
DetailedEncryptionResult,
EncryptionKey,
KeyDerivationOptions,
} from '@metamask/browser-passworder';
import {
encrypt,
encryptWithDetail,
Expand All @@ -10,9 +15,8 @@ import {
importKey,
exportKey,
generateSalt,
EncryptionKey,
KeyDerivationOptions,
} from '@metamask/browser-passworder';
import type { Encryptor } from '@metamask/keyring-controller';
import {
KeyringController,
KeyringControllerMessenger,
Expand All @@ -35,7 +39,7 @@ const encryptFactory =
data: unknown,
key?: EncryptionKey | CryptoKey,
salt?: string,
) =>
): Promise<string> =>
encrypt(password, data, key, salt, {
algorithm: 'PBKDF2',
params: {
Expand All @@ -52,7 +56,11 @@ const encryptFactory =
*/
const encryptWithDetailFactory =
(iterations: number) =>
async (password: string, object: unknown, salt?: string) =>
async (
password: string,
object: unknown,
salt?: string,
): Promise<DetailedEncryptionResult> =>
encryptWithDetail(password, object, salt, {
algorithm: 'PBKDF2',
params: {
Expand All @@ -77,7 +85,7 @@ const keyFromPasswordFactory =
salt: string,
exportable?: boolean,
opts?: KeyDerivationOptions,
) =>
): Promise<EncryptionKey> =>
keyFromPassword(
password,
salt,
Expand All @@ -97,13 +105,15 @@ const keyFromPasswordFactory =
* @param iterations - The number of iterations to use for the PBKDF2 algorithm.
* @returns A function that checks if the vault was encrypted with the given number of iterations.
*/
const isVaultUpdatedFactory = (iterations: number) => (vault: string) =>
isVaultUpdated(vault, {
algorithm: 'PBKDF2',
params: {
iterations,
},
});
const isVaultUpdatedFactory =
(iterations: number) =>
(vault: string): boolean =>
isVaultUpdated(vault, {
algorithm: 'PBKDF2',
params: {
iterations,
},
});

/**
* A factory function that returns an encryptor with the given number of iterations.
Expand All @@ -114,7 +124,7 @@ const isVaultUpdatedFactory = (iterations: number) => (vault: string) =>
* @param iterations - The number of iterations to use for the PBKDF2 algorithm.
* @returns An encryptor set with the given number of iterations.
*/
const encryptorFactory = (iterations: number) => ({
const encryptorFactory = (iterations: number): Encryptor => ({
encrypt: encryptFactory(iterations),
encryptWithKey,
encryptWithDetail: encryptWithDetailFactory(iterations),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
MessengerActions,
MessengerEvents,
} from '@metamask/messenger';
import type { NetworkControllerOptions } from '@metamask/network-controller';
import {
NetworkController,
NetworkControllerMessenger,
Expand All @@ -24,36 +25,38 @@ export const networkController: InitializationConfiguration<
name: 'NetworkController',
init: ({ state, messenger, options }) => {
// TODO: This was gutted to simplify implementation for now.
const getRpcServiceOptions = () => {
const maxRetries = DEFAULT_MAX_RETRIES;
const getRpcServiceOptions: NetworkControllerOptions['getRpcServiceOptions'] =
Comment thread
FrederikBolding marked this conversation as resolved.
() => {
const maxRetries = DEFAULT_MAX_RETRIES;

const isOffline = (): boolean => {
const connectivityState = messenger.call(
'ConnectivityController:getState',
);
return (
connectivityState.connectivityStatus === CONNECTIVITY_STATUSES.Offline
);
};
const isOffline = (): boolean => {
const connectivityState = messenger.call(
'ConnectivityController:getState',
);
return (
connectivityState.connectivityStatus ===
CONNECTIVITY_STATUSES.Offline
);
};

return {
fetch: globalThis.fetch.bind(globalThis),
btoa: globalThis.btoa.bind(globalThis),
isOffline,
policyOptions: {
// Ensure that the "cooldown" period after breaking the circuit is short.
circuitBreakDuration: inMilliseconds(30, Duration.Second),
maxRetries,
// Ensure that if the endpoint continually responds with errors, we
// break the circuit relatively fast (but not prematurely).
//
// Note that the circuit will break much faster if the errors are
// retriable (e.g. 503) than if not (e.g. 500), so we attempt to strike
// a balance here.
maxConsecutiveFailures: (maxRetries + 1) * 3,
},
return {
fetch: globalThis.fetch.bind(globalThis),
btoa: globalThis.btoa.bind(globalThis),
isOffline,
policyOptions: {
// Ensure that the "cooldown" period after breaking the circuit is short.
circuitBreakDuration: inMilliseconds(30, Duration.Second),
maxRetries,
// Ensure that if the endpoint continually responds with errors, we
// break the circuit relatively fast (but not prematurely).
//
// Note that the circuit will break much faster if the errors are
// retriable (e.g. 503) than if not (e.g. 500), so we attempt to strike
// a balance here.
maxConsecutiveFailures: (maxRetries + 1) * 3,
},
};
};
};

// TODO: Add the rest of the arguments.
const instance = new NetworkController({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export const remoteFeatureFlagController: InitializationConfiguration<
state,
messenger,
clientVersion: options.clientVersion,
clientConfigApiService: options.clientConfigApiService,
getMetaMetricsId: options.getMetaMetricsId,
});

return {
Expand Down
Loading
Loading