feat: `engine_newPayloadV3`: validate, execute & store block #222

fmoletta · 2024-08-05T19:25:55Z

Motivation

Being able to fully validate execute and store blocks received by engine_newPayloadV3

Description

Add block validation, execution & storing to engine_newPayloadV3 endpoint

Fixes:

Genesis.get_block:
- Now uses the INITIAL_BASE_FEE as base_fee_per_gas
- Now computes roots instead of using zero as default
- Now uses default values for cancun fields instead of leaving them as None

(With these fixes the genesis' block hash now matches the parentBlockHash of the next block when running with kurtosis)

beacon_root_contract_call now sets the block's gas_limit to avoid tx validation errors

Misc:

compute_transactions_root is now a standalone function matching the other compute functions
Log more steps of the engine_newPayloadV3 endpoint
Improve api of ExecutionPayloadV3 & PayloadStatus

Other
We can now execute payloads when running with kurtosis 🚀

Disclaimer: We are still getting some execution errors in later blocks that we need to look into (They are all currently passing the block validations)

Closes #51

…re-cancun-time

…idation (#220) **Motivation** Fetch cancun time from DB when validating payload v3 timestamp  **Description** * Store cancun_time in db * Use the stored cancun_time when validating payload timestamp in `eth_newPayloadV3` * Replace update methods for chain data in `Store` with `set_chain_config` Bonus: * Move `NewPayloadV3Request` to its corresponding module and update is parsing to match the rest of the codebase   Closes None, but is part of #51

**Motivation** Having a way to obtain lates/earliest/pending/etc block numbers  **Description** * Add get and update methods for earliest, latest, finalized, safe & pending block number to `Store` & `StoreEngine` * Resolve block numbers from tag in rpc methods   Closes None, but fixes many and enables others

…pts & withdrawals (#225) **Motivation** These roots are currently being calculated using `from_sorted_iter` but without being sorted beforehand. This PR replaces this behavior with inserting directly into the trie to ensure that it is ordered, then computing the root (The same fix that has been previously applied to storage root) **Description** Fixes `compute_transactions_root`, `compute_receipts_root` & `compute_withdrawals_root`  **Notes** After this change, the payloads created by kurtosis local net now pass the block hash validations in `engine_NewPayloadV3`  Closes None, but is needed for #51 Co-authored-by: Federica Moletta <[email protected]>

…date-payload

**Motivation**  - Make `get_state_transitions` we use un LEVM return the same as the one in REVM. **Description**  - I made changes in the past to make `get_state_transitions` for both LEVM and REVM the same for comparison but I missed one aspect. We only want to show the code in an `AccountUpdate` if the code itself has been modified, not just the `AccountInfo`. Before we were returning the code in the `AccountUpdate` even if only the nonce of the contract changed for example.  Closes #issue_number

) **Motivation** Currently during batch processing, the state transitions are calculated for every block and then merged, when it would be more performant to calculate them once at the end. **Description** This PR removes the account updates from the execution result and makes every consumer manually request them.  Closes #2504

…2556) **Motivation** When processing withdrawals with levm, accounts that are not cached are fetched directly from the `Store` (aka our DB) using the block's parent hash instead of using the `StoreWrapper` api that already knows which block's state to read accounts from (as we do for all other DB reads). This works fine when executing one block at a time as the block that the StoreWrapper reads from is the block's parent. But when we execute blocks in batch, the StoreWrapper reads from the parent of the first block in the batch, as changes from the following blocks will be recorded in the cache, so when processing withdrawals we may not have the state of the current block's parent in the Store. This PR fixes this issue by using the `StoreWrapper` to read uncached accounts from the batch's parent block instead of looking for an accounts in a parent state that may not exist. It also removes the method `get_account_info_by_hash` so we don't run into the same issue in the future  **Description** * Remove misleading method `get_account_info_by_hash` from levm Database trait (this can lead us to read state from a block that is not the designated parent block and which's state may not exist leading to Inconsistent Trie errors) * Remove the argument `parent_block_hash` from `process_withdrawals`   Closes #issue_number

**Motivation** - Make running and modifying EF Tests a better experience **Description**  - Remove spinner and just use prints - We now can filter the tests we want to run by fork name(we do the filtering in the parsing). Default is all forks. - Upgrade tests to more recent version - Run some Legacy Tests that we weren't running before. This adds a lot of tests more, it is the folder Cancun under LegacyTests. There will be repeated tests with the folder GeneralStateTests, we may want to find a solution for that so that it takes less time to execute. - Create docs in `README.md` - Implement some nits in the runner, making code easier to understand. - Ignore a few tests that take too long to run so that we can check for breaking changes fast. - Fix comparison report against LEVM, they weren't working correctly mostly because we were mishandling our Cache - Tidy the report, now it is much more clear and easier for debugging. Also the code is easier to follow and more concise too! - Fix some tests with REVM, basically now using constructor of `BlobExcessGasAndPrice` and setting chain id to 1 (as we do in LEVM). - Changed `get_state_transitions` [here](#2518) so that REVM and LEVM account updates are mostly the same and the comparison is more accurate for the person who is debugging any test.  Closes #issue_number

**Motivation**  - merge main changes to my other PR because main had a huge refactor and my PR was also a huge refactor :) **Description**   Closes #issue_number --------- Co-authored-by: Julian Ventura <[email protected]> Co-authored-by: Avila Gastón <[email protected]> Co-authored-by: fmoletta <[email protected]> Co-authored-by: Matías Onorato <[email protected]> Co-authored-by: Edgar <[email protected]> Co-authored-by: Tomás Arjovsky <[email protected]> Co-authored-by: Martin Paulucci <[email protected]> Co-authored-by: Lucas Fiegl <[email protected]> Co-authored-by: Estéfano Bargas <[email protected]> Co-authored-by: Javier Rodríguez Chatruc <[email protected]> Co-authored-by: VolodymyrBg <[email protected]> Co-authored-by: Tomás Paradelo <[email protected]> Co-authored-by: Mauro Toscano <[email protected]> Co-authored-by: Cypher Pepe <[email protected]>

**Motivation** Add instructions on how to set up ethrex along with a consensus node and start syncing with holesky or other testnets  **Description**   Closes #issue_number --------- Co-authored-by: Javier Rodríguez Chatruc <[email protected]>

**Motivation**  Currently the ProverServer component have the responsibility for both act as a server for the ProverClient (i.e., send blocks to prove and receive proofs) and send proofs to the L1 contract to verify blocks. This tasks can be parallel and decoupled one from the other. **Description**  A new struct `L1ProofSender` is created that periodically checks if there're new proofs to send to the L1, removing that job from the `ProverServer`. Also, components were renamed for better clarity. Note that the config names were not changed as there's a WIP PR (#2501) doing a full refactor of it  --------- Co-authored-by: Javier Rodríguez Chatruc <[email protected]> Co-authored-by: Javier Chatruc <[email protected]> Co-authored-by: Ivan Litteri <[email protected]>

**Motivation**  The flamegraphs are displayed half-sized on GH Pages. **Description**  In the CI, remove the line that cause the problem. Added Zed editor config directory to `.gitignore` btw.

**Motivation**  - Temporarily comment Invalid Missing Ancestor Syncing ReOrg and Invlalid P9 and P10. Should be fixed later, the issue is [this one](#2565) **Description**   Closes #issue_number

…0% now (#2568) **Motivation** - Fix CI **Description**  - London tests don't pass 100% because new tests have been added and it seems that there is an edge case we are not currently passing. For now I wanted to disable the check that sees if all tests from Prague to London passed and set it to look only for Prague to Paris. - Added `workflow_dispatch` to this workflow so that we can run it manually.  Closes #issue_number

**Motivation**  - stop popping callframe in finalize execution **Description**   Closes #issue_number

**Motivation** Now that the L2 has fully defaulted to levm, both for execution and proving, these modules became unused. **Description**   Closes #issue_number

Validium is a [scaling solution](https://ethereum.org/en/developers/docs/scaling/) that enforces integrity of transactions using validity proofs like [ZK-rollups](https://ethereum.org/en/developers/docs/scaling/zk-rollups/), but doesn’t store transaction data on the Ethereum Mainnet. **Description** - Replace EIP 4844 transactions for EIP 1559 - Modify OnChainProposer contract so that it supports validium. It is not the most efficient way of doing it but the simplest. - Now the config.toml has a validium field. Note: I'm not 100% sure about the changes that I made to the OnChainProposer, there may be a mistake in the additions that I made. I will review it though but I still consider worth opening this PR.  Closes #2313 --------- Co-authored-by: ilitteri <[email protected]> Co-authored-by: Ivan Litteri <[email protected]>

**Motivation** - We were just checking that the post state root matched. This checks that logs match too. **Description**  - This PR for now just compares the logs root hash with the provided by the `EFTest` but we might also want to compare against REVM's logs so that the log diff is debuggable in a follow up. This has to be added.  Closes #issue_number Co-authored-by: Javier Rodríguez Chatruc <[email protected]>

…sync hive test (#2605) **Motivation** PR #2426 changed how fork choice & new payload interact with the syncer and also introduced a bug. If snap sync is enabled, then fork choice update will never attempt to trigger a sync, so the sync process never gets started. This PR fixes the bug and also refactors the sync manager api to better suit the new use cases  * Combine commonly used together `SyncManager` methods `set_head` & `start_sync` into `sync_to_head` * Remove unused `SyncManager` method `status` and associated struct * Make sure sync is triggered during fcu when needed even if snap sync is enabled * Re-enable snap sync hive test suite **Description**   Closes #2521

…ad endpoint (#2561) **Motivation** This fixes the failing consume-engine hive tests on Prague and Cancun  **Description** * Return `INVALID` payload status when we fail to decode transactions as per spec ([Step 6 item 1](https://github.com/ethereum/execution-apis/blob/main/src/engine/paris.md#engine_newpayloadv1))   Closes #issue_number

**Motivation** There is currently a bug in the storage healer causing fetched paths to not be properly updated. This makes storage healing virtually infinite as fetched paths are constantly being added back to the queue. This fix should restore regular storage healing behaviour  **Description** * Fix logic error when updating pending paths for the next fetch during storage healing  **Other info** This bug was unknowingly introduced by #2288  Closes #issue_number

**Motivation** After recent changes in main, rebuilding now takes a lot longer than state sync. This PR aims to mitigate this hit by introducing other performance upgrades  **Description** * Increase parallelism when rebuilding storages * Reduce intermediate hashing when rebuilding state tries  These changes have increased storage rebuild speed to around the same as before the changes to store, and has reduced time estimates for state rebuild, but doesn't manage to make the state rebuild keep up with the state sync. These changes have not affected state sync speed  Closes #issue_number

…or encoding/decoding (#2454) **Motivation** Implements the refactor specified in the linked issues. Adding a single `code` method for the RLPxMessage enum was not enough for both encoding and decoding (as we would need to create the struct to call the method) so an associated constant was also added to support both needs. This solution fulfills the purpose of the issue, to have only one instance of each message code that we can use for encoding/decoding of messages, but its implementation is more complex than what we would have liked. If the complexity is not acceptable, we should close both this PR and the originating issue.  **Description** * Implement `code` method for `RLPxMessage` * Add `CODE` associated constant to `RLPxEncode` trait   Closes #1035 (Also #1034)

**Motivation**  **Description**   Closes #issue_number --------- Co-authored-by: Javier Chatruc <[email protected]>

**Motivation**  **Description**   Closes #issue_number

…test (#2615) **Motivation** Due to feeding so many txs, the base fee keeps increasing so when it goes beyond the load test txs max fee per gas, the block will have 0 txs due to them all having the same max fee per gas. This pr increasing the load test max fee per has to u64 MAX and lowers priority fee per gas to decreasing (realistically removing) the chance of 0 block txs in load tests **Description**   Closes #2523

**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.  Closes #issue_number

**Motivation** Snap sync hive test has been flaky lately, after running with debug output on the CI the problem seems to be a timeout when fetching the latest block. A [PR](lambdaclass/hive#22) was added to our hive fork to extend this timeout so the test doesn't fail. The timeout for the whole sync process has been left unchanged  **Description** * Extends the timeout for fetching the latest block on the sync hive test suite * Update Hive ref   Closes #issue_number

…2597) **Motivation** Allow running hive tests with snap sync using the available Makefile targets and passing the optional SYNCMODE=snap argument  **Description** * Add `SYNCOMDE` variable to Makefile with default value "full" * Set `syncmode` ethrex flag on hive Makefile targets according to above variable   Closes #issue_number

**Motivation** The client version was hardcoded in the rpc crate It was used in the client RPC msg, in the admin_info RPC msg and in the helloMsg in P2P **Description** Added vergen crate to include more environment variables at build time in the ethrex main package. It can be tested using the following cast commands ```shell cast client --rpc-url localhost:8545 cast rpc admin_nodeInfo --rpc-url http://localhost:8545 ``` Modified the `P2PContext` struct to include the client_info Also added it in the struct `RLPxConnection` to pass it to the helloMessage struct when doing the handshake Modified the test to use the functions with a dummy client_info The version can now be retrieved by using ethrex --version  Closes #2548

**Motivation**  - We were just propagating Internal errors but we also want to propagate DatabaseErrors. Before this we were just reverting the transaction and that is wrong. **Description**   Closes #issue_number

**Motivation** The `Store` methods `set_latest_valid_ancestor` & `get_latest_valid_ancestor` can be confusing without proper documentation. These methods were properly documented on the `StoreEngine` trait, but they were not documented in the `Store` structure where they will be most often called from. This PR adds documentation for these methods on the `Store` implementation while also simplifying it, as the internal trait documentation provides more information on the context and design choices/requirements for the implementation which are not necessary for the top-level methods.  **Description** * Add doc comments for `Store` methods `set_latest_valid_ancestor` & `get_latest_valid_ancestor`   Closes None

**Motivation**  - Try to remove `account_exists` if possible because it adds complexity and unnecessary checks to the DB. - Try to finally remove `get_account_no_push_cache`, which is related to the previous thing too. **Description** - We now ignore a specific test because [EIP-7702 spec has changed](ethereum/EIPs#9710) and we no longer need to check if the account exists in the trie. - Remove `Option` from `specific_tests` - Remove `get_account_no_push_cache` and the usage of `account_exists` in LEVM. This method is not deleted from the Database because it's used in `get_state_transitions`, and even here it could be removed but I think it is better to keep it in this PR and maybe decide later what to do with this function. (If we remove it it wouldn't make a difference to the state though). - We were able to remove a SpuriousDragon check because we don't support pre-merge forks now Note: `account_exists` hasn't been completely removed from `Database` because we use it in `get_state_transitions` but that is going to change soon and we'll be able to remove it.  Closes #issue_number

**Motivation** Some levm validation errors use the enum variant's name as display message instead of displaying a proper error message.  **Description** * Add error messages for some levm validation errors   Closes #issue_number

fmoletta added 11 commits August 2, 2024 11:43

wip

2f4b506

Insert cancun time from genesis + update newpayloadv3 parsing

d08ee6b

fmt

14b8bae

Merge branch 'main' of https://github.com/lambdaclass/ethrex into sto…

f395676

…re-cancun-time

Update

507e969

Merge branch 'main' of https://github.com/lambdaclass/ethrex into sto…

a618b61

…re-cancun-time

replace update_x with set_chain_config method for StoreEngine

688fb6e

Merge branch 'main' of https://github.com/lambdaclass/ethrex into sto…

bbb4e52

…re-cancun-time

Add payload status methods + validate block header

e81c9cd

Fix

938ae60

Execute & store block

7c43792

fmoletta changed the base branch from main to store-cancun-time August 5, 2024 19:26

Store block number

00ab2e4

Base automatically changed from store-cancun-time to main August 6, 2024 14:11

Federica Moletta added 2 commits August 6, 2024 13:43

Remove unwrap

72c4d29

Fix root computation

5ce36ce

Federica Moletta and others added 11 commits August 7, 2024 15:10

Use base fee for block 0 on validation

2cea78c

Compute roots for genesis block

934a3fd

Set initial base fee for genesis block

b085177

Update default values for genesis block

904f86a

Revert change to block validation

38b2ef7

Fix gas limit for beacon root contract call

96f0da7

Add more tracing

abf5a8f

clippy

ee655a0

Update test

4b03b11

Merge branch 'main' of github.com:lambdaclass/ethereum_rust into vali…

45afb13

…date-payload

Add test

e39caf5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `engine_newPayloadV3`: validate, execute & store block #222

feat: `engine_newPayloadV3`: validate, execute & store block #222

fmoletta commented Aug 5, 2024 •

edited

Loading

feat: engine_newPayloadV3: validate, execute & store block #222

feat: engine_newPayloadV3: validate, execute & store block #222

Conversation

fmoletta commented Aug 5, 2024 • edited Loading

feat: `engine_newPayloadV3`: validate, execute & store block #222

feat: `engine_newPayloadV3`: validate, execute & store block #222

fmoletta commented Aug 5, 2024 •

edited

Loading