Merge pull request #849 from ethereumjs/rewrite-tests

[VM] update VM test runner

.github/workflows/vm-pr.yml

@@ -76,16 +76,14 @@ jobs:
       - run: npm run build:vm
         working-directory: ${{github.workspace}}
 
-      - run: npm run test:state -- --fork=${{ matrix.fork }}
+      - run: npm run test:state -- --fork=${{ matrix.fork }} --verify-test-amount-alltests
 
   test-vm-blockchain:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        # This is the most fair division among 1 directory vs. everything else
-        args: ['--excludeDir=stTimeConsuming', '--dir=GeneralStateTests/stTimeConsuming']
-        # Run specific fork tests
-        fork: ['MuirGlacier', 'Istanbul', 'Chainstart', 'Berlin']
+        # Args to pass to the tester. Note that some have splitted the slow tests and only running those: these are only on forks where that is applicable (see PR #489 for numbers on these)
+        args: ['--fork=Chainstart --expected-test-amount=4385', '--fork=Homestead --expected-test-amount=6997', '--fork=Petersburg --excludeDir=stTimeConsuming --expected-test-amount=17174', '--fork=Petersburg --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561', '--fork=Istanbul --excludeDir=stTimeConsuming --expected-test-amount=19817', '--fork=Istanbul --dir=GeneralStateTests/stTimeConsuming --expected-test-amount=15561', '--fork=Berlin --expected-test-amount=33']
       fail-fast: false
     steps:
       - uses: actions/setup-node@v1
@@ -110,7 +108,7 @@ jobs:
       - run: npm run build:vm
         working-directory: ${{github.workspace}}
 
-      - run: npm run test:blockchain -- ${{ matrix.args }} --fork=${{ matrix.fork }}
+      - run: npm run test:blockchain -- ${{ matrix.args }}
 
   vm-benchmarks:
     runs-on: ubuntu-latest
@@ -163,4 +161,4 @@ jobs:
 
       # Re-apply git stash to prepare for saving back to cache.
       # Avoids exit code 1 by checking if there are changes to be stashed first
-      - run: STASH_LIST=`git stash list` && [ ! -z $STASH_LIST ] && git stash apply || echo "No files to stash-apply. Skipping…"
+      - run: STASH_LIST=`git stash list` && [ ! -z $STASH_LIST ] && git stash apply || echo "No files to stash-apply. Skipping…"

packages/blockchain/src/index.ts

@@ -302,7 +302,13 @@ export default class Blockchain implements BlockchainInterface {
 
     await this._lock.wait()
     await this._putBlockOrHeader(block, isGenesis)
-    this._lock.release()
+      .then(() => {
+        this._lock.release()
+      })
+      .catch((reason) => {
+        this._lock.release()
+        throw reason
+      })
   }
 
   /**
@@ -328,7 +334,13 @@ export default class Blockchain implements BlockchainInterface {
 
     await this._lock.wait()
     await this._putBlockOrHeader(header)
-    this._lock.release()
+      .then(() => {
+        this._lock.release()
+      })
+      .catch((reason) => {
+        this._lock.release()
+        throw reason
+      })
   }
 
   /**
@@ -706,7 +718,13 @@ export default class Blockchain implements BlockchainInterface {
 
     await this._lock.wait()
     await this._delBlock(blockHash)
-    this._lock.release()
+      .then(() => {
+        this._lock.release()
+      })
+      .catch((reason) => {
+        this._lock.release()
+        throw reason
+      })
   }
 
   /**

packages/vm/lib/index.ts

@@ -194,6 +194,10 @@ export default class VM extends AsyncEventEmitter {
     this._emit = promisify(this.emit.bind(this))
   }
 
+  _updateOpcodes() {
+    this._opcodes = getOpcodesForHF(this._common)
+  }
+
   async init(): Promise<void> {
     if (this.isInitialized) {
       return

packages/vm/package.json

@@ -16,12 +16,12 @@
     "coverage:test": "tape './tests/api/**/*.js' ./tests/tester.js --state --dist",
     "docs:build": "typedoc --options typedoc.js",
     "test:state": "ts-node ./tests/tester --state",
