Skip to content

perf(prewarm): hint the EIP-2935 parent-hash slot to the prewarmer - #12410

Merged
svlachakis merged 7 commits into
masterfrom
perf/warm-coinbase-system-slots
Jul 13, 2026
Merged

perf(prewarm): hint the EIP-2935 parent-hash slot to the prewarmer#12410
svlachakis merged 7 commits into
masterfrom
perf/warm-coinbase-system-slots

Conversation

@svlachakis

@svlachakis svlachakis commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Changes

BlockhashStore implements IHasAccessList: the hint covers the 2935 history contract and the exact ring-buffer slot the block writes, only when EIP-2935 is active, the block is not genesis, and the contract is deployed. The parent-hash write runs in the serial segment before the transaction loop, so without the hint its account and slot are always cold un-overlapped reads on the critical path.

Per review:

  • The system access-list providers (beacon-root handler, blockhash store) are injected into BlockCachePreWarmer by DI instead of threading through BranchProcessor — whose beacon-handler parameter existed only for that forwarding and is gone, together with the params on PreWarmCaches.
  • IBlockhashStore is unchanged (no interface widening; the class implements IHasAccessList), which also keeps other IBlockhashStore implementors unaffected.
  • Guards and slot computation are shared between ApplyBlockhashStateChanges and the hint through one helper, so the write and the hint cannot drift apart.
  • The hint is built through an exactly-sized single-entry factory (AccessList.ForSingleStorageCell) — no builder object, no list growth; one small sized list pair once per block, the same cost class as the beacon-root hint.

The beneficiary warming was split out to #12415 per review.

Testing

  • Consolidated GetAccessList cases in BlockhashProviderTests (deployed/pre-fork/genesis/undeployed, exact-slot assertion).
  • Full solution 121/0 plus explicit Nethermind.Xdc build (46/0); Nethermind.Blockchain.Test 1556/1556; Nethermind.Consensus.Test 110/110; dotnet format clean.

Types of changes

  • Optimization

@claude

claude Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Claude finished @svlachakis's task in 4m 1s —— View job


PR Review: warm the beneficiary and the EIP-2935 parent-hash slot

  • Read changed files & diff
  • Analyze correctness/edge cases
  • Analyze performance & thread-safety
  • Post review

I reviewed the full diff plus the surrounding prewarming machinery, DI wiring, and every BranchProcessor / IBlockhashStore construction site. This is a clean, well-targeted optimization. No Critical / High / Medium issues found.

✅ Correctness

  • BlockhashStore.GetAccessList precisely mirrors ApplyBlockhashStateChanges — same guards (IsEip2935Enabled, !IsGenesis, ParentHash is not null, IsContract), same address resolution (Eip2935ContractAddress ?? BlockHashHistoryAddress), and the same slot (number-1) % Eip2935RingBufferSize. The hint targets exactly the slot the block writes. 👍
  • Null access lists are handled safely. GetAccessList can return null (pre-fork / genesis / undeployed), and in the Cancun→Prague window the list will be [beaconList, null]. WorldState.WarmUp(AccessList?) guards with accessList?.IsEmpty == false, so the null entry is a no-op — no NPE. This matches the pre-existing pattern where BeaconBlockRootHandler.GetAccessList already returns null at genesis.
  • Dispose handling is correct. The guard widened to SystemTxAccessLists is not null || beneficiary is not null, and the finally now uses SystemTxAccessLists?.Dispose(). No double-dispose and no leak: when the list is null nothing needs disposing; when it's non-null it is disposed exactly once (or in the cancellation early-return).
  • Thread-safety. GetAccessList reads the shared main IWorldState via IsContract, but this runs synchronously in PrepareWarm on the main thread before the address warmer is queued and before the tx loop — identical to how the beacon handler already calls AccountExists. No race introduced.
  • Beneficiary warming is harmless if "wrong". It's a read-warm only; a non-existent account returns false and WarmupSender swallows MissingTrieNodeException.

✅ Wiring / build

  • IBlockhashStore and IBranchProcessor are both AddScoped in BlockProcessingModule, so the new BranchProcessor constructor param auto-resolves in the same scope. BalRecordingBranchProcessor is a decorator (no direct construction). Both manual test constructions (BlockProcessorTests, ReorgTests) were updated with the new positional arg in the correct position. No other new BranchProcessor(...) sites exist.

🟡 Low / minor observations (non-blocking)

  • Per-block scope build — flagged inline: GasBeneficiary is non-null for virtually every real block, so the warmer now builds a read-only env/scope on every block even when there are no system access lists. Cheap and clearly intended, just noting.
  • Duplicated guard/slot logic — the guards and (number-1) % ring computation are now duplicated between ApplyBlockhashStateChanges and GetAccessList. Acceptable (extracting a shared helper would add surface for two ~4-line blocks), but if the ring/slot rule ever changes, both must move together.
  • Bytecode not hintedApplyBlockhashStateChanges also calls RecordBytecodeAccess, which GetAccessList doesn't mirror. I checked: for the standard (non-stateless) WorldState path RecordBytecodeAccess is a no-op (it only matters for witness generation), so there is nothing cold to prewarm there. No action needed.

