From 718da53fd7eeca290c70e77187e4603e980f1905 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Wed, 3 Jul 2024 09:31:23 +0100 Subject: [PATCH 1/6] fix: Currently ledger keyring throw `unknown error` when user didn't turn on `eth` app in ledger. this code change provide better error message to indicate what happen. --- .gitignore | 3 +++ src/ledger-keyring.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index d54c2ba0..344f55d9 100644 --- a/.gitignore +++ b/.gitignore @@ -76,3 +76,6 @@ node_modules/ !.yarn/releases !.yarn/sdks !.yarn/versions + +# exclude webstorm profile file +.idea/ diff --git a/src/ledger-keyring.ts b/src/ledger-keyring.ts index ee8a7f68..067767ce 100644 --- a/src/ledger-keyring.ts +++ b/src/ledger-keyring.ts @@ -212,7 +212,7 @@ export class LedgerKeyring extends EventEmitter { hdPath: path, }); } catch (error) { - throw error instanceof Error ? error : new Error('Unknown error'); + throw error instanceof Error ? error : new Error('Unlock your Ledger device and open the ETH app'); } if (updateHdk && payload.chainCode) { From 612fa6865f8f8f77406fe81c8e4b917e418be620 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Mon, 8 Jul 2024 09:52:36 +0800 Subject: [PATCH 2/6] fix: Currently ledger keyring throw `unknown error` when user lock ledger or didn't turn on `eth` app in ledger in MM extension. this code change provide better error message to indicate what happen. --- src/ledger-keyring.test.ts | 2 +- src/ledger-keyring.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ledger-keyring.test.ts b/src/ledger-keyring.test.ts index 3f60d5a7..c19980c1 100644 --- a/src/ledger-keyring.test.ts +++ b/src/ledger-keyring.test.ts @@ -353,7 +353,7 @@ describe('LedgerKeyring', function () { it('throws an error when the bridge getPublicKey method throws an error and it is not an error type', async function () { keyring.setHdPath(`m/44'/60'/0'/0`); jest.spyOn(bridge, 'getPublicKey').mockRejectedValue('Some error'); - await expect(keyring.unlock()).rejects.toThrow('Unknown error'); + await expect(keyring.unlock()).rejects.toThrow('Unlock your Ledger device and open the ETH app'); }); }); diff --git a/src/ledger-keyring.ts b/src/ledger-keyring.ts index 067767ce..e507640c 100644 --- a/src/ledger-keyring.ts +++ b/src/ledger-keyring.ts @@ -212,7 +212,9 @@ export class LedgerKeyring extends EventEmitter { hdPath: path, }); } catch (error) { - throw error instanceof Error ? error : new Error('Unlock your Ledger device and open the ETH app'); + throw error instanceof Error + ? error + : new Error('Unlock your Ledger device and open the ETH app'); } if (updateHdk && payload.chainCode) { From 66fb23a271d77b8756c9b2c053b642a9d2ea6a67 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Mon, 8 Jul 2024 09:52:56 +0800 Subject: [PATCH 3/6] fix: Currently ledger keyring throw `unknown error` when user lock ledger or didn't turn on `eth` app in ledger in MM extension. this code change provide better error message to indicate what happen. --- src/ledger-keyring.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ledger-keyring.test.ts b/src/ledger-keyring.test.ts index c19980c1..8244939d 100644 --- a/src/ledger-keyring.test.ts +++ b/src/ledger-keyring.test.ts @@ -353,7 +353,9 @@ describe('LedgerKeyring', function () { it('throws an error when the bridge getPublicKey method throws an error and it is not an error type', async function () { keyring.setHdPath(`m/44'/60'/0'/0`); jest.spyOn(bridge, 'getPublicKey').mockRejectedValue('Some error'); - await expect(keyring.unlock()).rejects.toThrow('Unlock your Ledger device and open the ETH app'); + await expect(keyring.unlock()).rejects.toThrow( + 'Unlock your Ledger device and open the ETH app', + ); }); }); From b2505bb70907a9fa542607af15e00b8a9f66c08d Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Thu, 11 Jul 2024 21:49:09 +0800 Subject: [PATCH 4/6] feat: Remove duplicate tests. --- src/ledger-keyring.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/ledger-keyring.test.ts b/src/ledger-keyring.test.ts index 7ea1c173..c8598cee 100644 --- a/src/ledger-keyring.test.ts +++ b/src/ledger-keyring.test.ts @@ -337,7 +337,7 @@ describe('LedgerKeyring', function () { 'Unlock your Ledger device and open the ETH app', ); }); - + it('does not update hdk.publicKey if updateHdk is false', async function () { // @ts-expect-error we want to bypass the publicKey property set method keyring.hdk = { publicKey: 'ABC' }; @@ -374,12 +374,6 @@ describe('LedgerKeyring', function () { .mockRejectedValue(new Error('Some error')); await expect(keyring.unlock()).rejects.toThrow('Some error'); }); - - it('throws an error when the bridge getPublicKey method throws an error and it is not an error type', async function () { - keyring.setHdPath(`m/44'/60'/0'/0`); - jest.spyOn(bridge, 'getPublicKey').mockRejectedValue('Some error'); - await expect(keyring.unlock()).rejects.toThrow('Unknown error'); - }); }); describe('addAccounts', function () { From c75965d1ecea140dc31e5761479261c8abf4c25e Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Fri, 12 Jul 2024 16:03:51 +0800 Subject: [PATCH 5/6] feat: Change the error message to `Ledger Ethereum app closed. Open it to unlock.` --- src/ledger-keyring.test.ts | 2 +- src/ledger-keyring.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ledger-keyring.test.ts b/src/ledger-keyring.test.ts index c8598cee..553fcf4d 100644 --- a/src/ledger-keyring.test.ts +++ b/src/ledger-keyring.test.ts @@ -334,7 +334,7 @@ describe('LedgerKeyring', function () { keyring.setHdPath(`m/44'/60'/0'/0`); jest.spyOn(bridge, 'getPublicKey').mockRejectedValue('Some error'); await expect(keyring.unlock()).rejects.toThrow( - 'Unlock your Ledger device and open the ETH app', + 'ULedger Ethereum app closed. Open it to unlock.', ); }); diff --git a/src/ledger-keyring.ts b/src/ledger-keyring.ts index 1c26ca04..b2104799 100644 --- a/src/ledger-keyring.ts +++ b/src/ledger-keyring.ts @@ -215,7 +215,7 @@ export class LedgerKeyring extends EventEmitter { } catch (error) { throw error instanceof Error ? error - : new Error('Unlock your Ledger device and open the ETH app'); + : new Error('Ledger Ethereum app closed. Open it to unlock.'); } if (updateHdk && payload.chainCode) { From 27bbbcd4f08b845deb69769179bc488871b89f88 Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Fri, 12 Jul 2024 16:24:45 +0800 Subject: [PATCH 6/6] feat: Change the error message to `Ledger Ethereum app closed. Open it to unlock.` --- src/ledger-keyring.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ledger-keyring.test.ts b/src/ledger-keyring.test.ts index 553fcf4d..f2845840 100644 --- a/src/ledger-keyring.test.ts +++ b/src/ledger-keyring.test.ts @@ -334,7 +334,7 @@ describe('LedgerKeyring', function () { keyring.setHdPath(`m/44'/60'/0'/0`); jest.spyOn(bridge, 'getPublicKey').mockRejectedValue('Some error'); await expect(keyring.unlock()).rejects.toThrow( - 'ULedger Ethereum app closed. Open it to unlock.', + 'Ledger Ethereum app closed. Open it to unlock.', ); });