Skip to content

Commit

Permalink
fix(dep): use ulidx instead of ulid
Browse files Browse the repository at this point in the history
  • Loading branch information
@montelaidev
montelaidev committed Jan 22, 2025
1 parent 0e8b42a commit d76a0d8
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 86 deletions.
2 changes: 1 addition & 1 deletion packages/keyring-controller/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"async-mutex": "^0.5.0",
"ethereumjs-wallet": "^1.0.1",
"immer": "^9.0.6",
"ulid": "^2.3.0"
"ulidx": "^2.4.1"
},
"devDependencies": {
"@ethereumjs/common": "^3.2.0",
Expand Down
145 changes: 71 additions & 74 deletions packages/keyring-controller/src/KeyringController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jest.mock('uuid', () => {
};
});

jest.mock('ulid', () => ({
jest.mock('ulidx', () => ({
ulid: () => 'ULID01234567890ABCDEFGHIJKLMN',
monotonicFactory: () => () => 'ULID01234567890ABCDEFGHIJKLMN',
}));
Expand Down Expand Up @@ -186,12 +186,10 @@ describe('KeyringController', () => {
it('should not add a new account if called twice with the same accountCount param', async () => {
await withController(async ({ controller, initialState }) => {
const accountCount = initialState.keyrings[0].accounts.length;
const firstAccountAdded = await controller.addNewAccount(
accountCount,
);
const secondAccountAdded = await controller.addNewAccount(
accountCount,
);
const firstAccountAdded =
await controller.addNewAccount(accountCount);
const secondAccountAdded =
await controller.addNewAccount(accountCount);
expect(firstAccountAdded).toBe(secondAccountAdded);
expect(controller.state.keyrings[0].accounts).toHaveLength(
accountCount + 1,
Expand Down Expand Up @@ -225,13 +223,11 @@ describe('KeyringController', () => {

const accountCount = initialState.keyrings[0].accounts.length;
// We add a new account for "index 1" (not existing yet)
const firstAccountAdded = await controller.addNewAccount(
accountCount,
);
const firstAccountAdded =
await controller.addNewAccount(accountCount);
// Adding an account for an existing index will return the existing account's address
const secondAccountAdded = await controller.addNewAccount(
accountCount,
);
const secondAccountAdded =
await controller.addNewAccount(accountCount);
expect(firstAccountAdded).toBe(secondAccountAdded);
expect(controller.state.keyrings[0].accounts).toHaveLength(
accountCount + 1,
Expand Down Expand Up @@ -263,7 +259,7 @@ describe('KeyringController', () => {
undefined,
keyringId,
);

expect(initialState.keyrings).toHaveLength(1);
expect(initialState.keyrings[0].accounts).not.toStrictEqual(
controller.state.keyrings[0].accounts,
Expand All @@ -277,31 +273,31 @@ describe('KeyringController', () => {
);
});
});

it('should throw error if keyringId is invalid', async () => {
await withController(async ({ controller }) => {
await expect(
controller.addNewAccount(undefined, 'invalid-id'),
).rejects.toThrow('No HD keyring found');
});
});

it('should throw error if accountCount is out of sequence for specified keyring', async () => {
await withController(async ({ controller, initialState }) => {
const keyringId = initialState.keyringsMetadata[0].id;
const accountCount = initialState.keyrings[0].accounts.length;

await expect(
controller.addNewAccount(accountCount + 1, keyringId),
).rejects.toThrow('Account out of sequence');
});
});

it('should return existing account if called twice with same accountCount for specified keyring', async () => {
await withController(async ({ controller, initialState }) => {
const keyringId = initialState.keyringsMetadata[0].id;
const accountCount = initialState.keyrings[0].accounts.length;

const firstAccountAdded = await controller.addNewAccount(
accountCount,
keyringId,
Expand All @@ -310,7 +306,7 @@ describe('KeyringController', () => {
accountCount,
keyringId,
);

expect(firstAccountAdded).toBe(secondAccountAdded);
expect(controller.state.keyrings[0].accounts).toHaveLength(
accountCount + 1,
Expand All @@ -327,9 +323,8 @@ describe('KeyringController', () => {
const [primaryKeyring] = controller.getKeyringsByType(
KeyringTypes.hd,
) as Keyring<Json>[];
const addedAccountAddress = await controller.addNewAccountForKeyring(
primaryKeyring,
);
const addedAccountAddress =
await controller.addNewAccountForKeyring(primaryKeyring);
expect(initialState.keyrings).toHaveLength(1);
expect(initialState.keyrings[0].accounts).not.toStrictEqual(
controller.state.keyrings[0].accounts,
Expand Down Expand Up @@ -375,9 +370,8 @@ describe('KeyringController', () => {
const [primaryKeyring] = controller.getKeyringsByType(
KeyringTypes.hd,
) as Keyring<Json>[];
const addedAccountAddress = await controller.addNewAccountForKeyring(
primaryKeyring,
);
const addedAccountAddress =
await controller.addNewAccountForKeyring(primaryKeyring);
expect(initialState.keyrings).toHaveLength(1);
expect(initialState.keyrings[0].accounts).not.toStrictEqual(
controller.state.keyrings[0].accounts,
Expand Down Expand Up @@ -476,9 +470,8 @@ describe('KeyringController', () => {
await withController(
{ cacheEncryptionKey },
async ({ controller, initialState }) => {
const currentSeedWord = await controller.exportSeedPhrase(
password,
);
const currentSeedWord =
await controller.exportSeedPhrase(password);

await controller.createNewVaultAndRestore(
password,
Expand Down Expand Up @@ -544,9 +537,8 @@ describe('KeyringController', () => {
async ({ controller }) => {
await controller.createNewVaultAndKeychain(password);

const currentSeedPhrase = await controller.exportSeedPhrase(
password,
);
const currentSeedPhrase =
await controller.exportSeedPhrase(password);

expect(currentSeedPhrase.length).toBeGreaterThan(0);
expect(
Expand Down Expand Up @@ -634,17 +626,15 @@ describe('KeyringController', () => {
await withController(
{ cacheEncryptionKey },
async ({ controller, initialState }) => {
const initialSeedWord = await controller.exportSeedPhrase(
password,
);
const initialSeedWord =
await controller.exportSeedPhrase(password);
expect(initialSeedWord).toBeDefined();
const initialVault = controller.state.vault;

await controller.createNewVaultAndKeychain(password);

const currentSeedWord = await controller.exportSeedPhrase(
password,
);
const currentSeedWord =
await controller.exportSeedPhrase(password);
expect(initialState).toStrictEqual(controller.state);
expect(initialSeedWord).toBe(currentSeedWord);
expect(initialVault).toStrictEqual(controller.state.vault);
Expand Down Expand Up @@ -736,19 +726,19 @@ describe('KeyringController', () => {
expect(seed).not.toBe('');
});
});

it('should export seed phrase with valid keyringId', async () => {
await withController(async ({ controller, initialState }) => {
const keyringId = initialState.keyringsMetadata[0].id;
const seed = await controller.exportSeedPhrase(password, keyringId);
expect(seed).not.toBe('');
});
});

it('should throw error if keyringId is invalid', async () => {
await withController(async ({ controller }) => {
await expect(
controller.exportSeedPhrase(password, 'invalid-id')
controller.exportSeedPhrase(password, 'invalid-id'),
).rejects.toThrow('Keyring not found');
});
});
Expand All @@ -767,15 +757,17 @@ describe('KeyringController', () => {
});

it('should throw invalid password error with valid keyringId', async () => {
await withController(async ({ controller, encryptor, initialState }) => {
const keyringId = initialState.keyringsMetadata[0].id;
sinon
.stub(encryptor, 'decrypt')
.throws(new Error('Invalid password'));
await expect(
controller.exportSeedPhrase('', keyringId)
).rejects.toThrow('Invalid password');
});
await withController(
async ({ controller, encryptor, initialState }) => {
const keyringId = initialState.keyringsMetadata[0].id;
sinon
.stub(encryptor, 'decrypt')
.throws(new Error('Invalid password'));
await expect(
controller.exportSeedPhrase('', keyringId),
).rejects.toThrow('Invalid password');
},
);
});
});
});
Expand Down Expand Up @@ -863,10 +855,12 @@ describe('KeyringController', () => {
await withController(async ({ controller, initialState }) => {
const keyringId = initialState.keyringsMetadata[0].id;
const keyringAccounts = await controller.getAccounts(keyringId);
expect(keyringAccounts).toStrictEqual(initialState.keyrings[0].accounts);
expect(keyringAccounts).toStrictEqual(
initialState.keyrings[0].accounts,
);
});
});

it('should throw error if keyringId is invalid', async () => {
await withController(async ({ controller }) => {
await expect(controller.getAccounts('invalid-id')).rejects.toThrow(
Expand Down Expand Up @@ -1110,7 +1104,10 @@ describe('KeyringController', () => {
const modifiedState = {
...initialState,
keyrings: [initialState.keyrings[0], newKeyring],
keyringsMetadata: [initialState.keyringsMetadata[0], initialState.keyringsMetadata[0]],
keyringsMetadata: [
initialState.keyringsMetadata[0],
initialState.keyringsMetadata[0],
],
};
expect(controller.state).toStrictEqual(modifiedState);
expect(importedAccountAddress).toBe(address);
Expand Down Expand Up @@ -1184,7 +1181,10 @@ describe('KeyringController', () => {
const modifiedState = {
...initialState,
keyrings: [initialState.keyrings[0], newKeyring],
keyringsMetadata: [initialState.keyringsMetadata[0], initialState.keyringsMetadata[0]],
keyringsMetadata: [
initialState.keyringsMetadata[0],
initialState.keyringsMetadata[0],
],
};
expect(controller.state).toStrictEqual(modifiedState);
expect(importedAccountAddress).toBe(address);
Expand Down Expand Up @@ -2589,52 +2589,52 @@ describe('KeyringController', () => {
const fn = jest.fn();
const keyring = controller.getKeyringsByType(KeyringTypes.hd)[0];
const selector = { id: initialState.keyringsMetadata[0].id };

await controller.withKeyring(selector, fn);

expect(fn).toHaveBeenCalledWith(keyring);
});
});

it('should return the result of the function', async () => {
await withController(async ({ controller, initialState }) => {
const fn = async () => Promise.resolve('hello');
const selector = { id: initialState.keyringsMetadata[0].id };

expect(await controller.withKeyring(selector, fn)).toBe('hello');
});
});

it('should throw an error if the callback returns the selected keyring', async () => {
await withController(async ({ controller, initialState }) => {
await withController(async ({ controller, initialState }) => {
const selector = { id: initialState.keyringsMetadata[0].id };

await expect(
controller.withKeyring(selector, async (selectedKeyring) => {
return selectedKeyring;
}),
).rejects.toThrow(KeyringControllerError.UnsafeDirectKeyringAccess);
});
});

describe('when the keyring is not found', () => {
it('should throw an error if the keyring is not found and `createIfMissing` is false', async () => {
await withController(async ({ controller, initialState }) => {
const selector = { id: 'non-existent-id' };
const fn = jest.fn();

await expect(controller.withKeyring(selector, fn)).rejects.toThrow(
KeyringControllerError.KeyringNotFound,
);
expect(fn).not.toHaveBeenCalled();
});
});

it('should throw an error even if `createIfMissing` is true', async () => {
await withController(async ({ controller, initialState }) => {
const selector = { id: 'non-existent-id' };
const fn = jest.fn();

await expect(
controller.withKeyring(selector, fn, { createIfMissing: true }),
).rejects.toThrow(KeyringControllerError.KeyringNotFound);
Expand Down Expand Up @@ -2732,21 +2732,18 @@ describe('KeyringController', () => {
),
);

const firstPage = await signProcessKeyringController.connectQRHardware(
0,
);
const firstPage =
await signProcessKeyringController.connectQRHardware(0);
expect(firstPage).toHaveLength(5);
expect(firstPage[0].index).toBe(0);

const secondPage = await signProcessKeyringController.connectQRHardware(
1,
);
const secondPage =
await signProcessKeyringController.connectQRHardware(1);
expect(secondPage).toHaveLength(5);
expect(secondPage[0].index).toBe(5);

const goBackPage = await signProcessKeyringController.connectQRHardware(
-1,
);
const goBackPage =
await signProcessKeyringController.connectQRHardware(-1);
expect(goBackPage).toStrictEqual(firstPage);

await signProcessKeyringController.unlockQRHardwareWalletAccount(0);
Expand Down
14 changes: 10 additions & 4 deletions packages/keyring-controller/src/KeyringController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import type { MutexInterface } from 'async-mutex';
import Wallet, { thirdparty as importers } from 'ethereumjs-wallet';
import type { Patch } from 'immer';
// When generating a ULID within the same millisecond, monotonicFactory provides some guarantees regarding sort order.
import { monotonicFactory } from 'ulid';
import { monotonicFactory } from 'ulidx';

import { KeyringControllerError } from './constants';

Expand Down Expand Up @@ -2021,8 +2021,11 @@ export class KeyringController extends BaseController<
if (this.state.keyringsMetadata.length > this.#keyrings.length) {
this.update((state) => {
// remove metadata from the end of the array to have the same length as the keyrings array
state.keyringsMetadata = state.keyringsMetadata.slice(0, -1 * (state.keyringsMetadata.length - this.#keyrings.length));
})
state.keyringsMetadata = state.keyringsMetadata.slice(
0,
-1 * (state.keyringsMetadata.length - this.#keyrings.length),
);
});
}
}

Expand Down Expand Up @@ -2193,7 +2196,10 @@ export class KeyringController extends BaseController<
state.encryptionSalt = JSON.parse(updatedState.vault as string).salt;
}
if (updatedKeyrings.length < state.keyringsMetadata.length) {
state.keyringsMetadata = state.keyringsMetadata.slice(0, -1 * (state.keyringsMetadata.length - updatedKeyrings.length));
state.keyringsMetadata = state.keyringsMetadata.slice(
0,
-1 * (state.keyringsMetadata.length - updatedKeyrings.length),
);
}
});

Expand Down
Loading

0 comments on commit d76a0d8

Please sign in to comment.