Initial support for runevm contract #513
Conversation
Codecov Report
@@ Coverage Diff @@
## master #513 +/- ##
==========================================
- Coverage 69.85% 68.29% -1.57%
==========================================
Files 7 7
Lines 962 984 +22
Branches 125 126 +1
==========================================
Hits 672 672
- Misses 265 286 +21
- Partials 25 26 +1 |
Codecov Report
@@ Coverage Diff @@
## master #513 +/- ##
==========================================
- Coverage 52.17% 51.86% -0.32%
==========================================
Files 8 8
Lines 1311 1342 +31
Branches 129 130 +1
==========================================
+ Hits 684 696 +12
- Misses 600 619 +19
Partials 27 27 |
s1na
force-pushed
the
runevm-test
branch
2 times, most recently
from
c8bd9bf
to
48c8ae7
Compare
|
Updated, can you have a look @axic? |
| // This is the order of preference. | ||
| #if HERA_BINARYEN | ||
| new BinaryenEngine | ||
| BinaryenEngine::create |
There was a problem hiding this comment.
@chfast hmm, why did we had the static create() if we only ever used the default constructor?
There was a problem hiding this comment.
I don't remember anything about it. If the default constructor works for this case it's good choice.
| @@ -425,7 +505,8 @@ evmc_set_option_result hera_set_option( | |||
| if (strcmp(name, "engine") == 0) { | |||
| auto it = wasm_engine_map.find(value); | |||
| if (it != wasm_engine_map.end()) { | |||
| hera->engine = it->second(); | |||
| wasmEngineCreateFn = it->second; | |||
| hera->engine = wasmEngineCreateFn(); | |||
There was a problem hiding this comment.
Sorry, you were correct, .execute() is stateless at the moment. Would you mind if I remove the changes regarding the engines and merge this PR?
And then could you create a new PR changing the handling of the engine, because that might be a bug currently, but want to have @chfast's opinion.
There was a problem hiding this comment.
Sure, feel free to modify the PR.
axic
changed the title
|
@chfast can you please review this PR? |
|
|
||
| bytes ret; | ||
| evmc_status_code status = result.isRevert ? EVMC_REVERT : EVMC_SUCCESS; | ||
| if (status == EVMC_SUCCESS && result.returnValue.size() > 0) |
There was a problem hiding this comment.
| if (status == EVMC_SUCCESS && result.returnValue.size() > 0) | |
| if (status == EVMC_SUCCESS && !result.returnValue.empty()) |
but the returnValue is not really needed.
This PR adds runevm as a system contract. I wanted to be able to test runevm via
testeth, and made these changes to facilitate that. Not sure if this is something that you'd want merged.Just now when I wanted to push my branch name conflicted with https://github.com/ewasm/hera/tree/runevm and I found this commit 866e12c (Update: just saw the PR #375) which would have saved me a day! :)
Three points about the PR:
context->host->callto work, and directly use theWasmEngineto call runevm.module.memory.initial = 20to avoid that. Not sure how to properly handle this.disable-interface-metering, as runevm currently measures all gas costs itself (again, not sure if this is good or not).