fix: legacy transaction address recovery (#2195)

* chore: update snap * fix: legacy transaction address recovery * chore: changeset * update * chore: update vectors
wevm · Apr 28, 2024 · 79ec577 · 79ec577
1 parent 8d4e21b
commit 79ec577
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 25 deletions.
diff --git a/.changeset/angry-files-kiss.md b/.changeset/angry-files-kiss.md
@@ -0,0 +1,5 @@
+---
+"viem": patch
+---
+
+Fixed legacy transaction address recovery.
diff --git a/src/utils/signature/recoverTransactionAddress.test.ts b/src/utils/signature/recoverTransactionAddress.test.ts
@@ -14,6 +14,7 @@ import { kzg } from '../../../test/src/kzg.js'
 import type {
   TransactionSerializable,
   TransactionSerializableEIP4844,
+  TransactionSerializedLegacy,
 } from '../../types/transaction.js'
 import { sidecarsToVersionedHashes } from '../blob/sidecarsToVersionedHashes.js'
 import { toBlobSidecars } from '../blob/toBlobSidecars.js'
@@ -132,3 +133,12 @@ test('via `getTransaction`', async () => {
   })
   expect(address.toLowerCase()).toBe(transaction.from)
 })
+
+test('legacy', async () => {
+  expect(
+    await recoverTransactionAddress({
+      serializedTransaction:
+        '0xf8a90c8507558bdb0082d57a948813f5bcbe6c7071d8bd32d2a4f07599bb5797b080b844a9059cbb00000000000000000000000068674fb6a9ee3749d5d8f71eeed5f254a75ffeea0000000000000000000000000000000000000000000001894b59bd5cd2fc000026a00d39b9cb3369c546185f4ddd6ee6908052c39fe856642726fa93f9a2a83db755a06a8c3928a80275ecef8a0ff00486483f8db6274b5ffd44fbcb8765418e9bec26' as TransactionSerializedLegacy,
+    }),
+  ).toMatchInlineSnapshot(`"0xb03B8ffAB1f3Ac3CabE4A0B2ED441fDFd3C96C8E"`)
+})
diff --git a/src/utils/signature/recoverTransactionAddress.ts b/src/utils/signature/recoverTransactionAddress.ts
@@ -12,10 +12,7 @@ import {
   type RecoverAddressErrorType,
   recoverAddress,
 } from './recoverAddress.js'
-import {
-  type SignatureToHexErrorType,
-  signatureToHex,
-} from './signatureToHex.js'
+import type { SignatureToHexErrorType } from './signatureToHex.js'
 
 export type RecoverTransactionAddressParameters = {
   serializedTransaction: TransactionSerialized
@@ -38,14 +35,12 @@ export async function recoverTransactionAddress(
 
   const transaction = parseTransaction(serializedTransaction)
 
-  const signature =
-    signature_ ??
-    signatureToHex({
-      r: transaction.r!,
-      s: transaction.s!,
-      v: transaction.v!,
-      yParity: transaction.yParity!,
-    })
+  const signature = signature_ ?? {
+    r: transaction.r!,
+    s: transaction.s!,
+    v: transaction.v!,
+    yParity: transaction.yParity!,
+  }
 
   const serialized = serializeTransaction({
     ...transaction,

diff --git a/src/utils/signature/signatureToHex.test.ts b/src/utils/signature/signatureToHex.test.ts
@@ -32,4 +32,46 @@ test('default', () => {
   ).toMatchInlineSnapshot(
     '"0x6e100a352ec6ad1b70802290e18aeed190704973570f3b8ed42cb9808e2ea6bf4a90a229a244495b41890987806fcbd2d5d23fc0dbe5f5256c2613c039d76db81b"',
   )
+
+  expect(
+    signatureToHex({
+      r: toHex(
+        49782753348462494199823712700004552394425719014458918871452329774910450607807n,
+      ),
+      s: toHex(
+        33726695977844476214676913201140481102225469284307016937915595756355928419768n,
+      ),
+      v: 27n,
+    }),
+  ).toMatchInlineSnapshot(
+    '"0x6e100a352ec6ad1b70802290e18aeed190704973570f3b8ed42cb9808e2ea6bf4a90a229a244495b41890987806fcbd2d5d23fc0dbe5f5256c2613c039d76db81b"',
+  )
+
+  expect(
+    signatureToHex({
+      r: toHex(
+        49782753348462494199823712700004552394425719014458918871452329774910450607807n,
+      ),
+      s: toHex(
+        33726695977844476214676913201140481102225469284307016937915595756355928419768n,
+      ),
+      v: 28n,
+    }),
+  ).toMatchInlineSnapshot(
+    '"0x6e100a352ec6ad1b70802290e18aeed190704973570f3b8ed42cb9808e2ea6bf4a90a229a244495b41890987806fcbd2d5d23fc0dbe5f5256c2613c039d76db81c"',
+  )
+
+  expect(
+    signatureToHex({
+      r: toHex(
+        49782753348462494199823712700004552394425719014458918871452329774910450607807n,
+      ),
+      s: toHex(
+        33726695977844476214676913201140481102225469284307016937915595756355928419768n,
+      ),
+      v: 35n,
+    }),
+  ).toMatchInlineSnapshot(
+    '"0x6e100a352ec6ad1b70802290e18aeed190704973570f3b8ed42cb9808e2ea6bf4a90a229a244495b41890987806fcbd2d5d23fc0dbe5f5256c2613c039d76db81b"',
+  )
 })
diff --git a/src/utils/signature/signatureToHex.ts b/src/utils/signature/signatureToHex.ts
@@ -6,7 +6,7 @@ import {
   type HexToBigIntErrorType,
   hexToBigInt,
 } from '../../utils/encoding/fromHex.js'
-import { type ToHexErrorType } from '../../utils/encoding/toHex.js'
+import type { ToHexErrorType } from '../../utils/encoding/toHex.js'
 
 export type SignatureToHexErrorType =
   | HexToBigIntErrorType
@@ -28,13 +28,13 @@ export type SignatureToHexErrorType =
  * // "0x6e100a352ec6ad1b70802290e18aeed190704973570f3b8ed42cb9808e2ea6bf4a90a229a244495b41890987806fcbd2d5d23fc0dbe5f5256c2613c039d76db81c"
  */
 export function signatureToHex({ r, s, v, yParity }: Signature): Hex {
-  const vHex = (() => {
-    if (v === 27n || yParity === 0) return '1b'
-    if (v === 28n || yParity === 1) return '1c'
-    throw new Error('Invalid v value')
+  const yParity_ = (() => {
+    if (yParity === 0 || yParity === 1) return yParity
+    if (v && (v === 27n || v === 28n || v >= 35n)) return v % 2n === 0n ? 1 : 0
+    throw new Error('Invalid `v` or `yParity` value')
   })()
   return `0x${new secp256k1.Signature(
     hexToBigInt(r),
     hexToBigInt(s),
-  ).toCompactHex()}${vHex}`
+  ).toCompactHex()}${yParity_ === 0 ? '1b' : '1c'}`
 }
diff --git a/src/utils/transaction/parseTransaction.test.ts b/src/utils/transaction/parseTransaction.test.ts
@@ -710,7 +710,7 @@ describe('legacy', () => {
     expect(parseTransaction(serialized)).toEqual({
       ...baseLegacy,
       ...signature,
-      yParity: undefined,
+      yParity: 1,
       type: 'legacy',
     })
   })
@@ -728,7 +728,7 @@ describe('legacy', () => {
     expect(parseTransaction(serialized)).toEqual({
       ...args,
       ...signature,
-      yParity: undefined,
+      yParity: 0,
       type: 'legacy',
       v: 173n,
     })
@@ -803,6 +803,7 @@ describe('legacy', () => {
           "type": "legacy",
           "v": 27n,
           "value": 0n,
+          "yParity": 0,
         }
       `)
     })

diff --git a/src/utils/transaction/parseTransaction.ts b/src/utils/transaction/parseTransaction.ts
@@ -422,10 +422,10 @@ function parseTransactionLegacy(
   if (chainId > 0) transaction.chainId = chainId
   else if (v !== 27n && v !== 28n) throw new InvalidLegacyVError({ v })
 
-  delete transaction.yParity
   transaction.v = v
   transaction.s = s as Hex
   transaction.r = r as Hex
+  transaction.yParity = v % 2n === 0n ? 1 : 0
 
   return transaction
 }

diff --git a/src/utils/transaction/serializeTransaction.test.ts b/src/utils/transaction/serializeTransaction.test.ts
@@ -705,7 +705,7 @@ describe('legacy', () => {
     expect(parseTransaction(serialized)).toEqual({
       ...baseLegacy,
       ...signature,
-      yParity: undefined,
+      yParity: 1,
       type: 'legacy',
     })
   })
@@ -755,7 +755,7 @@ describe('legacy', () => {
     expect(parseTransaction(serialized)).toEqual({
       ...args,
       ...signature,
-      yParity: undefined,
+      yParity: 0,
       type: 'legacy',
       v: 173n,
     })

diff --git a/vectors/src/transaction.json.gz b/vectors/src/transaction.json.gz
diff --git a/vectors/src/transaction.ts b/vectors/src/transaction.ts
@@ -241,7 +241,7 @@ export async function generateTransactionVectors() {
 
     const privateKey = generatePrivateKey()
     const serializedSigned = await signTransaction({ privateKey, transaction })
-    const { r, s, v } = parseTransaction(serializedSigned)
+    const { r, s, v, yParity } = parseTransaction(serializedSigned)
 
     writer.write(
       stringify(
@@ -251,7 +251,7 @@ export async function generateTransactionVectors() {
           transaction,
           serialized,
           serializedSigned,
-          signature: { r, s, v },
+          signature: { r, s, v, yParity },
         },
         null,
         2,