Skip to content

Commit 83cfce6

Browse files
dawnseeker8dawnseeker8
authored
feat(keyring-eth-ledger-bridge): enhance error handling for Ledger signing operations (#294)
Added static method to identify Ledger-specific errors and improved error handling in the LedgerKeyring class. Now, specific error messages are thrown for user-rejected transactions and when blind signing is disabled, providing clearer feedback to users during transaction signing. <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> ## Examples <!-- Are there any examples of this change being used in another repository? When considering changes to the MetaMask module template, it's strongly preferred that the change be experimented with in another repository first. This gives reviewers a better sense of how the change works, making it less likely the change will need to be reverted or adjusted later. -->
1 parent f9f56e8 commit 83cfce6

3 files changed

Lines changed: 199 additions & 6 deletions

File tree

‎packages/keyring-eth-ledger-bridge/jest.config.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,10 @@ module.exports = merge(baseConfig, {
2323
// An object that configures minimum threshold enforcement for coverage results
2424
coverageThreshold: {
2525
global: {
26-
branches: 91.89,
26+
branches: 92.77,
2727
functions: 97.87,
28-
lines: 97.16,
29-
statements: 97.19,
28+
lines: 97.28,
29+
statements: 97.31,
3030
},
3131
},
3232
});

‎packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Common, Chain, Hardfork } from '@ethereumjs/common';
22
import { RLP } from '@ethereumjs/rlp';
33
import { TransactionFactory } from '@ethereumjs/tx';
44
import * as ethUtil from '@ethereumjs/util';
5+
import { TransportStatusError } from '@ledgerhq/hw-transport';
56
import * as sigUtil from '@metamask/eth-sig-util';
67
import { bytesToHex, Hex, remove0x } from '@metamask/utils';
78
import EthereumTx from 'ethereumjs-tx';
@@ -784,6 +785,60 @@ describe('LedgerKeyring', function () {
784785
keyring.signTransaction(fakeAccounts[0], fakeTx),
785786
).rejects.toThrow('Ledger: The transaction signature is not valid');
786787
});
788+
789+
it('throws user rejection error when TransportStatusError with code 27013 is thrown', async function () {
790+
await basicSetupToUnlockOneAccount();
791+
const transportError = {
792+
statusCode: 27013,
793+
message: 'Ledger device: (denied by the user?) (0x6985)',
794+
name: 'TransportStatusError',
795+
};
796+
797+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
798+
jest
799+
.spyOn(keyring.bridge, 'deviceSignTransaction')
800+
.mockRejectedValue(transportError);
801+
802+
await expect(
803+
keyring.signTransaction(fakeAccounts[0], fakeTx),
804+
).rejects.toThrow('Ledger: User rejected the transaction');
805+
});
806+
807+
it('throws blind signing error when TransportStatusError with code 27264 is thrown', async function () {
808+
await basicSetupToUnlockOneAccount();
809+
const transportError = {
810+
statusCode: 27264,
811+
message: 'Ledger device: Invalid data received (0x6a80)',
812+
name: 'TransportStatusError',
813+
};
814+
815+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
816+
jest
817+
.spyOn(keyring.bridge, 'deviceSignTransaction')
818+
.mockRejectedValue(transportError);
819+
820+
await expect(
821+
keyring.signTransaction(fakeAccounts[0], fakeTx),
822+
).rejects.toThrow('Ledger: Blind signing must be enabled');
823+
});
824+
825+
it('re-throws TransportStatusError with unknown status code', async function () {
826+
await basicSetupToUnlockOneAccount();
827+
const transportError = {
828+
statusCode: 12345,
829+
message: 'Some other transport error',
830+
name: 'TransportStatusError',
831+
};
832+
833+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
834+
jest
835+
.spyOn(keyring.bridge, 'deviceSignTransaction')
836+
.mockRejectedValue(transportError);
837+
838+
await expect(
839+
keyring.signTransaction(fakeAccounts[0], fakeTx),
840+
).rejects.toThrow(transportError);
841+
});
787842
});
788843

789844
describe('signPersonalMessage', function () {
@@ -858,6 +913,40 @@ describe('LedgerKeyring', function () {
858913
'Ledger: The signature doesnt match the right address',
859914
);
860915
});
916+
917+
it('throws user rejection error when TransportStatusError with code 27013 is thrown in signPersonalMessage', async function () {
918+
await basicSetupToUnlockOneAccount();
919+
const transportError = {
920+
statusCode: 27013,
921+
message: 'Ledger device: (denied by the user?) (0x6985)',
922+
name: 'TransportStatusError',
923+
};
924+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
925+
jest
926+
.spyOn(keyring.bridge, 'deviceSignMessage')
927+
.mockRejectedValue(transportError);
928+
929+
await expect(
930+
keyring.signPersonalMessage(fakeAccounts[0], 'some message'),
931+
).rejects.toThrow('Ledger: User rejected the transaction');
932+
});
933+
934+
it('re-throws TransportStatusError with unknown status code in signPersonalMessage', async function () {
935+
await basicSetupToUnlockOneAccount();
936+
const transportError = {
937+
statusCode: 12345,
938+
message: 'Some other transport error',
939+
name: 'TransportStatusError',
940+
};
941+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
942+
jest
943+
.spyOn(keyring.bridge, 'deviceSignMessage')
944+
.mockRejectedValue(transportError);
945+
946+
await expect(
947+
keyring.signPersonalMessage(fakeAccounts[0], 'some message'),
948+
).rejects.toThrow(transportError);
949+
});
861950
});
862951

