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

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
govendor fetch github.com/ethereum/evmc/bindings/go/evmc@=v6.0.2
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 <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
As noted in the comment, this may not be the right
gitm commit -S -s -m core/vm:

Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
@meowsbits
Copy link
Contributor Author

meowsbits commented Jul 9, 2020

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
Signed-off-by: meows <b5c6@protonmail.com>
…/evmc.git

And rm -rf evmc/.git .gitignore

Signed-off-by: meows <b5c6@protonmail.com>
…lative uses

Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Docker image complaining about 'bash' not being
available.

Signed-off-by: meows <b5c6@protonmail.com>
'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 <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
@meowsbits
Copy link
Contributor Author

meowsbits commented Jul 9, 2020

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

meowsbits added 13 commits July 21, 2020 07:33
Since emvc/ is now a copy-cloned package instead
of a submodule, this is no longer necessary.

Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
This is congruent to go-ethereum implementation,
although it does seem a little redundant/tangential.

Signed-off-by: meows <b5c6@protonmail.com>
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 <b5c6@protonmail.com>
This improves CI readability.

Signed-off-by: meows <b5c6@protonmail.com>
Remove the subjectivity a little, more
descriptive.

Signed-off-by: meows <b5c6@protonmail.com>
…patible

Test failed on Travis because unknown operator.

https://travis-ci.org/github/etclabscore/core-geth/jobs/710376273
Signed-off-by: meows <b5c6@protonmail.com>
… on Travis

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

Signed-off-by: meows <b5c6@protonmail.com>
…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 <b5c6@protonmail.com>
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 <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
…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 <b5c6@protonmail.com>
@meowsbits meowsbits merged commit e386608 into master Jul 21, 2020
@meowsbits meowsbits deleted the feat/evmc6 branch July 21, 2020 20:55
@meowsbits meowsbits mentioned this pull request Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants