[VM] update VM test runner #849
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| const { setupPreConditions, verifyPostConditions, getDAOCommon } = require('./util.js') | ||
| 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 | ||
|
|
@@ -39,6 +39,7 @@ module.exports = async function runBlockchainTest(options, testData, t) { | |
| } | ||
|
|
||
| const { common } = options | ||
| common.setHardforkByBlockNumber(0) | ||
|
|
||
| const blockchain = new Blockchain({ | ||
| db: blockchainDB, | ||
|
|
@@ -86,7 +87,7 @@ module.exports = async function runBlockchainTest(options, testData, t) { | |
|
|
||
| await blockchain.putGenesis(genesisBlock) | ||
|
|
||
| async function handleError(error, expectException) { | ||
| if (expectException) { | ||
| t.pass(`Expected exception ${expectException}`) | ||
| } else { | ||
|
|
@@ -95,25 +96,41 @@ module.exports = async function runBlockchainTest(options, testData, t) { | |
| } | ||
| } | ||
|
|
||
| let currentFork = common.hardfork() | ||
| let currentBlock | ||
| let lastBlock = 0 | ||
| for (const raw of testData.blocks) { | ||
| 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 { | ||
| let block = new Block(Buffer.from(raw.rlp.slice(2), 'hex'), { | ||
| common | ||
| }) | ||
| currentBlock = bufferToInt(block.header.number) | ||
| } catch(e) { | ||
| handleError(e, expectException) | ||
| continue | ||
| } | ||
|
|
||
| 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() | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. This can be simplified with Common.setHardforkByBlockNumber() There was a problem hiding this comment. Yeah there is some weird stuff going on over here, because if I re-instantiate a VM after I've updated Common (assuming that is what you mean here) then suddenly a lot more tests start failing. This used to be the way how most blockchain/state tests passed. 🤔 There was a problem hiding this comment. It's worth investigating why this happens though as it should definitely work if we re-instantiate the VM with a correctly-set Common 👍 |
||
|
|
@@ -127,15 +144,9 @@ module.exports = async function runBlockchainTest(options, testData, t) { | |
| // 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 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 | ||
|
|
@@ -159,13 +170,13 @@ module.exports = async function runBlockchainTest(options, testData, t) { | |
| } catch (error) { | ||
| // 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() | ||
| } | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not completely getting this re-org scenario: is this actually triggered somewhere in the tests? Then some of the tests should fail according to the code here I would assume? Or is this a "just in case" implementation? 😛
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that if you disable this, most tests pass (see the skipped tests which have a few tests which are added), but not all of these blocks are evaluated (these are the re-org blocks). So the test is only executed partially. There is only one test which actually fails due to this. I just don't like that tests are only partially executed. One of the features of these tests seems to be that it does this reorg stuff, but we are not testing this, so I do not think we should just allow the tests and make them pass while we know that they are only partially executed.
It is rather cumbersome that these tests setup the canonical chain, then do some reorg, and then basically "reorg back" to the initial canonical chain, which means that the tip block of the chain in our implementation is also the tip block which is expected by the test suite, which is also the reason why these tests pass. The blocks in the reorg are not evaluated. If not clear let me know 😄 Related issue which tracks this problem: #879