Skip to content

feat: obtain chain config from network in EF tests + use it to obtain spec id#247

Merged
fmoletta merged 8 commits into
mainfrom
get-chain-config-from-network-ef
Aug 13, 2024
Merged

feat: obtain chain config from network in EF tests + use it to obtain spec id#247
fmoletta merged 8 commits into
mainfrom
get-chain-config-from-network-ef

Conversation

@fmoletta

@fmoletta fmoletta commented Aug 12, 2024

Copy link
Copy Markdown
Contributor

Note
I based these changes on the assumption that we are only (for now) supporting post-merge blocks. This can also be extended to support older forks if we need them in the future.

Motivation

Obtaining the chain config from the network according to the EF tests decomentation and deriving SpecId from the chain config

Description

  • Add Network enum and use it to deserialize EF test units. (Only includes forks from Merge onwards)
  • Implement chain_config method for Network, which returns the ChainConfig as defined in the EF tests documentation for each network
  • Implement getter and store shanghai_time in DB from genesis config
  • Implement spec_id method to determine the correct SpecId according to the storedChainConfig. (Only includes forks from Merge onwards, will asume at least Merge fork is active)
  • Remove spec_id argument from execute_block (now uses the above method instead)
  • Obtain and store the ChainConfig when executing EF test units

Closes #231

@fmoletta fmoletta marked this pull request as ready for review August 12, 2024 22:19
@fmoletta fmoletta requested a review from a team as a code owner August 12, 2024 22:19
Comment thread crates/evm/evm.rs

/// Returns the spec id according to the block timestamp and the stored chain config
/// WARNING: Assumes at least Merge fork is active
pub fn spec_id(store: &Store, block_timestamp: u64) -> Result<SpecId, StoreError> {

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.

imo this calculation should be done outside the evm and passed as spec_id. I don't think it's a good idea to couple the evm crate to our store implementation.

Thinking more broadly we should have a import_block/process_block that is in core, calls this function and makes updates the state.

If that's the case, would it be possible to remove the storage crate dependency from this crate?

Thoughts?

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.

We cannot import neither the evm crate nor the storage crate from core due to cyclic dependencies. We cannot remove the storage dependency from the evm crate either as the evm reads from the Store during execution

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.

We could move it into an EvmState method so it doesn't appear as disconnected?

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.

Hmm, ok, this needs some more thought. Let's merge this one as is and we can think some further refactor down the line.

@fmoletta fmoletta added this pull request to the merge queue Aug 13, 2024
Merged via the queue into main with commit 4301f48 Aug 13, 2024
@fmoletta fmoletta deleted the get-chain-config-from-network-ef branch August 13, 2024 13:51
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.

Set proper chain config depending on the network value

2 participants