Testing

The new GetAccessList matrix (deployed / pre-fork / genesis / undeployed + exact-slot assertion) is well-parameterized and covers the guard combinations. Verify CI is green (I could not run the build in this environment due to sandbox permissions).

Verdict: mergeable. No blocking findings.

The fee write to the beneficiary and the 2935 ring-buffer write run in
serial segments the prewarmer never covers: the beneficiary is not a
tx sender/target and the blockhash store had no access-list hint. Both
are computable before processing starts, so warm them with the rest of
the addresses.
@svlachakis
svlachakis force-pushed the perf/warm-coinbase-system-slots branch from c2ae9df to 126a2be Compare July 12, 2026 23:29
@github-actions

Copy link
Copy Markdown
Contributor

EXPB Benchmark Comparison

Run: View workflow run

superblocks

Scenario: nethermind-flat-superblocks-perf-warm-coinbase-system-slots-delay0s

Client Processing (SSE)

Metric PR Master (cached) Delta
AVG (ms) 1028.61 992.34 +3.65%
MEDIAN (ms) 954.4 929.5 +2.68%
P90 (ms) 1237.4 1232.6 +0.39%
P95 (ms) 1622.7 1619.2 +0.22%
P99 (ms) 3526.0 2334.1 +51.06%
MIN (ms) 663.7 630.6 +5.25%
MAX (ms) 3526.0 2334.1 +51.06%
K6 TTFB
Metric PR Master (cached) Delta
AVG (ms) 1238.63 1170.50 +5.82%
MEDIAN (ms) 1132.11 1081.03 +4.73%
P90 (ms) 1551.39 1478.23 +4.95%
P95 (ms) 1968.02 1787.74 +10.08%
P99 (ms) 3880.99 2651.28 +46.38%
MIN (ms) 783.35 722.73 +8.39%
MAX (ms) 5154.89 3016.35 +70.90%

realblocks

Scenario: nethermind-flat-realblocks-perf-warm-coinbase-system-slots-delay0s

Client Processing (SSE)

Metric PR Master (cached) Delta
AVG (ms) 25.80 25.86 -0.23%
MEDIAN (ms) 21.7 21.6 +0.46%
P90 (ms) 44.3 44.3 +0.00%
P95 (ms) 49.8 53.8 -7.43%
P99 (ms) 100.4 100.2 +0.20%
MIN (ms) 0.4 0.4 +0.00%
MAX (ms) 194.6 193.1 +0.78%
K6 TTFB
Metric PR Master (cached) Delta
AVG (ms) 29.59 29.73 -0.47%
MEDIAN (ms) 25.41 25.55 -0.55%
P90 (ms) 47.96 48.17 -0.44%
P95 (ms) 56.67 61.10 -7.25%
P99 (ms) 102.64 102.83 -0.18%
MIN (ms) 2.09 1.98 +5.56%
MAX (ms) 198.20 196.28 +0.98%

@asdacap asdacap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Split to two PR

Comment thread src/Nethermind/Nethermind.Blockchain/Blocks/BlockhashStore.cs Outdated
Comment thread src/Nethermind/Nethermind.Blockchain/Blocks/BlockhashStore.cs Outdated
…ciary change

Split per review: the beneficiary warming moves to its own PR.
@svlachakis svlachakis changed the title perf(prewarm): warm the beneficiary and the EIP-2935 parent-hash slot perf(prewarm): hint the EIP-2935 parent-hash slot to the prewarmer Jul 13, 2026
@svlachakis
svlachakis requested a review from asdacap July 13, 2026 10:06
Comment thread src/Nethermind/Nethermind.Blockchain/Blocks/IBlockhashStore.cs Outdated
Comment thread src/Nethermind/Nethermind.Consensus/Processing/BranchProcessor.cs Outdated
…rmer

Per review: the providers arrive by DI instead of threading through
BranchProcessor, whose beacon-handler parameter existed only for the
forwarding. IBlockhashStore stays unchanged (fixes the Xdc implementor
break); the BlockhashStore class implements IHasAccessList.
@svlachakis
svlachakis requested a review from asdacap July 13, 2026 10:53
Chains that bind their own store (Xdc) broke on the hard cast in the
hint registration; a null-object provider contributes nothing instead.
@svlachakis
svlachakis merged commit 7f9788c into master Jul 13, 2026
714 of 718 checks passed
@svlachakis
svlachakis deleted the perf/warm-coinbase-system-slots branch July 13, 2026 11:48
svlachakis added a commit that referenced this pull request Jul 13, 2026
…12424)

* perf(xdc): forward the blockhash prewarm hint through the decorator

The parent-hash write is delegated to the inner store, so its 2935
prewarm hint (#12410) is exact for Xdc too; without the forwarding the
decorator silently drops it.

* test(xdc): guard the hint forwarding; order usings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants