bench: no metrics or logs while holding tx lock

[carneiro-cw]

PR Type

Enhancement, Tests

---

Description

• Replace Changes with generic State<S> model

• Adapt storage layers to propagate State changes

• Update executor and EVM outputs to include state

• Refactor and update tests for new state mechanism

---

Diagram Walkthrough

flowchart LR
  A["TransactionExecutionResult includes state"] -- "passed to" --> B["Miner.save_execution(tx, state)"]
  B -- "writes to" --> C["InMemoryTemporaryStorage.state"]
  C -- "on finish" --> D["StratusStorage.finish_pending_block returns (block, State)"]
  D -- "commit" --> E["RocksPermanentStorage.save_block(block, State)"]

Loading

File Walkthrough

	Relevant files
Enhancement	19 files stratus_storage.rs Refactor `StratusStorage` to use generic `State`                  +54/-196 values.rs Implement generic `Change` trait for values                            +108/-152 mod.rs Introduce `State` struct and stage markers                        +194/-0  mod.rs Update `Executor` to propagate new state                                  +33/-88  transaction_execution.rs Extend `TransactionExecutionOutput` with state                      +51/-56  transaction.rs Integrate `State` in temporary transaction storage              +31/-30  mod.rs Remove call storage; use `State` for transactions                +13/-31  resolve_pending.rs Simplify pending resolution logic                                                +17/-86  rocks_state.rs Adapt RocksDB batch writes to use `State`                  +26/-43  rocks_permanent.rs Update permanent storage to accept `State` changes              +6/-15    cache.rs Remove pending cache; use latest-only caching                        +15/-69  miner.rs Pass `State` to miner save and commit flows                            +18/-31  mod.rs Clean up EVM input and state handling                                        +13/-26  transaction_execution.rs Update `TransactionExecution` to use input/output                +16/-16  events.rs Adjust event builder for new execution output                        +12/-12  transaction_mined.rs Map mined transactions to use `State` output                          +19/-21  values.rs Refine `Change` implementations for slot and account          +108/-152 transaction_execution.rs Derive `Deref` for execution output outcome                            +51/-56  transaction.rs Remove old `block_changes` clone logic                                      +31/-30
Tests	1 files mod.rs Update importer tests to use new `State`                                  +13/-13
Additional files	28 files Cargo.toml +2/-2      session.rs +11/-0    call_execution.rs +7/-25    mod.rs +1/-0      call_execution.rs +4/-0      util.rs +4/-4      evm_worker_pool.rs +2/-4      mod.rs +2/-9      task.rs +2/-7      block_with_changes.rs +3/-3      fake_leader.rs +9/-29    replication.rs +3/-3      server.rs +1/-2      log_filter_input.rs +1/-1      mod.rs +0/-1      account.rs +19/-0    block_changes.rs +14/-0    call.rs +0/-127  block.rs +3/-3      pending_block_header.rs +1/-1      execution_kind.rs +0/-58    mod.rs +0/-1      unix_time_now.rs +1/-1      transaction_input.rs +2/-10    transaction_mined.rs +4/-4      transaction_stage.rs +4/-4      metrics_definitions.rs +1/-1      audits.toml +6/-0

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Silent Error The new EntityRead::read_temp returns Option<Self> instead of Result<Option<Self>, StorageError>, silently dropping any errors from temporary reads and masking storage failures. fn read_temp(s: &StratusStorage, key: Self::Key) -> Option<Self>; /// Reads from permanent storage at the resolved mined point. fn read_perm(s: &StratusStorage, key: Self::Key, point: MinedPointInTime<'_>) -> Result<Self, StorageError>; /// Caches the value as a latest (mined tip) entry, if not already cached. fn cache_latest_if_missing(s: &StratusStorage, key: Self::Key, value: Self); } Error Swallowed Methods like read_account return Option<Account> with no error propagation, losing any underlying StorageError and making failures invisible. pub fn read_account(&self, address: Address) -> Option<Account> { match self.pending_block.read().state.accounts.get(&address) { Some(pending_account) => Some(pending_account.clone().to_account(address)), None => self .latest_block Expensive Cloning finish_pending_block and downstream calls return owned State<Complete>, leading to repeated deep clones of the entire state. For large state this can cause high CPU and memory overhead—consider swapping or using references to avoid unnecessary clones. pub fn finish_pending_block(&self) -> (PendingBlock, State<Complete>) { #[cfg(feature = "tracing")] let _span = tracing::info_span!("storage::finish_pending_block", block_number = tracing::field::Empty).entered(); tracing::debug!(storage = %label::TEMP, "finishing pending block"); let result = timed(|| self.temp.finish_pending_block()).with(|m| { metrics::inc_storage_finish_pending_block(m.elapsed); }); Span::with(|s| s.rec_str("block_number", &result.0.header.number));

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Reintroduce block normalization The removed normalization can cause spurious mismatches due to per-transaction fields that don't survive replication. Reintroduce a normalization step so only persistent fields are compared. This restores correct equivalence checking. src/eth/follower/importer/importers/fake_leader.rs [63-66] -if mined_block != expected_block { - tracing::error!(?mined_block, ?expected_block, "block mismatch between leader and fake leader"); +let normalized_mined_block = normalize_for_replication_compare(&mined_block); +if normalized_mined_block != expected_block { + tracing::error!(?normalized_mined_block, ?expected_block, "block mismatch between leader and fake leader"); bail!("block mismatch between leader and fake leader") } Suggestion importance[1-10]: 9 __ Why: The removal of normalize_for_replication_compare breaks equivalence checks by including non-persistent fields, so reintroducing normalization fixes a critical correctness regression.	High
	Return correct U256 default The function signature returns revm::primitives::U256, but Uint::default() may be a different type. Return U256::default() (or U256::ZERO) to match the expected type and avoid conversion issues. src/eth/executor/evm/session.rs [90-92] if address.is_ignored() { - return Ok(Uint::default()); + return Ok(U256::default()); } Suggestion importance[1-10]: 8 __ Why: Returning Uint::default() mismatches the U256 return type of storage_ref, causing potential type errors; using U256::default() ensures the correct primitive is returned.	Medium
