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.

Sign up for GitHub

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

Jump to bottom

Feat/evmc6 #57

Merged
merged 182 commits into from
Jul 21, 2020
Merged

Feat/evmc6 #57

merged 182 commits into from
Jul 21, 2020

Conversation

meowsbits
Copy link
Contributor

Rel #55 Support EVMC6

chfast and others added 30 commits November 8, 2019 19:22
@chfast
vendor: Add EVMC 6.0.2
5af0927 
govendor fetch github.com/ethereum/evmc/bindings/go/evmc@=v6.0.2
@chfast
core/vm: Add EVMC support
b517d31
@chfast
Upgrade EVMC to 6.1.1
e862a67
@chfast
evmc: Add support for Petersburg revision
6d7511f
@chfast
evm: Reserve space for both EVM and EWASM interpreters
3f4678b
@chfast
evm: Create EVMC or default EVM (no failover)
9976611
@chfast
evmc: Refactor code
cff2b3f
@chfast
evmc: Replace if-else with switch
5091fd4
@chfast
vendor: Upgrade EVMC to 6.2.0 unstable
a8b6f87
@chfast
evmc: Adjust after EVMC upgrade
46352a4
@chfast
evmc: Seperate instances for Ewasm and EVM
567d3aa
@chfast
evmc: Separate EVMC module initalization code, use sync.Once
37eb880
@chfast
evmc: Add InitEVMC() for EVMC initialization at startup
4bea7d1
@chfast
evmc: Simplify EVMC initialization
afc097f
@chfast
vendor: Upgrade EVMC to 6.3.0
da04eb3
@chfast
evmc: Rename SuicideRefundGas -> SelfdestructRefundGas
e09ecc6
@chfast
evmc: Add Istanbul support
1ca8b76
@chfast
evmc: Update for v7 EVMC API
3dfb3bf
@chfast
tests: Fix EVMC initialization in tests
b36c49e
@chfast
evmc: Add Istanbul support
769e062
@meowsbits
Merge remote-tracking branch 'ewasm/evmc-v6' into feat/evmc6
4bdec40
@meowsbits
Init merge ewasm evmc6 support
8825915 
Submods, gomods, oh my!
(Mostly vendor/dep management)

I used a submodule with go mod's replace
directive because I want to first
establish an MVP implementation of the
existing PR as-is.

Issues found during testing, design and
architectural questions are unstable and still
open for discussion.

Signed-off-by: meows <[email protected]>
@meowsbits
core/vm: implement EIP2200 in our style
7b359ff 
Signed-off-by: meows <[email protected]>
@meowsbits
implement EIP158/161 in our style
62a2904 
Signed-off-by: meows <[email protected]>
@meowsbits
core/vm: fix params/var import path
414ca2c 
Signed-off-by: meows <[email protected]>
@meowsbits
core/vm: fix ChainID
7c11f44 
Signed-off-by: meows <[email protected]>
@meowsbits
core/vm: fix IsHomestead/GetEthashEIP2Transition
0f05562 
Signed-off-by: meows <[email protected]>
@meowsbits
core/vm: adjust Fork/Fork-feature logic for evmc.Revision
1641f68 
As noted in the comment, this may not be the right
gitm commit -S -s -m core/vm:

Signed-off-by: meows <[email protected]>
@meowsbits
tests: remove unused import
64553bc 
Signed-off-by: meows <[email protected]>
@meowsbits
core/vm,tests: pass: 'go test ./tests/...'
6a81e07 
Signed-off-by: meows <[email protected]>
@meowsbits
Copy link
Contributor Author

meowsbits commented Jul 9, 2020 •
edited
Loading

Just a little walkaround of what we've got here.

This introduces a new submodule at evmc/ with remote at https://github.com/ethereum/evmc.git, pegged at version v6.3.1. This is the latest tagged version supporting EVMC v6 (later version bump to v7).

Consequences of this are:

  • Builds depends on this submodule. This means go [build|install] depends on the user having cloned the submodules, via git submodule --init, git submodule --init evmc, git submodule --init --recursive, or git --recurse-submodules clone .... This is (obviously) a notable additional requirement for building.

I don't love this. I would prefer build (and test) requirements to be as simple and standard as possible. Since we're eternally-pegged at v6.3.1, a solve for this would be to include the dependency as an in-place fork. Precedent for this pattern exists in the codebase already in log/ package, which is a fork of github.com/inconshreveable/log15.

  • Tests are dependent on this submodule as well, but further, they depend on a generated artifact from the submodule: evmc/binding/go/evmc/example_vm.so. This causes go test ./... to depend on an additional build step (ie go generate evmc/binding/go/evmc/).

Again, I think this is ugly. IMO go test ./... should, with pretty darn high priority, "just work."

meowsbits added 8 commits July 9, 2020 10:37
@meowsbits
.gitmodules,evmc: remove evmc as submodule
8b2e1dc 
Signed-off-by: meows <[email protected]>
@meowsbits
evmc: git clone --branch v6.3.1 --depth 1 https://github.com/ethereum…
6bd2cc2 
…/evmc.git