-    "test:state:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1",
-    "test:state:selectedForks": "echo 'Homestead TangerineWhistle SpuriousDragon Petersburg' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1",
+    "test:state:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1 --verify-test-amount-alltests",
+    "test:state:selectedForks": "echo 'Homestead TangerineWhistle SpuriousDragon Petersburg' | xargs -n1 | xargs -I v1 npm run test:state -- --fork=v1 --verify-test-amount-alltests",
     "test:state:slow": "npm run test:state -- --runSkipped=slow",
     "test:buildIntegrity": "npm run test:state -- --test='stackOverflow'",
     "test:blockchain": "node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain",
-    "test:blockchain:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin' | xargs -n1 | xargs -I v1 node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=v1",
+    "test:blockchain:allForks": "echo 'Chainstart Homestead dao TangerineWhistle SpuriousDragon Byzantium Constantinople Petersburg Istanbul MuirGlacier Berlin ByzantiumToConstantinopleFixAt5 EIP158ToByzantiumAt5 FrontierToHomesteadAt5 HomesteadToDaoAt5 HomesteadToEIP150At5' | xargs -n1 | xargs -I v1 node -r ts-node/register --stack-size=1500 ./tests/tester --blockchain --fork=v1 --verify-test-amount-alltests",
     "test:API": "tape -r ts-node/register --stack-size=1500 './tests/api/**/*.js'",
     "test:API:browser": "npm run build && karma start karma.conf.js",
     "test": "echo \"[INFO] Generic test cmd not used. See package.json for more specific test run cmds.\"",

packages/vm/tests/BlockchainTestsRunner.js

@@ -1,5 +1,5 @@
 const { setupPreConditions, verifyPostConditions, getDAOCommon } = require('./util.js')
-const { addHexPrefix } = require('ethereumjs-util')
+const { addHexPrefix, rlp, bufferToInt } = require('ethereumjs-util')
 const Trie = require('merkle-patricia-tree').SecureTrie
 const { Block, BlockHeader } = require('@ethereumjs/block')
 const Blockchain = require('@ethereumjs/blockchain').default
@@ -15,39 +15,40 @@ module.exports = async function runBlockchainTest(options, testData, t) {
     return
   }
 
+  if (testData.lastblockhash.substr(0, 2) === '0x') {
+    // fix for BlockchainTests/GeneralStateTests/stRandom/*
+    testData.lastblockhash = testData.lastblockhash.substr(2)
+  }
+
   const blockchainDB = levelMem()
   const cacheDB = level('./.cachedb')
   const state = new Trie()
 
-  let validate = false
+  let validatePow = false
   // Only run with block validation when sealEngine present in test file
   // and being set to Ethash PoW validation
   if (testData.sealEngine && testData.sealEngine === 'Ethash') {
-    validate = true
+    validatePow = true
   }
-
+  
   let eips = []
   if (options.forkConfigVM == 'berlin') {
     // currently, the BLS tests run on the Berlin network, but our VM does not activate EIP2537 
     // if you run the Berlin HF
     eips = [2537]
   }
 
-  let common
-  if (options.forkConfigTestSuite == "HomesteadToDaoAt5") {
-    common = getDAOCommon(5)
-  } else {
-    common = new Common({ chain: 'mainnet', hardfork: options.forkConfigVM, eips })
-  }
+  const { common } = options
+  common.setHardforkByBlockNumber(0)
 
   const blockchain = new Blockchain({
     db: blockchainDB,
     common,
-    validateBlocks: validate,
-    validatePow: validate,
+    validateBlocks: true,
+    validatePow
   })
 
-  if (validate) {
+  if (validatePow) {
     blockchain.ethash.cacheDB = cacheDB
   }
 
@@ -86,59 +87,66 @@ module.exports = async function runBlockchainTest(options, testData, t) {
 
   await blockchain.putGenesis(genesisBlock)
 
-  async function handleError(error, expectException, cacheDB) {
+  async function handleError(error, expectException) {
     if (expectException) {
       t.pass(`Expected exception ${expectException}`)
     } else {
       console.log(error)
       t.fail(error)
     }
-    await cacheDB.close()
   }
 
-  const numBlocks = testData.blocks.length
-  let currentBlock = 0
-  let lastBlock = false
+  let currentFork = common.hardfork()
+  let currentBlock
+  let lastBlock = 0
   for (const raw of testData.blocks) {
-    currentBlock++
-    lastBlock = (currentBlock == numBlocks)
+    lastBlock = currentBlock
     const paramFork = `expectException${options.forkConfigTestSuite}`
     // Two naming conventions in ethereum/tests to indicate "exception occurs on all HFs" semantics
     // Last checked: ethereumjs-testing v1.3.1 (2020-05-11)
     const paramAll1 = 'expectExceptionALL'
     const paramAll2 = 'expectException'
     const expectException = raw[paramFork] ? raw[paramFork] : raw[paramAll1] || raw[paramAll2] || raw.blockHeader == undefined
 
+    // here we convert the rlp to block only to extract the number
+    // we have to do this again later because the common might run on a new hardfork
     try {
-      const block = new Block(Buffer.from(raw.rlp.slice(2), 'hex'), {
+      let block = new Block(Buffer.from(raw.rlp.slice(2), 'hex'), {
         common
       }) 
+      currentBlock = bufferToInt(block.header.number)
+    } catch(e) {
+      handleError(e, expectException)
+      continue
+    }
 
-      try {
-        await blockchain.putBlock(block)
-      } catch (error) {
-        await handleError(error, expectException, cacheDB)
-        return
+    if (currentBlock < lastBlock) {
+      // "re-org": rollback the blockchain to currentBlock (i.e. delete that block number in the blockchain plus the children)
+      t.fail("re-orgs are not supported by the test suite")
+      return
+    }
+    try {
+
+      // check if we should update common.
+      let newFork = common.setHardforkByBlockNumber(currentBlock)
+      if (newFork != currentFork) {
+        currentFork = newFork
+        vm._updateOpcodes()
       }
 
+      const block = new Block(Buffer.from(raw.rlp.slice(2), 'hex'), {
+        common
+      }) 
+      await blockchain.putBlock(block)
+
       // This is a trick to avoid generating the canonical genesis
       // state. Generating the genesis state is not needed because
       // blockchain tests come with their own `pre` world state.
       // TODO: Add option to `runBlockchain` not to generate genesis state.
       vm._common.genesis().stateRoot = vm.stateManager._trie.root
-
       await vm.runBlockchain()
-
       const headBlock = await vm.blockchain.getHead()
 
-      if (testData.lastblockhash.substr(0, 2) === '0x') {
-        // fix for BlockchainTests/GeneralStateTests/stRandom/*
-        testData.lastblockhash = testData.lastblockhash.substr(2)
-      }
-      if (expectException !== undefined && lastBlock) { // only check last block hash on last block
-        t.equal(headBlock.hash().toString('hex'), testData.lastblockhash, 'last block hash')
-      }
-
       // if the test fails, then block.header is the prej because
       // vm.runBlock has a check that prevents the actual postState from being
       // imported if it is not equal to the expected postState. it is useful
@@ -152,19 +160,25 @@ module.exports = async function runBlockchainTest(options, testData, t) {
       if (options.debug) {
         await verifyPostConditions(state, testData.postState, t)
       }
-      if (expectException !== undefined && lastBlock) {
-        t.equal(
-          blockchain.meta.rawHead.toString('hex'),
-          testData.lastblockhash,
-          'correct header block',
-        )
-      }
+
       await cacheDB.close()
+
+      if (expectException) {
+        t.fail("expected exception but test did not throw an exception: " + expectException)
+        return
+      }
     } catch (error) {
-      await handleError(error, expectException, cacheDB)
-      return
+      // caught an error, reduce block number
+      currentBlock--
+      await handleError(error, expectException)
     }
   }
+  t.equal(
+    blockchain.meta.rawHead.toString('hex'),
+    testData.lastblockhash,
+    'correct last header block',
+  )
+  await cacheDB.close()
 }
 
 function formatBlockHeader(data) {