Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VM: add selectHardforkByBlockNumber option #966

Merged
merged 2 commits into from Nov 26, 2020

Conversation

jochem-brouwer
Copy link
Member

This PR adds an option to make the VM automatically switch to the right hardfork in runBlock.

@codecov
Copy link

codecov bot commented Nov 25, 2020

Codecov Report

Merging #966 (d148349) into master (a61be5d) will decrease coverage by 0.04%.
The diff coverage is 83.33%.

Impacted file tree graph

Flag Coverage Δ
block 77.41% <ø> (-0.28%) ⬇️
blockchain 77.39% <ø> (ø)
common 92.11% <ø> (ø)
ethash 82.08% <ø> (ø)
tx 86.04% <ø> (-0.22%) ⬇️
vm 87.43% <83.33%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

packages/vm/lib/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, nice test case!

@ryanio ryanio added the type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR. label Nov 26, 2020
@ryanio
Copy link
Contributor

ryanio commented Nov 26, 2020

next time I will try adding type: test all hardforks label before the commit, didn't run with only a re-run

@ryanio ryanio merged commit 93b476e into master Nov 26, 2020
@ryanio ryanio deleted the select-hardfork-blocknumber branch November 26, 2020 01:19
*
* Default: `true`
*/
selectHardforkByBlockNumber?: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

???

We can't having this default to true, this is a giga-breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right, will default this to false in a new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: vm PR state: needs review type: test all hardforks This special label enables VM state and blockchain tests for all hardforks on the respective PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants