Skip to content

Commit

Permalink
refactor: rewrite getKeyringForAddress
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikesposito
mikesposito committed Jan 17, 2025
1 parent 666360f commit 5a86062
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 39 deletions.
6 changes: 3 additions & 3 deletions packages/keyring-controller/jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 92.94,
branches: 94.07,
functions: 100,
lines: 98.61,
statements: 98.63,
lines: 98.94,
statements: 98.96,
},
},

Expand Down
25 changes: 14 additions & 11 deletions packages/keyring-controller/src/KeyringController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,9 +694,12 @@ describe('KeyringController', () => {
it('should throw error', async () => {
await withController(async ({ controller }) => {
await expect(
controller.exportAccount(password, ''),
controller.exportAccount(
password,
'0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045',
),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
});
});
Expand Down Expand Up @@ -838,7 +841,7 @@ describe('KeyringController', () => {
await expect(
controller.decryptMessage(messageParams),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
});
});
Expand Down Expand Up @@ -896,7 +899,7 @@ describe('KeyringController', () => {
'0x0000000000000000000000000000000000000000',
),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
});
});
Expand Down Expand Up @@ -1215,13 +1218,13 @@ describe('KeyringController', () => {
'0x0000000000000000000000000000000000000000',
),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
'KeyringController - No keyring found. Error info: There are 2 keyrings available, but none match the address',
);

await expect(
controller.removeAccount('0xDUMMY_INPUT'),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
'KeyringController - No keyring found. Error info: There are 2 keyrings available, but none match the address',
);
});
});
Expand Down Expand Up @@ -1279,7 +1282,7 @@ describe('KeyringController', () => {
from: '',
}),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
});
});
Expand Down Expand Up @@ -1347,7 +1350,7 @@ describe('KeyringController', () => {
from: '',
}),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
});
});
Expand Down Expand Up @@ -1699,7 +1702,7 @@ describe('KeyringController', () => {
expect(unsignedEthTx.v).toBeUndefined();
await controller.signTransaction(unsignedEthTx, '');
}).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
});
});
Expand Down Expand Up @@ -2446,7 +2449,7 @@ describe('KeyringController', () => {
await expect(
controller.withKeyring(selector, fn),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
`KeyringController - No keyring found. Error info: There are ${controller.state.keyrings.length} keyrings available, but none match the address`,
);
expect(fn).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -3473,7 +3476,7 @@ describe('KeyringController', () => {
await expect(
controller.exportAccount(password, mockAddress),
).rejects.toThrow(
'KeyringController - No keyring found. Error info: There are keyrings, but none match the address',
'KeyringController - No keyring found. Error info: There are 1 keyrings available, but none match the address',
);
},
);
Expand Down
39 changes: 14 additions & 25 deletions packages/keyring-controller/src/KeyringController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import type {
} from '@metamask/utils';
import {
add0x,
assert,
assertIsStrictHexString,
bytesToHex,
hasProperty,
Expand Down Expand Up @@ -898,33 +899,24 @@ export class KeyringController extends BaseController<
*/
async getKeyringForAccount(account: string): Promise<unknown> {
this.#assertIsUnlocked();
const address = normalize(account);

const candidates = await Promise.all(
this.#keyrings.map(async (keyring) => {
return Promise.all([keyring, keyring.getAccounts()]);
}),
const address = normalize(account);
assert(
address,
`${KeyringControllerError.NoKeyring}. Error infor: Invalid account address`,
);

const winners = candidates.filter((candidate) => {
const accounts = candidate[1].map(normalize);
return accounts.includes(address);
});
const keyringIndex = this.state.keyrings.findIndex((keyring) =>
keyring.accounts.includes(address),
);

if (winners.length && winners[0]?.length) {
return winners[0][0];
if (keyringIndex === -1 || !this.#keyrings[keyringIndex]) {
throw new Error(
`${KeyringControllerError.NoKeyring}. Error info: There are ${this.#keyrings.length} keyrings available, but none match the address`,
);
}

// Adding more info to the error
let errorInfo = '';
if (!candidates.length) {
errorInfo = 'There are no keyrings';
} else if (!winners.length) {
errorInfo = 'There are keyrings, but none match the address';
}
throw new Error(
`${KeyringControllerError.NoKeyring}. Error info: ${errorInfo}`,
);
return this.#keyrings[keyringIndex];
}

/**
Expand Down Expand Up @@ -1874,10 +1866,7 @@ export class KeyringController extends BaseController<
const primaryKeyring = this.getKeyringsByType(KeyringTypes.hd)[0] as
| EthKeyring<Json>
| undefined;
if (!primaryKeyring) {
throw new Error('No HD keyring found.');
}

assert(primaryKeyring, 'No HD keyring found.');
assertHasUint8ArrayMnemonic(primaryKeyring);

const seedWords = primaryKeyring.mnemonic;
Expand Down

0 comments on commit 5a86062

Please sign in to comment.