Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: refactor to enable additional bridge #210

8 changes: 4 additions & 4 deletions jest.config.js
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
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
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
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
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
Loading