Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Commit

Permalink
feat!: refactor to enable additional bridge (#210)
Browse files Browse the repository at this point in the history 
- BREAKING: `LedgerIframeBridge` class constructor now takes an options object with `bridgeUrl`. 
- BREAKING: `LedgerBridge` `init` function now takes no parameters.
- BREAKING: `LedgerBridgeKeyringOptions` no longer contain `bridgeUrl`. 
- BREAKING: `LedgerBridge` interface is now parameterized over its option type
-  Add `getOptions` and `setOptions` methods to `LedgerBridge` interface
  • Loading branch information
@stanleyyconsensys
stanleyyconsensys authored Feb 27, 2024
1 parent 7749409 commit 2cc856a
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 38 deletions.
8 changes: 4 additions & 4 deletions jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ module.exports = {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 68.33,
functions: 88.23,
lines: 81.46,
statements: 81.38,
branches: 69.76,
functions: 88.73,
lines: 81.93,
statements: 81.84,
},
},

Expand Down
21 changes: 17 additions & 4 deletions src/ledger-bridge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,27 @@ export type LedgerSignTypedDataResponse = Awaited<
ReturnType<LedgerHwAppEth['signEIP712HashedMessage']>
>;

// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
export interface LedgerBridge {
export type LedgerBridgeOptions = Record<string, unknown>;

export type 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>;
Expand All @@ -51,4 +64,4 @@ export interface LedgerBridge {
deviceSignTypedData(
params: LedgerSignTypedDataParams,
): Promise<LedgerSignTypedDataResponse>;
}
};
117 changes: 113 additions & 4 deletions src/ledger-iframe-bridge.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ async function simulateIFrameLoad(iframe?: HTMLIFrameElementShim) {
}

const LEDGER_IFRAME_ID = 'LEDGER-IFRAME';

const BRIDGE_URL = 'https://metamask.github.io/eth-ledger-bridge-keyring';
const INVALID_URL_ERROR = 'bridgeURL is not a valid URL';
describe('LedgerIframeBridge', function () {
let bridge: LedgerIframeBridge;

Expand All @@ -75,22 +76,57 @@ 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);
});

afterEach(function () {
jest.clearAllMocks();
});

describe('constructor', function () {
describe('when configurate not given', function () {
it('should use the default bridgeUrl', async function () {
bridge = new LedgerIframeBridge();
expect(await bridge.getOptions()).toHaveProperty(
'bridgeUrl',
BRIDGE_URL,
);
});
});

describe('when configurate given', function () {
it('should set the given bridgeUrl', async function () {
bridge = new LedgerIframeBridge({
bridgeUrl: 'https://metamask.io',
});
expect(await bridge.getOptions()).toHaveProperty(
'bridgeUrl',
'https://metamask.io',
);
});

it('should throw error if given url is empty', async function () {
expect(
() =>
new LedgerIframeBridge({
bridgeUrl: '',
}),
).toThrow(INVALID_URL_ERROR);
});
});
});

describe('init', function () {
it('sets up the listener and iframe', async function () {
bridge = new LedgerIframeBridge();

const addEventListenerSpy = jest.spyOn(global.window, 'addEventListener');

await bridge.init('bridgeUrl');
await bridge.init();

expect(addEventListenerSpy).toHaveBeenCalledTimes(1);
expect(bridge.iframeLoaded).toBe(false);
Expand Down Expand Up @@ -480,4 +516,77 @@ 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();
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: 'https://metamask.io' });
});

it('should set bridgeUrl correctly', async function () {
expect(await bridge.getOptions()).toHaveProperty(
'bridgeUrl',
'https://metamask.io',
);
});

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 () {
it('should throw error', async function () {
await expect(bridge.setOptions({ bridgeUrl: '' })).rejects.toThrow(
INVALID_URL_ERROR,
);
});
});
});
});

describe('getOption', function () {
it('return instance options', async function () {
const result = await bridge.getOptions();
expect(result).toStrictEqual({
bridgeUrl: BRIDGE_URL,
});
});
});
});
46 changes: 42 additions & 4 deletions src/ledger-iframe-bridge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,19 @@ type IFramePostMessage<TAction extends IFrameMessageAction> =
target: typeof LEDGER_IFRAME_ID;
};

export class LedgerIframeBridge implements LedgerBridge {
export type LedgerIframeBridgeOptions = {
bridgeUrl: string;
};

export class LedgerIframeBridge
implements LedgerBridge<LedgerIframeBridgeOptions>
{
iframe?: HTMLIFrameElement;

iframeLoaded = false;

#opts: LedgerIframeBridgeOptions;

eventListener?: (eventMessage: {
origin: string;
data: IFrameMessageResponse;
Expand All @@ -108,10 +116,21 @@ export class LedgerIframeBridge implements LedgerBridge {
transportType: string;
};

async init(bridgeUrl: string) {
this.#setupIframe(bridgeUrl);
constructor(
opts: LedgerIframeBridgeOptions = {
bridgeUrl: 'https://metamask.github.io/eth-ledger-bridge-keyring',
},
) {
this.#validateConfiguration(opts);
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);
}
Expand All @@ -122,6 +141,19 @@ export class LedgerIframeBridge implements LedgerBridge {
}
}

async getOptions(): Promise<LedgerIframeBridgeOptions> {
return this.#opts;
}

async setOptions(opts: LedgerIframeBridgeOptions): Promise<void> {
this.#validateConfiguration(opts);
if (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(
Expand Down Expand Up @@ -324,4 +356,10 @@ export class LedgerIframeBridge implements LedgerBridge {

this.iframe.contentWindow.postMessage(postMsg, '*');
}

#validateConfiguration(opts: LedgerIframeBridgeOptions): void {
if (typeof opts.bridgeUrl !== 'string' || opts.bridgeUrl.length === 0) {
throw new Error('bridgeURL is not a valid URL');
}
}
}
17 changes: 7 additions & 10 deletions src/ledger-keyring.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -85,8 +88,7 @@ const fakeTypeTwoTx = TransactionFactory.fromTxData(

describe('LedgerKeyring', function () {
let keyring: LedgerKeyring;
let bridge: LedgerBridge;

let bridge: LedgerBridge<LedgerIframeBridgeOptions>;
/**
* Sets up the keyring to unlock one account.
*
Expand Down Expand Up @@ -140,7 +142,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');
});
Expand All @@ -161,9 +164,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);
Expand All @@ -188,9 +188,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);
});
Expand Down
Loading

0 comments on commit 2cc856a

Please sign in to comment.