tx: set default EIP2930 tx HF to the Common default for backwards-compatibility (EIP2930 tx will throw without explicitly being activated)

Author: holgerd77

packages/tx/src/baseTransaction.ts

@@ -8,7 +8,7 @@ import {
   ecsign,
   publicToAddress,
 } from 'ethereumjs-util'
-import { BaseTransactionData, BaseTxOptions, DEFAULT_COMMON, JsonTx } from './types'
+import { BaseTransactionData, BaseTxOptions, JsonTx } from './types'
 
 export abstract class BaseTransaction<TransactionObject> {
   public readonly nonce: BN
@@ -41,7 +41,7 @@ export abstract class BaseTransaction<TransactionObject> {
     this.common =
       (txOptions.common &&
         Object.assign(Object.create(Object.getPrototypeOf(txOptions.common)), txOptions.common)) ??
-      DEFAULT_COMMON
+      new Common({ chain: 'mainnet' })
   }
 
   protected validateExceedsMaxInteger(validateCannotExceedMaxInteger: { [key: string]: BN }) {

packages/tx/src/types.ts

@@ -184,5 +184,3 @@ export interface JsonTx {
   accessList?: JsonAccessListItem[]
   type?: string
 }
-
-export const DEFAULT_COMMON = new Common({ chain: 'mainnet', hardfork: 'berlin' })

packages/tx/test/base.spec.ts

@@ -6,16 +6,19 @@ import { BaseTransaction } from '../src/baseTransaction'
 import { privateToPublic } from 'ethereumjs-util'
 
 tape('[BaseTransaction]', function (t) {
+  // EIP-2930 is not enabled in Common by default (2021-03-06)
+  const common = new Common({ chain: 'mainnet', hardfork: 'berlin' })
+
   const legacyFixtures: TxsJsonEntry[] = require('./json/txs.json')
   const legacyTxs: BaseTransaction<Transaction>[] = []
   legacyFixtures.slice(0, 4).forEach(function (tx: any) {
-    legacyTxs.push(Transaction.fromTxData(tx.data))
+    legacyTxs.push(Transaction.fromTxData(tx.data, { common }))
   })
 
   const eip2930Fixtures = require('./json/eip2930txs.json')
   const eip2930Txs: BaseTransaction<AccessListEIP2930Transaction>[] = []
   eip2930Fixtures.forEach(function (tx: any) {
-    eip2930Txs.push(AccessListEIP2930Transaction.fromTxData(tx.data))
+    eip2930Txs.push(AccessListEIP2930Transaction.fromTxData(tx.data, { common }))
   })
 
   const zero = Buffer.alloc(0)
@@ -38,58 +41,58 @@ tape('[BaseTransaction]', function (t) {
 
   t.test('Initialization', function (st) {
     for (const txType of txTypes) {
-      let tx = txType.class.fromTxData({})
+      let tx = txType.class.fromTxData({}, { common })
       st.equal(
         tx.common.hardfork(),
         'berlin',
         `${txType.name}: should initialize with correct default HF`
       )
       st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)
 
-      const common = new Common({
+      const initCommon = new Common({
         chain: 'mainnet',
         hardfork: 'istanbul',
         eips: [2718, 2929, 2930],
       })
-      tx = txType.class.fromTxData({}, { common })
+      tx = txType.class.fromTxData({}, { common: initCommon })
       st.equal(
         tx.common.hardfork(),
         'istanbul',
         `${txType.name}: should initialize with correct HF provided`
       )
 
-      common.setHardfork('byzantium')
+      initCommon.setHardfork('byzantium')
       st.equal(
         tx.common.hardfork(),
         'istanbul',
         `${txType.name}: should stay on correct HF if outer common HF changes`
       )
 
-      tx = txType.class.fromTxData({}, { freeze: false })
-      tx = txType.class.fromTxData({}, { freeze: false })
+      tx = txType.class.fromTxData({}, { common, freeze: false })
+      tx = txType.class.fromTxData({}, { common, freeze: false })
       st.ok(
         !Object.isFrozen(tx),
         `${txType.name}: tx should not be frozen when freeze deactivated in options`
       )
 
       // Perform the same test as above, but now using a different construction method. This also implies that passing on the
       // options object works as expected.
-      tx = txType.class.fromTxData({}, { freeze: false })
+      tx = txType.class.fromTxData({}, { common, freeze: false })
       const rlpData = tx.serialize()
 
-      tx = txType.class.fromRlpSerializedTx(rlpData)
+      tx = txType.class.fromRlpSerializedTx(rlpData, { common })
       st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)
 
-      tx = txType.class.fromRlpSerializedTx(rlpData, { freeze: false })
+      tx = txType.class.fromRlpSerializedTx(rlpData, { common, freeze: false })
       st.ok(
         !Object.isFrozen(tx),
         `${txType.name}: tx should not be frozen when freeze deactivated in options`
       )
 
-      tx = txType.class.fromValuesArray(txType.values)
+      tx = txType.class.fromValuesArray(txType.values, { common })
       st.ok(Object.isFrozen(tx), `${txType.name}: tx should be frozen by default`)
 
-      tx = txType.class.fromValuesArray(txType.values, { freeze: false })
+      tx = txType.class.fromValuesArray(txType.values, { common, freeze: false })
       st.ok(
         !Object.isFrozen(tx),
         `${txType.name}: tx should not be frozen when freeze deactivated in options`
@@ -102,11 +105,11 @@ tape('[BaseTransaction]', function (t) {
     for (const txType of txTypes) {
       txType.txs.forEach(function (tx: any) {
         st.ok(
-          txType.class.fromRlpSerializedTx(tx.serialize()),
+          txType.class.fromRlpSerializedTx(tx.serialize(), { common }),
           `${txType.name}: should do roundtrip serialize() -> fromRlpSerializedTx()`
         )
         st.ok(
-          txType.class.fromSerializedTx(tx.serialize()),
+          txType.class.fromSerializedTx(tx.serialize(), { common }),
           `${txType.name}: should do roundtrip serialize() -> fromRlpSerializedTx()`
         )
       })
@@ -118,7 +121,7 @@ tape('[BaseTransaction]', function (t) {
     for (const txType of txTypes) {
       txType.txs.forEach(function (tx: any) {
         st.ok(
-          txType.class.fromValuesArray(tx.raw(true)),
+          txType.class.fromValuesArray(tx.raw(true), { common }),
           `${txType.name}: should do roundtrip raw() -> fromValuesArray()`
         )
       })
@@ -140,7 +143,7 @@ tape('[BaseTransaction]', function (t) {
       txType.fixtures.slice(0, 4).forEach(function (txFixture: any) {
         // set `s` to zero
         txFixture.data.s = `0x` + '0'.repeat(16)
-        const tx = txType.class.fromTxData(txFixture.data)
+        const tx = txType.class.fromTxData(txFixture.data, { common })
         st.equals(tx.verifySignature(), false, `${txType.name}: signature should not be valid`)
         st.ok(
           (<string[]>tx.validate(true)).includes('Invalid Signature'),

packages/tx/test/eip2930.spec.ts

@@ -19,6 +19,13 @@ const chainId = new BN(1)
 
 tape('[AccessListEIP2930Transaction]', function (t) {
   t.test('Initialization / Getter', function (t) {
+    t.ok(AccessListEIP2930Transaction.fromTxData({}, { common }), 'should initialize correctly')
+
+    const nonEIP2930Common = new Common({ chain: 'mainnet', hardfork: 'istanbul' })
+    t.throws(() => {
+      AccessListEIP2930Transaction.fromTxData({}, { common: nonEIP2930Common })
+    }, 'should throw on a pre-Berlin Harfork (EIP-2930 not activated)')
+
     t.throws(() => {
       AccessListEIP2930Transaction.fromTxData(
         {

packages/tx/test/legacy.spec.ts

@@ -11,6 +11,15 @@ const txFixturesEip155: VitaliksTestsDataEntry[] = require('./json/ttTransaction
 tape('[Transaction]', function (t) {
   const transactions: Transaction[] = []
 
+  t.test('Initialization', function (st) {
+    const nonEIP2930Common = new Common({ chain: 'mainnet', hardfork: 'istanbul' })
+    st.ok(
+      Transaction.fromTxData({}, { common: nonEIP2930Common }),
+      'should initialize on a pre-Berlin Harfork (EIP-2930 not activated)'
+    )
+    st.end()
+  })
+
   t.test('Initialization -> decode with fromValuesArray()', function (st) {
     txFixtures.slice(0, 4).forEach(function (tx: any) {
       const txData = tx.raw.map(toBuffer)