Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/crate_reviews/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Crate Complexity & Concurrency Reviews

This directory hosts the finished per-crate reviews and the tracker that summarizes their core metrics.

## Tracker

| Crate | Main LOC | Test LOC | Complexity Score | Report |
| --- | --- | --- | --- | --- |
| `crates/networking/p2p` | 11,644 | 1,613 | 5 / 5 | [ethrex_p2p_review.md](ethrex_p2p_review.md) |
| `crates/vm/levm` | 7,884 | 201 | 4 / 5 | [ethrex_levm_review.md](ethrex_levm_review.md) |
| `crates/storage` | 4,619 | 912 | 4 / 5 | [ethrex_storage_review.md](ethrex_storage_review.md) |
| `crates/blockchain` | 2,419 | 598 | 4 / 5 | [ethrex_blockchain_review.md](ethrex_blockchain_review.md) |
| `crates/networking/rpc` | 6,827 | 1,284 | 3 / 5 | [ethrex_rpc_review.md](ethrex_rpc_review.md) |
| `crates/common/trie` | 2,082 | 1,837 | 3 / 5 | [ethrex_trie_review.md](ethrex_trie_review.md) |
| `crates/common` | 6,161 | 1,985 | 2 / 5 | [ethrex_common_review.md](ethrex_common_review.md) |
| `crates/vm` | 1,118 | 0 | 2 / 5 | [ethrex_vm_review.md](ethrex_vm_review.md) |

## Toolkit

Workflow checklists, analyzer scripts, and the report template now live under `toolkit/`. See `toolkit/README.md` for usage details when preparing a new review.
48 changes: 48 additions & 0 deletions docs/crate_reviews/ethrex_blockchain_review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# ethrex-blockchain Complexity & Concurrency Review

Date: 2025-10-01
Commit: 31e19504485904c70bd5294aa65becf91358d0e3
Target crate: `crates/blockchain`

## 1. Quantitative Snapshot

| Type | Code | Blank | Doc comments | Comments | Total |
| --- | --- | --- | --- | --- | --- |
| Main | 2419 | 361 | 103 | 209 | 3092 |
| Tests | 598 | 108 | 0 | 41 | 747 |
| Total | 3017 | 469 | 103 | 250 | 3839 |