863952
describe('signMessage', function () {
@@ -1207,6 +1296,60 @@ describe('LedgerKeyring', function () {
12071296
'0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e3200',
12081297
);
12091298
});
1299+
1300+
it('throws user rejection error when TransportStatusError with code 27013 is thrown in signTypedData', async function () {
1301+
const transportError = {
1302+
statusCode: 27013,
1303+
message: 'Ledger device: (denied by the user?) (0x6985)',
1304+
name: 'TransportStatusError',
1305+
};
1306+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
1307+
jest
1308+
.spyOn(keyring.bridge, 'deviceSignTypedData')
1309+
.mockRejectedValue(transportError);
1310+
1311+
await expect(
1312+
keyring.signTypedData(fakeAccounts[15], fixtureData, {
1313+
version: sigUtil.SignTypedDataVersion.V4,
1314+
}),
1315+
).rejects.toThrow('Ledger: User rejected the transaction');
1316+
});
1317+
1318+
it('throws blind signing error when TransportStatusError with code 27264 is thrown in signTypedData', async function () {
1319+
const transportError = {
1320+
statusCode: 27264,
1321+
message: 'Ledger device: Invalid data received (0x6a80)',
1322+
name: 'TransportStatusError',
1323+
};
1324+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
1325+
jest
1326+
.spyOn(keyring.bridge, 'deviceSignTypedData')
1327+
.mockRejectedValue(transportError);
1328+
1329+
await expect(
1330+
keyring.signTypedData(fakeAccounts[15], fixtureData, {
1331+
version: sigUtil.SignTypedDataVersion.V4,
1332+
}),
1333+
).rejects.toThrow('Ledger: Blind signing must be enabled');
1334+
});
1335+
1336+
it('re-throws TransportStatusError with unknown status code in signTypedData', async function () {
1337+
const transportError = {
1338+
statusCode: 12345,
1339+
message: 'Some other transport error',
1340+
name: 'TransportStatusError',
1341+
};
1342+
Object.setPrototypeOf(transportError, TransportStatusError.prototype);
1343+
jest
1344+
.spyOn(keyring.bridge, 'deviceSignTypedData')
1345+
.mockRejectedValue(transportError);
1346+
1347+
await expect(
1348+
keyring.signTypedData(fakeAccounts[15], fixtureData, {
1349+
version: sigUtil.SignTypedDataVersion.V4,
1350+
}),
1351+
).rejects.toThrow(transportError);
1352+
});
12101353
});
12111354

12121355
describe('destroy', function () {

‎packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
type TypedTransaction,
66
} from '@ethereumjs/tx';
77
import { publicToAddress } from '@ethereumjs/util';
8+
import { TransportStatusError } from '@ledgerhq/hw-transport';
89
import type { MessageTypes, TypedMessage } from '@metamask/eth-sig-util';
910
import {
1011
recoverPersonalSignature,
@@ -413,7 +414,26 @@ export class LedgerKeyring implements Keyring {
413414
tx: remove0x(rawTxHex),
414415
hdPath,
415416
});
416-
} catch (error) {
417+
} catch (error: unknown) {
418+
// check whether error is TransportError
419+
if (error instanceof TransportStatusError) {
420+
// cast error to TransportError
421+
const transportError: TransportStatusError = error;
422+
if (
423+
transportError.statusCode === 27013 &&
424+
transportError.message.includes('(denied by the user?) (0x6985)')
425+
) {
426+
throw new Error('Ledger: User rejected the transaction');
427+
}
428+
429+
if (
430+
transportError.statusCode === 27264 &&
431+
transportError.message.includes('Invalid data received (0x6a80)')
432+
) {
433+
throw new Error('Ledger: Blind signing must be enabled');
434+
}
435+
}
436+
417437
throw error instanceof Error
418438
? error
419439
: new Error('Ledger: Unknown error while signing transaction');
@@ -448,7 +468,20 @@ export class LedgerKeyring implements Keyring {
448468
hdPath,
449469
message: remove0x(message),
450470
});
451-
} catch (error) {
471+
} catch (error: unknown) {
472+
if (error instanceof TransportStatusError) {
473+
const transportError: TransportStatusError = error;
474+
/**
475+
* for user rejected the transaction error
476+
*/
477+
if (
478+
transportError.statusCode === 27013 &&
479+
transportError.message.includes('(denied by the user?) (0x6985)')
480+
) {
481+
throw new Error('Ledger: User rejected the transaction');
482+
}
483+
}
484+
452485
throw error instanceof Error
453486
? error
454487
: new Error('Ledger: Unknown error while signing message');
@@ -540,7 +573,24 @@ export class LedgerKeyring implements Keyring {
540573
message,
541574
},
542575
});
543-
} catch (error) {
576+
} catch (error: unknown) {
577+
if (error instanceof TransportStatusError) {
578+
// cast error to TransportError
579+
const transportError: TransportStatusError = error;
580+
if (
581+
transportError.statusCode === 27013 &&
582+
transportError.message.includes('(denied by the user?) (0x6985)')
583+
) {
584+
throw new Error('Ledger: User rejected the transaction');
585+
}
586+
587+
if (
588+
transportError.statusCode === 27264 &&
589+
transportError.message.includes('Invalid data received (0x6a80)')
590+
) {
591+
throw new Error('Ledger: Blind signing must be enabled');
592+
}
593+
}
544594
throw error instanceof Error
545595
? error
546596
: new Error('Ledger: Unknown error while signing message');

0 commit comments

Comments
 (0)