Skip to content

Commit c4b59ee

Browse files
committed
fix: normalize signature v value for proper recovery
Ledger devices may return v as 0 or 1 (modern format), but signature recovery functions like `recoverPersonalSignature` and `recoverTypedSignature` expect v to be 27 or 28 (legacy format per EIP-191/EIP-712). This caused "The signature doesnt match the right address" errors when Ledger returned v=0 or v=1, as the hex conversion produced "00"/"01" instead of "1b"/"1c". The fix normalizes v before hex conversion in both `signPersonalMessage` and `signTypedData` methods.
1 parent f199df0 commit c4b59ee

File tree

2 files changed

+127
-4
lines changed

2 files changed

+127
-4
lines changed

src/ledger-keyring.test.ts

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,82 @@ describe('LedgerKeyring', function () {
840840
'Ledger: The signature doesnt match the right address',
841841
);
842842
});
843+
844+
it('normalizes v=0 to v=27 for proper signature recovery', async function () {
845+
await basicSetupToUnlockOneAccount();
846+
jest
847+
.spyOn(keyring.bridge, 'deviceSignMessage')
848+
.mockResolvedValue({ v: 0, r: 'aabbccdd', s: '11223344' });
849+
850+
jest
851+
.spyOn(sigUtil, 'recoverPersonalSignature')
852+
.mockReturnValue(fakeAccounts[0]);
853+
854+
const result = await keyring.signPersonalMessage(
855+
fakeAccounts[0],
856+
'some message',
857+
);
858+
859+
// v=0 should be normalized to v=27 (0x1b)
860+
expect(result).toBe('0xaabbccdd112233441b');
861+
});
862+
863+
it('normalizes v=1 to v=28 for proper signature recovery', async function () {
864+
await basicSetupToUnlockOneAccount();
865+
jest
866+
.spyOn(keyring.bridge, 'deviceSignMessage')
867+
.mockResolvedValue({ v: 1, r: 'aabbccdd', s: '11223344' });
868+
869+
jest
870+
.spyOn(sigUtil, 'recoverPersonalSignature')
871+
.mockReturnValue(fakeAccounts[0]);
872+
873+
const result = await keyring.signPersonalMessage(
874+
fakeAccounts[0],
875+
'some message',
876+
);
877+
878+
// v=1 should be normalized to v=28 (0x1c)
879+
expect(result).toBe('0xaabbccdd112233441c');
880+
});
881+
882+
it('preserves v=27 unchanged', async function () {
883+
await basicSetupToUnlockOneAccount();
884+
jest
885+
.spyOn(keyring.bridge, 'deviceSignMessage')
886+
.mockResolvedValue({ v: 27, r: 'aabbccdd', s: '11223344' });
887+
888+
jest
889+
.spyOn(sigUtil, 'recoverPersonalSignature')
890+
.mockReturnValue(fakeAccounts[0]);
891+
892+
const result = await keyring.signPersonalMessage(
893+
fakeAccounts[0],
894+
'some message',
895+
);
896+
897+
// v=27 should remain as 0x1b
898+
expect(result).toBe('0xaabbccdd112233441b');
899+
});
900+
901+
it('preserves v=28 unchanged', async function () {
902+
await basicSetupToUnlockOneAccount();
903+
jest
904+
.spyOn(keyring.bridge, 'deviceSignMessage')
905+
.mockResolvedValue({ v: 28, r: 'aabbccdd', s: '11223344' });
906+
907+
jest
908+
.spyOn(sigUtil, 'recoverPersonalSignature')
909+
.mockReturnValue(fakeAccounts[0]);
910+
911+
const result = await keyring.signPersonalMessage(
912+
fakeAccounts[0],
913+
'some message',
914+
);
915+
916+
// v=28 should remain as 0x1c
917+
expect(result).toBe('0xaabbccdd112233441c');
918+
});
843919
});
844920

845921
describe('signMessage', function () {
@@ -1043,7 +1119,9 @@ describe('LedgerKeyring', function () {
10431119
).rejects.toThrow('Ledger: Unknown error while signing message');
10441120
});
10451121

1046-
it('returns signature when recoveryId length < 2', async function () {
1122+
it('normalizes v=0 to v=27 for proper signature recovery', async function () {
1123+
// Ledger may return v as 0 or 1 (modern format), but signature
1124+
// recovery expects 27 or 28 (legacy format)
10471125
jest.spyOn(keyring.bridge, 'deviceSignTypedData').mockResolvedValue({
10481126
v: 0,
10491127
r: '72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b9',
@@ -1054,8 +1132,41 @@ describe('LedgerKeyring', function () {
10541132
fixtureData,
10551133
options,
10561134
);
1135+
// v=0 should be normalized to v=27 (0x1b)
10571136
expect(result).toBe(
1058-
'0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e3200',
1137+
'0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b',
1138+
);
1139+
});
1140+
1141+
it('normalizes v=1 to v=28 for proper signature recovery', async function () {
1142+
jest.spyOn(keyring.bridge, 'deviceSignTypedData').mockResolvedValue({
1143+
v: 1,
1144+
r: '72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b9',
1145+
s: '46759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e32',
1146+
});
1147+
1148+
// v=1 should be normalized to v=28 (0x1c), but this will fail
1149+
// address recovery since the correct v for this signature is 27
1150+
await expect(
1151+
keyring.signTypedData(fakeAccounts[15], fixtureData, options),
1152+
).rejects.toThrow(
1153+
'Ledger: The signature doesnt match the right address',
1154+
);
1155+
});
1156+
1157+
it('preserves v=27 unchanged', async function () {
1158+
jest.spyOn(keyring.bridge, 'deviceSignTypedData').mockResolvedValue({
1159+
v: 27,
1160+
r: '72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b9',
1161+
s: '46759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e32',
1162+
});
1163+
const result = await keyring.signTypedData(
1164+
fakeAccounts[15],
1165+
fixtureData,
1166+
options,
1167+
);
1168+
expect(result).toBe(
1169+
'0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b',
10591170
);
10601171
});
10611172
});

src/ledger-keyring.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,13 @@ export class LedgerKeyring extends EventEmitter {
425425
: new Error('Ledger: Unknown error while signing message');
426426
}
427427

428-
let modifiedV = parseInt(String(payload.v), 10).toString(16);
428+
let recoveryParam = parseInt(String(payload.v), 10);
429+
// Normalize: Ledger may return 0 or 1 (modern format), but signature
430+
// recovery expects 27 or 28 (legacy format per EIP-191)
431+
if (recoveryParam === 0 || recoveryParam === 1) {
432+
recoveryParam += 27;
433+
}
434+
let modifiedV = recoveryParam.toString(16);
429435
if (modifiedV.length < 2) {
430436
modifiedV = `0${modifiedV}`;
431437
}
@@ -511,7 +517,13 @@ export class LedgerKeyring extends EventEmitter {
511517
: new Error('Ledger: Unknown error while signing message');
512518
}
513519

514-
let recoveryId = parseInt(String(payload.v), 10).toString(16);
520+
let recoveryParam = parseInt(String(payload.v), 10);
521+
// Normalize: Ledger may return 0 or 1 (modern format), but signature
522+
// recovery expects 27 or 28 (legacy format per EIP-712)
523+
if (recoveryParam === 0 || recoveryParam === 1) {
524+
recoveryParam += 27;
525+
}
526+
let recoveryId = recoveryParam.toString(16);
515527
if (recoveryId.length < 2) {
516528
recoveryId = `0${recoveryId}`;
517529
}

0 commit comments

Comments
 (0)