General	filter unchanged storage slots Filter out unchanged storage slots before inserting them into the execution state. This avoids polluting the state with slots that were read but not modified. src/eth/executor/evm/types/output/transaction_execution.rs [301-304] let storage = std::mem::take(&mut revm_account.storage); -let account_slots = storage.into_iter().map(|(index, value)| (index.into(), value.into())).collect(); +let account_slots = storage.into_iter() + .filter_map(|(index, value)| { + if value.is_changed() { + Some((index.into(), CompleteValue::Changed(value.present_value.into()))) + } else { + None + } + }) + .collect(); execution_changes.insert_slots(address, account_slots); Suggestion importance[1-10]: 6 __ Why: Filtering out entries where value.is_changed() prevents storing original slots in State<Complete>, reducing unnecessary data and improving clarity without breaking functionality.	Low
	Log full change sets Include both the actual and expected change sets in the error log to make debugging mismatches easier. This provides full context on what diverged. src/eth/follower/importer/importers/fake_leader.rs [58-61] if final_changes != final_expected_changes { - tracing::error!(?mined_block, "execution changes result mismatch between leader and fake leader"); + tracing::error!( + ?mined_block, + ?final_changes, + ?final_expected_changes, + "execution changes result mismatch between leader and fake leader" + ); bail!("execution changes mismatch between leader and fake leader") } Suggestion importance[1-10]: 6 __ Why: Adding ?final_changes and ?final_expected_changes to the error log provides valuable debugging context without altering functionality.	Low

[stratus-benchmark]

Benchmark:
Run ID: bench-85dffaf3

Git Info:

• Branch: bench_no_metrics_logs
• Commit: c6cb000
• PR: bench: no metrics or logs while holding tx lock #2651

Leader Stats:
RPS Stats: Max: 6543.00, Min: 2041.00, Avg: 4130.48, StdDev: 435.48
TPS Stats: Max: 4827.00, Min: 2.00, Avg: 4061.67, StdDev: 620.90

Follower Stats:
Imported Blocks/s: Max: 6.00, Min: 2.00, Avg: 4.21, StdDev: 1.01
Imported Transactions/s: Max: 24786.00, Min: 2684.00, Avg: 17092.86, StdDev: 4279.30

Plots:

• View Leader Plot
• View Follower Plot