From 3447b4667b3161bd295bb65d6dbe31e6cace3e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Thu, 28 Nov 2024 16:39:57 -0300 Subject: [PATCH 1/3] fix(l2): use nonce diff in state diff (#1210) **Motivation** We were sending the new nonce instead of nonce diff. **Description** Use the previous block state to calculate the nonce diff. Closes #1098 --------- Co-authored-by: fborello-lambda Co-authored-by: Federico Borello <156438142+fborello-lambda@users.noreply.github.com> --- crates/l2/proposer/errors.rs | 2 ++ crates/l2/proposer/l1_committer.rs | 30 ++++++++++++++++++++++++------ crates/l2/proposer/state_diff.rs | 6 +++--- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/l2/proposer/errors.rs b/crates/l2/proposer/errors.rs index d33cbe731..3eae74dac 100644 --- a/crates/l2/proposer/errors.rs +++ b/crates/l2/proposer/errors.rs @@ -80,6 +80,8 @@ pub enum CommitterError { FailedToParseLastCommittedBlock(#[from] FromStrRadixErr), #[error("Committer failed retrieve block from storage: {0}")] FailedToRetrieveBlockFromStorage(#[from] StoreError), + #[error("Committer failed retrieve data from storage")] + FailedToRetrieveDataFromStorage, #[error("Committer failed to generate blobs bundle: {0}")] FailedToGenerateBlobsBundle(#[from] BlobsBundleError), #[error("Committer failed to get information from storage")] diff --git a/crates/l2/proposer/l1_committer.rs b/crates/l2/proposer/l1_committer.rs index 3633d15cf..c157c0ba0 100644 --- a/crates/l2/proposer/l1_committer.rs +++ b/crates/l2/proposer/l1_committer.rs @@ -17,7 +17,7 @@ use ethrex_core::{ }, Address, H256, U256, }; -use ethrex_storage::Store; +use ethrex_storage::{error::StoreError, Store}; use ethrex_vm::{evm_state, execute_block, get_state_transitions}; use keccak_hash::keccak; use secp256k1::SecretKey; @@ -132,7 +132,7 @@ impl Committer { deposits, )?; - let blobs_bundle = self.generate_blobs_bundle(state_diff.clone())?; + let blobs_bundle = self.generate_blobs_bundle(&state_diff)?; let head_block_hash = block_to_commit.hash(); match self @@ -242,18 +242,36 @@ impl Committer { let account_updates = get_state_transitions(&mut state); let mut modified_accounts = HashMap::new(); - account_updates.iter().for_each(|account_update| { + for account_update in &account_updates { + let prev_nonce = match state + .database() + .ok_or(CommitterError::FailedToRetrieveDataFromStorage)? + // If we want the state_diff of a batch, we will have to change the -1 with the `batch_size` + // and we may have to keep track of the latestCommittedBlock (last block of the batch), + // the batch_size and the latestCommittedBatch in the contract. + .get_account_info(block.header.number - 1, account_update.address) + .map_err(StoreError::from)? + { + Some(acc) => acc.nonce, + None => 0, + }; + modified_accounts.insert( account_update.address, AccountStateDiff { new_balance: account_update.info.clone().map(|info| info.balance), - nonce_diff: account_update.info.clone().map(|info| info.nonce as u16), + nonce_diff: (account_update + .info + .clone() + .ok_or(CommitterError::FailedToRetrieveDataFromStorage)? + .nonce + - prev_nonce) as u16, storage: account_update.added_storage.clone().into_iter().collect(), bytecode: account_update.code.clone(), bytecode_hash: None, }, ); - }); + } let state_diff = StateDiff { modified_accounts, @@ -287,7 +305,7 @@ impl Committer { /// Generate the blob bundle necessary for the EIP-4844 transaction. pub fn generate_blobs_bundle( &self, - state_diff: StateDiff, + state_diff: &StateDiff, ) -> Result { let blob_data = state_diff.encode().map_err(CommitterError::from)?; diff --git a/crates/l2/proposer/state_diff.rs b/crates/l2/proposer/state_diff.rs index 966181310..c246bb285 100644 --- a/crates/l2/proposer/state_diff.rs +++ b/crates/l2/proposer/state_diff.rs @@ -8,7 +8,7 @@ use super::errors::StateDiffError; #[derive(Clone)] pub struct AccountStateDiff { pub new_balance: Option, - pub nonce_diff: Option, + pub nonce_diff: u16, pub storage: Vec<(H256, U256)>, pub bytecode: Option, pub bytecode_hash: Option, @@ -125,9 +125,9 @@ impl AccountStateDiff { encoded.extend_from_slice(buf); } - if let Some(nonce_diff) = self.nonce_diff { + if self.nonce_diff != 0 { r#type += AccountStateDiffType::NonceDiff as u8; - encoded.extend(nonce_diff.to_be_bytes()); + encoded.extend(self.nonce_diff.to_be_bytes()); } if !self.storage.is_empty() { From 2036de875bcec9e92ccaec61dbb5e2eb3040890e Mon Sep 17 00:00:00 2001 From: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Date: Thu, 28 Nov 2024 17:01:50 -0300 Subject: [PATCH 2/3] feat(l1, l2, levm): print loc diff in loc report message (#1342) Resolves #1300 --- .github/workflows/loc.yaml | 22 +++++++- cmd/loc/Cargo.toml | 3 +- cmd/loc/src/main.rs | 110 +++++++++++++++++++++++++++++++------ 3 files changed, 116 insertions(+), 19 deletions(-) diff --git a/.github/workflows/loc.yaml b/.github/workflows/loc.yaml index 4724c3f25..44a58b929 100644 --- a/.github/workflows/loc.yaml +++ b/.github/workflows/loc.yaml @@ -1,4 +1,4 @@ -name: Weekly LoC +name: Daily Lines of Code Report on: schedule: @@ -25,10 +25,30 @@ jobs: - name: Add Rust Cache uses: Swatinem/rust-cache@v2 + - name: Restore cache + id: cache-loc-report + uses: actions/cache@v3 + with: + path: loc_report.json + key: loc-report-${{ github.ref_name }} + restore-keys: | + loc-report- + + - name: Rename cached loc_report.json to loc_report.json.old + if: steps.cache-loc-report.outputs.cache-hit == 'true' + run: mv loc_report.json loc_report.json.old + - name: Generate the loc report run: | make loc + - name: Save new loc_report.json to cache + if: success() + uses: actions/cache@v3 + with: + path: loc_report.json + key: loc-report-${{ github.ref_name }} + - name: Post results in summary run: | echo "# `ethrex` lines of code report" >> $GITHUB_STEP_SUMMARY diff --git a/cmd/loc/Cargo.toml b/cmd/loc/Cargo.toml index 55addc9d9..8c81c97b3 100644 --- a/cmd/loc/Cargo.toml +++ b/cmd/loc/Cargo.toml @@ -5,4 +5,5 @@ edition.workspace = true [dependencies] tokei = "12.1.2" -colored = "2.1.0" +serde = "1.0.215" +serde_json = "1.0.133" diff --git a/cmd/loc/src/main.rs b/cmd/loc/src/main.rs index 3eb15c074..506ea752d 100644 --- a/cmd/loc/src/main.rs +++ b/cmd/loc/src/main.rs @@ -1,8 +1,17 @@ +use serde::{Deserialize, Serialize}; use std::path::PathBuf; use tokei::{Config, LanguageType, Languages}; const CARGO_MANIFEST_DIR: &str = std::env!("CARGO_MANIFEST_DIR"); +#[derive(Default, Serialize, Deserialize, Clone, Copy)] +pub struct LinesOfCodeReport { + ethrex: usize, + ethrex_l1: usize, + ethrex_l2: usize, + levm: usize, +} + fn main() { let ethrex = PathBuf::from(CARGO_MANIFEST_DIR).join("../../"); let levm = PathBuf::from(CARGO_MANIFEST_DIR).join("../../crates/vm"); @@ -22,41 +31,108 @@ fn main() { languages.get_statistics(&[ethrex_l2], &["tests"], &config); let ethrex_l2_loc = &languages.get(&LanguageType::Rust).unwrap(); + let new_report = LinesOfCodeReport { + ethrex: ethrex_loc.code, + ethrex_l1: ethrex_loc.code - ethrex_l2_loc.code - levm_loc.code, + ethrex_l2: ethrex_l2_loc.code, + levm: levm_loc.code, + }; + + std::fs::write( + "loc_report.json", + serde_json::to_string(&new_report).unwrap(), + ) + .expect("loc_report.json could not be written"); + + let old_report: LinesOfCodeReport = std::fs::read_to_string("loc_report.json.old") + .map(|s| serde_json::from_str(&s).unwrap()) + .unwrap_or_default(); + std::fs::write( "loc_report_slack.txt", - slack_message(ethrex_loc.code, ethrex_l2_loc.code, levm_loc.code), + slack_message(old_report, new_report), ) .unwrap(); std::fs::write( "loc_report_github.txt", - github_step_summary(ethrex_loc.code, ethrex_l2_loc.code, levm_loc.code), + github_step_summary(old_report, new_report), ) .unwrap(); } -fn slack_message(ethrex_loc: usize, ethrex_l2_loc: usize, levm_loc: usize) -> String { +fn slack_message(old_report: LinesOfCodeReport, new_report: LinesOfCodeReport) -> String { + let ethrex_l1_diff = new_report.ethrex_l1.abs_diff(old_report.ethrex_l1); + let ethrex_l2_diff = new_report.ethrex_l2.abs_diff(old_report.ethrex_l2); + let levm_diff = new_report.levm.abs_diff(old_report.levm); + let ethrex_diff_total = ethrex_l1_diff + ethrex_l2_diff + levm_diff; + format!( - r#"*ethrex L1:* {}\n*ethrex L2:* {}\n*levm:* {}\n*ethrex (total):* {}"#, - ethrex_loc - ethrex_l2_loc - levm_loc, - ethrex_l2_loc, - levm_loc, - ethrex_loc, + r#"*ethrex L1:* {} {}\n*ethrex L2:* {} {}\n*levm:* {} {}\n*ethrex (total):* {} {}"#, + new_report.ethrex_l1, + if new_report.ethrex > old_report.ethrex { + format!("(+{ethrex_l1_diff})") + } else { + format!("(-{ethrex_l1_diff})") + }, + new_report.ethrex_l2, + if new_report.ethrex_l2 > old_report.ethrex_l2 { + format!("(+{ethrex_l2_diff})") + } else { + format!("(-{ethrex_l2_diff})") + }, + new_report.levm, + if new_report.levm > old_report.levm { + format!("(+{levm_diff})") + } else { + format!("(-{levm_diff})") + }, + new_report.ethrex, + if new_report.ethrex > old_report.ethrex { + format!("(+{ethrex_diff_total})") + } else { + format!("(-{ethrex_diff_total})") + }, ) } -fn github_step_summary(ethrex_loc: usize, ethrex_l2_loc: usize, levm_loc: usize) -> String { +fn github_step_summary(old_report: LinesOfCodeReport, new_report: LinesOfCodeReport) -> String { + let ethrex_l1_diff = new_report.ethrex_l1.abs_diff(old_report.ethrex_l1); + let ethrex_l2_diff = new_report.ethrex_l2.abs_diff(old_report.ethrex_l2); + let levm_diff = new_report.levm.abs_diff(old_report.levm); + let ethrex_diff_total = ethrex_l1_diff + ethrex_l2_diff + levm_diff; + format!( r#"``` ethrex loc summary ==================== -ethrex L1: {} -ethrex L2: {} -levm: {} -ethrex (total): {} +ethrex L1: {} {} +ethrex L2: {} {} +levm: {} ({}) +ethrex (total): {} {} ```"#, - ethrex_loc - ethrex_l2_loc - levm_loc, - ethrex_l2_loc, - levm_loc, - ethrex_loc, + new_report.ethrex_l1, + if new_report.ethrex > old_report.ethrex { + format!("(+{ethrex_l1_diff})") + } else { + format!("(-{ethrex_l1_diff})") + }, + new_report.ethrex_l2, + if new_report.ethrex_l2 > old_report.ethrex_l2 { + format!("(+{ethrex_l2_diff})") + } else { + format!("(-{ethrex_l2_diff})") + }, + new_report.levm, + if new_report.levm > old_report.levm { + format!("(+{levm_diff})") + } else { + format!("(-{levm_diff})") + }, + new_report.ethrex, + if new_report.ethrex > old_report.ethrex { + format!("(+{ethrex_diff_total})") + } else { + format!("(-{ethrex_diff_total})") + }, ) } From 31626499d49b7a074a39a8eae03ee5049d030d48 Mon Sep 17 00:00:00 2001 From: Martin Paulucci Date: Thu, 28 Nov 2024 21:07:22 +0100 Subject: [PATCH 3/3] ci(core): fix CI skipped triggering. (#1346) **Motivation** We want to skip this on any code changes, and have it if there is **only** doc changes. Dunno if there's a better way to do it. --- .github/workflows/ci_skipped.yaml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci_skipped.yaml b/.github/workflows/ci_skipped.yaml index c650d209b..876951ed0 100644 --- a/.github/workflows/ci_skipped.yaml +++ b/.github/workflows/ci_skipped.yaml @@ -2,11 +2,15 @@ name: CI Skipped on: pull_request: branches: ["**"] - paths: - - 'README.md' - - 'LICENSE' - - "**/README.md" - - "**/docs/**" + paths-ignore: + - '**/*.rs' + - '**/*.toml' + - '**/*.yaml' + - '**/*.sh' + - '**/*.json' + - '**/*.rlp' + - 'Dockefile*' + jobs: lint: # "Lint" is a required check, don't change the name