feat: check that the block extends the current chain when executing payloads#253
Merged
Conversation
mpaulucci
reviewed
Aug 14, 2024
| let last_block_number = storage | ||
| .get_latest_block_number() | ||
| .map_err(|_| RpcErr::Internal)?; | ||
| if let Some(latest) = last_block_number { |
Collaborator
There was a problem hiding this comment.
there should always be a last_block_number, right?
mpaulucci
approved these changes
Aug 16, 2024
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 19, 2024
**Motivation** The conditional should check the case were the block number is already part of the chain (therefore causing a potential reorg if the block is not the one we have on the state), but instead of checking that the incoming block number is lower or equal to the last block number, it checks that the last block number is lower or equal to the incoming one which doesn't make any sense **Description** * Fix conditional check in when checking that the incoming block extends the canonical chain in `engine_NewPaylaodV3` <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves #111, Resolves #222 --> Closes None, but fixes #253
This was referenced Feb 6, 2026
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Feb 9, 2026
## Motivation eBPF profiling on a live Hoodi node shows secp256k1 at ~11% of total CPU, with 53% from P2P transaction validation. `Transaction::sender()` recomputes ECDSA public key recovery on every call — when the same transaction has `sender()` called multiple times (mempool checks, block building), each triggers a fresh recovery. ## Description Add `sender_cache: OnceCell<Address>` to all transaction types, following the same pattern as the existing `inner_hash: OnceCell<H256>` cache for `Transaction::hash()`. The first `sender()` call computes and caches; subsequent calls return the cached value. Changes: - Added `sender_cache: OnceCell<Address>` field to all 7 transaction structs (Legacy, EIP2930, EIP1559, EIP4844, EIP7702, PrivilegedL2, FeeToken) - `sender()` now delegates to `get_or_try_init(compute_sender)` — zero behavior change - Field uses `#[rkyv(with=rkyv::with::Skip)]` (same as `inner_hash`) - All construction sites updated (RLPDecode impls + l1_watcher explicit construction) - Sites using `..Default::default()` automatically pick up the new field ## EXPB Benchmark Results **Fast scenario** (vs main baselines #253/#258): | Metric | Main | cache-tx-sender | Change | |--------|------|-----------------|--------| | latency_avg | 26.23 ms | 25.69 ms | **-2.1%** | | mgas_avg | 775.78 | 795.35 | **+2.5%** | **Gigablocks scenario** (vs main #254): | Metric | Main | cache-tx-sender | Change | |--------|------|-----------------|--------| | latency_avg | 754.51 ms | 723.45 ms | **-4.1%** | | mgas_avg | 1547.14 | 1616.66 | **+4.5%** | Fast shows ~2% improvement (noise range). Gigablocks shows a meaningful **4.5% mgas improvement** — more transactions per block means more cache hits. ## How to Test ```bash cargo test -p ethrex-common -p ethrex-blockchain -p ethrex-p2p ```
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Feb 10, 2026
…tion (#6150) ## Motivation eBPF profiling on Hoodi steady-state (main branch, 5-minute capture at 49 Hz) showed **~51% of CPU samples** in BLS12-381/KZG blob proof verification during P2P transaction handling. The expensive `verify_kzg_proof_batch`/`verify_cell_kzg_proof_batch` calls in `PooledTransactions::validate_requested()` run for every blob tx received from every peer, before the mempool deduplication check. When the same blob transaction is received from N peers, KZG runs N times — all but the first are redundant. ## Description Add `validate_cheap()` to `BlobsBundle` that performs all structural checks (max blobs per block, version/fork compatibility, lengths, versioned hash SHA256 verification) but **skips the KZG proof verification**. Use it in `validate_requested()` so the P2P layer only does cheap validation. Full KZG verification still happens in `add_blob_transaction_to_pool()`, which checks for duplicates **before** validating KZG: ``` 1. P2P: validate_cheap() — structural checks only (fast) 2. Mempool: dedup check — if already in pool, return early (no KZG) 3. Mempool: validate() — full KZG only for new unique blobs ``` This means KZG runs **exactly once per unique blob transaction**, regardless of how many peers send it. ### eBPF profiling results (simultaneous 5-min captures, same block range) Both servers running Hoodi on Ryzen 9 9950X3D, profiled during the same 5-minute window (same blocks, same P2P traffic): | Metric | Baseline (office-2, main) | Test (office-3, this branch) | Change | |--------|---------------------------|------------------------------|--------| | Total CPU samples | 2,692 | 675 | -74.9% | | **KZG samples (absolute)** | **1,365** | **180** | **-86.8%** | | KZG % of total | 50.7% | 26.7% | -24.0pp | KZG is pure CPU (BLS12-381 pairings), so the RAM difference between servers (126 GB vs 62 GB) does not affect this comparison. The remaining ~27% KZG is the irreducible minimum: each unique blob tx must be validated once at mempool insertion. ### EXPB benchmark results (no regression) Synthetic `engine_newPayload` benchmarks on ethrex-office-5, comparing against fresh main baselines at the same commit: **Fast scenario** (main #253 vs ours #247): | Metric | main | This branch | Change | |--------|------|-------------|--------| | latency avg | 26.23ms | 26.33ms | +0.4% | | latency med | 22.30ms | 22.83ms | +2.4% | | latency p95 | 49.98ms | 50.02ms | +0.1% | | mgas avg | 775.78 | 766.31 | -1.2% | **Gigablocks scenario** (main #254 vs ours #248): | Metric | main | This branch | Change | |--------|------|-------------|--------| | latency avg | 754.51ms | 755.88ms | +0.2% | | latency med | 721.06ms | 720.65ms | -0.1% | | latency p95 | 1150.00ms | 1160.00ms | +0.9% | | mgas avg | 1547.14 | 1545.62 | -0.1% | All metrics within noise (<2.5%), confirming no regression in block execution performance. This is expected since the optimization only affects the P2P blob validation path, not `engine_newPayload` processing. ## Test plan - [x] Node runs correctly on Hoodi (processing blocks at chain speed, ~1.6 Ggas/s) - [x] Simultaneous eBPF profiling confirms -86.8% KZG sample reduction - [x] EXPB fast benchmark: no regression (latency avg +0.4%, mgas avg -1.2%) - [x] EXPB gigablocks benchmark: no regression (latency avg +0.2%, mgas avg -0.1%) - [ ] Verify blob transactions still propagate correctly through the network
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
We should check that the payload received constitutes a block that is a direct successor of the head block
Description
engine_newPayloadV3: Compare the payload's block number against the latest block number, if it is a direct successor, proceed to execution, if it is an older block first check if we already have it on the state or else return an error and warn about potential reorgCloses #100