Skip to content

Commit

Permalink
fix: add wildcard for custody keyring (#3899)
Browse files Browse the repository at this point in the history
## Explanation

This pr resolves the special case when passing a custody keyring type to
the function keyringTypeToName.

## References

Fixes: MetaMask/accounts-planning#233

## Changelog

### `@metamask/keyring-controller`

- **CHANGED**:  Custody keyring type. 

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Gustavo Antunes <[email protected]>
  • Loading branch information
montelaidev and gantunesr authored Feb 15, 2024
1 parent 3ea7a48 commit 67a602d
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,7 @@ describe('AccountsController', () => {
KeyringTypes.ledger,
KeyringTypes.lattice,
KeyringTypes.qr,
KeyringTypes.custody,
'Custody - JSON - RPC',
])('should add accounts for %s type', async (keyringType) => {
mockUUID.mockReturnValue('mock-id');

Expand Down
11 changes: 7 additions & 4 deletions packages/accounts-controller/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { KeyringTypes } from '@metamask/keyring-controller';
import { isCustodyKeyring, KeyringTypes } from '@metamask/keyring-controller';
import { sha256FromString } from 'ethereumjs-util';
import { v4 as uuid } from 'uuid';

Expand All @@ -9,6 +9,12 @@ import { v4 as uuid } from 'uuid';
* @returns The name of the keyring type.
*/
export function keyringTypeToName(keyringType: string): string {
// Custody keyrings are a special case, as they are not a single type
// they just start with the prefix `Custody`
if (isCustodyKeyring(keyringType)) {
return 'Custody';
}

switch (keyringType) {
case KeyringTypes.simple: {
return 'Account';
Expand All @@ -31,9 +37,6 @@ export function keyringTypeToName(keyringType: string): string {
case KeyringTypes.snap: {
return 'Snap Account';
}
case KeyringTypes.custody: {
return 'Custody';
}
default: {
throw new Error(`Unknown keyring ${keyringType}`);
}
Expand Down
15 changes: 15 additions & 0 deletions packages/keyring-controller/src/KeyringController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
AccountImportStrategy,
KeyringController,
KeyringTypes,
isCustodyKeyring,
keyringBuilderFactory,
} from './KeyringController';

Expand Down Expand Up @@ -2791,6 +2792,20 @@ describe('KeyringController', () => {
});
});

describe('isCustodyKeyring', () => {
it('should return true if keyring is custody keyring', () => {
expect(isCustodyKeyring('Custody JSON-RPC')).toBe(true);
});

it('should not return true if keyring is not custody keyring', () => {
expect(isCustodyKeyring(KeyringTypes.hd)).toBe(false);
});

it("should not return true if the keyring doesn't start with custody", () => {
expect(isCustodyKeyring('NotCustody')).toBe(false);
});
});

describe('actions', () => {
beforeEach(() => {
jest
Expand Down
11 changes: 10 additions & 1 deletion packages/keyring-controller/src/KeyringController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,18 @@ export enum KeyringTypes {
ledger = 'Ledger Hardware',
lattice = 'Lattice Hardware',
snap = 'Snap Keyring',
custody = 'Custody - JSONRPC',
}

/**
* Custody keyring types are a special case, as they are not a single type
* but they all start with the prefix "Custody".
* @param keyringType - The type of the keyring.
* @returns Whether the keyring type is a custody keyring.
*/
export const isCustodyKeyring = (keyringType: string): boolean => {
return keyringType.startsWith('Custody');
};

/**
* @type KeyringControllerState
*
Expand Down

0 comments on commit 67a602d

Please sign in to comment.