refactor(levm): use ethrex account types in LEVM#2629
Merged
Conversation
Lines of code reportTotal lines added: Detailed view |
Benchmark Results ComparisonPR ResultsBenchmark Results: Factorial
Benchmark Results: Factorial - Recursive
Benchmark Results: Fibonacci
Benchmark Results: ManyHashes
Benchmark Results: BubbleSort
Benchmark Results: ERC20 - Transfer
Benchmark Results: ERC20 - Mint
Benchmark Results: ERC20 - Approval
Main ResultsBenchmark Results: Factorial
Benchmark Results: Factorial - Recursive
Benchmark Results: Fibonacci
Benchmark Results: ManyHashes
Benchmark Results: BubbleSort
Benchmark Results: ERC20 - Transfer
Benchmark Results: ERC20 - Mint
Benchmark Results: ERC20 - Approval
|
EF Tests Comparison
|
tomip01
reviewed
Apr 29, 2025
tomip01
approved these changes
Apr 29, 2025
tomip01
left a comment
Contributor
There was a problem hiding this comment.
Very good! Just one suggestion
JulianVentura
suggested changes
Apr 29, 2025
JulianVentura
approved these changes
Apr 29, 2025
pedrobergamini
pushed a commit
to pedrobergamini/ethrex
that referenced
this pull request
Aug 24, 2025
**Motivation** - Stop using the Account and AccountInfo types defined in LEVM and start using the ones defined in the L1 client. - Biggest changes are that AccountInfo no longer has code, so we can't use it with that purpose and also we don't have our struct StorageSlot anymore, so we have to keep track of original values somewhere else. **Description** - Now we use the structs of the L1 client but they are different from the ones that we used so I had to make changes: - `get_account_info` is now `get_account` because we also need the code of the account and `AccountInfo` has the `code_hash` only. This makes changes on every structure that implements `LevmDatabase` trait. - Now that we don't have `StorageSlot` that had the `current_value` and `original_value` of a storage slot (`original_value` being the value pre-tx) I had to make some changes to logic and store those original values into an auxiliary `HashMap` on `VM`. - Added new function `get_original_storage()` for getting the original storage value. - Make some tiny changes in SSTORE, mostly organize it better. Storage changes deep description: - Now every time we want to get the `original_value` we will look up in the original values stored in the VM struct. These intends to store the storage values previous to starting the execution of a particular transaction. For efficiency and performance, we only update this new field when actually getting the original value. - Let me clarify: At the beginning of the transaction the `CacheDB` could have a lot of accounts with their storage but the `VM.storage_original_values`will start empty on every transaction. When `SSTORE` opcode is executed and we actually care for the original value of a storage slot we will look at `storage_original_values` and it won’t find it (the first time), so then it will see what the value in the `CacheDB` is, and if it’s not there it will finally check on the actual `Database`. After retrieving the value, it will be added to `storage_original_values` , but ONLY the FIRST time. That means that if the value keeps on changing the `original_value` won’t change because once it’s added it’s not modified. <!-- Link to issues: Resolves lambdaclass#111, Resolves lambdaclass#222 --> Closes #issue_number
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
get_account_infois nowget_accountbecause we also need the code of the account andAccountInfohas thecode_hashonly. This makes changes on every structure that implementsLevmDatabasetrait.StorageSlotthat had thecurrent_valueandoriginal_valueof a storage slot (original_valuebeing the value pre-tx) I had to make some changes to logic and store those original values into an auxiliaryHashMaponVM.get_original_storage()for getting the original storage value.Storage changes deep description:
original_valuewe will look up in the original values stored in the VM struct. These intends to store the storage values previous to starting the execution of a particular transaction. For efficiency and performance, we only update this new field when actually getting the original value.CacheDBcould have a lot of accounts with their storage but theVM.storage_original_valueswill start empty on every transaction. WhenSSTOREopcode is executed and we actually care for the original value of a storage slot we will look atstorage_original_valuesand it won’t find it (the first time), so then it will see what the value in theCacheDBis, and if it’s not there it will finally check on the actualDatabase. After retrieving the value, it will be added tostorage_original_values, but ONLY the FIRST time. That means that if the value keeps on changing theoriginal_valuewon’t change because once it’s added it’s not modified.Closes #issue_number