Skip to content

feat: check that the block extends the current chain when executing payloads#253

Merged
fmoletta merged 3 commits into
mainfrom
check-payload-extends-chain
Aug 16, 2024
Merged

feat: check that the block extends the current chain when executing payloads#253
fmoletta merged 3 commits into
mainfrom
check-payload-extends-chain

Conversation

@fmoletta

Copy link
Copy Markdown
Contributor

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 reorg

Closes #100

@fmoletta fmoletta marked this pull request as ready for review August 14, 2024 14:42
@fmoletta fmoletta requested a review from a team as a code owner August 14, 2024 14:42
Comment thread crates/rpc/engine/mod.rs Outdated
let last_block_number = storage
.get_latest_block_number()
.map_err(|_| RpcErr::Internal)?;
if let Some(latest) = last_block_number {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should always be a last_block_number, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@fmoletta fmoletta added this pull request to the merge queue Aug 16, 2024
Merged via the queue into main with commit e01125e Aug 16, 2024
@fmoletta fmoletta deleted the check-payload-extends-chain branch August 16, 2024 15:03
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate blocks that come from consensus

2 participants