VM: fix selectHardforkByBlockNumber default value #967
Conversation
jochem-brouwer
added
PR state: needs review
package: vm
type: test all hardforks
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Can you also remove the options in the tests from the old PR which are not seeing the value to the default value? |
| const vm = new VM({ common: common1 }) | ||
| const vm_noSelect = new VM({ common: common2, selectHardforkByBlockNumber: false }) | ||
| const vm = new VM({ common: common1, selectHardforkByBlockNumber: true }) | ||
| const vm_noSelect = new VM({ common: common2 }) |
There was a problem hiding this comment.
This checks that the default value is indeed false? @holgerd77
There was a problem hiding this comment.
Also don't want to make this a blocker, just meant that the selectHardforkByBlockNumber: false initializations in the tests like https://github.com/ethereumjs/ethereumjs-vm/pull/966/files#diff-0ecbdf07c84f7c9ab5c8d5aa79ff9e4f4d38b935c0af79214e575d86f10deae8R74 could now be removed.
But just some cosmetics, just leave if you want and directly merge.
|
Ah yes you are right, I will add this. |
|
Huh this is weird... 🤔 Tests fail. |
|
Okay this makes no sense! I just re-ran the tests and now they pass? Failing test |
|
It seems the failing test was a random event, worth investigating, but in a different PR. |
Fixes the default value of the
selectHardforkByBlockNumberas pointed out by @holgerd77 in #966.