And rm -rf evmc/.git .gitignore

Signed-off-by: meows <[email protected]>
@meowsbits
evmc: skip tests if generated artifact example_vm.so is not present
6a2070a 
Signed-off-by: meows <[email protected]>
@meowsbits
evmc/README_COREGETH.md: documentation for included source and its re…
09efacd 
…lative uses

Signed-off-by: meows <[email protected]>
@meowsbits
.travis.yml: remove Travis EVMC submodule actions
b21e9fc 
Signed-off-by: meows <[email protected]>
@meowsbits
build/evmc-example_vm.so.sh: shebang: use 'sh' instead of 'bash'
aba05aa 
Docker image complaining about 'bash' not being
available.

Signed-off-by: meows <[email protected]>
@meowsbits
Makefile: remove 'evmc' command from 'test' command
4eb48c5 
'make test' should run the standard go tests.
It shouldn't do anything fancy.

This moves the evmc-specific testing to one place, ensuring
that tests both with and without the example_vm.so
are run.

Signed-off-by: meows <[email protected]>
@meowsbits
Makefile: make clean-evmc should also remove example_vm.so
1b22aa8 
Signed-off-by: meows <[email protected]>
@meowsbits
Copy link
Contributor Author

meowsbits commented Jul 9, 2020 •
edited
Loading

Commits 8b2e1dc .. 1b22aa8 were presented and merged from #141

meowsbits added 13 commits July 21, 2020 07:33
@meowsbits
README.md: remove unnecessary submodules flag from clone cmd
ecd9db9 
Since emvc/ is now a copy-cloned package instead
of a submodule, this is no longer necessary.

Signed-off-by: meows <[email protected]>
@meowsbits
docs/core/evmc.md: minor syntax tweaks to emvc6 doc
f42633e 
Signed-off-by: meows <[email protected]>
@meowsbits
tests: remove whitespace-only changes
d4c348a 
Signed-off-by: meows <[email protected]>
@meowsbits
Makefile: reinstate all as predicate for test cmd
f32e090 
This is congruent to go-ethereum implementation,
although it does seem a little redundant/tangential.

Signed-off-by: meows <[email protected]>
@meowsbits
.travis.yml: init move evmc-test to travis
9359d66 
I'd like to remove Github Actions as the CI
for EVMC6 tests, since that's all its currently used for,
and I think it'd be good to have our CI tests
consolidated at one provider, if possible.

This keeps CI visibility simple and predictable.

Signed-off-by: meows <[email protected]>
@meowsbits
.travis.yml: add job names for readability
63052ae 
This improves CI readability.

Signed-off-by: meows <[email protected]>
@meowsbits
.travis.yml: better names for travis jobs
a0b5fd0 
Remove the subjectivity a little, more
descriptive.

Signed-off-by: meows <[email protected]>
@meowsbits
build/evmc-example_vm.so.sh: fix example_vm.so script to be POSIX com…
0c7a433 
…patible

Test failed on Travis because unknown operator.

https://travis-ci.org/github/etclabscore/core-geth/jobs/710376273
Signed-off-by: meows <[email protected]>
@meowsbits
.travis.yml,docs/core/evmc.md: abort trying to get evmc tests to pass…
9f50dc3 
… on Travis

We can stick with Github Actions for now to avoid
the CI headache.

Signed-off-by: meows <[email protected]>
@meowsbits
Makefile,build/evmone.sh,tests: install EVMOne 0.2.0 --evmc.evm .so S…
6c74ead 
…tateTest

This installs a new test for --evmc.evm flag.
It uses the ethereum/evmcone C++ standalone EVM
.so artifact.

Tests for Constantinople, Istanbul, and Phoenix are
skipped because they are not supported by this latest-possible
evmcv6-compatible EVMOne version.

Signed-off-by: meows <[email protected]>
@meowsbits
main,tests: revise skipForks to be a method params for StateTest type
fb496f8 
This moves the logic of skipping forks from adhoc in the
StateTest runner (where it awkwardly used the testMatcher
method in a 'special' way), to still using the testMatcher
in a special way, but now more descriptively as a logic
facet of the StateTest type itself in the Subtests method.

Signed-off-by: meows <[email protected]>
@meowsbits
tests: (lint) goimports -w
764fd54 
Signed-off-by: meows <[email protected]>
@meowsbits
tests: remove unnecessary IstanbulBlock removal from difficulty test …
6d9e363 
…config

This change is not related to the EVMC feature,
and does not change test behavior, so striking it.

IstanbulBlock existing is a no-op, since difficulty
did not change at that fork anyways.

Signed-off-by: meows <[email protected]>
@meowsbits meowsbits merged commit e386608 into master Jul 21, 2020
@meowsbits meowsbits deleted the feat/evmc6 branch July 21, 2020 20:55
This was referenced Jul 23, 2020
Closed
Closed
@meowsbits meowsbits mentioned this pull request Jan 11, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzdybal tzdybal Awaiting requested review from tzdybal

Assignees
No one assigned
Labels
M4-core ⛓ M4-evm 🧮
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@meowsbits @tzdybal @chfast