perf: remove inmemory call storage

[carneiro-cw]

PR Type

Enhancement

---

Description

• Remove in-memory call storage and TxCount

• Extract TransactionExecutionResult from execution outputs

• Propagate execution state separately through executor and storage

• Simplify pending resolution and RPC call paths

---

File Walkthrough

	Relevant files
Cleanup	3 files call_execution.rs Remove pending-block constructor and TxCount                          +0/-15    mod.rs Remove unused `TxCount` export                                                      +0/-1      mod.rs Remove call storage, unify transaction temp store                +10/-29
Enhancement	9 files mod.rs Export `TransactionExecutionResult` type                                  +1/-0      call_execution.rs Add `is_success` helper method                                                      +4/-0      transaction_execution.rs Split output into `outcome` and `state`                                    +36/-20  mod.rs Pass `state` through executor and miner                                    +24/-26  transaction_execution.rs Replace output with `TransactionExecutionResult`                  +3/-3      fake_leader.rs Finalize and compare full state changes                                    +5/-22    miner.rs Extend `save_execution` with state parameter                          +2/-2      transaction_mined.rs Map to `TransactionExecutionResult`                                            +2/-4      pending_block_header.rs Derive `Copy` for pending header                                                  +1/-1
Tests	2 files mod.rs Use `TransactionExecutionResult` in tests                                +5/-6      rocks_state.rs Update tests to use `TransactionExecutionResult`                  +3/-3
Bug fix	2 files server.rs Fix RPC call return type to `CallExecutionOutput`                +1/-2      log_filter_input.rs Correct pending-header read without TxCount                            +1/-1
Refactoring	4 files cache.rs Simplify cache API signature                                                          +2/-2      resolve_pending.rs Drop `TxCount` and call-pending logic                                        +5/-66    stratus_storage.rs Update `save_execution` signature with state                          +18/-22  transaction.rs Merge state updates without TxCount                                            +19/-24
Additional files	6 files Cargo.toml +2/-3      call.rs +0/-124  execution_kind.rs +0/-58    mod.rs +0/-1      unix_time_now.rs +1/-1      transaction_stage.rs +2/-2

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 1567219)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Missing Pending Call Support The removal of from_pending_block from CallExecutionInput and its use in the RPC path means that eth_call with a Pending filter can no longer build an input against the in-flight block state. RPC calls using PointInTime::Pending will now fall through to try_from_mined_block, which reads from a finalized block and ignores pending state, causing unexpected behavior or errors when clients request eth_call on the pending block. impl CallExecutionInput { /// Creates from a call that was sent directly to Stratus with `eth_call` or `eth_estimateGas` for a mined block. pub fn try_from_mined_block(input: CallInput, block: Block, point_in_time: PointInTime) -> anyhow::Result<Self, StratusError> { let kind = match point_in_time { PointInTime::Latest => ExecutionKind::CallLatest(block.number()), PointInTime::Past(number) => ExecutionKind::CallPast(number), PointInTime::Pending => return Err(anyhow!("call execution cannot be created on mined block with PointInTime::Pending").into()), }; Ok(Self {

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 1567219
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add is_failure() method For symmetry and convenience, add an is_failure method that returns the negated success state, so callers don't have to negate is_success() themselves. src/eth/executor/evm/types/output/call_execution.rs [24-26] impl CallExecutionOutput { pub fn is_success(&self) -> bool { self.success } + pub fn is_failure(&self) -> bool { + !self.success + } } Suggestion importance[1-10]: 4 __ Why: Introducing is_failure is logically correct and convenient, but it's a minor API enhancement with limited impact on functionality.	Low

---

Previous suggestions

Suggestions up to commit 9c069f1

Category	Suggestion	Impact
Possible issue	Properly unwrap EVM execution result Destructure and unwrap the result of self.evms.execute directly to obtain evm_result and propagate errors, instead of pattern‐matching on a borrowed Result. This ensures evm_result is in scope when you build the tuple. src/eth/executor/mod.rs [182-196] -let evm_execution = self.evms.execute::<TransactionExecutionOutput>(EvmRoute::Transaction(evm_input.clone())); - +let (evm_result, evm_metrics) = self.evms.execute::<TransactionExecutionOutput>( + EvmRoute::Transaction(evm_input.clone()) +)?; #[cfg(feature = "metrics")] -if let Ok((evm_result, evm_metrics)) = &evm_execution { +{ *block_metrics += evm_metrics.slot_access; metrics::inc_executor_external_transaction(start.elapsed(), tx_contract, tx_function); - metrics::inc_executor_external_transaction_account_reads(evm_metrics.slot_access.account_reads, tx_contract, tx_function); - metrics::inc_executor_external_transaction_slot_reads(evm_metrics.slot_access.slot_reads, tx_contract, tx_function); - metrics::inc_executor_external_transaction_gas(evm_result.gas_used.as_u64() as usize, tx_contract, tx_function); + metrics::inc_executor_external_transaction_account_reads( + evm_metrics.slot_access.account_reads, tx_contract, tx_function + ); + metrics::inc_executor_external_transaction_slot_reads( + evm_metrics.slot_access.slot_reads, tx_contract, tx_function + ); + metrics::inc_executor_external_transaction_gas( + evm_result.gas_used.as_u64() as usize, tx_contract, tx_function + ); } ( - TransactionExecution::new(tx_input.transaction_info, tx_input.signature, evm_input, evm_result.outcome), + TransactionExecution::new( + tx_input.transaction_info, + tx_input.signature, + evm_input, + evm_result.outcome, + ), evm_result.state, ) Suggestion importance[1-10]: 9 __ Why: The external transaction branch currently swallows errors and uses an out-of-scope evm_result, causing potential bugs; unwrapping with ? ensures proper error propagation and correct variable scope.	High
General	Add execution success helper Add a convenience is_success method to TransactionExecutionResult so callers do not have to inspect result directly each time. src/eth/executor/evm/types/output/transaction_execution.rs [42-54] pub struct TransactionExecutionResult { /// Status of the execution. pub result: ExecutionResult, /// Output returned by the function execution (can be the function output or an exception). pub output: Bytes, /// Logs emitted by the function execution. pub logs: Vec<Log>, /// Consumed gas. pub gas_used: Gas, /// The contract address if the executed transaction deploys a contract. pub deployed_contract_address: Option<Address>, } +impl TransactionExecutionResult { + /// Returns true if the execution completed without revert or halt. + pub fn is_success(&self) -> bool { + matches!(self.result, ExecutionResult::Success) + } +} + Suggestion importance[1-10]: 5 __ Why: A is_success method on TransactionExecutionResult would simplify checks for consumers, improving ergonomics without affecting core functionality.	Low

[stratus-benchmark]

Benchmark:
Run ID: bench-a2c41bf2

Git Info:

• Branch: remove_call_storage
• Commit: 9c069f1
• PR: perf: remove inmemory call storage #2648

Leader Stats:
RPS Stats: Max: 10121.00, Min: 101.00, Avg: 3213.87, StdDev: 473.17
TPS Stats: Max: 3525.00, Min: 15.00, Avg: 3164.60, StdDev: 350.71

Follower Stats:
Imported Blocks/s: Max: 8.00, Min: 2.00, Avg: 5.33, StdDev: 1.19
Imported Transactions/s: Max: 24709.00, Min: 741.00, Avg: 16877.84, StdDev: 4105.36

Plots:

• View Leader Plot
• View Follower Plot

[cloudwalk-review-agent]

Summary

LGTM based on the provided diff context. The refactor consistently separates transaction execution outcome from state (TransactionExecutionResult vs State<Complete>) and threads the new state parameter through executor → miner → storage paths, while removing the in-memory call-specific TxCount machinery.

I did not find a concrete correctness/security/data-loss issue in the shown changes. Concurrency-sensitive paths in finish_pending_block still preserve lock ordering and now return cloned finalized state from the same stored snapshot, which is consistent with the new model.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1567219143

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Persistent review updated to latest commit 1567219

[cloudwalk-review-agent]

Summary

I found one blocking correctness issue in the eth_call path after removing call-specific temporary storage.

Blocking

PointInTime::Pending is now converted into ExecutionKind::CallLatest in CallExecutionInput::from_mined_block, and rpc_call always builds call input from read_block(point_in_time.into()) using a mined BlockHeader. This means pending calls are no longer anchored to the pending header/state snapshot and can observe latest-mined semantics instead of pending semantics, which changes RPC behavior for eth_call(..., "pending").

Please restore an explicit pending-input path (or equivalent header/state source) so pending calls are executed against pending state, not coerced through mined/latest header construction.

[cloudwalk-review-agent]

Summary

I found one blocking correctness issue in the eth_call(..., "pending") path after the in-memory call storage removal.

Blocking

PointInTime::Pending is now coerced into ExecutionKind::CallLatest in CallExecutionInput::from_mined_block, and Executor::execute_local_call now always builds call input from read_block(point_in_time.into()) using a mined block header. This changes pending-call semantics from pending snapshot/state to latest-mined semantics (or equivalent mined-path behavior), which is a user-visible RPC contract regression for eth_call/eth_estimateGas with pending.

Please restore an explicit pending-header/state input path for pending calls (instead of mapping Pending to CallLatest through mined block construction).

[cloudwalk-review-agent]

Summary

I found one blocking RPC correctness regression in the pending-call path after removing in-memory call storage.

Blocking

eth_call/eth_estimateGas with "pending" is now coerced into ExecutionKind::CallLatest via CallExecutionInput::from_mined_block (PointInTime::Pending => CallLatest(block.number)), while Executor::execute_local_call always builds call input from read_block(point_in_time.into()) using a mined BlockHeader. This changes the API contract: pending-tagged calls no longer execute against pending snapshot semantics, and instead follow latest-mined input construction.

Please restore an explicit pending-header/state input path for pending calls (or an equivalent route that preserves pending snapshot semantics) instead of mapping Pending through mined-block call construction.

[stratus-benchmark]

Benchmark:
Run ID: bench-5d4d5627

Git Info:

• Branch: remove_call_storage
• Commit: 9df243d
• PR: perf: remove inmemory call storage #2648

Leader Stats:
RPS Stats: Max: 8249.00, Min: 1367.00, Avg: 3228.26, StdDev: 359.61
TPS Stats: Max: 3475.00, Min: 170.00, Avg: 3169.38, StdDev: 321.55

Follower Stats:
Imported Blocks/s: Max: 7.00, Min: 2.00, Avg: 5.07, StdDev: 1.35
Imported Transactions/s: Max: 23012.00, Min: 2962.00, Avg: 16058.22, StdDev: 4531.35

Plots:

• View Leader Plot
• View Follower Plot

[cloudwalk-review-agent]

Found one blocking API correctness regression around pending-call semantics.

Blocking: eth_call/eth_estimateGas with "pending" no longer uses pending snapshot semantics

In this PR, call-specific temp storage and ExecutionKind::CallPending were removed, and pending calls are now mapped to ExecutionKind::CallLatest:

• CallExecutionInput::from_pending_block sets kind: CallLatest(block.number.prev().unwrap_or_default())
• resolve_pending::resolve_call_point now resolves only by block number, no tx position/pinning
• EntityRead::read_temp no longer receives ExecutionKind, so reads cannot distinguish pending-call position

Impact: a request explicitly targeting pending can observe latest/mined-style resolution instead of a stable pending-point view. That is a user-visible RPC contract change and can return different results under concurrent mining/tx ingestion.

Suggested fix:

1. Reintroduce an execution kind/path that preserves pending context (including call position/pinning), or
2. Introduce an equivalent pending snapshot mechanism used by eth_call(..., pending) and eth_estimateGas(..., pending) so resolution is stable and not coerced through latest semantics.

Also recommend adding regression tests specifically for:

• eth_call(..., pending) consistency while pending tx set changes
• behavior when pending block gets mined during a call (no silent switch to a different state basis).

[cloudwalk-review-agent]

Summary

I found one blocking RPC correctness regression in the pending call path.

Blocking

eth_call/eth_estimateGas with "pending" is now mapped to ExecutionKind::CallLatest and loses pending-snapshot semantics. In CallExecutionInput::from_pending_block, pending calls are assigned CallLatest(block.number.prev().unwrap_or_default()), and in from_mined_block PointInTime::Pending is also coerced to CallLatest; combined with removal of CallPending/TxCount and read_temp(..., kind) this removes the ability to pin reads to a pending tx position under concurrent execution/mining.

This is a user-visible API contract change for pending-tagged calls and can return different results than the pending view clients requested.

The merge-base changed after approval.

[cloudwalk-review-agent]

Summary

I found one blocking RPC correctness issue that is still present in the current diff.

Blocking

eth_call/eth_estimateGas with "pending" is no longer executed with pending semantics. In CallExecutionInput::from_pending_block, pending calls are mapped to ExecutionKind::CallLatest(block.number.prev().unwrap_or_default()), and from_mined_block also maps PointInTime::Pending to CallLatest; combined with removal of CallPending/TxCount and read_temp(..., kind), this drops the pending-position-aware resolution path. This is a user-visible API contract regression under concurrent tx ingestion/mining (pending-tagged calls can observe latest-mined behavior instead of the requested pending view).

Please restore an explicit pending call execution/resolution path (or equivalent pinning semantics) instead of coercing pending into CallLatest.

[cloudwalk-review-agent]

Summary

I found one blocking RPC correctness regression still present in this diff after removing call-specific temporary storage.

Blocking

eth_call / eth_estimateGas with "pending" no longer preserves pending semantics. In CallExecutionInput::from_pending_block, pending calls are converted to ExecutionKind::CallLatest(block.number.prev().unwrap_or_default()), and from_mined_block also maps PointInTime::Pending to CallLatest; combined with the removal of CallPending + TxCount and read_temp(..., kind), this eliminates the per-pending-position path and coerces pending-tagged calls into latest/mined-style resolution.

This is a user-visible API contract change under concurrent tx ingestion/mining: callers asking for pending can observe different state than the pending view they requested.

[cloudwalk-review-agent]

Summary

I found one blocking RPC correctness regression in the current diff.

Blocking

eth_call/eth_estimateGas with "pending" is still being coerced to latest/mined semantics after removing call-specific temporary storage. In CallExecutionInput::from_pending_block, pending calls are now created with ExecutionKind::CallLatest(block.number.prev().unwrap_or_default()), and from_mined_block also maps PointInTime::Pending to CallLatest; combined with removal of CallPending + TxCount and read_temp(..., kind), this removes pending-position-aware behavior.

This is a user-visible API contract change under concurrent tx ingestion/mining: callers asking for pending can observe a different state view than pending.