diff --git a/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts b/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts index c6b1ec7b1..453da5d06 100644 --- a/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts +++ b/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts @@ -49,16 +49,33 @@ export class LedgerSigner implements Signer { try { const validatedDerivationPath = await this.getValidatedDerivationPath() await this.checkForKnownToken(encodedTx) - const signature = await this.ledger!.signTransaction( + let { + r, + s, + v: _v, + } = await this.ledger!.signTransaction( validatedDerivationPath, trimLeading0x(encodedTx.rlpEncode), // the ledger requires the rlpEncode without the leading 0x null ) + if (typeof _v === 'string' && (_v === '' || _v === '0x')) { + console.warn( + `ledger-signer@signTransaction: signature \`v\` was malformed \`${_v}\`. Replaced with "0x0"` + ) + _v = '0x0' + } + const v = typeof _v === 'string' ? parseInt(ensureLeading0x(_v), 16) : _v + if (isNaN(v)) { + throw new Error( + `ledger-signer@signTransaction: signature \`v\` was malformed and was parsed to NaN \`${_v}\`` + ) + } + return { - v: parseInt(signature.v, 16), - r: ethUtil.toBuffer(ensureLeading0x(signature.r)), - s: ethUtil.toBuffer(ensureLeading0x(signature.s)), + v, + r: ethUtil.toBuffer(ensureLeading0x(r)), + s: ethUtil.toBuffer(ensureLeading0x(s)), } } catch (error: unknown) { if (error instanceof TransportStatusError) { diff --git a/packages/viem-account-ledger/src/ledger-to-account.test.ts b/packages/viem-account-ledger/src/ledger-to-account.test.ts index 1788614ed..0fdb43fb6 100644 --- a/packages/viem-account-ledger/src/ledger-to-account.test.ts +++ b/packages/viem-account-ledger/src/ledger-to-account.test.ts @@ -2,7 +2,7 @@ import { recoverTransaction } from '@celo/wallet-base' import TransportNodeHid from '@ledgerhq/hw-transport-node-hid' import { beforeAll, describe, expect, it, test, vi } from 'vitest' import { ledgerToAccount } from './ledger-to-account.js' -import { mockLedger, TEST_CHAIN_ID } from './test-utils.js' +import { mockLedger, TEST_CHAIN_ID, test_ledger } from './test-utils.js' import { generateLedger } from './utils.js' const USE_PHYSICAL_LEDGER = process.env.USE_PHYSICAL_LEDGER === 'true' @@ -13,24 +13,24 @@ const transport = USE_PHYSICAL_LEDGER ? TransportNodeHid.open('') : Promise.resolve(undefined as unknown as TransportNodeHid) +vi.mock('./utils.js', async () => { + const module = await vi.importActual('./utils.js') + + return { + ...module, + generateLedger: vi.fn((...args) => + // @ts-expect-error + USE_PHYSICAL_LEDGER ? module.generateLedger(...args) : Promise.resolve(mockLedger()) + ), + } +}) + syntheticDescribe('ledgerToAccount (mocked ledger)', () => { let account: Awaited> beforeAll(async () => { account = await ledgerToAccount({ transport: await transport, }) - - vi.mock('./utils.js', async () => { - const module = await vi.importActual('./utils.js') - - return { - ...module, - generateLedger: vi.fn((...args) => - // @ts-expect-error - USE_PHYSICAL_LEDGER ? module.generateLedger(...args) : Promise.resolve(mockLedger()) - ), - } - }) }) it('can be setup', async () => { @@ -47,7 +47,7 @@ syntheticDescribe('ledgerToAccount (mocked ledger)', () => { maxPriorityFeePerGas: BigInt(100), } as const - describe('eip1559', async () => { + describe('eip1559', () => { test('v=0', async () => { const txHash = await account.signTransaction(txData) expect(txHash).toMatchInlineSnapshot( @@ -70,7 +70,7 @@ syntheticDescribe('ledgerToAccount (mocked ledger)', () => { }) }) - describe('cip64', async () => { + describe('cip64', () => { test('v=0', async () => { const account = await ledgerToAccount({ transport: await transport, @@ -100,6 +100,32 @@ syntheticDescribe('ledgerToAccount (mocked ledger)', () => { expect(decoded.yParity).toMatchInlineSnapshot(`1`) }) }) + + describe('malformed v values', () => { + test('empty string', async () => { + const test_vs_0_and_1 = [ + [0, '', '00', '0x', '0x0', '0x00', '0x1b', 27], // yParity 0 + [1, '1', '0x1', '0x01', '01', '0x1c', 28], // vParity 1 + ] + for (const expectedyParity in test_vs_0_and_1) { + const test_vs = test_vs_0_and_1[+expectedyParity] + for (const v of test_vs) { + vi.spyOn(test_ledger, 'signTransaction').mockImplementationOnce(() => + // @ts-expect-error + Promise.resolve({ + v, + r: '0x1', + s: '0x1', + }) + ) + const txHash = await account.signTransaction(txData) + const [recovered] = recoverTransaction(txHash) + // @ts-expect-error + expect(recovered.yParity).toBe(+expectedyParity) + } + } + }) + }) }) it('signs messages', async () => { diff --git a/packages/viem-account-ledger/src/ledger-to-account.ts b/packages/viem-account-ledger/src/ledger-to-account.ts index 99f55424a..7f0d89f69 100644 --- a/packages/viem-account-ledger/src/ledger-to-account.ts +++ b/packages/viem-account-ledger/src/ledger-to-account.ts @@ -46,11 +46,23 @@ export async function ledgerToAccount({ }) const hash = serializeTransaction(transaction) - const { r, s, v } = await ledger!.signTransaction(derivationPath, trimLeading0x(hash), null) + let { r, s, v: _v } = await ledger!.signTransaction(derivationPath, trimLeading0x(hash), null) + if (typeof _v === 'string' && (_v === '' || _v === '0x')) { + console.warn(`Ledger signature \`v\` was malformed \`${_v}\`. Replaced with "0x0"`) + _v = '0x0' + } + let v: bigint + try { + v = BigInt(typeof _v === 'string' ? ensureLeading0x(_v) : _v) + } catch (err) { + throw new Error( + `Ledger signature \`v\` was malformed and couldn't be parsed \`${_v}\` (Original error: ${err})` + ) + } return serializeTransaction(transaction, { r: ensureLeading0x(r), s: ensureLeading0x(s), - v: BigInt(ensureLeading0x(v)), + v, }) }, diff --git a/packages/viem-account-ledger/src/test-utils.ts b/packages/viem-account-ledger/src/test-utils.ts index e3320812b..63ad8a615 100644 --- a/packages/viem-account-ledger/src/test-utils.ts +++ b/packages/viem-account-ledger/src/test-utils.ts @@ -92,115 +92,126 @@ const TYPED_DATA = { interface Config extends Partial>> {} -export const mockLedger = (config?: Config) => { - const _ledger = { - getAddress: async (derivationPath: string) => { - if (ledgerAddresses[derivationPath]) { - const { address, privateKey } = ledgerAddresses[derivationPath] - return { - address, - derivationPath, - publicKey: privateKeyToAccount(privateKey).publicKey, - } - } +export const test_ledger = { + isMock: true, + getAddress: async (derivationPath: string) => { + if (ledgerAddresses[derivationPath]) { + const { address, privateKey } = ledgerAddresses[derivationPath] return { - address: '', + address, derivationPath, - publicKey: '', + publicKey: privateKeyToAccount(privateKey).publicKey, } - }, - signTransaction: async (derivationPath: string, data: string) => { - if (ledgerAddresses[derivationPath]) { - const hash = getHashFromEncoded(ensureLeading0x(data)) - const { r, s, v } = signTransaction(hash, ledgerAddresses[derivationPath].privateKey) - - return { - v: v.toString(16), - r: r.toString('hex'), - s: s.toString('hex'), - } - } - throw new Error('Invalid Path') - }, - signPersonalMessage: async (derivationPath: string, data: string) => { - if (ledgerAddresses[derivationPath]) { - const dataBuff = ethUtil.toBuffer(ensureLeading0x(data)) - const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff) + } + return { + address: '', + derivationPath, + publicKey: '', + } + }, + signTransaction: async (derivationPath: string, data: string) => { + if (ledgerAddresses[derivationPath]) { + const hash = getHashFromEncoded(ensureLeading0x(data)) + const { r, s, v } = signTransaction(hash, ledgerAddresses[derivationPath].privateKey) - const trimmedKey = trimLeading0x(ledgerAddresses[derivationPath].privateKey) - const pkBuffer = Buffer.from(trimmedKey, 'hex') - const signature = ethUtil.ecsign(msgHashBuff, pkBuffer) - return { - v: Number(signature.v), - r: signature.r.toString('hex'), - s: signature.s.toString('hex'), - } + return { + v: v.toString(16), + r: r.toString('hex'), + s: s.toString('hex'), } - throw new Error('Invalid Path') - }, - signEIP712HashedMessage: async ( - derivationPath: string, - _domainSeparator: string, - _structHash: string - ) => { - const messageHash = generateTypedDataHash(TYPED_DATA) + } + throw new Error('Invalid Path') + }, + signPersonalMessage: async (derivationPath: string, data: string) => { + if (ledgerAddresses[derivationPath]) { + const dataBuff = ethUtil.toBuffer(ensureLeading0x(data)) + const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff) const trimmedKey = trimLeading0x(ledgerAddresses[derivationPath].privateKey) const pkBuffer = Buffer.from(trimmedKey, 'hex') - const signature = ethUtil.ecsign(messageHash, pkBuffer) + const signature = ethUtil.ecsign(msgHashBuff, pkBuffer) return { v: Number(signature.v), r: signature.r.toString('hex'), s: signature.s.toString('hex'), } - }, - getAppConfiguration: async () => { - return { - arbitraryDataEnabled: config?.arbitraryDataEnabled ?? 1, - version: config?.version ?? MIN_VERSION_EIP1559, - erc20ProvisioningNecessary: config?.erc20ProvisioningNecessary ?? 1, - starkEnabled: config?.starkEnabled ?? 1, - starkv2Supported: config?.starkv2Supported ?? 1, - } - }, - provideERC20TokenInformation: async (tokenData: string) => { - let pubkey: VerifyPublicKeyInput - const version = (await _ledger.getAppConfiguration()).version - if ( - meetsVersionRequirements(version, { - minimum: MIN_VERSION_EIP1559, - }) - ) { - // verify with new pubkey - const pubDir = dirname(require.resolve('@celo/ledger-token-signer')) - pubkey = { key: readFileSync(join(pubDir, 'pubkey.pem')).toString() } - } else { - // verify with oldpubkey - pubkey = { key: legacyLedgerPublicKeyHex } - } + } + throw new Error('Invalid Path') + }, + signEIP712HashedMessage: async ( + derivationPath: string, + _domainSeparator: string, + _structHash: string + ) => { + const messageHash = generateTypedDataHash(TYPED_DATA) - const verify = createVerify('sha256') - const tokenDataBuf = Buffer.from(trimLeading0x(tokenData), 'hex') - const BASE_DATA_LENGTH = - 20 + // contract address, 20 bytes - 4 + // decimals, uint32, 4 bytes - 4 // chainId, uint32, 4 bytes - // first byte of data is the ticker length, so we add that to base data length - const dataLen = BASE_DATA_LENGTH + tokenDataBuf.readUint8(0) + const trimmedKey = trimLeading0x(ledgerAddresses[derivationPath].privateKey) + const pkBuffer = Buffer.from(trimmedKey, 'hex') + const signature = ethUtil.ecsign(messageHash, pkBuffer) + return { + v: Number(signature.v), + r: signature.r.toString('hex'), + s: signature.s.toString('hex'), + } + }, + getAppConfiguration: async () => { + return { + arbitraryDataEnabled: 1, + version: MIN_VERSION_EIP1559, + erc20ProvisioningNecessary: 1, + starkEnabled: 1, + starkv2Supported: 1, + } + }, + provideERC20TokenInformation: async (tokenData: string) => { + let pubkey: VerifyPublicKeyInput + const version = (await test_ledger.getAppConfiguration()).version + if ( + meetsVersionRequirements(version, { + minimum: MIN_VERSION_EIP1559, + }) + ) { + // verify with new pubkey + const pubDir = dirname(require.resolve('@celo/ledger-token-signer')) + pubkey = { key: readFileSync(join(pubDir, 'pubkey.pem')).toString() } + } else { + // verify with oldpubkey + pubkey = { key: legacyLedgerPublicKeyHex } + } - // start at 1 since the first byte was just informative - const data = tokenDataBuf.slice(1, dataLen + 1) - verify.update(data) - verify.end() - // read from end of data til the end - const signature = tokenDataBuf.slice(dataLen + 1) - const verified = verify.verify(pubkey, signature) + const verify = createVerify('sha256') + const tokenDataBuf = Buffer.from(trimLeading0x(tokenData), 'hex') + const BASE_DATA_LENGTH = + 20 + // contract address, 20 bytes + 4 + // decimals, uint32, 4 bytes + 4 // chainId, uint32, 4 bytes + // first byte of data is the ticker length, so we add that to base data length + const dataLen = BASE_DATA_LENGTH + tokenDataBuf.readUint8(0) - if (!verified) { - throw new Error('couldnt verify data sent to MockLedger') - } - return verified - }, - } as unknown as Eth - return _ledger + // start at 1 since the first byte was just informative + const data = tokenDataBuf.slice(1, dataLen + 1) + verify.update(data) + verify.end() + // read from end of data til the end + const signature = tokenDataBuf.slice(dataLen + 1) + const verified = verify.verify(pubkey, signature) + + if (!verified) { + throw new Error('couldnt verify data sent to MockLedger') + } + return verified + }, +} as unknown as Eth + +export const mockLedger = (config?: Config) => { + test_ledger.getAppConfiguration = () => + Promise.resolve({ + arbitraryDataEnabled: config?.arbitraryDataEnabled ?? 1, + version: config?.version ?? MIN_VERSION_EIP1559, + erc20ProvisioningNecessary: config?.erc20ProvisioningNecessary ?? 1, + starkEnabled: config?.starkEnabled ?? 1, + starkv2Supported: config?.starkv2Supported ?? 1, + }) + + return test_ledger }