feat: Allow overwriting built-in keyring builders

[Gudahtt]

Explanation

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.

References

N/A

Changelog

@metamask/keyring-controller

Added

• Add support for overwriting built-in keyring builders for the Simple and HD keyring.

Checklist

• I've updated the test suite for new or updated code as appropriate
• 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

[cryptodev-2s]

LGTM! only a small change if it's relevant

[cryptodev-2s]

Nit:
We can also do:

function buildKeyringBuilderWithSpy(KeyringConstructor: KeyringClass<Json>): {
  (): EthKeyring<Json>;
  type: string;
} {
  const keyringBuilderWithSpy = jest
    .fn()
    .mockImplementation(
      (...args) => new KeyringConstructor(...args),
    ) as jest.Mock & { type: string };
  keyringBuilderWithSpy.type = KeyringConstructor.type;
  return keyringBuilderWithSpy;
}

[Gudahtt]

Hmm, yes that would also work but it seems equivalent to me (in terms of type safety and readability and whatnot).

I was trying to do it with no assertion.

[Gudahtt]

I could do this without an assertion by doing something like

keyringBuilderWithSpy.type = KeyringConstructor.type
if (!hasProperty(keyringBuilderWithSpy, 'type')) {
  throw new Error('Missing type');
}

TypeScript would be able to infer it then. But that would be very silly to have to do, because we already know that type is set by that point (KeyringConstructor.type is of type string, it's not optional).