From 1b96b4c52933bd47ef0f03cf56986def3319b956 Mon Sep 17 00:00:00 2001 From: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:40:01 -0300 Subject: [PATCH 01/10] fix(levm): makefile EVM EF tests run command (#1261) --- .github/workflows/ci_levm.yaml | 4 ++-- crates/vm/levm/Makefile | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci_levm.yaml b/.github/workflows/ci_levm.yaml index f5d3a3a77..ff8d5734e 100644 --- a/.github/workflows/ci_levm.yaml +++ b/.github/workflows/ci_levm.yaml @@ -34,12 +34,12 @@ jobs: - name: Download EF Tests run: | cd crates/vm/levm - make download-ef-tests + make download-evm-ef-tests - name: Run tests run: | cd crates/vm/levm - make run-ef-tests + make run-evm-ef-tests test: name: Tests runs-on: ubuntu-latest diff --git a/crates/vm/levm/Makefile b/crates/vm/levm/Makefile index b2947f224..552c4015d 100644 --- a/crates/vm/levm/Makefile +++ b/crates/vm/levm/Makefile @@ -29,13 +29,13 @@ $(SPECTEST_VECTORS_DIR): $(SPECTEST_ARTIFACT) tar -xzf $(SPECTEST_ARTIFACT) -C tmp mv tmp/tests-14.1/GeneralStateTests $(SPECTEST_VECTORS_DIR) -download-ef-tests: $(SPECTEST_VECTORS_DIR) ## 📥 Download EF Tests +download-evm-ef-tests: $(SPECTEST_VECTORS_DIR) ## 📥 Download EF Tests -run-ef-tests: ## 🏃‍♂️ Run EF Tests +run-evm-ef-tests: ## 🏃‍♂️ Run EF Tests cd ../../../ && \ - cargo test -p ef_tests-levm --tests test + time cargo test -p ef_tests-levm --test ef_tests_levm -clean-ef-tests: ## 🗑️ Clean test vectors +clean-evm-ef-tests: ## 🗑️ Clean test vectors rm -rf $(SPECTEST_VECTORS_DIR) ###### Benchmarks ###### From 18fbe437f8fa1ab36c8fcffe24afcebe6037e14b Mon Sep 17 00:00:00 2001 From: Maximo Palopoli <96491141+maximopalopoli@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:22:29 -0300 Subject: [PATCH 02/10] fix(levm): return error in returndatacopy in unexpected behavior (#1244) **Motivation** Fixes a bug found by [FuzzingLabs](https://github.com/FuzzingLabs) in returndatacopy opcode implementation. **Description** Now returns an error in `returndatacopy` if offset is larger than the return data size. Closes #1232 --- .../levm/src/opcode_handlers/environment.rs | 26 +++++++++---------- crates/vm/levm/tests/edge_case_tests.rs | 10 +++++++ 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/crates/vm/levm/src/opcode_handlers/environment.rs b/crates/vm/levm/src/opcode_handlers/environment.rs index f0d7e02c8..106317472 100644 --- a/crates/vm/levm/src/opcode_handlers/environment.rs +++ b/crates/vm/levm/src/opcode_handlers/environment.rs @@ -393,19 +393,19 @@ impl VM { } let sub_return_data_len = current_call_frame.sub_return_data.len(); - let data = if returndata_offset < sub_return_data_len { - current_call_frame.sub_return_data.slice( - returndata_offset - ..(returndata_offset - .checked_add(size) - .ok_or(VMError::Internal( - InternalError::ArithmeticOperationOverflow, - ))?) - .min(sub_return_data_len), - ) - } else { - vec![0u8; size].into() - }; + + if returndata_offset >= sub_return_data_len { + return Err(VMError::VeryLargeNumber); // Maybe can create a new error instead of using this one + } + let data = current_call_frame.sub_return_data.slice( + returndata_offset + ..(returndata_offset + .checked_add(size) + .ok_or(VMError::Internal( + InternalError::ArithmeticOperationOverflow, + ))?) + .min(sub_return_data_len), + ); current_call_frame.memory.store_bytes(dest_offset, &data)?; diff --git a/crates/vm/levm/tests/edge_case_tests.rs b/crates/vm/levm/tests/edge_case_tests.rs index e406e36a3..3da16d0de 100644 --- a/crates/vm/levm/tests/edge_case_tests.rs +++ b/crates/vm/levm/tests/edge_case_tests.rs @@ -1,6 +1,7 @@ use bytes::Bytes; use ethrex_core::U256; use ethrex_levm::{ + errors::{TxResult, VMError}, operations::Operation, utils::{new_vm_with_bytecode, new_vm_with_ops}, }; @@ -114,3 +115,12 @@ fn test_sdiv_zero_dividend_and_negative_divisor() { vm.execute(&mut current_call_frame); assert_eq!(current_call_frame.stack.pop().unwrap(), U256::zero()); } + +#[test] +fn test_non_compliance_returndatacopy() { + let mut vm = + new_vm_with_bytecode(Bytes::copy_from_slice(&[56, 56, 56, 56, 56, 56, 62, 56])).unwrap(); + let mut current_call_frame = vm.call_frames.pop().unwrap(); + let txreport = vm.execute(&mut current_call_frame); + assert_eq!(txreport.result, TxResult::Revert(VMError::VeryLargeNumber)); +} From 1c771bcedf8fc2daa020ea6988f0d4ca5c0067e3 Mon Sep 17 00:00:00 2001 From: Maximo Palopoli <96491141+maximopalopoli@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:39:50 -0300 Subject: [PATCH 03/10] fix(levm): remove the push for success in return (#1248) **Motivation** Fixes a bug found by [FuzzingLabs](https://github.com/FuzzingLabs) in return opcode implementation. **Description** Now return opcode does not pushes 1 in the stack in case of success, since it is not the documented behavior. Closes #1224 --- crates/vm/levm/src/opcode_handlers/system.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/crates/vm/levm/src/opcode_handlers/system.rs b/crates/vm/levm/src/opcode_handlers/system.rs index fe676f2bb..d2a883347 100644 --- a/crates/vm/levm/src/opcode_handlers/system.rs +++ b/crates/vm/levm/src/opcode_handlers/system.rs @@ -1,6 +1,5 @@ use crate::{ call_frame::CallFrame, - constants::SUCCESS_FOR_RETURN, errors::{InternalError, OpcodeSuccess, ResultReason, VMError}, gas_cost, vm::{word_to_address, VM}, @@ -184,9 +183,6 @@ impl VM { let return_data = current_call_frame.memory.load_range(offset, size)?.into(); current_call_frame.returndata = return_data; - current_call_frame - .stack - .push(U256::from(SUCCESS_FOR_RETURN))?; Ok(OpcodeSuccess::Result(ResultReason::Return)) } From a9d45d6fa9d4a233b6e035e711aec0a1bc51a799 Mon Sep 17 00:00:00 2001 From: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:41:51 -0300 Subject: [PATCH 04/10] feat(l1, l2, levm): improve loc reporter (#1264) - Print summary - Send summary to slack --- .github/workflows/loc.yaml | 23 +++++++++++++++++++++++ .gitignore | 2 ++ cmd/loc/src/main.rs | 25 +++++++++++++++---------- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/.github/workflows/loc.yaml b/.github/workflows/loc.yaml index b8a7046b4..c26b87b66 100644 --- a/.github/workflows/loc.yaml +++ b/.github/workflows/loc.yaml @@ -26,4 +26,27 @@ jobs: uses: Swatinem/rust-cache@v2 - name: Generate the loc report + id: loc-report run: make loc + echo "content=$(cat loc_report.md)" >> $GITHUB_OUTPUT + + - name: Post results in summary + run: | + echo "# `ethrex` lines of code report:\n\n" >> $GITHUB_STEP_SUMMARY + $(cat loc_report.md) >> $GITHUB_STEP_SUMMARY + + - name: Post results to slack + uses: slackapi/slack-github-action@v2.0.0 + with: + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + webhook-type: incoming-webhook + payload: | + blocks: + - type: "header" + text: + type: "plain_text" + text: ethrex lines of code report + - type: "section" + text: + type: "mrkdwn" + text: ${{steps.loc-report.outputs.content}} diff --git a/.gitignore b/.gitignore index 3ee1c729c..6cfabcf47 100644 --- a/.gitignore +++ b/.gitignore @@ -48,3 +48,5 @@ tests_v3.0.0.tar.gz .env levm_ef_tests_report.txt + +loc_report.md diff --git a/cmd/loc/src/main.rs b/cmd/loc/src/main.rs index c2a8eb30f..0b180b380 100644 --- a/cmd/loc/src/main.rs +++ b/cmd/loc/src/main.rs @@ -1,4 +1,3 @@ -use colored::Colorize; use std::path::PathBuf; use tokei::{Config, LanguageType, Languages}; @@ -23,14 +22,20 @@ fn main() { languages.get_statistics(&[ethrex_l2], &[], &config); let ethrex_l2_loc = &languages.get(&LanguageType::Rust).unwrap(); - println!("{}", "ethrex loc summary".bold()); - println!("{}", "====================".bold()); - println!( - "{}: {:?}", - "ethrex L1".bold(), - ethrex_loc.code - ethrex_l2_loc.code - levm_loc.code + let report = format!( + r#"``` +ethrex loc summary +==================== +ethrex L1: {} +ethrex L2: {} +levm: {} +ethrex (total): {} +```"#, + ethrex_loc.code - ethrex_l2_loc.code - levm_loc.code, + ethrex_l2_loc.code, + levm_loc.code, + ethrex_loc.code, ); - println!("{}: {:?}", "ethrex L2".bold(), ethrex_l2_loc.code); - println!("{}: {:?}", "levm".bold(), levm_loc.code); - println!("{}: {:?}", "ethrex (total)".bold(), ethrex_loc.code); + + std::fs::write("loc_report.md", report).unwrap(); } From 08d15bb65168bad219fed79b05fe56233755abd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jerem=C3=ADas=20Salom=C3=B3n?= <48994069+JereSalo@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:43:08 -0300 Subject: [PATCH 05/10] fix(levm): fix revm_runner test execution (#1265) **Motivation** - When running Cancun tests revm Halted because the spec_id was older than Cancun. **Description** Closes #issue_number --- cmd/ef_tests/levm/runner/revm_runner.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/ef_tests/levm/runner/revm_runner.rs b/cmd/ef_tests/levm/runner/revm_runner.rs index 9594d6d9d..9c7052558 100644 --- a/cmd/ef_tests/levm/runner/revm_runner.rs +++ b/cmd/ef_tests/levm/runner/revm_runner.rs @@ -7,7 +7,7 @@ use crate::{ use ethrex_core::{types::TxKind, Address}; use ethrex_levm::errors::{TransactionReport, TxResult}; use ethrex_storage::{error::StoreError, AccountUpdate}; -use ethrex_vm::{db::StoreWrapper, spec_id, EvmState, RevmAddress, RevmU256}; +use ethrex_vm::{db::StoreWrapper, EvmState, RevmAddress, RevmU256, SpecId}; use revm::{ db::State, inspectors::TracerEip3155 as RevmTracerEip3155, @@ -135,7 +135,7 @@ pub fn prepare_revm_for_tx<'state>( .with_block_env(block_env) .with_tx_env(tx_env) .modify_cfg_env(|cfg| cfg.chain_id = chain_spec.chain_id) - .with_spec_id(spec_id(&chain_spec, test.env.current_timestamp.as_u64())) + .with_spec_id(SpecId::CANCUN) .with_external_context( RevmTracerEip3155::new(Box::new(std::io::stderr())).without_summary(), ); From 450476a3b46b591f7e03e15533bf8ba8bebedff1 Mon Sep 17 00:00:00 2001 From: Maximo Palopoli <96491141+maximopalopoli@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:57:05 -0300 Subject: [PATCH 06/10] fix(levm): prevent inefficient memory allocation in extcodecopy (#1258) **Motivation** Fixes an memory inefficiency found by [FuzzingLabs](https://github.com/FuzzingLabs) in extcodecopy opcode implementation. **Description** Before, there was memory allocation even if the size to copy was zero. Now, if that happens, returns. Closes #1245 --- crates/vm/levm/src/opcode_handlers/environment.rs | 4 ++++ crates/vm/levm/tests/edge_case_tests.rs | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/crates/vm/levm/src/opcode_handlers/environment.rs b/crates/vm/levm/src/opcode_handlers/environment.rs index 106317472..7e1d90d34 100644 --- a/crates/vm/levm/src/opcode_handlers/environment.rs +++ b/crates/vm/levm/src/opcode_handlers/environment.rs @@ -314,6 +314,10 @@ impl VM { self.increase_consumed_gas(current_call_frame, gas_cost)?; + if size == 0 { + return Ok(OpcodeSuccess::Continue); + } + if !is_cached { self.cache_from_db(&address); }; diff --git a/crates/vm/levm/tests/edge_case_tests.rs b/crates/vm/levm/tests/edge_case_tests.rs index 3da16d0de..2ae94e7eb 100644 --- a/crates/vm/levm/tests/edge_case_tests.rs +++ b/crates/vm/levm/tests/edge_case_tests.rs @@ -124,3 +124,11 @@ fn test_non_compliance_returndatacopy() { let txreport = vm.execute(&mut current_call_frame); assert_eq!(txreport.result, TxResult::Revert(VMError::VeryLargeNumber)); } + +#[test] +fn test_non_compliance_extcodecopy() { + let mut vm = new_vm_with_bytecode(Bytes::copy_from_slice(&[88, 88, 88, 89, 60, 89])).unwrap(); + let mut current_call_frame = vm.call_frames.pop().unwrap(); + vm.execute(&mut current_call_frame); + assert_eq!(current_call_frame.stack.stack.pop().unwrap(), U256::zero()); +} From ec09fed25dfc8274c579090e4b6775f12aad578d Mon Sep 17 00:00:00 2001 From: Maximo Palopoli <96491141+maximopalopoli@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:41:48 -0300 Subject: [PATCH 07/10] fix(levm): add memory alignment in extcodecopy (#1263) **Motivation** Fixes an implementation error found by [FuzzingLabs](https://github.com/FuzzingLabs) in extcodecopy opcode implementation. **Description** Previously, in EXTCODECOPY the allocated memory size was not aligned to a multiple of 32 bytes, and was added in, for example, 12 bytes (should increase in blocks of 32). Now the increases are done in groups of 32. Closes #1251 --- crates/vm/levm/src/opcode_handlers/environment.rs | 8 ++++++-- crates/vm/levm/tests/edge_case_tests.rs | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/vm/levm/src/opcode_handlers/environment.rs b/crates/vm/levm/src/opcode_handlers/environment.rs index 7e1d90d34..0edce6a62 100644 --- a/crates/vm/levm/src/opcode_handlers/environment.rs +++ b/crates/vm/levm/src/opcode_handlers/environment.rs @@ -324,9 +324,13 @@ impl VM { let bytecode = self.get_account(&address).info.bytecode; - let new_memory_size = dest_offset.checked_add(size).ok_or(VMError::Internal( + let new_memory_size = (((!size).checked_add(1).ok_or(VMError::Internal( InternalError::ArithmeticOperationOverflow, - ))?; + ))?) & 31) + .checked_add(size) + .ok_or(VMError::Internal( + InternalError::ArithmeticOperationOverflow, + ))?; let current_memory_size = current_call_frame.memory.data.len(); if current_memory_size < new_memory_size { current_call_frame.memory.data.resize(new_memory_size, 0); diff --git a/crates/vm/levm/tests/edge_case_tests.rs b/crates/vm/levm/tests/edge_case_tests.rs index 2ae94e7eb..33845f3bd 100644 --- a/crates/vm/levm/tests/edge_case_tests.rs +++ b/crates/vm/levm/tests/edge_case_tests.rs @@ -132,3 +132,14 @@ fn test_non_compliance_extcodecopy() { vm.execute(&mut current_call_frame); assert_eq!(current_call_frame.stack.stack.pop().unwrap(), U256::zero()); } + +#[test] +fn test_non_compliance_extcodecopy_memory_resize() { + let mut vm = new_vm_with_bytecode(Bytes::copy_from_slice(&[ + 0x60, 12, 0x5f, 0x5f, 0x5f, 0x3c, 89, + ])) + .unwrap(); + let mut current_call_frame = vm.call_frames.pop().unwrap(); + vm.execute(&mut current_call_frame); + assert_eq!(current_call_frame.stack.pop().unwrap(), U256::from(32)); +} From 40e606805b6d6e46e69489dcb19090b17a69ea4c Mon Sep 17 00:00:00 2001 From: Federico Borello <156438142+fborello-lambda@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:50:50 -0300 Subject: [PATCH 08/10] chore(l2): rename cli's binary (#1271) **Motivation** Change the CLI's name so it has `ethrex` in the binary's name. --- cmd/ethrex_l2/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ethrex_l2/Cargo.toml b/cmd/ethrex_l2/Cargo.toml index 5b969aa03..dbc3cc791 100644 --- a/cmd/ethrex_l2/Cargo.toml +++ b/cmd/ethrex_l2/Cargo.toml @@ -34,5 +34,5 @@ ethrex-rlp.workspace = true ethrex-rpc.workspace = true [[bin]] -name = "l2" +name = "ethrex_l2" path = "./src/main.rs" From 2330a51c30061b6eac13b6b0c1343d4410e8f2fc Mon Sep 17 00:00:00 2001 From: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:55:23 -0300 Subject: [PATCH 09/10] fix(l1, l2, levm): error in loc reporter (#1272) --- .github/workflows/loc.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/loc.yaml b/.github/workflows/loc.yaml index c26b87b66..f72172d0c 100644 --- a/.github/workflows/loc.yaml +++ b/.github/workflows/loc.yaml @@ -27,8 +27,9 @@ jobs: - name: Generate the loc report id: loc-report - run: make loc - echo "content=$(cat loc_report.md)" >> $GITHUB_OUTPUT + run: | + make loc + echo "content=$(cat loc_report.md)" >> $GITHUB_OUTPUT - name: Post results in summary run: | From ae10d075b8b87d1fac60b2c53e17e8398a91f680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Arjovsky?= Date: Mon, 25 Nov 2024 22:03:24 +0100 Subject: [PATCH 10/10] fix(l1): fix hive report workflow (#1262) **Motivation** The step that builds the report and saves it to an output so the report is not being published. --- .github/workflows/hive_coverage.yaml | 22 +++++----------------- publish.sh | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 publish.sh diff --git a/.github/workflows/hive_coverage.yaml b/.github/workflows/hive_coverage.yaml index 8268c9c7f..6f9f856f4 100644 --- a/.github/workflows/hive_coverage.yaml +++ b/.github/workflows/hive_coverage.yaml @@ -51,25 +51,13 @@ jobs: id: report run: | cargo run -p hive_report > results.md - echo "content=$(cat results.md)" >> $GITHUB_OUTPUT - name: Post results in summary run: | - echo "# Hive coverage report\n\n" >> $GITHUB_STEP_SUMMARY - $(cat results.md) >> $GITHUB_STEP_SUMMARY + echo "# Hive coverage report" >> $GITHUB_STEP_SUMMARY + cat results.md >> $GITHUB_STEP_SUMMARY - name: Post results to slack - uses: slackapi/slack-github-action@v2.0.0 - with: - webhook: ${{ secrets.SLACK_WEBHOOK_URL }} - webhook-type: incoming-webhook - payload: | - blocks: - - type: "header" - text: - type: "plain_text" - text: Hive coverage report - - type: "section" - text: - type: "mrkdwn" - text: ${{steps.report.outputs.content}} + env: + url: ${{ secrets.SLACK_WEBHOOK_URL }} + run: sh publish.sh diff --git a/publish.sh b/publish.sh new file mode 100644 index 000000000..37d7bc37f --- /dev/null +++ b/publish.sh @@ -0,0 +1,22 @@ +curl -X POST $url \ +-H 'Content-Type: application/json; charset=utf-8' \ +--data @- <