From a1f4ca1d404cd225d19e00c66a9d94e2d98ad966 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Mon, 17 Jul 2023 22:02:18 -0230
Subject: [PATCH] Allow non-object keyring serialized state

The `addNewKeyring` method has been updated to support non-object
keyring serialized state, which had been supported already on versions
prior to v11.

This allows us to remove a special-case for the simple keyring, which
requires a JSON array as its serialized keyring state.

When constructing a simple keyring, the method `addNewKeying` now
expects the second parameter (`opts`) to be an array of private keys
rather than an object with a `privateKeys` property. This is how it
worked prior to v11.
---
 jest.config.js                |  6 +++---
 src/KeyringController.test.ts | 35 ++++++++++++-----------------------
 src/KeyringController.ts      | 18 ++----------------
 3 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/jest.config.js b/jest.config.js
index ab6571a5..4075bb3a 100644
--- a/jest.config.js
+++ b/jest.config.js
@@ -41,10 +41,10 @@ module.exports = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 72.41,
+      branches: 71.42,
       functions: 92.85,
-      lines: 90.87,
-      statements: 91.08,
+      lines: 90.68,
+      statements: 90.9,
     },
   },
   preset: 'ts-jest',
diff --git a/src/KeyringController.test.ts b/src/KeyringController.test.ts
index cd9a736f..86fc9cea 100644
--- a/src/KeyringController.test.ts
+++ b/src/KeyringController.test.ts
@@ -457,7 +457,7 @@ describe('KeyringController', () => {
       const previousAccounts = await keyringController.getAccounts();
       const keyring = await keyringController.addNewKeyring(
         KeyringType.Simple,
-        { privateKeys: [privateKey] },
+        [privateKey],
       );
 
       const keyringAccounts = await keyring?.getAccounts();
@@ -473,15 +473,6 @@ describe('KeyringController', () => {
       expect(allAccounts).toStrictEqual(expectedAllAccounts);
     });
 
-    it('should throw an error when attempting to add simple key pair without private keys', async () => {
-      const keyringController = await initializeKeyringController({
-        password: PASSWORD,
-      });
-      await expect(async () =>
-        keyringController.addNewKeyring(KeyringType.Simple),
-      ).rejects.toThrow('Private keys missing');
-    });
-
     it('should add HD Key Tree without mnemonic passed as an argument', async () => {
       const keyringController = await initializeKeyringController({
         password: PASSWORD,
@@ -663,9 +654,9 @@ describe('KeyringController', () => {
       const accountsBeforeAdding = await keyringController.getAccounts();
 
       // Add a new keyring with one account
-      await keyringController.addNewKeyring(KeyringType.Simple, {
-        privateKeys: [account.privateKey],
-      });
+      await keyringController.addNewKeyring(KeyringType.Simple, [
+        account.privateKey,
+      ]);
       expect(keyringController.keyrings).toHaveLength(2);
 
       // remove that account that we just added
@@ -688,9 +679,9 @@ describe('KeyringController', () => {
       };
 
       // Add a new keyring with one account
-      await keyringController.addNewKeyring(KeyringType.Simple, {
-        privateKeys: [account.privateKey],
-      });
+      await keyringController.addNewKeyring(KeyringType.Simple, [
+        account.privateKey,
+      ]);
 
       // We should have 2 keyrings
       expect(keyringController.keyrings).toHaveLength(2);
@@ -823,7 +814,7 @@ describe('KeyringController', () => {
 
       const keyring = await keyringController.addNewKeyring(
         KeyringType.Simple,
-        { privateKeys: [privateKey] },
+        [privateKey],
       );
 
       const getAppKeyAddressSpy = sinon.spy(
@@ -857,9 +848,7 @@ describe('KeyringController', () => {
       const address = '0x01560cd3bac62cc6d7e6380600d9317363400896';
       const privateKey =
         '0xb8a9c05beeedb25df85f8d641538cbffedf67216048de9c678ee26260eb91952';
-      await keyringController.addNewKeyring(KeyringType.Simple, {
-        privateKeys: [privateKey],
-      });
+      await keyringController.addNewKeyring(KeyringType.Simple, [privateKey]);
       const appKeyAddress = await keyringController.getAppKeyAddress(
         address,
         'someapp.origin.io',
@@ -1051,9 +1040,9 @@ describe('KeyringController', () => {
       };
 
       // Add a new keyring with one account
-      await keyringController.addNewKeyring(KeyringType.Simple, {
-        privateKeys: [account.privateKey],
-      });
+      await keyringController.addNewKeyring(KeyringType.Simple, [
+        account.privateKey,
+      ]);
       expect(await keyringController.getAccounts()).toHaveLength(4);
 
       // remove that account that we just added
diff --git a/src/KeyringController.ts b/src/KeyringController.ts
index 8aca0426..97c872e0 100644
--- a/src/KeyringController.ts
+++ b/src/KeyringController.ts
@@ -584,22 +584,8 @@ class KeyringController extends EventEmitter {
    * @param opts - The constructor options for the keyring.
    * @returns The new keyring.
    */
-  async addNewKeyring(
-    type: string,
-    opts?: Record<string, unknown>,
-  ): Promise<Keyring<Json>> {
-    let keyring: Keyring<Json>;
-    switch (type) {
-      case KeyringType.Simple:
-        if (!isObject(opts)) {
-          throw new Error('Private keys missing');
-        }
-        keyring = await this.#newKeyring(type, opts.privateKeys);
-        break;
-      default:
-        keyring = await this.#newKeyring(type, opts);
-        break;
-    }
+  async addNewKeyring(type: string, opts?: unknown): Promise<Keyring<Json>> {
+    const keyring = await this.#newKeyring(type, opts);
 
     if (!keyring) {
       throw new Error(KeyringControllerError.NoKeyring);