From ecfa698e493cf10942dc7f7339ad09384e8c33a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jerem=C3=ADas=20Salom=C3=B3n?= <48994069+JereSalo@users.noreply.github.com> Date: Fri, 27 Dec 2024 17:03:30 -0300 Subject: [PATCH] feat(levm): run hive tests with levm, generate and publish report (#1546) **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 in `account_updates` we'll have only the things that have changed from the account, so that we don't have much redundant information - 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. 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. --- .github/scripts/publish_levm_hive.sh | 22 ++++ .../workflows/daily_reports_levm_hive.yaml | 92 +++++++++++++ .gitignore | 1 + Dockerfile | 4 +- cmd/ethrex/Cargo.toml | 2 +- cmd/ethrex/ethrex.rs | 10 +- crates/blockchain/Cargo.toml | 2 +- crates/common/types/account.rs | 2 +- crates/l2/Makefile | 11 ++ crates/storage/store/storage.rs | 2 +- crates/vm/Cargo.toml | 2 +- crates/vm/levm/Makefile | 12 ++ crates/vm/vm.rs | 122 ++++++++++++------ 13 files changed, 241 insertions(+), 43 deletions(-) create mode 100644 .github/scripts/publish_levm_hive.sh create mode 100644 .github/workflows/daily_reports_levm_hive.yaml diff --git a/.github/scripts/publish_levm_hive.sh b/.github/scripts/publish_levm_hive.sh new file mode 100644 index 000000000..3f5db1c6c --- /dev/null +++ b/.github/scripts/publish_levm_hive.sh @@ -0,0 +1,22 @@ +curl -X POST $url \ +-H 'Content-Type: application/json; charset=utf-8' \ +--data @- < results.md + + - name: Post results in summary + run: | + echo "# LEVM Hive coverage report" >> $GITHUB_STEP_SUMMARY + cat results.md >> $GITHUB_STEP_SUMMARY + + - name: Post results to levm slack channel + env: + url: ${{ secrets.LEVM_SLACK_WEBHOOK }} + run: sh .github/scripts/publish_levm_hive.sh + + # Note: Leave this commented, as it is for testing purposes. + # - name: Post results to test channel for debugging + # env: + # url: ${{ secrets.TEST_CHANNEL_SLACK }} + # run: sh .github/scripts/publish_levm_hive.sh diff --git a/.gitignore b/.gitignore index 3076308ec..828116851 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,7 @@ levm_ef_tests_summary_slack.txt levm_ef_tests_summary_github.txt levm_ef_tests_summary.txt +results.md loc_report.md loc_report_slack.txt loc_report_github.txt diff --git a/Dockerfile b/Dockerfile index c8ddcf0bb..cb327d98b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,8 +21,10 @@ COPY --from=planner /ethrex/recipe.json recipe.json # Build dependencies only, these remained cached RUN cargo chef cook --release --recipe-path recipe.json +# Optional build flags +ARG BUILD_FLAGS="" COPY . . -RUN cargo build --release +RUN cargo build --release $BUILD_FLAGS FROM ubuntu:24.04 WORKDIR /usr/local/bin diff --git a/cmd/ethrex/Cargo.toml b/cmd/ethrex/Cargo.toml index d32c516c8..90030c459 100644 --- a/cmd/ethrex/Cargo.toml +++ b/cmd/ethrex/Cargo.toml @@ -45,4 +45,4 @@ dev = ["dep:ethrex-dev"] libmdbx = ["dep:libmdbx", "ethrex-storage/libmdbx"] redb = ["dep:redb", "ethrex-storage/redb"] l2 = ["dep:ethrex-l2", "ethrex-vm/l2"] -levm = ["ethrex-vm/levm", "ethrex-blockchain/levm"] +levm = ["default", "ethrex-vm/levm", "ethrex-blockchain/levm"] diff --git a/cmd/ethrex/ethrex.rs b/cmd/ethrex/ethrex.rs index c5eb55dd9..3a86c5ac7 100644 --- a/cmd/ethrex/ethrex.rs +++ b/cmd/ethrex/ethrex.rs @@ -356,7 +356,15 @@ fn import_blocks(store: &Store, blocks: &Vec) { } if let Some(last_block) = blocks.last() { let hash = last_block.hash(); - apply_fork_choice(store, hash, hash, hash).unwrap(); + cfg_if::cfg_if! { + if #[cfg(feature = "levm")] { + // We are allowing this not to unwrap so that tests can run even if block execution results in the wrong root hash with LEVM. + let _ = apply_fork_choice(store, hash, hash, hash); + } + else { + apply_fork_choice(store, hash, hash, hash).unwrap(); + } + } } info!("Added {} blocks to blockchain", size); } diff --git a/crates/blockchain/Cargo.toml b/crates/blockchain/Cargo.toml index 90aa8c6d1..c832e7f1b 100644 --- a/crates/blockchain/Cargo.toml +++ b/crates/blockchain/Cargo.toml @@ -28,10 +28,10 @@ path = "./blockchain.rs" [features] default = ["c-kzg"] +levm = ["default", "ethrex-vm/levm"] libmdbx = [ "ethrex-core/libmdbx", "ethrex-storage/default", "ethrex-vm/libmdbx", ] -levm = ["ethrex-vm/levm"] c-kzg =["ethrex-core/c-kzg"] diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index 28ac9b9bf..0e70323b7 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -37,7 +37,7 @@ pub struct Account { pub storage: HashMap, } -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Eq)] pub struct AccountInfo { pub code_hash: H256, pub balance: U256, diff --git a/crates/l2/Makefile b/crates/l2/Makefile index 52365e321..e1a915765 100644 --- a/crates/l2/Makefile +++ b/crates/l2/Makefile @@ -66,6 +66,17 @@ init-l1: ## 🚀 Initializes an L1 Lambda ethrex Client --authrpc.port ${L1_AUTH_PORT} \ --datadir ${ethrex_L1_DEV_LIBMDBX} +init-l1-levm: ## 🚀 Initializes an L1 Lambda ethrex Client with LEVM + cargo run --release \ + --manifest-path ../../Cargo.toml \ + --bin ethrex \ + --features dev,ethrex-blockchain/levm,ethrex-vm/levm -- \ + --network ${L1_GENESIS_FILE_PATH} \ + --http.port ${L1_PORT} \ + --http.addr 0.0.0.0 \ + --authrpc.port ${L1_AUTH_PORT} \ + --datadir ${ethrex_L1_DEV_LIBMDBX} + down-local-l1: ## 🛑 Shuts down the L1 Lambda ethrex Client docker compose -f ${ethrex_DEV_DOCKER_COMPOSE_PATH} down docker compose -f docker-compose-l2.yaml down diff --git a/crates/storage/store/storage.rs b/crates/storage/store/storage.rs index 695ffddf0..ffa2fa5bf 100644 --- a/crates/storage/store/storage.rs +++ b/crates/storage/store/storage.rs @@ -44,7 +44,7 @@ pub enum EngineType { RedB, } -#[derive(Default, Debug, Clone, Serialize, Deserialize)] +#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct AccountUpdate { pub address: Address, pub removed: bool, diff --git a/crates/vm/Cargo.toml b/crates/vm/Cargo.toml index c514443cf..ba614a44c 100644 --- a/crates/vm/Cargo.toml +++ b/crates/vm/Cargo.toml @@ -39,11 +39,11 @@ path = "./vm.rs" [features] default = ["c-kzg", "blst"] +levm = ["default", "ethrex-levm"] l2 = [] c-kzg = ["revm/c-kzg"] blst = ["revm/blst"] libmdbx = ["ethrex-storage/default", "ethrex-core/libmdbx"] -levm = ["ethrex-levm"] [profile.test] opt-level = 3 diff --git a/crates/vm/levm/Makefile b/crates/vm/levm/Makefile index 786514db7..63ae36a8e 100644 --- a/crates/vm/levm/Makefile +++ b/crates/vm/levm/Makefile @@ -76,3 +76,15 @@ build-revm-comparison: --bin levm_factorial \ --bin revm_fibonacci \ --bin levm_fibonacci + +###### Build Client with LEVM ###### + +FLAGS := "--features ethrex-blockchain/levm,ethrex-vm/levm,ethrex/levm" + +build-image-levm: ## 🐳 Build the Docker image with LEVM features + cd ../../../ && \ + docker build -t ethrex --build-arg BUILD_FLAGS=$(FLAGS) . + +run-hive-debug-levm: build-image-levm + $(MAKE) -C ../../../ setup-hive + cd ../../../hive && ./hive --sim $(SIMULATION) --client ethrex --sim.limit "$(TEST_PATTERN)" --docker.output diff --git a/crates/vm/vm.rs b/crates/vm/vm.rs index c2940edc3..ac201c253 100644 --- a/crates/vm/vm.rs +++ b/crates/vm/vm.rs @@ -88,6 +88,62 @@ cfg_if::cfg_if! { use std::{collections::HashMap, sync::Arc}; use ethrex_core::types::code_hash; + pub fn get_state_transitions_levm( + initial_state: &EvmState, + block_hash: H256, + new_state: &CacheDB, + ) -> Vec { + let current_db = match initial_state { + EvmState::Store(state) => state.database.store.clone(), + EvmState::Execution(_cache_db) => unreachable!("Execution state should not be passed here"), + }; + let mut account_updates: Vec = vec![]; + for (new_state_account_address, new_state_account) in new_state { + // This stores things that have changed in the account. + let mut account_update = AccountUpdate::new(*new_state_account_address); + + // Account state before block execution. + let initial_account_state = current_db + .get_account_info_by_hash(block_hash, *new_state_account_address) + .expect("Error getting account info by address") + .unwrap_or_default(); + // Account state after block execution. + let new_state_acc_info = AccountInfo { + code_hash: code_hash(&new_state_account.info.bytecode), + balance: new_state_account.info.balance, + nonce: new_state_account.info.nonce, + }; + + // Compare Account Info + if initial_account_state != new_state_acc_info { + account_update.info = Some(new_state_acc_info.clone()); + } + + // If code hash is different it means the code is different too. + if initial_account_state.code_hash != new_state_acc_info.code_hash { + account_update.code = Some(new_state_account.info.bytecode.clone()); + } + + let mut updated_storage = HashMap::new(); + for (key, storage_slot) in &new_state_account.storage { + // original_value in storage_slot is not the original_value on the DB, be careful. + let original_value = current_db.get_storage_at_hash(block_hash, *new_state_account_address, *key).unwrap().unwrap_or_default(); // Option inside result, I guess I have to assume it is zero. + + if original_value != storage_slot.current_value { + updated_storage.insert(*key, storage_slot.current_value); + } + } + account_update.added_storage = updated_storage; + + account_update.removed = new_state_account.is_empty(); + + if account_update != AccountUpdate::new(*new_state_account_address) { + account_updates.push(account_update); + } + } + account_updates + } + /// Executes all transactions in a block and returns their receipts. pub fn execute_block( block: &Block, @@ -103,56 +159,49 @@ cfg_if::cfg_if! { } } } - let mut receipts = Vec::new(); - let mut cumulative_gas_used = 0; let store_wrapper = Arc::new(StoreWrapper { store: state.database().unwrap().clone(), block_hash: block.header.parent_hash, }); - let mut account_updates: Vec = vec![]; + // Account updates are initialized like this because of the beacon_root_contract_call, it is going to be empty if it wasn't called. + let mut account_updates = get_state_transitions(state); - for transaction in block.body.transactions.iter() { - let result = execute_tx_levm(transaction, block_header, store_wrapper.clone()).unwrap(); - cumulative_gas_used += result.gas_used; - let receipt = Receipt::new( - transaction.tx_type(), - matches!(result.result, TxResult::Success), - cumulative_gas_used, - result.logs, - ); - receipts.push(receipt); + let mut receipts = Vec::new(); + let mut cumulative_gas_used = 0; + let mut block_cache: CacheDB = HashMap::new(); - for (address, account) in result.new_state { - let mut added_storage = HashMap::new(); + for tx in block.body.transactions.iter() { + let report = execute_tx_levm(tx, block_header, store_wrapper.clone(), block_cache.clone()).unwrap(); - for (key, value) in account.storage { - added_storage.insert(key, value.current_value); + let mut new_state = report.new_state.clone(); + + // Now original_value is going to be the same as the current_value, for the next transaction. + // It should have only one value but it is convenient to keep on using our CacheDB structure + for account in new_state.values_mut() { + for storage_slot in account.storage.values_mut() { + storage_slot.original_value = storage_slot.current_value; } + } - let code = if account.info.bytecode.is_empty() { - None - } else { - Some(account.info.bytecode.clone()) - }; + block_cache.extend(new_state); - let account_update = AccountUpdate { - address, - removed: false, - info: Some(AccountInfo { - code_hash: code_hash(&account.info.bytecode), - balance: account.info.balance, - nonce: account.info.nonce, - }), - code, - added_storage, - }; + // Currently, in LEVM, we don't substract refunded gas to used gas, but that can change in the future. + let gas_used = report.gas_used - report.gas_refunded; + cumulative_gas_used += gas_used; + let receipt = Receipt::new( + tx.tx_type(), + matches!(report.result.clone(), TxResult::Success), + cumulative_gas_used, + report.logs.clone(), + ); - account_updates.push(account_update); - } + receipts.push(receipt); } + account_updates.extend(get_state_transitions_levm(state, block.header.parent_hash, &block_cache)); + if let Some(withdrawals) = &block.body.withdrawals { process_withdrawals(state, withdrawals)?; } @@ -164,6 +213,7 @@ cfg_if::cfg_if! { tx: &Transaction, block_header: &BlockHeader, db: Arc, + block_cache: CacheDB ) -> Result { let gas_price : U256 = tx.effective_gas_price(block_header.base_fee_per_gas).ok_or(VMError::InvalidTransaction)?.into(); @@ -194,7 +244,7 @@ cfg_if::cfg_if! { tx.value(), tx.data().clone(), db, - CacheDB::default(), + block_cache, tx.access_list(), )?;