Skip to content

Commit

Permalink
Fix bug with GridPlus pairing after removing permissions (#23)
Browse files Browse the repository at this point in the history
* Fix bug with GridPlus pairing after removing permissions

* Bump version

* Add test for new edge case
  • Loading branch information
FrederikBolding authored Nov 23, 2021
1 parent a6bff71 commit 9178670
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 17 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@mycrypto/wallets",
"version": "1.4.0",
"version": "1.4.1",
"description": "Wallet abstractions to be used throughout the MyCrypto product suite.",
"repository": "MyCryptoHQ/wallets",
"author": "MyCrypto",
Expand Down
18 changes: 15 additions & 3 deletions src/implementations/deterministic/__mocks__/gridplus-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,24 @@ const convertPathToString = (path: number[]): string =>
export class Client {
isPaired = false;
hasActiveWallet = jest.fn().mockReturnValue(true);
pair = jest
.fn()
.mockImplementation(
(_secret: string, callback: (err: Error | null, hasActiveWallet: boolean) => void) => {
// For now we only pair and expect it to fail
callback(new Error('Failed to pair'), false);
}
);
connect = jest
.fn()
.mockImplementation(
(_deviceID: string, callback: (err: Error | null, isPaired: boolean) => void) => {
this.isPaired = true;
callback(null, true);
(deviceID: string, callback: (err: Error | null, isPaired: boolean) => void) => {
if (deviceID === 'foo') {
this.isPaired = true;
callback(null, true);
} else {
callback(null, false);
}
}
);
sign = jest
Expand Down
17 changes: 17 additions & 0 deletions src/implementations/deterministic/gridplus.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,23 @@ describe('GridPlusWalletInstance', () => {
await expect(instance.signTransaction(fTransactionRequest)).resolves.toBe(fSignedTx);
});

it('handles pairing using popup if invalid credentials', async () => {
const postMessage = jest.fn();
window.open = jest.fn().mockReturnValue({ postMessage });
window.addEventListener = jest.fn().mockImplementation((_type, callback) => {
callback({ origin, data: JSON.stringify(config) });
});

const wallet = new GridPlusWallet({ ...config, deviceID: 'somethingelse' });
const instance = await wallet.getWallet(DEFAULT_ETH, 0);

expect(window.open).toHaveBeenCalled();
expect(postMessage).toHaveBeenCalled();
expect(window.addEventListener).toHaveBeenCalled();

await expect(instance.signTransaction(fTransactionRequest)).resolves.toBe(fSignedTx);
});

it('rejects if credentials are not in response', async () => {
const postMessage = jest.fn();
window.open = jest.fn().mockReturnValue({ postMessage });
Expand Down
30 changes: 17 additions & 13 deletions src/implementations/deterministic/gridplus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ export interface GridPlusCredentials {
password?: string;
}

// @todo Cleanup / polish

const getPrivateKey = (config: GridPlusConfiguration) => {
const buf = Buffer.concat([
Buffer.from(config.password!),
Expand Down Expand Up @@ -83,30 +81,36 @@ const waitForPairing = (config: GridPlusConfiguration): Promise<GridPlusCredenti
});
};

const getClient = async (config: GridPlusConfiguration, client?: Client) => {
const getClient = async (
config: GridPlusConfiguration,
client?: Client
): Promise<{ config: GridPlusConfiguration; client: Client }> => {
if (client?.isPaired && client?.hasActiveWallet()) {
return { client, config };
}

const { deviceID, password, ...clientConfig } = config;

if (client === undefined && deviceID !== undefined && password !== undefined) {
const privKey = getPrivateKey(config);
client = new Client({ ...clientConfig, privKey, crypto });
}

if (client && deviceID !== undefined && password !== undefined) {
const connect = promisify(client.connect).bind(client);

const isPaired = await connect(deviceID);
const isPaired = await connect(deviceID).catch(wrapGridPlusError);
if (isPaired) {
return { client, config };
} else {
// Hack to dismiss pairing screen
const pair = promisify(client.pair).bind(client);
await pair('').catch(() => null);
}
} else if (deviceID === undefined || password === undefined) {
const result = await waitForPairing(config);
config = { ...config, ...result };
}

if (client === undefined) {
const privKey = getPrivateKey(config);
client = new Client({ ...clientConfig, privKey, crypto });
}

return { client, config };
const result = await waitForPairing(config);
return getClient({ ...clientConfig, ...result }, client);
};

export class GridPlusWalletInstance implements Wallet {
Expand Down
1 change: 1 addition & 0 deletions src/types/vendor/gridplus-sdk/client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ declare module 'gridplus-sdk' {
isPaired: boolean;
connect(deviceID: string, callback: (err: Error | null, isPaired: boolean) => void): void;
sign(opts: SignOpts, callback: (err: Error | null, data: SignResult) => void): void;
pair(secret: string, callback: (err: Error | null, hasActiveWallet: boolean) => void): void;
getAddresses(opts: AddressesOpts, callback: (err: Error | null, data: string[]) => void): void;
hasActiveWallet(): boolean;
}
Expand Down

0 comments on commit 9178670

Please sign in to comment.