-
Notifications
You must be signed in to change notification settings - Fork 33
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(levm): run hive tests with levm, generate and publish report #1546
Conversation
…. Basic tx now works as expected
…thereum_rust into levm/fixes_cache_db
This reverts commit 58d42b0.
**Motivation** <!-- Why does this pull request exist? What are its goals? --> Publish results of running Hive tests with LEVM to slack channel **Description** <!-- A clear and concise general description of the changes this PR introduces --> - We build the client image with LEVM, we run hive tests and then we publish them - Add to the Dockerfile the possibility to add build flags. - Avoid panicking if setup wasn't completed successfully, as no tests will run in that scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Left some suggestions.
crates/blockchain/Cargo.toml
Outdated
@@ -28,6 +28,7 @@ path = "./blockchain.rs" | |||
|
|||
[features] | |||
default = ["c-kzg"] | |||
default-levm = ["default", "levm"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's name the target to levm
default-levm = ["default", "levm"] | |
levm = ["default", "levm"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a levm target
levm = ["ethrex-vm/levm"]
Do you think I should just do: levm = ["default", "ethrex-vm/levm"]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Motivation
Description
We now can execute multiple transactions in a block and commit them after execution of them all.
It refactors
get_state_transitions_levm
, now inaccount_updates
we'll have only the things that have changed from the account, so that we don't have much redundant informationWe build the client image with LEVM, we run hive tests and then we publish them
Add to the Dockerfile the possibility to add build flags.
Avoid panicking if setup wasn't completed successfully, as no tests will run in that scenario.
We can run tests with hive but the problem is that these tests execute a lot of blocks at the beginning and we usually fail at some block because of a difference in state, I tried debugging some tests and I fixed some things in other PRs but at some point I got stuck and decided that it was a better idea to leave it as is and merge this to main.