From a74572e5374f9cc430858405c7424c3628914154 Mon Sep 17 00:00:00 2001
From: Holger Drewes <Holger.Drewes@gmail.com>
Date: Tue, 31 May 2022 11:21:22 +0200
Subject: [PATCH] tx: switch to earlier v validation and throwing before v
 common/EIP-155 determination logic

---
 packages/tx/src/legacyTransaction.ts | 23 +++++++++++------------
 packages/tx/test/legacy.spec.ts      |  5 ++++-
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/packages/tx/src/legacyTransaction.ts b/packages/tx/src/legacyTransaction.ts
index 2ebfe4537d..f93e054242 100644
--- a/packages/tx/src/legacyTransaction.ts
+++ b/packages/tx/src/legacyTransaction.ts
@@ -136,18 +136,6 @@ export default class Transaction extends BaseTransaction<Transaction> {
       }
     }
 
-    if (!this.supports(Capability.EIP155ReplayProtection)) {
-      // In case of a signed non-EIP155 tx, `ecrecover` does not get a `chainId` provided
-      // Internally, `ecrecover` will calculate the recovery bit by `v = v - 27`
-      // This recovery bit is either 0 or 1. Thus, any other value than `v = 27` or `v = 28` will
-      // throw upon calling `getSenderAddress` here. Thus throw early in the constructor
-      if (this.v !== undefined) {
-        if (!this.v.eqn(27) && !this.v.eqn(28)) {
-          throw new Error(`Non-EIP155 txs need either v = 27 or v = 28, got v = ${this.v}`)
-        }
-      }
-    }
-
     if (this.common.isActivatedEIP(3860)) {
       checkMaxInitCodeSize(this.common, this.data.length)
     }
@@ -390,6 +378,17 @@ export default class Transaction extends BaseTransaction<Transaction> {
    * Validates tx's `v` value
    */
   private _validateTxV(v?: BN, common?: Common): Common {
+    // Check for valid v values in the scope of a signed legacy tx
+    if (v !== undefined) {
+      // v is 1. not matching the EIP-155 chainId included case and...
+      // v is 2. not matching the classic v=27 or v=28 case
+      if (v.ltn(37) && !v.eqn(27) && !v.eqn(28)) {
+        throw new Error(
+          `Legacy txs need either v = 27/28 or v >= 37 (EIP-155 replay protection), got v = ${v}`
+        )
+      }
+    }
+
     let chainIdBN
     // No unsigned tx and EIP-155 activated and chain ID included
     if (
diff --git a/packages/tx/test/legacy.spec.ts b/packages/tx/test/legacy.spec.ts
index b6d30e8d06..766b7d4a0b 100644
--- a/packages/tx/test/legacy.spec.ts
+++ b/packages/tx/test/legacy.spec.ts
@@ -439,7 +439,7 @@ tape('[Transaction]', function (t) {
   )
 
   t.test(
-    'constructor: throw on non-EIP155 transactions which have v != 27 and v != 28',
+    'constructor: throw on legacy transactions which have v != 27 and v != 28 and v < 37',
     function (st) {
       function getTxData(v: number) {
         return {
@@ -450,8 +450,11 @@ tape('[Transaction]', function (t) {
         st.throws(() => Transaction.fromTxData(getTxData(n)))
       }
       st.throws(() => Transaction.fromTxData(getTxData(29)))
+      st.throws(() => Transaction.fromTxData(getTxData(36)))
+
       st.doesNotThrow(() => Transaction.fromTxData(getTxData(27)))
       st.doesNotThrow(() => Transaction.fromTxData(getTxData(28)))
+      st.doesNotThrow(() => Transaction.fromTxData(getTxData(37)))
       st.end()
     }
   )