- Files analyzed: 9 Rust sources (excludes `dev/` and `metrics/`)
- Functions: 134 total with 15 flagged as complex (line/branch heuristics)
- Longest routine(s): [crates/blockchain/blockchain.rs:184](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/blockchain.rs#L184) (`generate_witness_for_blocks`, 212 lines, 22 branches); [crates/blockchain/blockchain.rs:501](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/blockchain.rs#L501) (`add_blocks_in_batch`, 140 lines, 11 branches); [crates/blockchain/blockchain.rs:749](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/blockchain.rs#L749) (`validate_transaction`, 91 lines, 22 branches)
- Async/concurrency signals (crate-wide):
- `async fn`: 31 (production paths; +7 in `smoke_test.rs`)
- `.await`: 45 (production paths; +74 in `smoke_test.rs`)
- `tokio::spawn`: 0 (uses 1 `tokio::task::spawn` for payload builds)
- `spawn_blocking`: 1 (EVM tracing timeouts)
- `Arc<...>`: 3 (payload queue, witness logging)
- Mutexes: 1 `tokio::sync::Mutex` (payload ring); 1 `std::sync::RwLock` (mempool); several short-lived `std::sync::Mutex` guards inside tracing/witness helpers
- Atomics: 4 (`AtomicBool` sync flag)
- Other noteworthy primitives: `CancellationToken` for payload builders, `tokio::time::timeout`

## 2. High-Risk Components
- [crates/blockchain/blockchain.rs:184](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/blockchain.rs#L184) `generate_witness_for_blocks`: 212-line re-execution pipeline that interleaves trie logging, storage lookups, and manual witness assembly; error handling spans many branches and mixes sync + async storage access.
- [crates/blockchain/blockchain.rs:501](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/blockchain.rs#L501) `add_blocks_in_batch`: batch executor writes through shared `Store`, keeps mutable VM across loop, and performs partial cancellation handling; throughput logging and state validation live alongside persistence logic.
- [crates/blockchain/payload.rs:320](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/payload.rs#L320) `get_payload`: acquires `TokioMutex<Vec<_>>`, removes entry, then awaits `PayloadOrTask::to_payload` while still holding the guard—any concurrent `initiate_payload_build`/`get_payload` call must wait for the build task to finish.

## 3. Concurrency Observations
- Awaiting while holding the `payloads` `TokioMutexGuard` ([payload.rs:320-327](https://github.com/lambdaclass/ethrex/blob/31e19504485904c70bd5294aa65becf91358d0e3/crates/blockchain/payload.rs#L320-L327)) risks deadlock/priority inversion if the spawned builder needs to re-lock or if multiple requests try to materialize payloads simultaneously.
- Runtime code still leans on blocking `std::sync::RwLock` for the mempool; `validate_transaction` and payload construction invoke those methods from async contexts, so heavy contention could stall the Tokio scheduler.
- Witness generation uses `Arc<Mutex<_>>` loggers inside an async loop; guards are dropped before awaits today, but the pattern is fragile and complicates a future move toward actors.

## 4. Engineering Complexity Score
- **Score: 4 / 5** — Large, stateful workflows (witnessing, batch execution, payload assembly) intertwine storage IO, VM replays, and concurrency controls; several lengthy functions mix error handling, metrics, and business logic, increasing maintenance and refactor risk.

## 5. Recommendations
1. Refactor `get_payload` to drop the `TokioMutexGuard` before awaiting the builder (`swap_remove` + `drop(guard)` or restructure around `Arc<Mutex>` inside the task) to avoid executor stalls.
2. Split `generate_witness_for_blocks` into focused helpers (state harvesting, trie replay, code collection) and consider streaming witness assembly to shrink lock scopes and surface invariants.
3. Evaluate replacing the mempool `std::sync::RwLock` with an async-friendly primitive or routing access through an actor to prevent blocking the runtime during high-throughput validation.

## 6. Follow-Ups / Tooling Ideas
- Add tracing around `add_blocks_in_batch` (per-batch duration, gas throughput) to watch for stalls when the cancellation token trips.
- Capture integration tests exercising payload build + retrieval concurrency once the mutex fix lands.
49 changes: 49 additions & 0 deletions docs/crate_reviews/ethrex_common_review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# ethrex-common Complexity & Concurrency Review

Date: 2025-10-02
Commit: a25ab5cb61dba3c70210e0fca40353c91c88d0f1
Target crate: `crates/common` (excludes nested crates: `config/`, `crypto/`, `rlp/`, `trie/`)

## 1. Quantitative Snapshot

| Type | Code | Blank | Doc comments | Comments | Total |
| --- | --- | --- | --- | --- | --- |
| Main | 6161 | 761 | 204 | 200 | 7326 |
| Tests | 1985 | 109 | 0 | 31 | 2125 |
| Total | 8146 | 870 | 204 | 231 | 9451 |

- Files analyzed: 24 Rust sources (sibling crates under `crates/common/*` excluded)
- Functions: 395 total with 28 flagged as complex (heuristic: ≥60 lines or heavy branching)
- Longest routine(s): [crates/common/types/fork_id.rs:504](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/types/fork_id.rs#L504) (`sepolia_test_cases`, 151 lines), [crates/common/types/fork_id.rs:251](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/types/fork_id.rs#L251) (`holesky_test_cases`, 138 lines), [crates/common/types/transaction.rs:937](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/types/transaction.rs#L937) (`sender`, 116 lines)
- Async/concurrency signals (crate-wide):
- `async fn`: 0
- `.await`: 0
- `tokio::spawn`: 0
- `spawn_blocking`: 0
- `Arc<...>`: 1 (all other lock primitives absent)
- Mutexes: none (`std::sync`/Tokio locks 0 hits)
- Atomics: 0
- Other noteworthy primitives: `rayon` parallel iterators for signature recovery in `BlockBody`

## 2. High-Risk Components
- [crates/common/types/transaction.rs:937](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/types/transaction.rs#L937) — `Transaction::sender` duplicates variant-specific RLP encoding and signature packing across 100+ lines; drift risk and redundant allocations on a hot path.
- [crates/common/types/block_execution_witness.rs:216](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/types/block_execution_witness.rs#L216) — `apply_account_updates` orchestrates trie mutations with `expect` calls; a malformed witness panics instead of surfacing structured errors.
- [crates/common/serde_utils.rs:554](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/serde_utils.rs#L554) — `parse_duration` implements a bespoke parser with silent `None` failures on malformed input; edge cases (empty numeric buffer, stray unit chars) go unreported.

## 3. Concurrency Observations
- [crates/common/types/block.rs:259](https://github.com/lambdaclass/ethrex/blob/a25ab5cb61dba3c70210e0fca40353c91c88d0f1/crates/common/types/block.rs#L259) uses `rayon::par_iter` to parallelize sender recovery; good for throughput but can saturate CPU under large batches.
- No async runtime integration or shared-state mutexes detected; concurrency risk is low and mostly tied to CPU fan-out.
- `OnceCell` caches (e.g., transaction inner hashes) rely on single-writer semantics—ensure population occurs before sharing between threads to avoid redundant work.

## 4. Engineering Complexity Score
- **Score: 2 / 5** — Large dataset transformers with minimal concurrency; primary complexity lies in verbose data encoding/decoding and custom parsing rather than coordination.

## 5. Recommendations
1. Break `Transaction::sender` into reusable helpers (buffer builder + signature pack) and add cross-variant regression tests for encoding parity.
2. Replace `expect` chains in execution-witness updates with rich error propagation so corrupted tries fail gracefully.
3. Swap bespoke parsers (`parse_duration`, blob conversions) for reusable crates or bolster with targeted fuzz/unit coverage.

## 6. Follow-Ups / Tooling Ideas
- Add a targeted fuzz corpus for `parse_duration` and ABI decoders (`Deposit::from_abi_byte_array`) to catch silent failure cases.
- Profile `BlockBody::get_transactions_with_sender` under realistic batch sizes to confirm Rayon parallelism remains a net win.
- Consider a lint or CI check that flags new `expect` calls in state-mutating paths within this crate.
48 changes: 48 additions & 0 deletions docs/crate_reviews/ethrex_levm_review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# ethrex-levm Complexity & Concurrency Review

Date: 2025-10-02
Commit: 25ee6a95a6ccf329be87aecf903483fbc34796d0
Target crate: `crates/vm/levm`

## 1. Quantitative Snapshot

| Type | Code | Blank | Doc comments | Comments | Total |
| --- | --- | --- | --- | --- | --- |
| Main | 7884 | 1515 | 341 | 669 | 10409 |
| Tests | 201 | 49 | 0 | 8 | 258 |
| Total | 8085 | 1564 | 341 | 677 | 10667 |

- Files analyzed: 36 Rust sources (excluding the standalone bench and runner crates)
- Functions: 413 total with 49 flagged as complex (line/branch heuristics)
- Longest routine(s): [crates/vm/levm/src/opcodes.rs:183](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/opcodes.rs#L183) (`from`, 160 lines, 1 branch); [crates/vm/levm/src/opcodes.rs:389](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/opcodes.rs#L389) (`build_opcode_table_pre_shanghai`, 153 lines, 0 branches); [crates/vm/levm/src/opcode_handlers/system.rs:585](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/opcode_handlers/system.rs#L585) (`generic_create`, 117 lines, 11 branches)
- Async/concurrency signals (crate-wide):
- `async fn`: 0
- `.await`: 0
- `tokio::spawn`: 0
- `spawn_blocking`: 0
- `Arc<...>`: 3 (database façade only)
- Mutexes: 0 (`std`/`tokio`)
- Atomics: 0
- Other noteworthy primitives: heavy `Rc<RefCell<_>>` use inside the VM core

## 2. High-Risk Components
- [crates/vm/levm/src/opcode_handlers/system.rs:717](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/opcode_handlers/system.rs#L717) `VM::generic_call`: centralizes CALL-family execution, gas accounting, and precompile routing; its size and many side effects (value transfers, tracer hooks, backup management) make regressions likely when adding new forks or call semantics.
- [crates/vm/levm/src/db/gen_db.rs:160](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/db/gen_db.rs#L160) `GeneralizedDatabase::get_state_transitions`: constructs `AccountUpdate` payloads by diffing cached state against the backing store; destroyed-account handling, code lookups, and storage comparisons span dozens of branches with multiple early returns.
- [crates/vm/levm/src/hooks/default_hook.rs:29](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/hooks/default_hook.rs#L29) `DefaultHook::prepare_execution`: enforces fork-specific validation rules, intrinsic gas calculation, and upfront balance mutations; the growing matrix of EIP gates (Prague, Osaka, 4844, 7702) increases the chance of missing a precondition when specs shift.

## 3. Concurrency Observations
- `GeneralizedDatabase` shares an `Arc<dyn Database>` but mutates cache maps without synchronization; callers must ensure single-threaded access or provide their own locking ([crates/vm/levm/src/db/gen_db.rs:21](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/db/gen_db.rs#L21)).
- The VM core leans on `Rc<RefCell<_>>` for shared pools and substates, which makes the engine inherently !Send/!Sync and fragile if embedded in multi-threaded orchestrators ([crates/vm/levm/src/vm.rs:20](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/vm.rs#L20)).
- Precompiles execute CPU-heavy cryptography (BLS12-381, P-256, KZG) inline on the caller thread; there is no `spawn_blocking` escape hatch, so integrating with async runtimes will require careful isolation ([crates/vm/levm/src/precompiles.rs:1](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/precompiles.rs#L1)).

## 4. Engineering Complexity Score
- **Score: 4 / 5** — Large code surface with many fork-specific paths, deep call stack management, and cryptographic precompiles; changes demand extensive regression coverage across transaction types, gas rules, and state-diff machinery.

## 5. Recommendations
1. Break down `VM::generic_call` into smaller helpers (gas reservation, precompile dispatch, callframe instantiation) and add focused tests for each branch to reduce the blast radius of fork upgrades ([crates/vm/levm/src/opcode_handlers/system.rs:717](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/opcode_handlers/system.rs#L717)).
2. Augment `get_state_transitions` with property tests that cover destroyed-account recreation and storage diffing; consider caching lookups (code hashes, initial storage) to shrink the branching surface ([crates/vm/levm/src/db/gen_db.rs:160](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/db/gen_db.rs#L160)).
3. Document the fork/EIP matrix exercised by `DefaultHook::prepare_execution` and enforce it with table-driven tests so new protocol features can be introduced without touching the entire method ([crates/vm/levm/src/hooks/default_hook.rs:29](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/hooks/default_hook.rs#L29)).

## 6. Follow-Ups / Tooling Ideas
- Add microbenchmarks or profiling hooks around precompile entry points to detect regressions in cryptographic helpers before they impact block execution ([crates/vm/levm/src/precompiles.rs:1](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/precompiles.rs#L1)).
- Explore swapping `Rc<RefCell<_>>` pools for explicit actor-style ownership so the VM can run safely inside async or multi-threaded environments without extensive wrapping ([crates/vm/levm/src/vm.rs:20](https://github.com/lambdaclass/ethrex/blob/25ee6a95a6ccf329be87aecf903483fbc34796d0/crates/vm/levm/src/vm.rs#L20)).
Loading