Add petersburg aka constantinopleFix support #433
Conversation
danjm
force-pushed
the
update-ethereumjs-common-version
branch
from
2e63c71
to
e8f7d72
Compare
danjm
force-pushed
the
add-petersburg-aka-constantinople-fix-support
branch
from
e858f81
to
fb81943
Compare
tests/GeneralStateTestsRunner.js
Outdated
| @@ -126,7 +126,11 @@ function runTestCase (options, testData, t, cb) { | |||
|
|
|||
| module.exports = function runStateTest (options, testData, t, cb) { | |||
| try { | |||
| const testCases = parseTestCases(options.forkConfig, testData, options.data, options.gasLimit, options.value) | |||
| let aliasForkConfig | |||
| if (options.forkConfig === 'Petersburg') { | |||
There was a problem hiding this comment.
Some note here: when I added the fork config logic here to the tests some time ago I added it in a way that both capital and non-capital names can be used, see e.g. here, so e.g. Byzantiumorbyzantium`. Still not sure if this was a good idea, I actually already did a first take to remove this lately along some other PR and then stumbled over some unexpected stuff which made this a bit more complicated than expected.
So I would suggest also for now that we keep that behavior and concentrate on the Petersburg changes. But to keep things consistent we should then also do these two Petersburg comparisons in the PR case-agnostic and compare the lowercase versions.
There was a problem hiding this comment.
I have made changes in 948fcfa such that the fork config for tests can be provided 'petersburg' or 'Petersburg'.
However, I noticed that node ./tests/tester -s --fork="Byzantium" works but node ./tests/tester -s --fork="byzantium" does not. This is because the ethereum/tests assume that the fork names will be written in upper case. The 'petersburg'/'Petersburg' case is not affected because I've made the name check in the added hardfork alias logic case insensitive.
One way to address this would be the changes I pushed (with another branch) here: add-petersburg-aka-constantinople-fix-support...fix-test-fork-name-case-handling
danjm
force-pushed
the
add-petersburg-aka-constantinople-fix-support
branch
from
fb81943
to
fe0d62d
Compare
danjm
force-pushed
the
update-ethereumjs-common-version
branch
from
e8f7d72
to
6451b75
Compare
danjm
force-pushed
the
add-petersburg-aka-constantinople-fix-support
branch
from
7af05fe
to
020bc03
Compare
danjm
force-pushed
the
update-ethereumjs-common-version
branch
from
6451b75
to
da97a00
Compare
danjm
force-pushed
the
add-petersburg-aka-constantinople-fix-support
branch
from
020bc03
to
948fcfa
Compare
danjm
force-pushed
the
update-ethereumjs-common-version
branch
from
da97a00
to
e6be3e1
Compare
danjm
force-pushed
the
add-petersburg-aka-constantinople-fix-support
branch
from
948fcfa
to
1b31ead
Compare
…ltiple supported forks
danjm
force-pushed
the
add-petersburg-aka-constantinople-fix-support
branch
from
0aa7bd8
to
ceb1787
Compare
danjm
changed the base branch from
update-ethereumjs-common-version
to
master
Addressses #423
constantinoplehardfork (and not on prior or subsequent hardforks).This is branched from #432
For this PR to work, we will also have to merge ethereumjs/ethereumjs-blockchain#86 and ethereumjs/ethereumjs-block#64, release new versions of those repos to npm and then update this PR to use those new versions.