Feat: refactor to enable additional bridge

jest.config.js

@@ -41,10 +41,10 @@ module.exports = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 68.06,
-      functions: 88.57,
-      lines: 81.29,
-      statements: 81.21,
+      branches: 69.91,
+      functions: 88.88,
+      lines: 81.64,
+      statements: 81.56,
     },
   },
 

src/ledger-bridge.ts

@@ -26,14 +26,28 @@ export type LedgerSignTypedDataResponse = Awaited<
   ReturnType<LedgerHwAppEth['signEIP712HashedMessage']>
 >;
 
+export type LedgerBridgeOptions = Record<string, unknown>;
+
 // eslint-disable-next-line @typescript-eslint/consistent-type-definitions
-export interface LedgerBridge {
+export interface LedgerBridge<T extends LedgerBridgeOptions> {
   isDeviceConnected: boolean;
 
-  init(bridgeUrl: string): Promise<void>;
+  init(): Promise<void>;
 
   destroy(): Promise<void>;
 
+  /**
+   * Method to get the current configuration of the ledger bridge keyring.
+   */
+  getOptions(): Promise<T>;
+
+  /**
+   * Method to set the current configuration of the ledger bridge keyring.
+   *
+   * @param opts - An object contains configuration of the bridge.
+   */
+  setOptions(opts: T): Promise<void>;
+
   attemptMakeApp(): Promise<boolean>;
 
   updateTransportMethod(transportType: string): Promise<boolean>;

src/ledger-iframe-bridge.test.ts

@@ -51,6 +51,7 @@ async function simulateIFrameLoad(iframe?: HTMLIFrameElementShim) {
 }
 
 const LEDGER_IFRAME_ID = 'LEDGER-IFRAME';
+const BRIDGE_URL = 'BRIDGE_URL';
 
 describe('LedgerIframeBridge', function () {
   let bridge: LedgerIframeBridge;
@@ -75,8 +76,10 @@ describe('LedgerIframeBridge', function () {
   }
 
   beforeEach(async function () {
-    bridge = new LedgerIframeBridge();
-    await bridge.init('bridgeUrl');
+    bridge = new LedgerIframeBridge({
+      bridgeUrl: BRIDGE_URL,
+    });
+    await bridge.init();
     await simulateIFrameLoad(bridge.iframe);
   });
 
@@ -90,7 +93,7 @@ describe('LedgerIframeBridge', function () {
 
       const addEventListenerSpy = jest.spyOn(global.window, 'addEventListener');
 
-      await bridge.init('bridgeUrl');
+      await bridge.init();
 
       expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
       expect(bridge.iframeLoaded).toBe(false);
@@ -480,4 +483,91 @@ describe('LedgerIframeBridge', function () {
       expect(bridge.iframe?.contentWindow?.postMessage).toHaveBeenCalled();
     });
   });
+
+  describe('setOption', function () {
+    let removeEventListenerSpy: jest.SpyInstance;
+    let addEventListenerSpy: jest.SpyInstance;
+    const defaultIframeLoadedCounter = 1;
+
+    beforeEach(async () => {
+      removeEventListenerSpy = jest.spyOn(global.window, 'removeEventListener');
+      addEventListenerSpy = jest.spyOn(global.window, 'addEventListener');
+      bridge = new LedgerIframeBridge({
+        bridgeUrl: BRIDGE_URL,
+      });
+      await bridge.init();
+      await simulateIFrameLoad(bridge.iframe);
+    });
+
+    describe('when configurate bridge url', function () {
+      describe('when given bridge url is different with current', function () {
+        beforeEach(async () => {
+          await bridge.setOptions({ bridgeUrl: 'another url' });
+        });
+
+        it('should set bridgeUrl correctly', async function () {
+          expect(await bridge.getOptions()).toHaveProperty(
+            'bridgeUrl',
+            'another url',
+          );
+        });
+
+        it('should reload the iframe', async function () {
+          expect(addEventListenerSpy).toHaveBeenCalledTimes(
+            defaultIframeLoadedCounter + 1,
+          );
+          expect(removeEventListenerSpy).toHaveBeenCalledTimes(1);
+        });
+      });
+
+      describe('when given bridge url is same as current', function () {
+        beforeEach(async () => {
+          await bridge.setOptions({ bridgeUrl: BRIDGE_URL });
+        });
+
+        it('should not set bridgeUrl', async function () {
+          expect(await bridge.getOptions()).toHaveProperty(
+            'bridgeUrl',
+            BRIDGE_URL,
+          );
+        });
+
+        it('should not reload the iframe', async function () {
+          expect(addEventListenerSpy).toHaveBeenCalledTimes(
+            defaultIframeLoadedCounter,
+          );
+          expect(removeEventListenerSpy).toHaveBeenCalledTimes(0);
+        });
+      });
+
+      describe('when given bridge url is empty', function () {
+        beforeEach(async () => {
+          await bridge.setOptions({ bridgeUrl: '' });
+        });
+
+        it('should not set bridgeUrl', async function () {
+          expect(await bridge.getOptions()).toHaveProperty(
+            'bridgeUrl',
+            BRIDGE_URL,
+          );
+        });
+
+        it('should not reload the iframe', async function () {
+          expect(addEventListenerSpy).toHaveBeenCalledTimes(
+            defaultIframeLoadedCounter,
+          );
+          expect(removeEventListenerSpy).toHaveBeenCalledTimes(0);
+        });
+      });
+    });
+  });
+
+  describe('getOption', function () {
+    it('return instance options', async function () {
+      const result = await bridge.getOptions();
+      expect(result).toStrictEqual({
+        bridgeUrl: BRIDGE_URL,
+      });
+    });
+  });
 });

src/ledger-iframe-bridge.ts

@@ -85,11 +85,21 @@ type IFramePostMessage<TAction extends IFrameMessageAction> =
     target: typeof LEDGER_IFRAME_ID;
   };
 
-export class LedgerIframeBridge implements LedgerBridge {
+const BRIDGE_URL = 'https://metamask.github.io/eth-ledger-bridge-keyring';
+
+export type LedgerIframeBridgeOptions = {
+  bridgeUrl: string;
+};
+
+export class LedgerIframeBridge
+  implements LedgerBridge<LedgerIframeBridgeOptions>
+{
   iframe?: HTMLIFrameElement;
 
   iframeLoaded = false;
 
+  #opts: LedgerIframeBridgeOptions = { bridgeUrl: BRIDGE_URL };
+
   eventListener?: (eventMessage: {
     origin: string;
     data: IFrameMessageResponse;
@@ -108,10 +118,16 @@ export class LedgerIframeBridge implements LedgerBridge {
     transportType: string;
   };
 
-  async init(bridgeUrl: string) {
-    this.#setupIframe(bridgeUrl);
+  constructor(opts?: LedgerIframeBridgeOptions) {
+    if (opts?.bridgeUrl) {
+      this.#opts.bridgeUrl = opts.bridgeUrl;
+    }
+  }
+
+  async init() {
+    this.#setupIframe(this.#opts.bridgeUrl);
 
-    this.eventListener = this.#eventListener.bind(this, bridgeUrl);
+    this.eventListener = this.#eventListener.bind(this, this.#opts.bridgeUrl);
 
     window.addEventListener('message', this.eventListener);
   }
@@ -122,6 +138,18 @@ export class LedgerIframeBridge implements LedgerBridge {
     }
   }
 
+  async getOptions(): Promise<LedgerIframeBridgeOptions> {
+    return this.#opts;
+  }
+
+  async setOptions(opts: LedgerIframeBridgeOptions): Promise<void> {
+    if (opts.bridgeUrl && this.#opts.bridgeUrl !== opts.bridgeUrl) {
+      this.#opts.bridgeUrl = opts.bridgeUrl;
+      await this.destroy();
+      await this.init();
+    }
+  }
+
   async attemptMakeApp(): Promise<boolean> {
     return new Promise((resolve, reject) => {
       this.#sendMessage(

src/ledger-keyring.test.ts

@@ -7,7 +7,10 @@ import EthereumTx from 'ethereumjs-tx';
 import HDKey from 'hdkey';
 
 import { LedgerBridge } from './ledger-bridge';
-import { LedgerIframeBridge } from './ledger-iframe-bridge';
+import {
+  LedgerIframeBridge,
+  LedgerIframeBridgeOptions,
+} from './ledger-iframe-bridge';
 import { AccountDetails, LedgerKeyring } from './ledger-keyring';
 
 jest.mock('@metamask/eth-sig-util', () => {
@@ -83,10 +86,12 @@ const fakeTypeTwoTx = TransactionFactory.fromTxData(
   { common: commonEIP1559, freeze: false },
 );
 
+const BRIDGE_URL = 'BRIDGE_URL';
+
 describe('LedgerKeyring', function () {
   let keyring: LedgerKeyring;
-  let bridge: LedgerBridge;
-
+  let bridge: LedgerBridge<LedgerIframeBridgeOptions>;
+  const opts = { bridgeUrl: BRIDGE_URL };
   /**
    * Sets up the keyring to unlock one account.
    *
@@ -102,7 +107,7 @@ describe('LedgerKeyring', function () {
   }
 
   beforeEach(async function () {
-    bridge = new LedgerIframeBridge();
+    bridge = new LedgerIframeBridge(opts);
     keyring = new LedgerKeyring({ bridge });
     keyring.hdk = fakeHdKey;
     await keyring.deserialize();
@@ -128,7 +133,7 @@ describe('LedgerKeyring', function () {
   describe('constructor', function () {
     it('constructs', async function () {
       const ledgerKeyring = new LedgerKeyring({
-        bridge: new LedgerIframeBridge(),
+        bridge: new LedgerIframeBridge(opts),
       });
       expect(typeof ledgerKeyring).toBe('object');
 
@@ -140,7 +145,8 @@ describe('LedgerKeyring', function () {
       expect(
         () =>
           new LedgerKeyring({
-            bridge: undefined as unknown as LedgerBridge,
+            bridge:
+              undefined as unknown as LedgerBridge<LedgerIframeBridgeOptions>,
           }),
       ).toThrow('Bridge is a required dependency for the keyring');
     });
@@ -161,9 +167,6 @@ describe('LedgerKeyring', function () {
     it('serializes an instance', async function () {
       const output = await keyring.serialize();
 
-      expect(output.bridgeUrl).toBe(
-        'https://metamask.github.io/eth-ledger-bridge-keyring',
-      );
       expect(output.hdPath).toBe(`m/44'/60'/0'`);
       expect(Array.isArray(output.accounts)).toBe(true);
       expect(output.accounts).toHaveLength(0);
@@ -188,9 +191,6 @@ describe('LedgerKeyring', function () {
       const serialized = await keyring.serialize();
 
       expect(serialized.accounts).toHaveLength(1);
-      expect(serialized.bridgeUrl).toBe(
-        'https://metamask.github.io/eth-ledger-bridge-keyring',
-      );
       expect(serialized.hdPath).toBe(someHdPath);
       expect(serialized.accountDetails).toStrictEqual(accountDetails);
     });

src/ledger-keyring.ts

@@ -15,14 +15,12 @@ import type OldEthJsTransaction from 'ethereumjs-tx';
 import { EventEmitter } from 'events';
 import HDKey from 'hdkey';
 
-import { LedgerBridge } from './ledger-bridge';
+import { LedgerBridge, LedgerBridgeOptions } from './ledger-bridge';
 
 const pathBase = 'm';
 const hdPathString = `${pathBase}/44'/60'/0'`;
 const keyringType = 'Ledger Hardware';
 
-const BRIDGE_URL = 'https://metamask.github.io/eth-ledger-bridge-keyring';
-
 const MAX_INDEX = 1000;
 
 enum NetworkApiUrls {
@@ -33,7 +31,7 @@ enum NetworkApiUrls {
 }
 
 type SignTransactionPayload = Awaited<
-  ReturnType<LedgerBridge['deviceSignTransaction']>
+  ReturnType<LedgerBridge<LedgerBridgeOptions>['deviceSignTransaction']>
 >;
 
 export type AccountDetails = {
@@ -47,7 +45,6 @@ export type LedgerBridgeKeyringOptions = {
   accounts: readonly string[];
   accountDetails: Readonly<Record<string, AccountDetails>>;
   accountIndexes: Readonly<Record<string, number>>;
-  bridgeUrl: string;
   implementFullBIP44: boolean;
 };
 
@@ -95,11 +92,9 @@ export class LedgerKeyring extends EventEmitter {
 
   implementFullBIP44 = false;
 
-  bridgeUrl: string = BRIDGE_URL;
-
-  bridge: LedgerBridge;
+  bridge: LedgerBridge<LedgerBridgeOptions>;
 
-  constructor({ bridge }: { bridge: LedgerBridge }) {
+  constructor({ bridge }: { bridge: LedgerBridge<LedgerBridgeOptions> }) {
     super();
 
     if (!bridge) {
@@ -110,7 +105,7 @@ export class LedgerKeyring extends EventEmitter {
   }
 
   async init() {
-    return this.bridge.init(this.bridgeUrl);
+    return this.bridge.init();
   }
 
   async destroy() {
@@ -122,16 +117,15 @@ export class LedgerKeyring extends EventEmitter {
       hdPath: this.hdPath,
       accounts: this.accounts,
       accountDetails: this.accountDetails,
-      bridgeUrl: this.bridgeUrl,
       implementFullBIP44: false,
     };
   }
 
   async deserialize(opts: Partial<LedgerBridgeKeyringOptions> = {}) {
     this.hdPath = opts.hdPath ?? hdPathString;
-    this.bridgeUrl = opts.bridgeUrl ?? BRIDGE_URL;
     this.accounts = opts.accounts ?? [];
     this.accountDetails = opts.accountDetails ?? {};
+
     if (!opts.accountDetails) {
       this.#migrateAccountDetails(opts);
     }