Skip to content

Commit

Permalink
feat: Allow overwriting built-in keyring builders
Browse files Browse the repository at this point in the history
The KeyringController comes with two built-in keyrings: Simple and HD.
Unfortunately it's impossible to overwrite these because when we build
a new keyring, we look for the first keyring builder in the list that
matches the type we want to build, and the built-in keyrings are always
first.

The order has been switched so that built-in keyrings come second,
after custom keyring builders. This allows them to be overwritten.

This makes it possible to test behavior that is specific to simple or
HD keyrings. This is something that I wanted to do in the mobile
repository.
  • Loading branch information
Gudahtt committed Jun 3, 2024
1 parent 076a657 commit 7d3db4e
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
45 changes: 45 additions & 0 deletions packages/keyring-controller/src/KeyringController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SignTypedDataVersion,
encrypt,
} from '@metamask/eth-sig-util';
import SimpleKeyring from '@metamask/eth-simple-keyring/dist/simple-keyring';
import type { EthKeyring } from '@metamask/keyring-api';
import { wordlist } from '@metamask/scure-bip39/dist/wordlists/english';
import type { KeyringClass } from '@metamask/utils';
Expand Down Expand Up @@ -100,6 +101,32 @@ describe('KeyringController', () => {
}),
).toThrow(KeyringControllerError.UnsupportedEncryptionKeyExport);
});

it('allows overwriting the built-in Simple keyring builder', async () => {
const mockSimpleKeyringBuilder =
// @ts-expect-error The simple keyring doesn't yet conform to the KeyringClass type
buildKeyringBuilderWithSpy(SimpleKeyring);
await withController(
{ keyringBuilders: [mockSimpleKeyringBuilder] },
async ({ controller }) => {
await controller.addNewKeyring(KeyringTypes.simple);

expect(mockSimpleKeyringBuilder).toHaveBeenCalledTimes(1);
},
);
});

it('allows overwriting the built-in HD keyring builder', async () => {
const mockHdKeyringBuilder = buildKeyringBuilderWithSpy(HDKeyring);
await withController(
{ keyringBuilders: [mockHdKeyringBuilder] },
async () => {
// This is called as part of initializing the controller
// because the first keyring is assumed to always be an HD keyring
expect(mockHdKeyringBuilder).toHaveBeenCalledTimes(1);
},
);
});
});

describe('addNewAccount', () => {
Expand Down Expand Up @@ -3538,3 +3565,21 @@ async function withController<ReturnValue>(
messenger,
});
}

/**
* Construct a keyring builder with a spy.
*
* @param KeyringConstructor - The constructor to use for building the keyring.
* @returns A keyring builder that uses `jest.fn()` to spy on invocations.
*/
function buildKeyringBuilderWithSpy(KeyringConstructor: KeyringClass<Json>): {
(): EthKeyring<Json>;
type: string;
} {
const keyringBuilderWithSpy: { (): EthKeyring<Json>; type?: string } = jest
.fn()
.mockImplementation((...args) => new KeyringConstructor(...args));
keyringBuilderWithSpy.type = KeyringConstructor.type;
// Not sure why TypeScript isn't smart enough to infer that `type` is set here.
return keyringBuilderWithSpy as { (): EthKeyring<Json>; type: string };
}
2 changes: 1 addition & 1 deletion packages/keyring-controller/src/KeyringController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ export class KeyringController extends BaseController<
});

this.#keyringBuilders = keyringBuilders
? defaultKeyringBuilders.concat(keyringBuilders)
? keyringBuilders.concat(defaultKeyringBuilders)
: defaultKeyringBuilders;

this.#encryptor = encryptor;
Expand Down

0 comments on commit 7d3db4e

Please sign in to comment.