vm: add guard for EIP-3607 (#1691)

* add guard for EIP-3607 and tests
* 3607: update minimumHardfork to chainstart, since older HFs were added (see https://github.com/ethereum/tests/blob/develop/GeneralStateTests/stEIP3607/transactionCollidingWithNonEmptyAccount_calls.json#L114)
* enable EIP-3607 for blockchain tests to pass ethereum/tests
* organize lines
* fix comment typos
* simplify stripping hex prefix from testData.lastbockhash (uses `isHexPrefixed` which should be faster than `subtr(0, 2)`)

Author: ryanio

packages/common/src/eips/3607.json

@@ -0,0 +1,13 @@
+{
+  "name": "EIP-3607",
+  "number": 3607,
+  "comment": "Reject transactions from senders with deployed code",
+  "url": "https://eips.ethereum.org/EIPS/eip-3607",
+  "status": "Draft",
+  "minimumHardfork": "chainstart",
+  "requiredEIPs": [],
+  "gasConfig": {},
+  "gasPrices": {},
+  "vm": {},
+  "pow": {}
+}

packages/common/src/eips/index.ts

@@ -12,6 +12,7 @@ export const EIPs: eipsType = {
   3529: require('./3529.json'),
   3541: require('./3541.json'),
   3554: require('./3554.json'),
+  3607: require('./3607.json'),
   3675: require('./3675.json'),
   3855: require('./3855.json'),
   4345: require('./4345.json'),

packages/vm/src/index.ts

@@ -194,11 +194,11 @@ export default class VM extends AsyncEventEmitter {
     }
 
     if (opts.common) {
-      //EIPs
-      const supportedEIPs = [1559, 2315, 2537, 2565, 2718, 2929, 2930, 3198, 3529, 3541, 3855]
+      // Supported EIPs
+      const supportedEIPs = [1559, 2315, 2537, 2565, 2718, 2929, 2930, 3198, 3529, 3541, 3607, 3855]
       for (const eip of opts.common.eips()) {
         if (!supportedEIPs.includes(eip)) {
-          throw new Error(`${eip} is not supported by the VM`)
+          throw new Error(`EIP-${eip} is not supported by the VM`)
         }
       }
 

packages/vm/src/runTx.ts

@@ -303,10 +303,18 @@ async function _runTx(this: VM, opts: RunTxOpts): Promise<RunTxResult> {
   const { nonce, balance } = fromAccount
 
   // EIP-3607: Reject transactions from senders with deployed code
-  if (!fromAccount.codeHash.equals(KECCAK256_NULL)) {
-    const msg = _errorMsg('invalid sender address, address is not EOA (EIP-3607)', this, block, tx)
-    throw new Error(msg)
+  if (this._common.isActivatedEIP(3607)) {
+    if (!fromAccount.codeHash.equals(KECCAK256_NULL)) {
+      const msg = _errorMsg(
+        'invalid sender address, address is not EOA (EIP-3607)',
+        this,
+        block,
+        tx
+      )
+      throw new Error(msg)
+    }
   }
+
   if (!opts.skipBalance) {
     const cost = tx.getUpfrontCost(block.header.baseFeePerGas)
     if (balance.lt(cost)) {

packages/vm/tests/api/EIPs/eip-3607.spec.ts

@@ -0,0 +1,46 @@
+import tape from 'tape'
+import Common, { Chain, Hardfork } from '@ethereumjs/common'
+import { Transaction } from '@ethereumjs/tx'
+import { Address } from 'ethereumjs-util'
+import VM from '../../../src'
+
+tape('EIP-3607 tests', (t) => {
+  const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Berlin, eips: [3607] })
+  const commonNoEIP3607 = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Berlin, eips: [] })
+  const precompileAddr = Address.fromString('0x0000000000000000000000000000000000000001')
+
+  t.test('should reject txs from senders with deployed code when EIP is enabled', async (st) => {
+    const vm = new VM({ common })
+    await vm.stateManager.putContractCode(precompileAddr, Buffer.alloc(32, 1))
+    const tx = Transaction.fromTxData({ gasLimit: 100000 }, { freeze: false })
+    tx.getSenderAddress = () => precompileAddr
+    try {
+      await vm.runTx({ tx })
+      st.fail('runTx should have thrown')
+    } catch (error: any) {
+      if (error.message.includes('EIP-3607')) {
+        st.pass('threw correct error')
+      } else {
+        st.fail('did not throw correct error')
+      }
+    }
+    st.end()
+  })
+
+  t.test(
+    'should not reject txs from senders with deployed code when EIP is not enabled',
+    async (st) => {
+      const vm = new VM({ common: commonNoEIP3607 })
+      await vm.stateManager.putContractCode(precompileAddr, Buffer.alloc(32, 1))
+      const tx = Transaction.fromTxData({ gasLimit: 100000 }, { freeze: false })
+      tx.getSenderAddress = () => precompileAddr
+      try {
+        await vm.runTx({ tx })
+        st.ok('runTx successfully ran')
+      } catch (error: any) {
+        st.fail('threw an unexpected error')
+      }
+      st.end()
+    }
+  )
+})

packages/vm/tests/tester/config.ts

@@ -29,10 +29,10 @@ export const SKIP_BROKEN = [
  * Tests skipped due to system specifics / design considerations
  */
 export const SKIP_PERMANENT = [
-  'SuicidesMixingCoinbase', // sucides to the coinbase, since we run a blockLevel we create coinbase account.
-  'static_SuicidesMixingCoinbase', // sucides to the coinbase, since we run a blockLevel we create coinbase account.
-  'ForkUncle', // Only BlockchainTest, correct behaviour unspecified (?)
-  'UncleFromSideChain', // Only BlockchainTest, same as ForkUncle, the TD is the same for two diffent branches so its not clear which one should be the finally chain
+  'SuicidesMixingCoinbase', // suicides to the coinbase, since we run a blockLevel we create coinbase account.
+  'static_SuicidesMixingCoinbase', // suicides to the coinbase, since we run a blockLevel we create coinbase account.
+  'ForkUncle', // Only BlockchainTest, correct behavior unspecified (?)
+  'UncleFromSideChain', // Only BlockchainTest, same as ForkUncle, the TD is the same for two different branches so its not clear which one should be the finally chain
 ]
 
 /**
@@ -298,10 +298,13 @@ export function getCommon(targetNetwork: string) {
         })
       }
     }
-    const common = Common.custom({
-      hardforks: testHardforks,
-      defaultHardfork: hfName,
-    })
+    const common = Common.custom(
+      {
+        hardforks: testHardforks,
+        defaultHardfork: hfName,
+      },
+      { eips: [3607] }
+    )
     const eips = targetNetwork.match(/(?<=\+)(.\d+)/g)
     if (eips) {
       common.setEIPs(eips.map((e: string) => parseInt(e)))

packages/vm/tests/tester/index.ts

@@ -22,12 +22,12 @@ import blockchainTestsRunner from './runners/BlockchainTestsRunner'
  * --state: run state tests
  * --blockchain: run blockchain tests
  * --fork: fork to use for these tests
- * --skip: comma seperated list of tests to skip. choices of: all,broken,permanent,slow. Defaults to all
- * --runSkipped: comma seperated list of tests to skip if --skip is not set. choices of: all,broken,permanent,slow. Defaults to none
+ * --skip: comma-separated list of tests to skip. choices of: all,broken,permanent,slow. Defaults to all
+ * --runSkipped: comma-separated list of tests to skip if --skip is not set. choices of: all,broken,permanent,slow. Defaults to none
  * --file: test file to run
  * --test: test name to run
  * --dir: test directory to look for tests
- * --excludeDir: test directory to exlude from testing
+ * --excludeDir: test directory to exclude from testing
  * --testsPath: root directory of tests to look (default: '../ethereum-tests')
  * --customTestsPath: custom directory to look for tests (e.g. '../../my_custom_test_folder')
  * --customStateTest: run a file with a custom state test (not in test directory)

packages/vm/tests/tester/runners/BlockchainTestsRunner.ts

@@ -3,7 +3,7 @@ import { Block } from '@ethereumjs/block'
 import Blockchain from '@ethereumjs/blockchain'
 import Common, { ConsensusAlgorithm } from '@ethereumjs/common'
 import { TransactionFactory } from '@ethereumjs/tx'
-import { addHexPrefix, BN, toBuffer, rlp } from 'ethereumjs-util'
+import { addHexPrefix, BN, toBuffer, rlp, stripHexPrefix } from 'ethereumjs-util'
 import { SecureTrie as Trie } from 'merkle-patricia-tree'
 import { setupPreConditions, verifyPostConditions } from '../../util'
 
@@ -17,10 +17,8 @@ export default async function runBlockchainTest(options: any, testData: any, t:
     return
   }
 
-  if (testData.lastblockhash.substr(0, 2) === '0x') {
-    // fix for BlockchainTests/GeneralStateTests/stRandom/*
-    testData.lastblockhash = testData.lastblockhash.substr(2)
-  }
+  // fix for BlockchainTests/GeneralStateTests/stRandom/*
+  testData.lastblockhash = stripHexPrefix(testData.lastblockhash)
 
   const blockchainDB = levelMem()
   const cacheDB = level('./.cachedb')

packages/vm/tests/tester/runners/GeneralStateTestsRunner.ts

@@ -67,13 +67,12 @@ async function runTestCase(options: any, testData: any, t: tape.Test) {
     VM = require('../../../src').default
   }
   const begin = Date.now()
-  const state = new Trie()
-  const hardfork = options.forkConfigVM
-
-  const eips: number[] = []
 
+  const hardfork = options.forkConfigVM
+  const eips: number[] = [3607]
   const common = new Common({ chain: Chain.Mainnet, hardfork, eips })
 
+  const state = new Trie()
   const vm = new VM({ state, common })
 
   await setupPreConditions(vm.stateManager._trie, testData)