fix(simulate): route the EIP-7928 BAL path through the simulate tx adapter (#12692) - #12721
Merged
Conversation
Under EIP-7928, eth_simulateV1 runs transactions through the BlockAccessListManager's own tx processors, bypassing SimulateTransactionProcessorAdapter. That lost its GasCap budget clamp, its TotalGasLeft/BlockGasLeft accounting (block gasUsed reported as 0), and its validation:false handling (the BAL path always called Execute, never Trace). Inject the adapter via a new ITransactionProcessorAdapterFactory so the sequential BAL manager — the only one simulate drives — wraps each tx processor in the simulate adapter. The parallel manager always uses the default ExecuteTransactionProcessorAdapter: the stateful simulate adapter is sequential-only and simulate never triggers the parallel path (it attaches no BlockAccessList). The factory is an interface, not a delegate, so Autofac does not auto-synthesise one on the real block-production scope, where the optional parameter must stay null and fall back to the default adapter. The no-gas EIP-8037 execution-dimension default is a separate ordering issue (the per-tx inclusion check runs before the adapter) and is left as a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…delegate; add GasCap test Replace the ITransactionProcessorAdapterFactory interface with a required TransactionProcessorAdapterFactory delegate, mirroring CodeInfoRepositoryFactory: the block-processing module registers the default (ExecuteTransactionProcessorAdapter) and the simulate scope overrides it with the simulate adapter. This drops the optional/null-fallback and the auto-synthesis hazard an optional delegate would carry (ITransactionProcessorAdapter is registered on the block-processing scope, so Autofac would otherwise fill an optional delegate on the real path). Also add a regression test for the JsonRpc.GasCap budget (#12692 item 2): a two-call request whose cumulative gas exceeds the cap has its second call clamped below intrinsic gas and rejected; without the adapter the cap is not enforced and both calls run unclamped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hudem1
requested review from
LukaszRozmej,
MarekM25,
asdacap,
benaadams,
flcl42 and
svlachakis
as code owners
August 7, 2026 04:32
core-repository-dispatch-app
Bot
added
bug fix + reliability
eip
evm
refactoring
rpc
labels
Contributor
|
Claude finished @hudem1's task in 8m 50s —— View job Review: route the EIP-7928 BAL path through the simulate tx adapter
The core fix is right and the design is the good kind of minimal: a delegate seam where the adapter used to be a hard-coded
I also verified the gas-cap test discriminates rather than passing vacuously — details in the inline comment; the numbers work out because Amsterdam's EIP-2780 intrinsic is 14600 for these calls, not 21000. Findings
On #1: the parallel path genuinely cannot be reached today, so nothing is broken. My concern is durability — On #2: explicitly not a regression and not something I think you should fix here — the ask is to reword the No security or hot-path performance concerns: the delegate is invoked once per pooled processor at construction, and real block processing keeps the identical Verdict: nothing blocks correctness today. Reword the doc comment and land the contract note (or explicitly ack #1 and #2 with rationale) and this is good to merge. Note: I reviewed against the PR base commit |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
11 tasks
Contributor
EVM Opcode Benchmark DiffAggregated runs: base=3, pr=3 Regressions (1)
|
…cs, test dedup) - Set ProcessingOptions.ForceSequentialBlockAccessList in the simulate options so the stateful SimulateTransactionProcessorAdapter can never reach the parallel BAL pool (previously guaranteed only by simulate not attaching a BlockAccessList); documented the single-threaded contract on the adapter itself. - Register the default TransactionProcessorAdapterFactory with AddScoped + a method group (drops the lambda cast) and the simulate override with the typed-dependency AddScoped overload (no manual Resolve / captive singleton). - Reworded the factory <remarks> to describe what is actually wired (default Execute; other scopes still get the default on the BAL path) and dropped the overstated "gas defaulting" from the registration comment. - Extracted BuildAmsterdamBalChain test helper; pinned the EIP-2780 intrinsic in the GasCap test comment. Follow-up filed as #12723 (scopes overriding ITransactionProcessorAdapter still get Execute on the BAL path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
wurdum
approved these changes
Aug 7, 2026
Addresses LukaszRozmej's review: the TransactionProcessorAdapterFactory was a second registration axis alongside ITransactionProcessorAdapter, so scopes that only overrode the latter (block production, trace, proof) drifted to the default Execute adapter on the EIP-7928 BAL path — the same bug class this PR fixes for simulate (#12723), left live elsewhere. Notably block production silently downgraded its intended BuildUp semantics to Execute under Amsterdam. Make the factory the single source of truth: the root registers the default (Execute) plus a derivation ITransactionProcessorAdapter = factory(processor), and each scope overrides only the factory — production BuildUp, trace (factory param, dropping the generic T), proof Trace, simulate keeps its factory and drops the now-redundant AddSingleton<ITransactionProcessorAdapter>. Debug keeps its ChangeableTransactionProcessorAdapter (mutated at runtime; its direct registration overrides the derivation). This folds in #12723. Also removed ProcessingOptions.ForceSequentialBlockAccessList from the simulate options (redundant — simulate attaches no BlockAccessList, so ParallelExecutionEnabled is already false; the contract stays documented on SimulateTransactionProcessorAdapter). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closed
8 tasks
The manager's five construction-only dependencies (blockhash provider, spec provider, code-info/adapter/processor factories) collapse into one scoped BalTxProcessorFactory that builds the per-worker processor + adapter pairs. Scope overrides (decorated ITransactionProcessorFactory, scope-specific CodeInfoRepositoryFactory / TransactionProcessorAdapterFactory) flow through container resolution; the Ethereum defaults live on the factory ctor for manual construction sites. Also refresh the stale TransactionProcessorAdapterFactory remarks to describe the single-axis wiring from step 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LukaszRozmej
approved these changes
Aug 10, 2026
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
…simplification Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LukaszRozmej
approved these changes
Aug 11, 2026
AnkushinDaniil
added a commit
that referenced
this pull request
Aug 13, 2026
* fix(rpc-bench): run benchmarked nodes the way production runs them (#12625)
* diag(rpc-bench): run Nethermind with production-default runtime (no TC=0 pin)
* reword comment for master
* review: document expb divergence, add NODE_ENV_VARS escape hatch
- README: the 'Alignment with expb' section no longer claims the removed
env pins; documents the deliberate code-gen divergence and that JIT
warm-up now lands inside the measured window; dotTrace reports are not
comparable across this change
- start-node.sh: reword comment (no warm-up phase exists yet), add
NODE_ENV_VARS passthrough for deliberate one-off code-gen experiments
* trim comments to one-liners; rationale stays in the PR
* drop the Merge GC flags: inert here and misleading
GCKeeper only runs on Engine API calls; this harness parks the node at a
snapshot head and never sends newPayload, so the three flags changed
nothing while implying the node ran a non-production GC configuration.
* keep the image entrypoint for Nethermind
The override skipped entrypoint.sh, which applies host tuning and enables a
shipped PGO profile. Its comment claimed parity with expb, but expb only
overrides the entrypoint for dotTrace, so normal expb runs (and production)
do run entrypoint.sh - this harness was the outlier.
* Rename EIP-8037 regular gas dimension to execution gas (#12600)
* Auto-update fast sync settings (#12665)
Co-authored-by: rubo <rubo@users.noreply.github.com>
* refactor(net): namespace snap by version (#12606)
* refactor(net): namespace snap messages by version
Prepare the snap subprotocol for a second version by moving the concrete
snap/1 messages, message codes and protocol handler into a versioned
namespace, so a snap/2 handler can be added alongside without touching
snap/1 code.
Snap/Messages/* -> Snap/V1/Messages/*
Snap/SnapMessageCode -> Snap/V1/Snap1MessageCode
Snap/SnapProtocolHandler -> Snap/V1/Snap1ProtocolHandler
P2P/P2PMessageKey.cs -> P2P/VersionedProtocol.cs (file renamed to
match the type it declares)
SnapMessageBase and SnapSerializerBase deliberately stay in
Snap.Messages: they are shared by all protocol versions, not specific to
snap/1.
Introduce SnapVersions constants and ISnapSyncPeer.SnapProtocolVersion so
version checks can replace the magic numbers currently spelled as 1.
PeerInfoExtensions.CanGetSnapData is renamed to CanGetTrieNodes to say
what it actually tests. The rename is nominal - the peer probe is
unchanged, so behaviour is identical.
No functional change.
* refactor(net): remove Snap2 version constant from SnapVersions
* address review comments
* rename
* feat(sync): serve block access lists from the snap server (#12607)
* Refactor SnapServer and SnapStateServer integration
- Renamed SnapServer to SnapStateServer for clarity and consistency.
- Updated WorldStateManager to use SnapStateServer instead of SnapServer.
- Modified StateSyncFeedTestsBase to accommodate changes in SnapServer instantiation.
- Adjusted SnapProviderTests to reflect the new SnapStateServer type.
- Introduced SnapServerTests to validate SnapServer functionality.
- Added SnapStateServerTests to ensure robust testing of state management.
- Implemented new methods in SnapServer for handling bytecode and block access lists.
- Enhanced test coverage for account range retrieval and storage management.
* refactor: change SnapServer field type to interface ISnapServer
* test: enhance SnapServerTests with additional block access list scenarios
* chore: Update Dockerfiles (#12663)
Update Dockerfiles
Co-authored-by: rubo <rubo@users.noreply.github.com>
* test: make prewarmer env-return assertion pool-hit independent (#12616)
PreWarmCaches_ReturnsAddressWarmEnvWhenScopeBuildThrows asserts
Returned == Created, but with maxPoolSize 1 an env returned to the pool
can be rented again without a Create, so a pool hit legitimately
increments Returned twice against a single Create. Whether a hit occurs
depends on worker interleaving: flaky on CI, fails deterministically on
high-core machines (0/30 locally).
ThrowingBuildPolicy.Return now refuses retention, so every rental is a
fresh Create and the invariant holds under any interleaving (30/30
locally). Stack-trace instrumentation confirmed every rental is returned
exactly once - the prewarmer itself is correct.
* Update OP Superchain chains (#12664)
Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
* fix(receipts): restore the post-merge flag before regeneration (#12641)
* fix(receipts): restore the post-merge flag before regeneration
Stored headers do not carry IsPostMerge and regeneration bypasses the
recovery step that restores it, so post-merge blocks re-executed with
PREVRANDAO evaluating to the zeroed difficulty - any transaction reading
it produced receipts failing the root check (~7% of post-merge blocks
refused on mainnet archives deriving receipts from state).
* test(receipts): dispose buffer, pin logged value
* fix(receipts): classify post-merge via the switcher
A difficulty heuristic misreads chains that repurpose the field -
Taiko stores per-block ZK gas in Difficulty while AlwaysPoS - so ask
IPoSSwitcher instead, mirroring MergeProcessingRecoveryStep. On
mainnet the switcher's TD-null branch is the same difficulty check,
so behavior there is unchanged.
* test(receipts): pin the real switcher's TD-null derivation
The production failure arrived as a mainnet-shaped header with
TotalDifficulty unset; cover PoSSwitcher's TD-null branch end to end,
not only the honour-the-switcher contract.
* test(receipts): cover the switcher registration path
A hand-injected switcher cannot catch a composition regression that
leaves the container-resolved regenerator on the NoPoS default, so
resolve it from a graph whose IPoSSwitcher registration is overridden
the way a merge-enabled node overrides it.
* fix(tests): mark RecoverReceiptsBlockchain.Create as hiding
* Expose the node's ENR in admin_nodeInfo (#12631)
feat(rpc): expose the node's ENR in admin_nodeInfo
Nethermind is the only discv5-capable execution client whose
admin_nodeInfo omits the node record. Tooling that bootstraps
discv5-only networks reads the ENR from this endpoint and has to
special-case Nethermind to the enode instead, which is useless once
discv4 is disabled.
NodeRecordProvider already maintains a signed, sequence-numbered self
record, so surface it as an 'enr' field. Move INodeRecordProvider to
Nethermind.Network so Nethermind.JsonRpc can reference it without a new
dependency on Nethermind.Network.Discovery; the implementation stays in
Discovery. The provider is only registered when discovery is enabled, so
it is resolved optionally and the field is omitted otherwise.
* Validate ABI decode allocation bounds (#12588)
* ci: disable stateless glamsterdam-devnet-7 scheduled run (#12680)
* Fix EIP-7708 tracing with logs (#12577)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Naming
* fix(flat): guard the trie-warmer against a TransientResource recycle race (storage reads as 0x00) (#12429)
* fix(flatdb): warm the trie from persistence only
The trie warmer read the recyclable `_snapshots` and `_transientResource`
while the warm job held only a `ReadOnlySnapshotBundle` lease, which does not
cover them. A concurrent scope reset could recycle those under the running
warmer, so a warm read could return a torn or foreign node.
The warmer only needs to warm from persistence, so restrict its reads to the
trie node cache and the `ReadOnlySnapshotBundle` - exactly what the lease
covers. In-memory nodes are already hot and do not need warming.
Claude-Session: https://claude.ai/code/session_01S3BG321zDG9BNjAgWhZhCX
* fix(flat): warm the transient resource via a per-job lease
The trie warmer now warms nodes into the per-job-pinned TransientResource
(not just the trie node cache and ReadOnlySnapshotBundle), covered by a single
transient lease held for the whole warm traversal. It still never reads the
recyclable _snapshots. A warmer read on a torn-down bundle bails to a
persistence-only read instead of spinning for a lease that will never land.
Claude-Session: https://claude.ai/code/session_01S3BG321zDG9BNjAgWhZhCX
* refactor(flat): drop the warmer transient ThreadStatic capture
Measured TS-on vs always-per-node-lease on x10 (3 runs, heavy-block warmer
load): newPayload 178 vs 178 ms, getProof p99 20.1 vs 20.2 ms, RSS identical.
The ambient capture bought nothing, so keep only the per-node lease + ABA
re-check (the actual recycle-race fix) and drop the ThreadStatic + the
EnterWarmerTransientScope pins in the two warm-job callers.
* fix(flat): register the transient return owner at pool checkout
- ResourcePool.GetCachedResource now calls OnRented, so every checkout
carries a registered return owner; a final ReleaseLease without one
throws instead of silently dropping the resource (which leaked the
BloomFilter allocation on the public AddSnapshot path)
- re-check _isDisposed after the warmer's lease acquire: Dispose releases
the owner lease but leaves _transientResource pointing at the recycled
instance, so the identity re-check alone could latch a resource already
re-rented by another bundle
- document why ReleaseLease is distinct from Dispose: the pool contract
reserves Dispose for destroying an over-capacity resource
- run Nethermind.State.Flat.Test in the nethermind-tests.yml matrix; it
was compiled but not run by any CI job
- FlatDbManagerTests duplicate-snapshot test asserts the resource lands
back in the checkout pool; new ResourcePoolTests cover the final-release
return and the unregistered-release throw; refresh stale warmer test
comments
* fix(flat): pin the transient resource for prewarm dedupe reads
ShouldQueuePrewarm read _transientResource without pinning it. The dedupe
bloom lives on that recyclable resource and the call runs on prewarmer and
BAL threads, so the owner could retire the resource mid-read: the pool
Resets it and, on overflow, Disposes the BloomFilter, whose backing store is
native memory. Route both overloads through the same lease + ABA re-check
the warmer node reads already use, and decline the prewarm on a torn-down
bundle. Rename the helper accordingly, since it is no longer warmer-only.
The FlatWorldStateScopeProvider and FlatOverridableWorldScope test doubles
returned the committed resource to the pool directly instead of releasing
its lease, mirroring neither AddSnapshot implementation. That recycles the
resource while a warmer lease is outstanding and returns it a second time
when that lease is released; the scope provider double also returned the
wrong instance and then returned it again on teardown. Both now release the
lease, which is the single return-to-pool path.
Test changes:
- the persistence-only test now commits the written nodes into the bundle's
recyclable _snapshots before reading, so the warmer's Unknown result is a
genuine miss. Previously the node was still in the transient (SetStateNode
writes both) and was itself Unknown, so the assertion held either way.
- the churn test gives every epoch its own persisted node instance, so a
read served from another epoch's recycled transient is caught by identity
rather than by value, drives both recycle paths (CollectAndApplySnapshot
swap and Dispose), exercises ShouldQueuePrewarm alongside the node reads,
and joins the readers with a bounded wait instead of blocking forever.
- new test: a warmer read and a prewarm check on a disposed bundle fall back
to the leased persistence reader within a bounded wait, covering the
Dispose bail-out deterministically.
* fix(jsonrpc): synchronise SubscriptionManager per-client subscription bag (#12672)
* fix(jsonrpc): synchronise SubscriptionManager per-client bag
The per-client subscription bag is a HashSet mutated and enumerated from
multiple threads: concurrent subscribe requests (socket worker tasks),
unsubscribe, and the Closed handler that fires on connection teardown.
Concurrent HashSet access could corrupt it, dropping a subscription so
its event handlers stayed attached and leaked. Lock on the bag for every
add, remove, and snapshot-before-dispose.
Fixes #12668
* refactor(jsonrpc): dispose client subscriptions under bag lock without snapshot
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(jsonrpc): race unsubscribe path too; drop bag field comment
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* test: eth_createAccessList affordability with omitted fee fields (#12629)
* test(rpc): eth_createAccessList affordability with omitted fee fields (execution-apis #854)
execution-apis PR #854 clarifies that eth_createAccessList must not fail
solely because an unfunded sender cannot afford client-selected default fees
when all gas-fee fields are omitted. Nethermind already conforms (verified in
hive rpc-compat); this adds a regression test mirroring the conformance
fixture: unfunded sender, codeless recipient, zero value, no gas/fee fields
-> {"accessList":[],"gasUsed":"0x5208"}.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(rpc): run affordability test on London chain, assert no in-body error, reuse helper
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
* Fix stale transaction pool snapshots (#12685)
* Encode engine_newPayloadWithWitness witness as an RLP data string (#12635)
* fix
Signed-off-by: jsign <jsign.uy@gmail.com>
* Tighten witness RLP JSON encoding
---------
Signed-off-by: jsign <jsign.uy@gmail.com>
Co-authored-by: jsign <jsign.uy@gmail.com>
* perf(state): skip trie warmup for read-only BAL accounts in flat layout (#12681)
* perf(state): skip trie warmup for read-only BAL accounts in flat layout
With a suggested BAL the block's write set is known upfront, and trie
nodes are only needed at commit for written accounts. Gate address
trie-warm hints (HintBal, HintGet, HintWarmAccount) on the BAL write
set so read-only accounts no longer trigger state-trie path walks.
On BAL blocks dominated by cold account reads this removes up to ~8-10
wasted trie-node DB reads per unique read-only account from the
measured processing window.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Apply suggestions from code review
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
* refactor(state): extract QueueStateTrieWarmup and address review findings
- Extract the NeedsStateTrieWarmup + PushAddressJob + increment pattern
into QueueStateTrieWarmup, used by all three address warmup call sites
- Hoist CancelHintBal above the empty-BAL early return so a stale write
set never survives into the next block
- Drop the inaccurate bloom false-positive-rate comment
- Tests: parameterize warm-per-write-kind over balance/nonce/code/storage,
add empty-BAL reset regression test, split the HintWarmAccount test,
wrap scopes in using, use order-insensitive assertions
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Update src/Nethermind/Nethermind.State.Flat/ScopeProvider/FlatWorldStateScope.cs
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
* refactor(test): reuse TestContext for recording-warmer scope construction
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(state): bind BAL warmup gate to HintBal lifecycle; address review findings
- Keep _warmupWriteSet across StartWriteBatch: on the parallel BAL path the
BAL apply commits mid-block, concurrently with tx workers, so clearing the
gate in CancelHintBal collapsed it at a nondeterministic point. The gate is
now replaced only by the next HintBal.
- Drop the token from Task.Run in both HintBal implementations: a task
cancelled before being dequeued never ran the finally that returns the
pooled accountChanges array. The body already observes the token.
- Align TrieStoreScopeProvider.HintBal with the flat scope: a new hint
supersedes the previous one even when it carries no work itself.
- Remove the stale prestate-load mutation mention from ReadOnlyBlockAccessList
docs, state the immutability invariant, and seal the type.
- Tests: pin the gate surviving StartWriteBatch and a second BAL replacing
the previous write set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
* test: make can-never-fail tests assert what their names claim (#12690)
* test(xdc): drop redundant trailing Assert.Pass in SpecialTransactionsTests
* test(core): bound McsLock re-acquire test instead of passing unconditionally
SingleThreadAcquireRelease asserted nothing and is subsumed by
ReacquireAfterReleaseSucceeds, which now runs on a worker with a timeout
so a broken release fails in seconds instead of hanging the test host.
* test(flat): assert real postconditions instead of Assert.Pass
Add_ConcurrentWithMightContain_ShouldWork now verifies no write is lost
under concurrent readers (a bloom filter never false-negatives).
DisposeAsync test renamed: FlatDbManager does not dispose the repository
(the container owns it); it now asserts bounded drain completion and
idempotent double-dispose. ConstructorAcceptsPersistedRepository removed -
Not.Null on a fresh object, subsumed by every other test in the file.
* test(merge): assert pending-validation cleanup instead of catch-only assertions
The memory-leak test asserted only inside catch blocks and ended with
Assert.Pass, so it passed whenever nothing threw; it now drives the
handler directly and asserts the pending-validation count stays zero
across repeated timed-out payloads. The TrySet double-completion test
is removed: it was a weaker duplicate of the concurrent-calls test
above it (same scenario, but swallowing OperationCanceledException and
keying Assert.Fail on exception message text).
* test(merge): await header-sync test helpers
The helpers were async void and invoked without awaiting - including one
un-awaited call inside the other helper - so their assertions raced the
test body and failures could surface as host crashes or not at all.
* test(network): restore DisconnectsAnalyzer assertions with deterministic flush capture
All four tests had their assertions commented out as CI-flaky, leaving
them assertion-free. The flakiness came from racing the 10ms flush
timer: reports are now recorded while the default 10s interval is in
effect and only then is the interval shortened, so a flush cannot fire
mid-arrangement. Assertions scan captured flush reports, including that
counters aggregate, reset after each flush, and cleared categories do
not resurface (the analyzer double-buffers, so a lost clear shows up as
a stale count in every other flush, not as a doubled count).
* test: address review findings on strengthened tests
Will_clear_after_report no longer issues a second report at all - a
report can race the flush's enumerate-then-clear window (Timer.Stop does
not drain a queued Elapsed callback, so no test-side quiesce is airtight).
A lost clear is observable without it: the analyzer double-buffers, so a
stale count resurfaces in later flushes, and the test asserts the
category appears in exactly one flush across several more (mutation-
verified). Bloom capacity raised to 100k - at 10k the saturated filter's
~34% false-positive rate masked a single lost write - and misses are
collected into one assertion. The repeated-timeouts payload test is
dropped rather than parameterized: the pending dictionary is keyed by
block hash and the test resubmits one block, so the count can never
exceed one and iterations add no coverage. The FlatDbManager dispose
test states its intent with Assert.DoesNotThrowAsync and a corrected
comment (WaitAsync bounds the wait, not the drain). Wait timeouts are
named constants and polling uses Thread.Sleep(1).
* test: simplify comments per ASD-STE100 and drop dead times parameter
Comments now use short, active, single-topic sentences. The times
parameter of ShouldEventuallyReport had no remaining non-default call
site after the second-report removal.
* test: use SpinWait.SpinUntil instead of a custom poll helper
Keep one condition re-check after a timeout: a flush can land in
SpinUntil's final sleep tick.
* Stop parallel transaction execution once BAL validation rejects the block (#12697)
* fix(consensus): stop parallel tx execution once BAL validation rejects
The parallel BAL block validator ran the incremental validator alongside the
transaction workers, but a validator failure faulted neither the worker loop nor
its cancellation token. `ParallelUnbalancedWork` only stops fetching new indices
on caller cancellation or a transaction-worker fault, so a block rejected at an
early transaction index still executed every remaining transaction before the
foreground observed the failure at `GetResult()`. For a block whose invalidity is
decided by a cheap prefix, that turns a sub-second rejection into slot-scale CPU
work.
`IncrementalValidationWorkItem` now owns a cancellation source that is signalled
together with the stored exception, and the worker loop runs under that token, so
workers stop pulling transactions as soon as validation becomes terminal. The
resulting `OperationCanceledException` is translated back into the original
validation failure, keeping the returned error identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(consensus): signal BAL validation failure with a flag, not cancellation
Routing the stop signal through ParallelUnbalancedWork's cancellation token meant
`For` ended by throwing an OperationCanceledException that existed only to be
caught and translated back into the original validation failure. The token also
needed a per-block CancellationTokenSource, whose recycling in Schedule was only
safe because the previous block's validator had already been joined.
Replace all of it with a volatile read of the exception the work item already
stores: workers check `HasFailed` before doing any work, the loop drains its
remaining indices without executing anything, and `GetResult()` reports the
rejection on the normal return path. Same behaviour, no exception used as control
flow, no extra state to keep in sync, and no ordering constraint between Schedule
and the work that follows it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(test): trim comments and simplify the tail-cancellation test
Comments cut back to the non-obvious reasoning only. The regression test now
counts executions instead of collecting indices, so a single Is.InRange assertion
covers both requirements — the decisive prefix ran, the tail did not — and the
prefix array, the index bag and one assertion all go away. Transaction count is a
plain constant rather than derived from the canonical lead, which the test never
depended on: with uniform gas limits the tail sort is stable, so the schedule is
natural order regardless. CreateParallelValidationTransactions takes an optional
gas limit so the block no longer needs patching after construction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(consensus): address review — exempt iteration 0, loosen test bound
Iteration 0 (WaitForBalWarmup + ApplyStateChanges) is now exempt from the
HasFailed guard, so pre-execution keeps its previous semantics instead of being
droppable when validation fails before any worker starts. Skipping it was traced
as benign, but the fix only needs to stop transaction execution, so leaving the
pre-execution step alone keeps the behavioural change narrower at no cost.
The regression test's upper bound leaned on SpinWait outlasting exception
unwinding, because the fake validator releases its gate before the work item
stores the exception. Bound is now txCount / 8, which asserts the tail stopped
without depending on cancellation-propagation timing — a revert still executes
all 2048 and fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* test: pin eth/62-66 serializer wire encodings with hand-derived goldens (#12696)
* test(network): remove duplicate eth serializer tests
ZeroNewBlockMessageSerializerTests.Roundtrip2 and V63
NodeDataMessageSerializerTests.Zero_roundtrip were verbatim copies of
their Roundtrip siblings. Can_deserialize_own_eth_64 shared its body
and one payload with Can_deserialize_eth_64; its unique payload moves
there as another TestCase.
* test(network): pin eth/62-66 wire encodings with hand-derived goldens
Every serializer test file in Eth V62-V66 now asserts an exact wire
encoding somewhere. The goldens are derived from the RLP rules and
verified with an independent encoder (pyrlp + pycryptodome keccak);
values shared across files live in EthSerializerGoldens. ToString
smoke tests now assert the log line names the message type, and the
32-byte-selector test asserts the exact decoded hash.
* test(network): address review feedback on serializer goldens
- BlockBodies: pin the null-body framing (c1c0) and the empty-vs-absent
withdrawals distinction (e9e8...c0c0)
- Status To_string: assert against the independent Protocol.Eth constant
- V63 NodeData: rename Roundtrip_with_nulls to Roundtrip_with_empty_entry
(the data holds an empty array, not null)
* feat: EIP-8037 two-dimensional gas tracing (stateGasTracer + callTracer) (#12628)
* feat(tracing): add EIP-8037 stateGasTracer (execution-apis #852)
Implements the `stateGasTracer` named tracer specified in execution-apis
PR #852, returning the per-transaction two-dimensional gas summary
`{gasUsed, regularGasUsed, stateGasUsed, gasRefund}` (EIP-8037/EIP-7778).
The values are already computed for block-level gas accounting and carried
on `GasConsumed`; the tracer only reads and formats them. Adds the missing
applied EIP-3529 refund (capped) to `GasConsumed.GasRefund`, populated in
the success and top-level-halt refund paths. The native tracer factory is
threaded with `IReleaseSpec` so the fork is determined explicitly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(tracing): add EIP-8037 two-dimensional gas to callTracer (execution-apis #852)
Adds `regularGasUsed`, `stateGasUsed` and `gasRefund` to the callTracer
top-level frame for Amsterdam+ blocks, per execution-apis PR #852. The
fields are gated on `IReleaseSpec.IsEip8037Enabled` (MUST NOT appear before
the fork) and set only on the top frame (omitted on sub-frames). Values are
read from the transaction's `GasConsumed` result, matching the stateGasTracer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): address #12628 review feedback
- Restore native-tracer factory API back-compat: keep the public 4-arg
GethLikeNativeTracerFactoryDelegate/RegisterTracer/CreateTracer so external
plugin registrations stay source- and binary-compatible; built-ins receive
the (nullable) IReleaseSpec via an internal spec-aware factory.
- stateGasTracer: disable IsTracingOpLevelStorage/IsTracingStack so the
terminal-only tracer stops invoking per-opcode storage/stack callbacks.
- Consolidate the callTracer top-frame regularGasUsed/stateGasUsed/gasRefund
into a single TwoDimensionalGas? value, removing the coupled nullables and
the unsafe null-forgiving dereferences in the converter.
- Document that regularGasUsed is floor-clamped (block-accounting value) so
the two-dimensional invariant is exempted under the calldata floor.
- Add an end-to-end stateGasTracer test executing a real Amsterdam tx through
the TransactionProcessor (fresh SSTORE + in-tx reset) that exercises field
selection and the GasConsumed.GasRefund plumbing; assert
regularGasUsed + stateGasUsed == gasUsed + gasRefund.
- Test cleanups: multiple-assert scope and a shared helper for the two
callTracer Amsterdam cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): address #12628 re-review nits
- Document the spec-less CreateTracer overload's pre-fork fallback and cover it
with a factory test (back-compat public contract was untested).
- stateGasTracer hex test now forces the ambient NumberConversion.Raw so it
actually exercises StateGasTraceConverter's hex-quantity override.
- E2E test: use a `using` tracer and correct the refund assertion message
(the slot is reset to its original zero value within the tx, not pre-nonzero).
- Convert the remaining `//` member comments to XML doc.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: remove unused using in NativeStateGasTracerE2ETests
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): drop excess comments; non-nullable spec in native tracer factory
Address flcl42 review on #12628:
- Remove the newly added explanatory comments that restated the code (kept only a
few essential EIP-referenced DTO docs and the hex-conversion note).
- Make the release spec non-nullable through GethLikeNativeTracerFactory and the
native tracers: drop the unused spec-less 4-arg CreateTracer overload (the only
null source) — RegisterTracer, the actual plugin API, is unchanged. Also drops
the brittle regularGasUsed occurrence-count assertion flcl42 flagged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: collapse double blank line before DeepNesting test
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): skip EIP-3607 on the EIP-7928 BAL path in eth_simulateV1 (#12691)
* fix(simulate): skip EIP-3607 on the EIP-7928 BAL path in eth_simulateV1
eth_simulateV1 relaxes EIP-3607 so a state-overridden contract can be the tx
`from`. That relaxation covered only the main execution (via
SimulateTransactionProcessorAdapter). Under glamsterdam (EIP-7928),
BlockProcessor runs transactions through the BlockAccessListManager's own tx
processors, which bypass the adapter and re-enforce EIP-3607 — so a contract
sender is rejected with `-38024 sender has deployed code` instead of reaching
the normal balance/fee checks (hive rpc-compat divergence on glamsterdam-devnet-8:
ethSimulate-simple-send-from-contract*, ethSimulate-override-address-twice).
Relax EIP-3607 on the block execution context in
SimulateBlockValidationTransactionsExecutor instead:
ParallelBlockValidationTransactionsExecutor sets that context on both the main tx
processor and the BAL manager, so both paths skip the check — while BlockProcessor
still receives the unwrapped spec, preserving chain-specific release-spec
interfaces (Taiko / XDC / Optimism).
Verified end-to-end on a glamsterdam-at-genesis chain (the three hive fixtures
flip -38024 -> -38014/-38012, matching besu/erigon/reth/geth) and by an
integration test through the real EIP-7928 BAL path that fails -38024 without the
fix and passes with it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): preserve PrevRandao in context rebuild; harden 3607 BAL test
Address re-review on #12691:
- SimulateBlockValidationTransactionsExecutor is now the single context-rebuild
funnel for the simulate scope, so preserve the incoming PrevRandao (via
BlockExecutionContext.WithPrevRandao*) instead of re-deriving the default —
a BlockProcessor subclass (e.g. XdcBlockProcessor) may have supplied a
non-default value.
- Make eth_simulateV1_contract_sender_skips_eip3607_on_bal_path validation-enabled
so the -38014 expectation is fork-independent and stable across the #12692 fix
(with validation:false the -38014 relied on the BAL path ignoring NoValidation).
Pin the EIP-7928 premise with an explicit BlockLevelAccessListsEnabled assert.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): forward BlobBaseFee too in the context rebuild
Re-review follow-up on #12691: WithPrevRandao recomputes BlobBaseFee from the
header, so the no-override branch was newly lossy for a processor that forced a
non-derivable value (XdcBlockProcessor sets BlobBaseFee = 0 on a header cloned
with ExcessBlobGas = 0; recomputation yields MinBlobGasPrice = 1). Collapse to a
single WithPrevRandaoAndBlobBaseFee that forwards the incoming BlobBaseFee (or the
block override when present), leaving Spec as the only field the rebuild changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): relax EIP-3607 via execution flag, not spec wrapping
The eth_simulateV1 contract-sender fix relaxed EIP-3607 by wrapping the block
execution context spec with WithoutEip3607(), which returns a NoEip3607Spec
decorator implementing only IReleaseSpec. Because the tx processors read their
spec from that context (TransactionProcessor.GetSpec => BlockExecutionContext.Spec),
the decorator reached chain-specific processors that hard-cast it — e.g.
TaikoTransactionProcessor.PayFees does (ITaikoReleaseSpec)spec and
XdcTransactionProcessor casts on every tx — so eth_simulateV1 on Taiko/XDC threw
InvalidCastException/InvalidOperationException instead of returning a result
(Eip3607Transition defaults to 0, so the wrap is always active there).
Replace the spec wrapping with a dedicated ExecutionOptions.SkipSenderCodeCheck
policy flag gated in ValidateSender. The main simulate adapter ORs it into its
Process call; the EIP-7928 BAL path receives it via a small
BlockAccessListTxExecutionOptions injected into BlockAccessListManager and threaded
to its ExecuteTransactionProcessorAdapter. The release spec now keeps its concrete
runtime type on every path, so chain-specific interfaces survive.
Regression test: Nethermind.Taiko.Test exercises a code-bearing sender through
TaikoTransactionProcessor.PayFees — it passes with the flag and throws
InvalidCastException under the old spec-wrapping. eth_simulateV1 BAL-path and full
simulate suite remain green.
Follow-up to #12691; addresses the residual type-erasure raised in its review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(simulate): trim explanatory comments to essentials
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): carry EIP-3607 relaxation on BlockExecutionContext
Replace the ExecutionOptions.SkipSenderCodeCheck flag (and the machinery to thread
it into the EIP-7928 BAL path) with a SkipSenderCodeCheck field on
BlockExecutionContext. Both the main tx processor and the BAL manager's own
processors already share the context via SetBlockExecutionContext, so the field
reaches every path for free — no threading through BlockAccessListManager /
TxProcessorPool / ExecuteTransactionProcessorAdapter, and no injected policy type.
The context is the same shared channel #12691 originally relaxed on; this just uses
a flag instead of a spec decorator, keeping the spec's concrete runtime type (so
ITaikoReleaseSpec/IXdcReleaseSpec casts survive). Net simpler diff and consistent
with the existing IsGenesis flag on the same struct.
ValidateSender reads VirtualMachine.BlockExecutionContext.SkipSenderCodeCheck; the
simulate executor sets it in its context rebuild. Taiko regression test updated to
set the flag on the context (still throws InvalidCastException under spec-wrapping).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): carry EIP-3607 relaxation on TransactionProcessor
Per review preference (@LukaszRozmej), move the SkipSenderCodeCheck flag from
BlockExecutionContext onto TransactionProcessor. ValidateSender reads the
processor's SkipSenderCodeCheck property. The simulate scope sets it type-
preservingly on both creation paths: Intercept<ITransactionProcessor> for the
main processor, and a factory decorator for the EIP-7928 BAL processors — so each
chain keeps its concrete processor type (no spec wrapping, no processor-type swap).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: trim comments to the essential why
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): remove dead WithoutEip3607; address review polish
Follow-up to the SkipSenderCodeCheck switch (review by @claude on #12691):
- Remove the now-dead EIP-3607 spec wrapper — this PR dropped its last production
caller. Deletes WithoutEip3607, NoEip3607Spec, GetNoEip3607Spec (std + zkevm) and
the _noEip3607Specs cache; keeps WithoutEip158 (still live). Fixes two comments
that cited the removed decorator.
- Enforce the relaxation invariant: a shared Apply() throws if the resolved
ITransactionProcessor isn't a TransactionProcessorBase, instead of silently
no-op'ing back to -38024. Used by both the Intercept hook and the factory decorator.
- XML-doc SkipSenderCodeCheck with its set-before-use / unsynchronised-read invariant.
- Make SkipSenderCodeCheckTransactionProcessorFactory internal.
- Revert SimulateBlockValidationTransactionsExecutor to master (the fix no longer
touches it; its PrevRandao tweak was unrelated).
- Assert fee payment in the Taiko regression test (matches its name).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(dns): verify EIP-1459 subtree hashes (#12707)
* fix(dns): verify EIP-1459 subtree hashes
EIP-1459 serves every subtree entry from the subdomain
base32(keccak256(entry)[..16]). The root signature covers only the
enrtree-root entry, so this hash chain is the only binding between the
signed root and the branch and ENR records a client consumes. The
crawler never checked it, so any resolver or poisoned cache could
substitute arbitrary node records and steer the crawl to
attacker-chosen labels.
Verification follows go-ethereum: unpadded standard base32, prefix
comparison against keccak256 of the record, abbreviated labels accepted
down to 12 decoded bytes (minHashLength). The tree root is exempt, as
EIP-1459 serves it from the bare domain with no hash label. A mismatch
logs at Warn and skips the record.
No base32 decoder existed in src/Nethermind, so EnrTreeHash carries a
small one, tested against labels produced by the reference
implementation.
Validated against live trees: all.mainnet.ethdisco.net crawls 3000
records with zero rejections, so the check does not misfire on
anything the reference publisher emits.
* refactor(dns): simplify and harden EnrTreeHash
- Reuse Keccak.Size instead of a local MaxHashLength constant.
- Decode base32 arithmetically instead of scanning the alphabet string.
- Guard the decoded-length check against int overflow on hostile input.
- Hash UTF-8 bytes from a stackalloc/pooled buffer instead of allocating.
* refactor(dns): log hash-mismatch rejections at Debug, clarify base32 mapping
---------
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
* Selectable dotTrace profiling mode + dotnet-trace EventPipe sidecar for benchmark workflows (#12708)
* benchmarks: selectable dotTrace profiling mode (sampling | tracing | timeline)
The dottrace input on run-rpc-benchmarks and run-expb-reproducible-benchmarks
becomes a choice: false | sampling | tracing | timeline ('true' stays accepted
as a legacy alias for sampling on API dispatches). The mode maps to the
dotTrace CLI's --profiling-type: rpc-bench sets it on the wrapped entrypoint in
start-node.sh; the EXPB workflow passes --dottrace-mode to expb (only when
non-default, so pinned expb versions keep working for sampling runs; requires
execution-payloads-benchmarks feature/dottrace-profiling-modes for the new
modes).
Timeline snapshots cannot be converted to XML by Reporter.exe, so the
generate-dottrace-reports and summary jobs skip that mode - the raw .dtp
artifact is still collected and uploaded for the dotTrace UI. Line-by-line is
deliberately not offered: it needs PDBs the client docker images do not carry.
EXPB additionally rejects trace_blocks with timeline, since per-block
snapshots ride the MeasureProfiler API, which needs a performance session.
* expb workflow: collect a dotnet-trace EventPipe sidecar with every dotTrace run
Whenever dottrace is enabled the run also passes --dotnet-trace to expb: a
host-side EventPipe session records gc/contention/threading/exception events
(no CPU sampler - dotTrace owns the stacks) and the .nettrace ships in the same
dottrace-* artifact. Requires expb feature/dottrace-profiling-modes.
* rpcbench/expb: document the profiling modes and fix two review nits
Docs still described the pre-mode behaviour: the rpc-bench README documented a
dottrace command line with no --profiling-type and stated capture deliberately
runs in default sampling mode, and its input table listed dottrace as a boolean.
AGENTS.md's expb section had the same gap. Both now cover the four choices, when
to reach for each, that timeline yields no XML, and the dotnet-trace sidecar -
that section is what agents read to interpret run artifacts.
Timeline snapshots now save as .dtt rather than .dtp. The report job is gated off
for timeline either way, but the extension is what stops Reporter.exe's .dtp glob
from picking up a snapshot it cannot convert if that gate is ever relaxed.
The expb dottrace flag no longer special-cases sampling: the run already requires
an expb that understands --dotnet-trace, so there is no older-expb compatibility
left to preserve, and the flag string was spelled out twice in each of two
copy-pasted job bodies. The trace_blocks default guard becomes an explicit if -
as the last statement of its if body the AND-list left the block with status 1,
surviving only through errexit's AND-OR exemption.
* docs: scope the EventPipe sidecar to EXPB
The rpc-bench README claimed every profiled run drops a .nettrace into the
dottrace-rpcbench artifact, but --dotnet-trace was only wired into the EXPB
workflow - nothing in run-rpc-benchmarks.yml or scripts/rpc-bench collects one, so
the paragraph sent readers hunting for a file that is never produced, and promised
it precisely for timeline runs, which have no XML either. Say what a timeline
rpc-bench run actually yields: the .dtt snapshot alone.
* fix(jsonrpc): serialize receipt root as full-width DATA (#12706)
* fix(jsonrpc): serialize receipt root as full-width DATA
* test(jsonrpc): parameterize the receipt-root width cases
* test(jsonrpc): pin the whole-byte leading-zero root case
* test: add hand-derived golden tests for eth/71 and snap serializers (#12699)
* test(network): pin eth/71 and snap serializer wire encodings
Every serializer test file in Eth V71 and Snap/V1 now asserts an exact
wire encoding. The goldens are hand-derived from the RLP rules and
verified with an independent encoder (pyrlp + pycryptodome keccak).
Random request ids are pinned only in the goldened tests; ByteCodes
gains a sibling roundtrip that keeps the constructor-assigned id, and
GetAccountRange pins long.MaxValue to keep the eight-byte id path.
* test(network): share repeated snap golden fragments
The request-id, keccak-of-empty-string, and range-hash fragments moved
into SnapSerializerGoldens. Each golden fragment and its test input
come from one hex constant, so the expectation cannot drift from the
input.
* test(network): address review feedback on snap golden tests
- ByteCodes: one parameterized Roundtrip; long.MaxValue pins the
eight-byte request id deterministically instead of a random id
- GetAccountRange: independent field asserts in EnterMultipleScope
- SnapSerializerGoldens: expose RequestId1111 next to its RLP item;
the remarks state which fragments share hex with inputs and that
the keccak("") fragment is an independent literal on purpose
* `debug_trace*`: Fix phantom logs on frame revert (#12621)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Phantom log test
* Remove logs on a reverted frame
* Naming
* Fix leaking `ArrayPooList`
* Code cleanup
* Additional test
* Simplify tests
* Build fix
* Snap sync: reject storage range responses with unmatched slot lists (#12729)
* fix(snap): reject storage range responses with unmatched slot lists
A `StorageRanges` reply carries one slot list per requested account, but
nothing verified that the responder returned no more lists than were asked
for. `AddStorageRange` looped over the response count and indexed the request
account list with it, so a longer-than-requested reply read past the end of
the request.
Reject such a response up front and retry the range, which also settles the
active storage request accounting so that the range download phase can still
reach completion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(snap): pin the slot list count boundary
Cover equal and fewer slot lists than requested accounts so that widening the
guard to `>=` cannot pass, which would reject every well-formed response.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(metrics): durable sync-time and full-pruning duration metrics (#12590)
* feat(metrics): durable sync-time + per-stage breakdown and full-pruning duration
Sync:
- Fix UpdateAndGetSyncTime() so the total is retained after sync completes
instead of resetting to zero (EthSyncingInfo and Taiko override).
- Replace nethermind_sync_time with nethermind_sync_time_seconds (durable).
- Add nethermind_sync_time_in_mode_seconds{sync_mode} per-stage breakdown,
driven by a new SyncTimeInModeTracker on ISyncModeSelector.
Pruning:
- Add nethermind_full_pruning_last_duration_seconds and
nethermind_full_pruning_count, recorded on successful FullPruningDb.PruningFinished.
- Suffix in-memory trie pruning gauges with *Ms and document the unit.
* refactor(metrics): address PR review on sync/pruning time metrics
- Sync time no longer drops to 0 for one scrape when a stopped node
re-syncs: extract shared SyncTimeStopwatch that always returns the
retained total (used by EthSyncingInfo and Taiko override). Add
stop->resume regression tests.
- SyncTimeInModeTracker is now IDisposable (unsubscribes from
ISyncModeSelector.Changed), seeds its labels with TryAdd instead of
clobbering the shared static dictionary, and is owned by the container.
Timestamp seam moved to an internal ctor.
- SyncTimeInModeSeconds is get-only to match the other [KeyIsLabel] metrics.
- FullPruningCount uses the Interlocked backing-field pattern like the
other counters in Db/Metrics.
- Clarify that FullPruningLastDurationSeconds covers the trie copy+commit,
not the wait for a suitable state root.
- Make the full-pruning duration test assert the write actually happened.
* fix(metrics): avoid DI cycle wiring the sync-mode tracker; drop unused using
- Attach SyncTimeInModeTracker via Intercept<ISyncModeSelector> instead of
resolving it during IMonitoringService construction. Resolving it there
created a container cycle (IMonitoringService -> tracker -> ISyncModeSelector
-> ... -> DbTracker -> IMonitoringService) that crashed the node at startup.
This keeps the monitoring module free of outward dependencies, mirroring the
existing IEthSyncingInfo wiring.
- Remove unused `using System;` in SyncTimeInModeTrackerTests (IDE0005).
* Only accept the requested header in FetchHeaderFromPeer (#12730)
* fix(sync): only accept the requested header in FetchHeaderFromPeer
FetchHeaderFromPeer queries every initialized peer in parallel and takes
the first non-null response, but never checked that the returned header
is actually the one that was asked for. A peer that answers with some
other block had its header accepted, so the result depended on which
peer replied first rather than on what was requested.
Compare the returned header's hash against the requested hash on both
response paths. A non-matching response is treated like a missing one,
so the remaining in-flight peers can still supply the header instead of
the whole lookup failing.
The sibling lookups in StartingSyncPivotUpdater and PeerRefresher
already do this; FetchHeaderFromPeer was the one that did not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): cover the allocated-peer fallback and tighten assertions
Add a case where no peer answers the head-header request, so the
GetBlockHeaders fallback is the one that resolves the header. Nothing
pinned its success branch before, so it could have been broken without
a test noticing.
Assert on the header instance rather than its hash, so the negative
case cannot pass for a header that merely has no hash set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sync): report a peer that answers with a different block
A mismatched header was discarded silently, leaving it indistinguishable
from a peer that simply does not have the block. Requests are serialised
per peer by MessageQueue and a late response to a timed-out request is
disposed rather than handed to the next caller, so a mismatch is always
a protocol violation rather than a benign response race.
Report it as UnexpectedHeaderHash, matching HeadersSyncFeed and the
other header lookups.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): cover that an honest peer keeps its connection
Reporting a mismatch disconnects the peer, so the costly failure mode is
now an honest peer being dropped rather than a wrong header accepted.
Only the reported direction was asserted.
Cover all three answer shapes, including a peer that does not have the
block, which is the normal answer while a head is unknown and must not
cost a connection.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix doubled revert handling in some tracers (#12715)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Phantom log test
* Remove logs on a reverted frame
* Naming
* Fix leaking `ArrayPooList`
* Code cleanup
* Additional test
* Simplify tests
* Showcase test
* Direct fix
* More failing tracing tests
* Remove problematic `ReportActionRevert` > `ReportActionError` delegation from base
* Get rid of virtual-to-virtual calls in report revert/error
* Formatting
* Build fix
* Remove other `ReportActionRevert` -> `ReportActionError` calls
* Move common test codes to base class
* Fix `IsTracingActions` summary
* Small test fix
* Code cleanup
* test(era): anchor accumulator tests to EIP-7643 vectors, remove unused GetProof (#12718)
* test(era): anchor AccumulatorCalculator roots to derived spec vectors
Replaces the two-instances self-oracle, the inequality-only comparisons,
and the DoesNotThrow-only Add tests with parameterized roots derived by
an independent Python SSZ merkleization of the portal-network history
spec. Adds proof fold-up verification against the derived roots.
Deletes the byte-identical duplicate golden test in Era1.Test.
* test(era): assert the accumulator root the readers return
ReadAccumulator and ReadAccumulatorRoot tests discarded the root and
asserted only Throws.Nothing. They now assert the root equals the
accumulator of the written contents. The shared calculator loop moves
into a ComputeAccumulatorRoot helper in each file.
* test(era): apply review round on the accumulator vector tests
Parameterizes the proof verification and adds a three-entry index 2
case, so the upper tree levels exercise the right-hand sibling order.
Moves the proof length assert into the fold-up helper. Rewrites the
provenance comments in active voice and adds the spec link.
* test(era): cite EIP-7643 as the accumulator spec reference
The portal-network history spec was restructured and no longer defines
the accumulator. EIP-7643 defines HeaderRecord and the 8192-entry epoch
record. Also states the exact value of the index 2 proof case: it is
the only case that catches a wrong index shift.
* refactor(era): remove unused AccumulatorCalculator.GetProof
GetProof shipped with the EraE feature but no production code ever
called it; its only callers were its own tests. The removal also drops
the _totalDifficulties list, which only GetProof read, and the proof
tests in Nethermind.EraE.Test.
* docs(era): cite EIP-7643 on AccumulatorCalculator
The linked portal-network document was restructured and no longer
defines the accumulator. EIP-7643 defines HeaderRecord and the
8192-entry epoch record this class computes.
* test(era): apply removal-round polish
Converts the class comment to an XML summary per the documentation
rule. Inlines the two-entry root, which lost its second consumer with
the proof tests.
* test(era): consolidate the accumulator fixtures into Era1.Test
Both fixtures tested Nethermind.Era1.AccumulatorCalculator with
overlapping vectors. The Era1.Test table now carries every
discriminating single-entry case, a pinned empty-accumulator root, and
a Clear reset test against the writer-reuse contract. The EraE.Test
copy is deleted.
* test(era): state only true contracts in the fixture comments
The Clear comment claimed a multi-file writer-reuse path that does not
exist: Add throws after Finalize and each era file gets a fresh writer.
The vector comment claimed a pairwise single-input difference that two
case pairs violate. Both now state only what holds.
* test: strengthen mock-echo and vacuous tests in Abi, Optimism and Xdc test projects (#12720)
* test(abi): pin forwarding and return propagation in encoder extensions
The extension tests asserted only Received on the inner call and
ignored the extension return value. A stub on the exact unpacked
arguments plus an identity assert on the result covers both.
* test(xdc): assert the RocksDb config factory routing
The single test asserted Is.Not.Null on a result that cannot be null.
The factory contract is routing: Xdc databases get a PerTableDbConfig
without consulting the base factory, and every other database delegates
to it. Both branches are now pinned; the delegation branch was
untested.
* test(xdc): pin the routed timeout instance
The vote routing test pins the exact vote, but the timeout test used
Arg.Any, so a handler that routes the wrong timeout passed.
* test: apply the C11 review round
Removes an unused using that fails lint CI. Pins the factory options
against the provided IDbConfig, so a wrong database name in the special
branch turns the test red. Moves the extension rationale to class level
and renames the Abi tests to the project snake_case convention.
* test: state only true mechanisms in the C11 comments
NSubstitute returns an empty array, not null, for an unmatched call on
an array-returning member. The prefixed-options claim holds for only
half of the database prefixes, so the sentence is dropped.
* test(xdc): use a neutral database name in the delegation test
Review feedback on #12720: the delegation test used the literal
"State"/"Code", and "State" is the one name PerTableDbConfig
special-cases (StartsWith("State")). Use nameof(DbNames.Blocks) with a
null column so the test isolates the delegate-to-base contract without
that special-case, matching the sibling FlatRocksDbConfigAdjusterTests
idiom.
* test: anchor crypto and RLP tests to independent expectations (#12712)
* test(core): anchor the keccak span test to an independent vector
* test(core): anchor RLP ulong lengths to the spec
* test(core): compare decoded blocks to the original and drop the ignored file writer
* test(core): compare decoded header fields to the original block
* test(core): anchor the regression block decode to pyrlp-derived fields
* test(core): apply review round - drop dead using, widen anchors, guard indexed asserts
* test(core): apply round-2 review polish
* test(core): cover the header tail fields and sharpen the roundtrip comments
* test(core): apply confirm-round nits
* test(core): compare decoded uncle hashes in the block roundtrip
The body compared uncles by count only. The scenarios build two uncles
with distinct headers, so the hash comparison catches an order or
content error the count cannot see. The count guard moves outside the
multiple-assert scope like the transaction guard.
* Update OP Superchain chains (#12752)
Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com>
* Auto-update fast sync settings (#12751)
Co-authored-by: rubo <rubo@users.noreply.github.com>
* test: Db tests assert stored state instead of smoke-calling empty methods (#12693)
* test(db): assert stored state instead of smoke-calling empty methods
MemDb.Flush and Dispose (and the Snapshotable variants) are empty method
bodies, so the six *_does_not_cause_trouble tests could never fail;
removed. The no-assert MemDb creation tests now verify the value round-
trips. ReadOnlyDbProviderTests.Can_clear exercised an empty registry (no
db was ever registered), making ClearTempChanges a no-op; it now
registers a real MemDb-backed read-only db and asserts the overlay is
dropped while the wrapped db stays intact, plus the no-writes contract
when localChanges is off. Can_get_all_on_empty now asserts emptiness
instead of discarding the enumeration.
* test(db): assert overlay drop only where a write could land in the overlay
* test(db): group independent post-condition asserts in Assert.EnterMultipleScope
* Reject invalid fixed-size header RLP (#12579)
* Treat a null header answer as the block being absent (#12741)
* fix(sync): treat a null header answer as the block being absent
An empty list item decodes to a null header and is kept in the response
list, so the allocated-peer fallback could pass one into the hash
comparison and throw. FetchHeaderFromPeer only catches cancellation and
timeouts, so it would surface into engine_forkchoiceUpdated.
Handle it in Validate, which lets the head-header path drop its own null
check too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): name the mock switch after the answer it produces
The peer answers with a null header, not an empty one. Move it next to
the other answer switches so HeaderToReturn keeps its own doc.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Return only the requested header from GetHeadBlockHeader (#12740)
* fix(network): return only the requested header from GetHeadBlockHeader
GetHeadBlockHeader asks for one specific block but returned whatever
single header the peer sent back. Every caller wants the block it asked
for, and two of them re-checked the hash themselves afterwards; the peer
refresh in SyncPeerPool did not, so it would take a substituted header
and record the wrong head number and total difficulty for that peer.
Compare the response against the requested hash where the request is
made, and disconnect a peer that answers with a different block. A peer
that does not have the block answers with an empty list, which is the
normal response while a head is unknown and still yields null.
Drop the now-redundant check in StartingSyncPivotUpdater. The one in
PeerRefresher stays: that method validates its head/parent response
locally anyway, since GetBlockHeaders carries no such guarantee.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(network): treat a null header answer as the block being absent
An empty list item decodes to a null header and is kept in the response
list, so a peer can answer a single-header request with one null entry.
Dereferencing it to compare hashes threw, and two callers do not catch
that: the pivot updater would leave its update loop for good, and
FetchHeaderFromPeer would surface it into engine_forkchoiceUpdated.
Treat it the same as an empty list — the peer does not have the block —
rather than as a breach.
Also type the requested hash as nullable, matching _remoteHeadBlockHash
before the status handshake, and assert the absent cases on the result
itself rather than on its hash.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(network): make the requested-header guarantee unconditional
The hash check was skipped when neither the argument nor the announced
head was known, so the documented contract had a hole. Return early
instead: with no hash to ask for there is no meaningful request to send.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat: add built-in portfolio viewer UI at /portfolio (#12360)
* Handle failed sender recovery (#12757)
* fix(simulate): route the EIP-7928 BAL path through the simulate tx adapter (#12692) (#12721)
* fix(simulate): route EIP-7928 BAL path through the simulate tx adapter
Under EIP-7928, eth_simulateV1 runs transactions through the
BlockAccessListManager's own tx processors, bypassing
SimulateTransactionProcessorAdapter. That lost its GasCap budget clamp, its
TotalGasLeft/BlockGasLeft accounting (block gasUsed reported as 0), and its
validation:false handling (the BAL path always called Execute, never Trace).
Inject the adapter via a new ITransactionProcessorAdapterFactory so the
sequential BAL manager — the only one simulate drives — wraps each tx processor
in the simulate adapter. The parallel manager always uses the default
ExecuteTransactionProcessorAdapter: the stateful simulate adapter is
sequential-only and simulate never triggers the parallel path (it attaches no
BlockAccessList).
The factory is an interface, not a delegate, so Autofac does not
auto-synthesise one on the real block-production scope, where the optional
parameter must stay null and fall back to the default adapter.
The no-gas EIP-8037 execution-dimension default is a separate ordering issue
(the per-tx inclusion check runs before the adapter) and is left as a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): make the tx-processor-adapter factory a required delegate; add GasCap test
Replace the ITransactionProcessorAdapterFactory interface with a required
TransactionProcessorAdapterFactory delegate, mirroring CodeInfoRepositoryFactory:
the block-processing module registers the default (ExecuteTransactionProcessorAdapter)
and the simulate scope overrides it with the simulate adapter. This drops the
optional/null-fallback and the auto-synthesis hazard an optional delegate would
carry (ITransactionProcessorAdapter is registered on the block-processing scope,
so Autofac would otherwise fill an optional delegate on the real path).
Also add a regression test for the JsonRpc.GasCap budget (#12692 item 2): a
two-call request whose cumulative gas exceeds the cap has its second call clamped
below intrinsic gas and rejected; without the adapter the cap is not enforced and
both calls run unclamped.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: address #12721 feedback (enforce sequential BAL, DI style, docs, test dedup)
- Set ProcessingOptions.ForceSequentialBlockAccessList in the simulate options so the
stateful SimulateTransactionProcessorAdapter can never reach the parallel BAL pool
(previously guaranteed only by simulate not attaching a BlockAccessList); documented
the single-threaded contract on the adapter itself.
- Register the default TransactionProcessorAdapterFactory with AddScoped + a method group
(drops the lambda cast) and the simulate override with the typed-dependency AddScoped
overload (no manual Resolve / captive singleton).
- Reworded the factory <remarks> to describe what is actually wired (default Execute;
other scopes still get the default on the BAL path) and dropped the overstated
"gas defaulting" from the registration comment.
- Extracted BuildAmsterdamBalChain test helper; pinned the EIP-2780 intrinsic in the
GasCap test comment.
Follow-up filed as #12723 (scopes overriding ITransactionProcessorAdapter still get
Execute on the BAL path).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: single-axis tx-processor-adapter registration (step 1)
Addresses LukaszRozmej's review: the TransactionProcessorAdapterFactory was a
second registration axis alongside ITransactionProcessorAdapter, so scopes that
only overrode the latter (block production, trace, proof) drifted to the default
Execute adapter on the EIP-7928 BAL path — the same bug class this PR fixes for
simulate (#12723), left live elsewhere. Notably block production silently
downgraded its intended BuildUp semantics to Execute under Amsterdam.
Make the factory the single source of truth: the root registers the default
(Execute) plus a derivation ITransactionProcessorAdapter = factory(processor), and
each scope overrides only the factory — production BuildUp, trace (…
Marchhill
added a commit
that referenced
this pull request
Aug 19, 2026
* fix(rpc-bench): run benchmarked nodes the way production runs them (#12625)
* diag(rpc-bench): run Nethermind with production-default runtime (no TC=0 pin)
* reword comment for master
* review: document expb divergence, add NODE_ENV_VARS escape hatch
- README: the 'Alignment with expb' section no longer claims the removed
env pins; documents the deliberate code-gen divergence and that JIT
warm-up now lands inside the measured window; dotTrace reports are not
comparable across this change
- start-node.sh: reword comment (no warm-up phase exists yet), add
NODE_ENV_VARS passthrough for deliberate one-off code-gen experiments
* trim comments to one-liners; rationale stays in the PR
* drop the Merge GC flags: inert here and misleading
GCKeeper only runs on Engine API calls; this harness parks the node at a
snapshot head and never sends newPayload, so the three flags changed
nothing while implying the node ran a non-production GC configuration.
* keep the image entrypoint for Nethermind
The override skipped entrypoint.sh, which applies host tuning and enables a
shipped PGO profile. Its comment claimed parity with expb, but expb only
overrides the entrypoint for dotTrace, so normal expb runs (and production)
do run entrypoint.sh - this harness was the outlier.
* Rename EIP-8037 regular gas dimension to execution gas (#12600)
* Auto-update fast sync settings (#12665)
Co-authored-by: rubo <rubo@users.noreply.github.com>
* refactor(net): namespace snap by version (#12606)
* refactor(net): namespace snap messages by version
Prepare the snap subprotocol for a second version by moving the concrete
snap/1 messages, message codes and protocol handler into a versioned
namespace, so a snap/2 handler can be added alongside without touching
snap/1 code.
Snap/Messages/* -> Snap/V1/Messages/*
Snap/SnapMessageCode -> Snap/V1/Snap1MessageCode
Snap/SnapProtocolHandler -> Snap/V1/Snap1ProtocolHandler
P2P/P2PMessageKey.cs -> P2P/VersionedProtocol.cs (file renamed to
match the type it declares)
SnapMessageBase and SnapSerializerBase deliberately stay in
Snap.Messages: they are shared by all protocol versions, not specific to
snap/1.
Introduce SnapVersions constants and ISnapSyncPeer.SnapProtocolVersion so
version checks can replace the magic numbers currently spelled as 1.
PeerInfoExtensions.CanGetSnapData is renamed to CanGetTrieNodes to say
what it actually tests. The rename is nominal - the peer probe is
unchanged, so behaviour is identical.
No functional change.
* refactor(net): remove Snap2 version constant from SnapVersions
* address review comments
* rename
* feat(sync): serve block access lists from the snap server (#12607)
* Refactor SnapServer and SnapStateServer integration
- Renamed SnapServer to SnapStateServer for clarity and consistency.
- Updated WorldStateManager to use SnapStateServer instead of SnapServer.
- Modified StateSyncFeedTestsBase to accommodate changes in SnapServer instantiation.
- Adjusted SnapProviderTests to reflect the new SnapStateServer type.
- Introduced SnapServerTests to validate SnapServer functionality.
- Added SnapStateServerTests to ensure robust testing of state management.
- Implemented new methods in SnapServer for handling bytecode and block access lists.
- Enhanced test coverage for account range retrieval and storage management.
* refactor: change SnapServer field type to interface ISnapServer
* test: enhance SnapServerTests with additional block access list scenarios
* chore: Update Dockerfiles (#12663)
Update Dockerfiles
Co-authored-by: rubo <rubo@users.noreply.github.com>
* test: make prewarmer env-return assertion pool-hit independent (#12616)
PreWarmCaches_ReturnsAddressWarmEnvWhenScopeBuildThrows asserts
Returned == Created, but with maxPoolSize 1 an env returned to the pool
can be rented again without a Create, so a pool hit legitimately
increments Returned twice against a single Create. Whether a hit occurs
depends on worker interleaving: flaky on CI, fails deterministically on
high-core machines (0/30 locally).
ThrowingBuildPolicy.Return now refuses retention, so every rental is a
fresh Create and the invariant holds under any interleaving (30/30
locally). Stack-trace instrumentation confirmed every rental is returned
exactly once - the prewarmer itself is correct.
* Update OP Superchain chains (#12664)
Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
* fix(receipts): restore the post-merge flag before regeneration (#12641)
* fix(receipts): restore the post-merge flag before regeneration
Stored headers do not carry IsPostMerge and regeneration bypasses the
recovery step that restores it, so post-merge blocks re-executed with
PREVRANDAO evaluating to the zeroed difficulty - any transaction reading
it produced receipts failing the root check (~7% of post-merge blocks
refused on mainnet archives deriving receipts from state).
* test(receipts): dispose buffer, pin logged value
* fix(receipts): classify post-merge via the switcher
A difficulty heuristic misreads chains that repurpose the field -
Taiko stores per-block ZK gas in Difficulty while AlwaysPoS - so ask
IPoSSwitcher instead, mirroring MergeProcessingRecoveryStep. On
mainnet the switcher's TD-null branch is the same difficulty check,
so behavior there is unchanged.
* test(receipts): pin the real switcher's TD-null derivation
The production failure arrived as a mainnet-shaped header with
TotalDifficulty unset; cover PoSSwitcher's TD-null branch end to end,
not only the honour-the-switcher contract.
* test(receipts): cover the switcher registration path
A hand-injected switcher cannot catch a composition regression that
leaves the container-resolved regenerator on the NoPoS default, so
resolve it from a graph whose IPoSSwitcher registration is overridden
the way a merge-enabled node overrides it.
* fix(tests): mark RecoverReceiptsBlockchain.Create as hiding
* Expose the node's ENR in admin_nodeInfo (#12631)
feat(rpc): expose the node's ENR in admin_nodeInfo
Nethermind is the only discv5-capable execution client whose
admin_nodeInfo omits the node record. Tooling that bootstraps
discv5-only networks reads the ENR from this endpoint and has to
special-case Nethermind to the enode instead, which is useless once
discv4 is disabled.
NodeRecordProvider already maintains a signed, sequence-numbered self
record, so surface it as an 'enr' field. Move INodeRecordProvider to
Nethermind.Network so Nethermind.JsonRpc can reference it without a new
dependency on Nethermind.Network.Discovery; the implementation stays in
Discovery. The provider is only registered when discovery is enabled, so
it is resolved optionally and the field is omitted otherwise.
* Validate ABI decode allocation bounds (#12588)
* ci: disable stateless glamsterdam-devnet-7 scheduled run (#12680)
* Fix EIP-7708 tracing with logs (#12577)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Naming
* fix(flat): guard the trie-warmer against a TransientResource recycle race (storage reads as 0x00) (#12429)
* fix(flatdb): warm the trie from persistence only
The trie warmer read the recyclable `_snapshots` and `_transientResource`
while the warm job held only a `ReadOnlySnapshotBundle` lease, which does not
cover them. A concurrent scope reset could recycle those under the running
warmer, so a warm read could return a torn or foreign node.
The warmer only needs to warm from persistence, so restrict its reads to the
trie node cache and the `ReadOnlySnapshotBundle` - exactly what the lease
covers. In-memory nodes are already hot and do not need warming.
Claude-Session: https://claude.ai/code/session_01S3BG321zDG9BNjAgWhZhCX
* fix(flat): warm the transient resource via a per-job lease
The trie warmer now warms nodes into the per-job-pinned TransientResource
(not just the trie node cache and ReadOnlySnapshotBundle), covered by a single
transient lease held for the whole warm traversal. It still never reads the
recyclable _snapshots. A warmer read on a torn-down bundle bails to a
persistence-only read instead of spinning for a lease that will never land.
Claude-Session: https://claude.ai/code/session_01S3BG321zDG9BNjAgWhZhCX
* refactor(flat): drop the warmer transient ThreadStatic capture
Measured TS-on vs always-per-node-lease on x10 (3 runs, heavy-block warmer
load): newPayload 178 vs 178 ms, getProof p99 20.1 vs 20.2 ms, RSS identical.
The ambient capture bought nothing, so keep only the per-node lease + ABA
re-check (the actual recycle-race fix) and drop the ThreadStatic + the
EnterWarmerTransientScope pins in the two warm-job callers.
* fix(flat): register the transient return owner at pool checkout
- ResourcePool.GetCachedResource now calls OnRented, so every checkout
carries a registered return owner; a final ReleaseLease without one
throws instead of silently dropping the resource (which leaked the
BloomFilter allocation on the public AddSnapshot path)
- re-check _isDisposed after the warmer's lease acquire: Dispose releases
the owner lease but leaves _transientResource pointing at the recycled
instance, so the identity re-check alone could latch a resource already
re-rented by another bundle
- document why ReleaseLease is distinct from Dispose: the pool contract
reserves Dispose for destroying an over-capacity resource
- run Nethermind.State.Flat.Test in the nethermind-tests.yml matrix; it
was compiled but not run by any CI job
- FlatDbManagerTests duplicate-snapshot test asserts the resource lands
back in the checkout pool; new ResourcePoolTests cover the final-release
return and the unregistered-release throw; refresh stale warmer test
comments
* fix(flat): pin the transient resource for prewarm dedupe reads
ShouldQueuePrewarm read _transientResource without pinning it. The dedupe
bloom lives on that recyclable resource and the call runs on prewarmer and
BAL threads, so the owner could retire the resource mid-read: the pool
Resets it and, on overflow, Disposes the BloomFilter, whose backing store is
native memory. Route both overloads through the same lease + ABA re-check
the warmer node reads already use, and decline the prewarm on a torn-down
bundle. Rename the helper accordingly, since it is no longer warmer-only.
The FlatWorldStateScopeProvider and FlatOverridableWorldScope test doubles
returned the committed resource to the pool directly instead of releasing
its lease, mirroring neither AddSnapshot implementation. That recycles the
resource while a warmer lease is outstanding and returns it a second time
when that lease is released; the scope provider double also returned the
wrong instance and then returned it again on teardown. Both now release the
lease, which is the single return-to-pool path.
Test changes:
- the persistence-only test now commits the written nodes into the bundle's
recyclable _snapshots before reading, so the warmer's Unknown result is a
genuine miss. Previously the node was still in the transient (SetStateNode
writes both) and was itself Unknown, so the assertion held either way.
- the churn test gives every epoch its own persisted node instance, so a
read served from another epoch's recycled transient is caught by identity
rather than by value, drives both recycle paths (CollectAndApplySnapshot
swap and Dispose), exercises ShouldQueuePrewarm alongside the node reads,
and joins the readers with a bounded wait instead of blocking forever.
- new test: a warmer read and a prewarm check on a disposed bundle fall back
to the leased persistence reader within a bounded wait, covering the
Dispose bail-out deterministically.
* fix(jsonrpc): synchronise SubscriptionManager per-client subscription bag (#12672)
* fix(jsonrpc): synchronise SubscriptionManager per-client bag
The per-client subscription bag is a HashSet mutated and enumerated from
multiple threads: concurrent subscribe requests (socket worker tasks),
unsubscribe, and the Closed handler that fires on connection teardown.
Concurrent HashSet access could corrupt it, dropping a subscription so
its event handlers stayed attached and leaked. Lock on the bag for every
add, remove, and snapshot-before-dispose.
Fixes #12668
* refactor(jsonrpc): dispose client subscriptions under bag lock without snapshot
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(jsonrpc): race unsubscribe path too; drop bag field comment
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* test: eth_createAccessList affordability with omitted fee fields (#12629)
* test(rpc): eth_createAccessList affordability with omitted fee fields (execution-apis #854)
execution-apis PR #854 clarifies that eth_createAccessList must not fail
solely because an unfunded sender cannot afford client-selected default fees
when all gas-fee fields are omitted. Nethermind already conforms (verified in
hive rpc-compat); this adds a regression test mirroring the conformance
fixture: unfunded sender, codeless recipient, zero value, no gas/fee fields
-> {"accessList":[],"gasUsed":"0x5208"}.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(rpc): run affordability test on London chain, assert no in-body error, reuse helper
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
* Fix stale transaction pool snapshots (#12685)
* Encode engine_newPayloadWithWitness witness as an RLP data string (#12635)
* fix
Signed-off-by: jsign <jsign.uy@gmail.com>
* Tighten witness RLP JSON encoding
---------
Signed-off-by: jsign <jsign.uy@gmail.com>
Co-authored-by: jsign <jsign.uy@gmail.com>
* perf(state): skip trie warmup for read-only BAL accounts in flat layout (#12681)
* perf(state): skip trie warmup for read-only BAL accounts in flat layout
With a suggested BAL the block's write set is known upfront, and trie
nodes are only needed at commit for written accounts. Gate address
trie-warm hints (HintBal, HintGet, HintWarmAccount) on the BAL write
set so read-only accounts no longer trigger state-trie path walks.
On BAL blocks dominated by cold account reads this removes up to ~8-10
wasted trie-node DB reads per unique read-only account from the
measured processing window.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Apply suggestions from code review
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
* refactor(state): extract QueueStateTrieWarmup and address review findings
- Extract the NeedsStateTrieWarmup + PushAddressJob + increment pattern
into QueueStateTrieWarmup, used by all three address warmup call sites
- Hoist CancelHintBal above the empty-BAL early return so a stale write
set never survives into the next block
- Drop the inaccurate bloom false-positive-rate comment
- Tests: parameterize warm-per-write-kind over balance/nonce/code/storage,
add empty-BAL reset regression test, split the HintWarmAccount test,
wrap scopes in using, use order-insensitive assertions
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Update src/Nethermind/Nethermind.State.Flat/ScopeProvider/FlatWorldStateScope.cs
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
* refactor(test): reuse TestContext for recording-warmer scope construction
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(state): bind BAL warmup gate to HintBal lifecycle; address review findings
- Keep _warmupWriteSet across StartWriteBatch: on the parallel BAL path the
BAL apply commits mid-block, concurrently with tx workers, so clearing the
gate in CancelHintBal collapsed it at a nondeterministic point. The gate is
now replaced only by the next HintBal.
- Drop the token from Task.Run in both HintBal implementations: a task
cancelled before being dequeued never ran the finally that returns the
pooled accountChanges array. The body already observes the token.
- Align TrieStoreScopeProvider.HintBal with the flat scope: a new hint
supersedes the previous one even when it carries no work itself.
- Remove the stale prestate-load mutation mention from ReadOnlyBlockAccessList
docs, state the immutability invariant, and seal the type.
- Tests: pin the gate surviving StartWriteBatch and a second BAL replacing
the previous write set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
* test: make can-never-fail tests assert what their names claim (#12690)
* test(xdc): drop redundant trailing Assert.Pass in SpecialTransactionsTests
* test(core): bound McsLock re-acquire test instead of passing unconditionally
SingleThreadAcquireRelease asserted nothing and is subsumed by
ReacquireAfterReleaseSucceeds, which now runs on a worker with a timeout
so a broken release fails in seconds instead of hanging the test host.
* test(flat): assert real postconditions instead of Assert.Pass
Add_ConcurrentWithMightContain_ShouldWork now verifies no write is lost
under concurrent readers (a bloom filter never false-negatives).
DisposeAsync test renamed: FlatDbManager does not dispose the repository
(the container owns it); it now asserts bounded drain completion and
idempotent double-dispose. ConstructorAcceptsPersistedRepository removed -
Not.Null on a fresh object, subsumed by every other test in the file.
* test(merge): assert pending-validation cleanup instead of catch-only assertions
The memory-leak test asserted only inside catch blocks and ended with
Assert.Pass, so it passed whenever nothing threw; it now drives the
handler directly and asserts the pending-validation count stays zero
across repeated timed-out payloads. The TrySet double-completion test
is removed: it was a weaker duplicate of the concurrent-calls test
above it (same scenario, but swallowing OperationCanceledException and
keying Assert.Fail on exception message text).
* test(merge): await header-sync test helpers
The helpers were async void and invoked without awaiting - including one
un-awaited call inside the other helper - so their assertions raced the
test body and failures could surface as host crashes or not at all.
* test(network): restore DisconnectsAnalyzer assertions with deterministic flush capture
All four tests had their assertions commented out as CI-flaky, leaving
them assertion-free. The flakiness came from racing the 10ms flush
timer: reports are now recorded while the default 10s interval is in
effect and only then is the interval shortened, so a flush cannot fire
mid-arrangement. Assertions scan captured flush reports, including that
counters aggregate, reset after each flush, and cleared categories do
not resurface (the analyzer double-buffers, so a lost clear shows up as
a stale count in every other flush, not as a doubled count).
* test: address review findings on strengthened tests
Will_clear_after_report no longer issues a second report at all - a
report can race the flush's enumerate-then-clear window (Timer.Stop does
not drain a queued Elapsed callback, so no test-side quiesce is airtight).
A lost clear is observable without it: the analyzer double-buffers, so a
stale count resurfaces in later flushes, and the test asserts the
category appears in exactly one flush across several more (mutation-
verified). Bloom capacity raised to 100k - at 10k the saturated filter's
~34% false-positive rate masked a single lost write - and misses are
collected into one assertion. The repeated-timeouts payload test is
dropped rather than parameterized: the pending dictionary is keyed by
block hash and the test resubmits one block, so the count can never
exceed one and iterations add no coverage. The FlatDbManager dispose
test states its intent with Assert.DoesNotThrowAsync and a corrected
comment (WaitAsync bounds the wait, not the drain). Wait timeouts are
named constants and polling uses Thread.Sleep(1).
* test: simplify comments per ASD-STE100 and drop dead times parameter
Comments now use short, active, single-topic sentences. The times
parameter of ShouldEventuallyReport had no remaining non-default call
site after the second-report removal.
* test: use SpinWait.SpinUntil instead of a custom poll helper
Keep one condition re-check after a timeout: a flush can land in
SpinUntil's final sleep tick.
* Stop parallel transaction execution once BAL validation rejects the block (#12697)
* fix(consensus): stop parallel tx execution once BAL validation rejects
The parallel BAL block validator ran the incremental validator alongside the
transaction workers, but a validator failure faulted neither the worker loop nor
its cancellation token. `ParallelUnbalancedWork` only stops fetching new indices
on caller cancellation or a transaction-worker fault, so a block rejected at an
early transaction index still executed every remaining transaction before the
foreground observed the failure at `GetResult()`. For a block whose invalidity is
decided by a cheap prefix, that turns a sub-second rejection into slot-scale CPU
work.
`IncrementalValidationWorkItem` now owns a cancellation source that is signalled
together with the stored exception, and the worker loop runs under that token, so
workers stop pulling transactions as soon as validation becomes terminal. The
resulting `OperationCanceledException` is translated back into the original
validation failure, keeping the returned error identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(consensus): signal BAL validation failure with a flag, not cancellation
Routing the stop signal through ParallelUnbalancedWork's cancellation token meant
`For` ended by throwing an OperationCanceledException that existed only to be
caught and translated back into the original validation failure. The token also
needed a per-block CancellationTokenSource, whose recycling in Schedule was only
safe because the previous block's validator had already been joined.
Replace all of it with a volatile read of the exception the work item already
stores: workers check `HasFailed` before doing any work, the loop drains its
remaining indices without executing anything, and `GetResult()` reports the
rejection on the normal return path. Same behaviour, no exception used as control
flow, no extra state to keep in sync, and no ordering constraint between Schedule
and the work that follows it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(test): trim comments and simplify the tail-cancellation test
Comments cut back to the non-obvious reasoning only. The regression test now
counts executions instead of collecting indices, so a single Is.InRange assertion
covers both requirements — the decisive prefix ran, the tail did not — and the
prefix array, the index bag and one assertion all go away. Transaction count is a
plain constant rather than derived from the canonical lead, which the test never
depended on: with uniform gas limits the tail sort is stable, so the schedule is
natural order regardless. CreateParallelValidationTransactions takes an optional
gas limit so the block no longer needs patching after construction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(consensus): address review — exempt iteration 0, loosen test bound
Iteration 0 (WaitForBalWarmup + ApplyStateChanges) is now exempt from the
HasFailed guard, so pre-execution keeps its previous semantics instead of being
droppable when validation fails before any worker starts. Skipping it was traced
as benign, but the fix only needs to stop transaction execution, so leaving the
pre-execution step alone keeps the behavioural change narrower at no cost.
The regression test's upper bound leaned on SpinWait outlasting exception
unwinding, because the fake validator releases its gate before the work item
stores the exception. Bound is now txCount / 8, which asserts the tail stopped
without depending on cancellation-propagation timing — a revert still executes
all 2048 and fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* test: pin eth/62-66 serializer wire encodings with hand-derived goldens (#12696)
* test(network): remove duplicate eth serializer tests
ZeroNewBlockMessageSerializerTests.Roundtrip2 and V63
NodeDataMessageSerializerTests.Zero_roundtrip were verbatim copies of
their Roundtrip siblings. Can_deserialize_own_eth_64 shared its body
and one payload with Can_deserialize_eth_64; its unique payload moves
there as another TestCase.
* test(network): pin eth/62-66 wire encodings with hand-derived goldens
Every serializer test file in Eth V62-V66 now asserts an exact wire
encoding somewhere. The goldens are derived from the RLP rules and
verified with an independent encoder (pyrlp + pycryptodome keccak);
values shared across files live in EthSerializerGoldens. ToString
smoke tests now assert the log line names the message type, and the
32-byte-selector test asserts the exact decoded hash.
* test(network): address review feedback on serializer goldens
- BlockBodies: pin the null-body framing (c1c0) and the empty-vs-absent
withdrawals distinction (e9e8...c0c0)
- Status To_string: assert against the independent Protocol.Eth constant
- V63 NodeData: rename Roundtrip_with_nulls to Roundtrip_with_empty_entry
(the data holds an empty array, not null)
* feat: EIP-8037 two-dimensional gas tracing (stateGasTracer + callTracer) (#12628)
* feat(tracing): add EIP-8037 stateGasTracer (execution-apis #852)
Implements the `stateGasTracer` named tracer specified in execution-apis
PR #852, returning the per-transaction two-dimensional gas summary
`{gasUsed, regularGasUsed, stateGasUsed, gasRefund}` (EIP-8037/EIP-7778).
The values are already computed for block-level gas accounting and carried
on `GasConsumed`; the tracer only reads and formats them. Adds the missing
applied EIP-3529 refund (capped) to `GasConsumed.GasRefund`, populated in
the success and top-level-halt refund paths. The native tracer factory is
threaded with `IReleaseSpec` so the fork is determined explicitly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(tracing): add EIP-8037 two-dimensional gas to callTracer (execution-apis #852)
Adds `regularGasUsed`, `stateGasUsed` and `gasRefund` to the callTracer
top-level frame for Amsterdam+ blocks, per execution-apis PR #852. The
fields are gated on `IReleaseSpec.IsEip8037Enabled` (MUST NOT appear before
the fork) and set only on the top frame (omitted on sub-frames). Values are
read from the transaction's `GasConsumed` result, matching the stateGasTracer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): address #12628 review feedback
- Restore native-tracer factory API back-compat: keep the public 4-arg
GethLikeNativeTracerFactoryDelegate/RegisterTracer/CreateTracer so external
plugin registrations stay source- and binary-compatible; built-ins receive
the (nullable) IReleaseSpec via an internal spec-aware factory.
- stateGasTracer: disable IsTracingOpLevelStorage/IsTracingStack so the
terminal-only tracer stops invoking per-opcode storage/stack callbacks.
- Consolidate the callTracer top-frame regularGasUsed/stateGasUsed/gasRefund
into a single TwoDimensionalGas? value, removing the coupled nullables and
the unsafe null-forgiving dereferences in the converter.
- Document that regularGasUsed is floor-clamped (block-accounting value) so
the two-dimensional invariant is exempted under the calldata floor.
- Add an end-to-end stateGasTracer test executing a real Amsterdam tx through
the TransactionProcessor (fresh SSTORE + in-tx reset) that exercises field
selection and the GasConsumed.GasRefund plumbing; assert
regularGasUsed + stateGasUsed == gasUsed + gasRefund.
- Test cleanups: multiple-assert scope and a shared helper for the two
callTracer Amsterdam cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): address #12628 re-review nits
- Document the spec-less CreateTracer overload's pre-fork fallback and cover it
with a factory test (back-compat public contract was untested).
- stateGasTracer hex test now forces the ambient NumberConversion.Raw so it
actually exercises StateGasTraceConverter's hex-quantity override.
- E2E test: use a `using` tracer and correct the refund assertion message
(the slot is reset to its original zero value within the tx, not pre-nonzero).
- Convert the remaining `//` member comments to XML doc.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: remove unused using in NativeStateGasTracerE2ETests
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): drop excess comments; non-nullable spec in native tracer factory
Address flcl42 review on #12628:
- Remove the newly added explanatory comments that restated the code (kept only a
few essential EIP-referenced DTO docs and the hex-conversion note).
- Make the release spec non-nullable through GethLikeNativeTracerFactory and the
native tracers: drop the unused spec-less 4-arg CreateTracer overload (the only
null source) — RegisterTracer, the actual plugin API, is unchanged. Also drops
the brittle regularGasUsed occurrence-count assertion flcl42 flagged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: collapse double blank line before DeepNesting test
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): skip EIP-3607 on the EIP-7928 BAL path in eth_simulateV1 (#12691)
* fix(simulate): skip EIP-3607 on the EIP-7928 BAL path in eth_simulateV1
eth_simulateV1 relaxes EIP-3607 so a state-overridden contract can be the tx
`from`. That relaxation covered only the main execution (via
SimulateTransactionProcessorAdapter). Under glamsterdam (EIP-7928),
BlockProcessor runs transactions through the BlockAccessListManager's own tx
processors, which bypass the adapter and re-enforce EIP-3607 — so a contract
sender is rejected with `-38024 sender has deployed code` instead of reaching
the normal balance/fee checks (hive rpc-compat divergence on glamsterdam-devnet-8:
ethSimulate-simple-send-from-contract*, ethSimulate-override-address-twice).
Relax EIP-3607 on the block execution context in
SimulateBlockValidationTransactionsExecutor instead:
ParallelBlockValidationTransactionsExecutor sets that context on both the main tx
processor and the BAL manager, so both paths skip the check — while BlockProcessor
still receives the unwrapped spec, preserving chain-specific release-spec
interfaces (Taiko / XDC / Optimism).
Verified end-to-end on a glamsterdam-at-genesis chain (the three hive fixtures
flip -38024 -> -38014/-38012, matching besu/erigon/reth/geth) and by an
integration test through the real EIP-7928 BAL path that fails -38024 without the
fix and passes with it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): preserve PrevRandao in context rebuild; harden 3607 BAL test
Address re-review on #12691:
- SimulateBlockValidationTransactionsExecutor is now the single context-rebuild
funnel for the simulate scope, so preserve the incoming PrevRandao (via
BlockExecutionContext.WithPrevRandao*) instead of re-deriving the default —
a BlockProcessor subclass (e.g. XdcBlockProcessor) may have supplied a
non-default value.
- Make eth_simulateV1_contract_sender_skips_eip3607_on_bal_path validation-enabled
so the -38014 expectation is fork-independent and stable across the #12692 fix
(with validation:false the -38014 relied on the BAL path ignoring NoValidation).
Pin the EIP-7928 premise with an explicit BlockLevelAccessListsEnabled assert.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): forward BlobBaseFee too in the context rebuild
Re-review follow-up on #12691: WithPrevRandao recomputes BlobBaseFee from the
header, so the no-override branch was newly lossy for a processor that forced a
non-derivable value (XdcBlockProcessor sets BlobBaseFee = 0 on a header cloned
with ExcessBlobGas = 0; recomputation yields MinBlobGasPrice = 1). Collapse to a
single WithPrevRandaoAndBlobBaseFee that forwards the incoming BlobBaseFee (or the
block override when present), leaving Spec as the only field the rebuild changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): relax EIP-3607 via execution flag, not spec wrapping
The eth_simulateV1 contract-sender fix relaxed EIP-3607 by wrapping the block
execution context spec with WithoutEip3607(), which returns a NoEip3607Spec
decorator implementing only IReleaseSpec. Because the tx processors read their
spec from that context (TransactionProcessor.GetSpec => BlockExecutionContext.Spec),
the decorator reached chain-specific processors that hard-cast it — e.g.
TaikoTransactionProcessor.PayFees does (ITaikoReleaseSpec)spec and
XdcTransactionProcessor casts on every tx — so eth_simulateV1 on Taiko/XDC threw
InvalidCastException/InvalidOperationException instead of returning a result
(Eip3607Transition defaults to 0, so the wrap is always active there).
Replace the spec wrapping with a dedicated ExecutionOptions.SkipSenderCodeCheck
policy flag gated in ValidateSender. The main simulate adapter ORs it into its
Process call; the EIP-7928 BAL path receives it via a small
BlockAccessListTxExecutionOptions injected into BlockAccessListManager and threaded
to its ExecuteTransactionProcessorAdapter. The release spec now keeps its concrete
runtime type on every path, so chain-specific interfaces survive.
Regression test: Nethermind.Taiko.Test exercises a code-bearing sender through
TaikoTransactionProcessor.PayFees — it passes with the flag and throws
InvalidCastException under the old spec-wrapping. eth_simulateV1 BAL-path and full
simulate suite remain green.
Follow-up to #12691; addresses the residual type-erasure raised in its review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(simulate): trim explanatory comments to essentials
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): carry EIP-3607 relaxation on BlockExecutionContext
Replace the ExecutionOptions.SkipSenderCodeCheck flag (and the machinery to thread
it into the EIP-7928 BAL path) with a SkipSenderCodeCheck field on
BlockExecutionContext. Both the main tx processor and the BAL manager's own
processors already share the context via SetBlockExecutionContext, so the field
reaches every path for free — no threading through BlockAccessListManager /
TxProcessorPool / ExecuteTransactionProcessorAdapter, and no injected policy type.
The context is the same shared channel #12691 originally relaxed on; this just uses
a flag instead of a spec decorator, keeping the spec's concrete runtime type (so
ITaikoReleaseSpec/IXdcReleaseSpec casts survive). Net simpler diff and consistent
with the existing IsGenesis flag on the same struct.
ValidateSender reads VirtualMachine.BlockExecutionContext.SkipSenderCodeCheck; the
simulate executor sets it in its context rebuild. Taiko regression test updated to
set the flag on the context (still throws InvalidCastException under spec-wrapping).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): carry EIP-3607 relaxation on TransactionProcessor
Per review preference (@LukaszRozmej), move the SkipSenderCodeCheck flag from
BlockExecutionContext onto TransactionProcessor. ValidateSender reads the
processor's SkipSenderCodeCheck property. The simulate scope sets it type-
preservingly on both creation paths: Intercept<ITransactionProcessor> for the
main processor, and a factory decorator for the EIP-7928 BAL processors — so each
chain keeps its concrete processor type (no spec wrapping, no processor-type swap).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: trim comments to the essential why
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): remove dead WithoutEip3607; address review polish
Follow-up to the SkipSenderCodeCheck switch (review by @claude on #12691):
- Remove the now-dead EIP-3607 spec wrapper — this PR dropped its last production
caller. Deletes WithoutEip3607, NoEip3607Spec, GetNoEip3607Spec (std + zkevm) and
the _noEip3607Specs cache; keeps WithoutEip158 (still live). Fixes two comments
that cited the removed decorator.
- Enforce the relaxation invariant: a shared Apply() throws if the resolved
ITransactionProcessor isn't a TransactionProcessorBase, instead of silently
no-op'ing back to -38024. Used by both the Intercept hook and the factory decorator.
- XML-doc SkipSenderCodeCheck with its set-before-use / unsynchronised-read invariant.
- Make SkipSenderCodeCheckTransactionProcessorFactory internal.
- Revert SimulateBlockValidationTransactionsExecutor to master (the fix no longer
touches it; its PrevRandao tweak was unrelated).
- Assert fee payment in the Taiko regression test (matches its name).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(dns): verify EIP-1459 subtree hashes (#12707)
* fix(dns): verify EIP-1459 subtree hashes
EIP-1459 serves every subtree entry from the subdomain
base32(keccak256(entry)[..16]). The root signature covers only the
enrtree-root entry, so this hash chain is the only binding between the
signed root and the branch and ENR records a client consumes. The
crawler never checked it, so any resolver or poisoned cache could
substitute arbitrary node records and steer the crawl to
attacker-chosen labels.
Verification follows go-ethereum: unpadded standard base32, prefix
comparison against keccak256 of the record, abbreviated labels accepted
down to 12 decoded bytes (minHashLength). The tree root is exempt, as
EIP-1459 serves it from the bare domain with no hash label. A mismatch
logs at Warn and skips the record.
No base32 decoder existed in src/Nethermind, so EnrTreeHash carries a
small one, tested against labels produced by the reference
implementation.
Validated against live trees: all.mainnet.ethdisco.net crawls 3000
records with zero rejections, so the check does not misfire on
anything the reference publisher emits.
* refactor(dns): simplify and harden EnrTreeHash
- Reuse Keccak.Size instead of a local MaxHashLength constant.
- Decode base32 arithmetically instead of scanning the alphabet string.
- Guard the decoded-length check against int overflow on hostile input.
- Hash UTF-8 bytes from a stackalloc/pooled buffer instead of allocating.
* refactor(dns): log hash-mismatch rejections at Debug, clarify base32 mapping
---------
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
* Selectable dotTrace profiling mode + dotnet-trace EventPipe sidecar for benchmark workflows (#12708)
* benchmarks: selectable dotTrace profiling mode (sampling | tracing | timeline)
The dottrace input on run-rpc-benchmarks and run-expb-reproducible-benchmarks
becomes a choice: false | sampling | tracing | timeline ('true' stays accepted
as a legacy alias for sampling on API dispatches). The mode maps to the
dotTrace CLI's --profiling-type: rpc-bench sets it on the wrapped entrypoint in
start-node.sh; the EXPB workflow passes --dottrace-mode to expb (only when
non-default, so pinned expb versions keep working for sampling runs; requires
execution-payloads-benchmarks feature/dottrace-profiling-modes for the new
modes).
Timeline snapshots cannot be converted to XML by Reporter.exe, so the
generate-dottrace-reports and summary jobs skip that mode - the raw .dtp
artifact is still collected and uploaded for the dotTrace UI. Line-by-line is
deliberately not offered: it needs PDBs the client docker images do not carry.
EXPB additionally rejects trace_blocks with timeline, since per-block
snapshots ride the MeasureProfiler API, which needs a performance session.
* expb workflow: collect a dotnet-trace EventPipe sidecar with every dotTrace run
Whenever dottrace is enabled the run also passes --dotnet-trace to expb: a
host-side EventPipe session records gc/contention/threading/exception events
(no CPU sampler - dotTrace owns the stacks) and the .nettrace ships in the same
dottrace-* artifact. Requires expb feature/dottrace-profiling-modes.
* rpcbench/expb: document the profiling modes and fix two review nits
Docs still described the pre-mode behaviour: the rpc-bench README documented a
dottrace command line with no --profiling-type and stated capture deliberately
runs in default sampling mode, and its input table listed dottrace as a boolean.
AGENTS.md's expb section had the same gap. Both now cover the four choices, when
to reach for each, that timeline yields no XML, and the dotnet-trace sidecar -
that section is what agents read to interpret run artifacts.
Timeline snapshots now save as .dtt rather than .dtp. The report job is gated off
for timeline either way, but the extension is what stops Reporter.exe's .dtp glob
from picking up a snapshot it cannot convert if that gate is ever relaxed.
The expb dottrace flag no longer special-cases sampling: the run already requires
an expb that understands --dotnet-trace, so there is no older-expb compatibility
left to preserve, and the flag string was spelled out twice in each of two
copy-pasted job bodies. The trace_blocks default guard becomes an explicit if -
as the last statement of its if body the AND-list left the block with status 1,
surviving only through errexit's AND-OR exemption.
* docs: scope the EventPipe sidecar to EXPB
The rpc-bench README claimed every profiled run drops a .nettrace into the
dottrace-rpcbench artifact, but --dotnet-trace was only wired into the EXPB
workflow - nothing in run-rpc-benchmarks.yml or scripts/rpc-bench collects one, so
the paragraph sent readers hunting for a file that is never produced, and promised
it precisely for timeline runs, which have no XML either. Say what a timeline
rpc-bench run actually yields: the .dtt snapshot alone.
* fix(jsonrpc): serialize receipt root as full-width DATA (#12706)
* fix(jsonrpc): serialize receipt root as full-width DATA
* test(jsonrpc): parameterize the receipt-root width cases
* test(jsonrpc): pin the whole-byte leading-zero root case
* test: add hand-derived golden tests for eth/71 and snap serializers (#12699)
* test(network): pin eth/71 and snap serializer wire encodings
Every serializer test file in Eth V71 and Snap/V1 now asserts an exact
wire encoding. The goldens are hand-derived from the RLP rules and
verified with an independent encoder (pyrlp + pycryptodome keccak).
Random request ids are pinned only in the goldened tests; ByteCodes
gains a sibling roundtrip that keeps the constructor-assigned id, and
GetAccountRange pins long.MaxValue to keep the eight-byte id path.
* test(network): share repeated snap golden fragments
The request-id, keccak-of-empty-string, and range-hash fragments moved
into SnapSerializerGoldens. Each golden fragment and its test input
come from one hex constant, so the expectation cannot drift from the
input.
* test(network): address review feedback on snap golden tests
- ByteCodes: one parameterized Roundtrip; long.MaxValue pins the
eight-byte request id deterministically instead of a random id
- GetAccountRange: independent field asserts in EnterMultipleScope
- SnapSerializerGoldens: expose RequestId1111 next to its RLP item;
the remarks state which fragments share hex with inputs and that
the keccak("") fragment is an independent literal on purpose
* `debug_trace*`: Fix phantom logs on frame revert (#12621)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Phantom log test
* Remove logs on a reverted frame
* Naming
* Fix leaking `ArrayPooList`
* Code cleanup
* Additional test
* Simplify tests
* Build fix
* Snap sync: reject storage range responses with unmatched slot lists (#12729)
* fix(snap): reject storage range responses with unmatched slot lists
A `StorageRanges` reply carries one slot list per requested account, but
nothing verified that the responder returned no more lists than were asked
for. `AddStorageRange` looped over the response count and indexed the request
account list with it, so a longer-than-requested reply read past the end of
the request.
Reject such a response up front and retry the range, which also settles the
active storage request accounting so that the range download phase can still
reach completion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(snap): pin the slot list count boundary
Cover equal and fewer slot lists than requested accounts so that widening the
guard to `>=` cannot pass, which would reject every well-formed response.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(metrics): durable sync-time and full-pruning duration metrics (#12590)
* feat(metrics): durable sync-time + per-stage breakdown and full-pruning duration
Sync:
- Fix UpdateAndGetSyncTime() so the total is retained after sync completes
instead of resetting to zero (EthSyncingInfo and Taiko override).
- Replace nethermind_sync_time with nethermind_sync_time_seconds (durable).
- Add nethermind_sync_time_in_mode_seconds{sync_mode} per-stage breakdown,
driven by a new SyncTimeInModeTracker on ISyncModeSelector.
Pruning:
- Add nethermind_full_pruning_last_duration_seconds and
nethermind_full_pruning_count, recorded on successful FullPruningDb.PruningFinished.
- Suffix in-memory trie pruning gauges with *Ms and document the unit.
* refactor(metrics): address PR review on sync/pruning time metrics
- Sync time no longer drops to 0 for one scrape when a stopped node
re-syncs: extract shared SyncTimeStopwatch that always returns the
retained total (used by EthSyncingInfo and Taiko override). Add
stop->resume regression tests.
- SyncTimeInModeTracker is now IDisposable (unsubscribes from
ISyncModeSelector.Changed), seeds its labels with TryAdd instead of
clobbering the shared static dictionary, and is owned by the container.
Timestamp seam moved to an internal ctor.
- SyncTimeInModeSeconds is get-only to match the other [KeyIsLabel] metrics.
- FullPruningCount uses the Interlocked backing-field pattern like the
other counters in Db/Metrics.
- Clarify that FullPruningLastDurationSeconds covers the trie copy+commit,
not the wait for a suitable state root.
- Make the full-pruning duration test assert the write actually happened.
* fix(metrics): avoid DI cycle wiring the sync-mode tracker; drop unused using
- Attach SyncTimeInModeTracker via Intercept<ISyncModeSelector> instead of
resolving it during IMonitoringService construction. Resolving it there
created a container cycle (IMonitoringService -> tracker -> ISyncModeSelector
-> ... -> DbTracker -> IMonitoringService) that crashed the node at startup.
This keeps the monitoring module free of outward dependencies, mirroring the
existing IEthSyncingInfo wiring.
- Remove unused `using System;` in SyncTimeInModeTrackerTests (IDE0005).
* Only accept the requested header in FetchHeaderFromPeer (#12730)
* fix(sync): only accept the requested header in FetchHeaderFromPeer
FetchHeaderFromPeer queries every initialized peer in parallel and takes
the first non-null response, but never checked that the returned header
is actually the one that was asked for. A peer that answers with some
other block had its header accepted, so the result depended on which
peer replied first rather than on what was requested.
Compare the returned header's hash against the requested hash on both
response paths. A non-matching response is treated like a missing one,
so the remaining in-flight peers can still supply the header instead of
the whole lookup failing.
The sibling lookups in StartingSyncPivotUpdater and PeerRefresher
already do this; FetchHeaderFromPeer was the one that did not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): cover the allocated-peer fallback and tighten assertions
Add a case where no peer answers the head-header request, so the
GetBlockHeaders fallback is the one that resolves the header. Nothing
pinned its success branch before, so it could have been broken without
a test noticing.
Assert on the header instance rather than its hash, so the negative
case cannot pass for a header that merely has no hash set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sync): report a peer that answers with a different block
A mismatched header was discarded silently, leaving it indistinguishable
from a peer that simply does not have the block. Requests are serialised
per peer by MessageQueue and a late response to a timed-out request is
disposed rather than handed to the next caller, so a mismatch is always
a protocol violation rather than a benign response race.
Report it as UnexpectedHeaderHash, matching HeadersSyncFeed and the
other header lookups.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): cover that an honest peer keeps its connection
Reporting a mismatch disconnects the peer, so the costly failure mode is
now an honest peer being dropped rather than a wrong header accepted.
Only the reported direction was asserted.
Cover all three answer shapes, including a peer that does not have the
block, which is the normal answer while a head is unknown and must not
cost a connection.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix doubled revert handling in some tracers (#12715)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Phantom log test
* Remove logs on a reverted frame
* Naming
* Fix leaking `ArrayPooList`
* Code cleanup
* Additional test
* Simplify tests
* Showcase test
* Direct fix
* More failing tracing tests
* Remove problematic `ReportActionRevert` > `ReportActionError` delegation from base
* Get rid of virtual-to-virtual calls in report revert/error
* Formatting
* Build fix
* Remove other `ReportActionRevert` -> `ReportActionError` calls
* Move common test codes to base class
* Fix `IsTracingActions` summary
* Small test fix
* Code cleanup
* test(era): anchor accumulator tests to EIP-7643 vectors, remove unused GetProof (#12718)
* test(era): anchor AccumulatorCalculator roots to derived spec vectors
Replaces the two-instances self-oracle, the inequality-only comparisons,
and the DoesNotThrow-only Add tests with parameterized roots derived by
an independent Python SSZ merkleization of the portal-network history
spec. Adds proof fold-up verification against the derived roots.
Deletes the byte-identical duplicate golden test in Era1.Test.
* test(era): assert the accumulator root the readers return
ReadAccumulator and ReadAccumulatorRoot tests discarded the root and
asserted only Throws.Nothing. They now assert the root equals the
accumulator of the written contents. The shared calculator loop moves
into a ComputeAccumulatorRoot helper in each file.
* test(era): apply review round on the accumulator vector tests
Parameterizes the proof verification and adds a three-entry index 2
case, so the upper tree levels exercise the right-hand sibling order.
Moves the proof length assert into the fold-up helper. Rewrites the
provenance comments in active voice and adds the spec link.
* test(era): cite EIP-7643 as the accumulator spec reference
The portal-network history spec was restructured and no longer defines
the accumulator. EIP-7643 defines HeaderRecord and the 8192-entry epoch
record. Also states the exact value of the index 2 proof case: it is
the only case that catches a wrong index shift.
* refactor(era): remove unused AccumulatorCalculator.GetProof
GetProof shipped with the EraE feature but no production code ever
called it; its only callers were its own tests. The removal also drops
the _totalDifficulties list, which only GetProof read, and the proof
tests in Nethermind.EraE.Test.
* docs(era): cite EIP-7643 on AccumulatorCalculator
The linked portal-network document was restructured and no longer
defines the accumulator. EIP-7643 defines HeaderRecord and the
8192-entry epoch record this class computes.
* test(era): apply removal-round polish
Converts the class comment to an XML summary per the documentation
rule. Inlines the two-entry root, which lost its second consumer with
the proof tests.
* test(era): consolidate the accumulator fixtures into Era1.Test
Both fixtures tested Nethermind.Era1.AccumulatorCalculator with
overlapping vectors. The Era1.Test table now carries every
discriminating single-entry case, a pinned empty-accumulator root, and
a Clear reset test against the writer-reuse contract. The EraE.Test
copy is deleted.
* test(era): state only true contracts in the fixture comments
The Clear comment claimed a multi-file writer-reuse path that does not
exist: Add throws after Finalize and each era file gets a fresh writer.
The vector comment claimed a pairwise single-input difference that two
case pairs violate. Both now state only what holds.
* test: strengthen mock-echo and vacuous tests in Abi, Optimism and Xdc test projects (#12720)
* test(abi): pin forwarding and return propagation in encoder extensions
The extension tests asserted only Received on the inner call and
ignored the extension return value. A stub on the exact unpacked
arguments plus an identity assert on the result covers both.
* test(xdc): assert the RocksDb config factory routing
The single test asserted Is.Not.Null on a result that cannot be null.
The factory contract is routing: Xdc databases get a PerTableDbConfig
without consulting the base factory, and every other database delegates
to it. Both branches are now pinned; the delegation branch was
untested.
* test(xdc): pin the routed timeout instance
The vote routing test pins the exact vote, but the timeout test used
Arg.Any, so a handler that routes the wrong timeout passed.
* test: apply the C11 review round
Removes an unused using that fails lint CI. Pins the factory options
against the provided IDbConfig, so a wrong database name in the special
branch turns the test red. Moves the extension rationale to class level
and renames the Abi tests to the project snake_case convention.
* test: state only true mechanisms in the C11 comments
NSubstitute returns an empty array, not null, for an unmatched call on
an array-returning member. The prefixed-options claim holds for only
half of the database prefixes, so the sentence is dropped.
* test(xdc): use a neutral database name in the delegation test
Review feedback on #12720: the delegation test used the literal
"State"/"Code", and "State" is the one name PerTableDbConfig
special-cases (StartsWith("State")). Use nameof(DbNames.Blocks) with a
null column so the test isolates the delegate-to-base contract without
that special-case, matching the sibling FlatRocksDbConfigAdjusterTests
idiom.
* test: anchor crypto and RLP tests to independent expectations (#12712)
* test(core): anchor the keccak span test to an independent vector
* test(core): anchor RLP ulong lengths to the spec
* test(core): compare decoded blocks to the original and drop the ignored file writer
* test(core): compare decoded header fields to the original block
* test(core): anchor the regression block decode to pyrlp-derived fields
* test(core): apply review round - drop dead using, widen anchors, guard indexed asserts
* test(core): apply round-2 review polish
* test(core): cover the header tail fields and sharpen the roundtrip comments
* test(core): apply confirm-round nits
* test(core): compare decoded uncle hashes in the block roundtrip
The body compared uncles by count only. The scenarios build two uncles
with distinct headers, so the hash comparison catches an order or
content error the count cannot see. The count guard moves outside the
multiple-assert scope like the transaction guard.
* Update OP Superchain chains (#12752)
Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com>
* Auto-update fast sync settings (#12751)
Co-authored-by: rubo <rubo@users.noreply.github.com>
* test: Db tests assert stored state instead of smoke-calling empty methods (#12693)
* test(db): assert stored state instead of smoke-calling empty methods
MemDb.Flush and Dispose (and the Snapshotable variants) are empty method
bodies, so the six *_does_not_cause_trouble tests could never fail;
removed. The no-assert MemDb creation tests now verify the value round-
trips. ReadOnlyDbProviderTests.Can_clear exercised an empty registry (no
db was ever registered), making ClearTempChanges a no-op; it now
registers a real MemDb-backed read-only db and asserts the overlay is
dropped while the wrapped db stays intact, plus the no-writes contract
when localChanges is off. Can_get_all_on_empty now asserts emptiness
instead of discarding the enumeration.
* test(db): assert overlay drop only where a write could land in the overlay
* test(db): group independent post-condition asserts in Assert.EnterMultipleScope
* Reject invalid fixed-size header RLP (#12579)
* Treat a null header answer as the block being absent (#12741)
* fix(sync): treat a null header answer as the block being absent
An empty list item decodes to a null header and is kept in the response
list, so the allocated-peer fallback could pass one into the hash
comparison and throw. FetchHeaderFromPeer only catches cancellation and
timeouts, so it would surface into engine_forkchoiceUpdated.
Handle it in Validate, which lets the head-header path drop its own null
check too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): name the mock switch after the answer it produces
The peer answers with a null header, not an empty one. Move it next to
the other answer switches so HeaderToReturn keeps its own doc.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Return only the requested header from GetHeadBlockHeader (#12740)
* fix(network): return only the requested header from GetHeadBlockHeader
GetHeadBlockHeader asks for one specific block but returned whatever
single header the peer sent back. Every caller wants the block it asked
for, and two of them re-checked the hash themselves afterwards; the peer
refresh in SyncPeerPool did not, so it would take a substituted header
and record the wrong head number and total difficulty for that peer.
Compare the response against the requested hash where the request is
made, and disconnect a peer that answers with a different block. A peer
that does not have the block answers with an empty list, which is the
normal response while a head is unknown and still yields null.
Drop the now-redundant check in StartingSyncPivotUpdater. The one in
PeerRefresher stays: that method validates its head/parent response
locally anyway, since GetBlockHeaders carries no such guarantee.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(network): treat a null header answer as the block being absent
An empty list item decodes to a null header and is kept in the response
list, so a peer can answer a single-header request with one null entry.
Dereferencing it to compare hashes threw, and two callers do not catch
that: the pivot updater would leave its update loop for good, and
FetchHeaderFromPeer would surface it into engine_forkchoiceUpdated.
Treat it the same as an empty list — the peer does not have the block —
rather than as a breach.
Also type the requested hash as nullable, matching _remoteHeadBlockHash
before the status handshake, and assert the absent cases on the result
itself rather than on its hash.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(network): make the requested-header guarantee unconditional
The hash check was skipped when neither the argument nor the announced
head was known, so the documented contract had a hole. Return early
instead: with no hash to ask for there is no meaningful request to send.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat: add built-in portfolio viewer UI at /portfolio (#12360)
* Handle failed sender recovery (#12757)
* fix(simulate): route the EIP-7928 BAL path through the simulate tx adapter (#12692) (#12721)
* fix(simulate): route EIP-7928 BAL path through the simulate tx adapter
Under EIP-7928, eth_simulateV1 runs transactions through the
BlockAccessListManager's own tx processors, bypassing
SimulateTransactionProcessorAdapter. That lost its GasCap budget clamp, its
TotalGasLeft/BlockGasLeft accounting (block gasUsed reported as 0), and its
validation:false handling (the BAL path always called Execute, never Trace).
Inject the adapter via a new ITransactionProcessorAdapterFactory so the
sequential BAL manager — the only one simulate drives — wraps each tx processor
in the simulate adapter. The parallel manager always uses the default
ExecuteTransactionProcessorAdapter: the stateful simulate adapter is
sequential-only and simulate never triggers the parallel path (it attaches no
BlockAccessList).
The factory is an interface, not a delegate, so Autofac does not
auto-synthesise one on the real block-production scope, where the optional
parameter must stay null and fall back to the default adapter.
The no-gas EIP-8037 execution-dimension default is a separate ordering issue
(the per-tx inclusion check runs before the adapter) and is left as a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): make the tx-processor-adapter factory a required delegate; add GasCap test
Replace the ITransactionProcessorAdapterFactory interface with a required
TransactionProcessorAdapterFactory delegate, mirroring CodeInfoRepositoryFactory:
the block-processing module registers the default (ExecuteTransactionProcessorAdapter)
and the simulate scope overrides it with the simulate adapter. This drops the
optional/null-fallback and the auto-synthesis hazard an optional delegate would
carry (ITransactionProcessorAdapter is registered on the block-processing scope,
so Autofac would otherwise fill an optional delegate on the real path).
Also add a regression test for the JsonRpc.GasCap budget (#12692 item 2): a
two-call request whose cumulative gas exceeds the cap has its second call clamped
below intrinsic gas and rejected; without the adapter the cap is not enforced and
both calls run unclamped.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: address #12721 feedback (enforce sequential BAL, DI style, docs, test dedup)
- Set ProcessingOptions.ForceSequentialBlockAccessList in the simulate options so the
stateful SimulateTransactionProcessorAdapter can never reach the parallel BAL pool
(previously guaranteed only by simulate not attaching a BlockAccessList); documented
the single-threaded contract on the adapter itself.
- Register the default TransactionProcessorAdapterFactory with AddScoped + a method group
(drops the lambda cast) and the simulate override with the typed-dependency AddScoped
overload (no manual Resolve / captive singleton).
- Reworded the factory <remarks> to describe what is actually wired (default Execute;
other scopes still get the default on the BAL path) and dropped the overstated
"gas defaulting" from the registration comment.
- Extracted BuildAmsterdamBalChain test helper; pinned the EIP-2780 intrinsic in the
GasCap test comment.
Follow-up filed as #12723 (scopes overriding ITransactionProcessorAdapter still get
Execute on the BAL path).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: single-axis tx-processor-adapter registration (step 1)
Addresses LukaszRozmej's review: the TransactionProcessorAdapterFactory was a
second registration axis alongside ITransactionProcessorAdapter, so scopes that
only overrode the latter (block production, trace, proof) drifted to the default
Execute adapter on the EIP-7928 BAL path — the same bug class this PR fixes for
simulate (#12723), left live elsewhere. Notably block production silently
downgraded its intended BuildUp semantics to Execute under Amsterdam.
Make the factory the single source of truth: the root registers the default
(Execute) plus a derivation ITransactionProcessorAdapter = factory(processor), and
each scope overrides only the factory — production BuildUp, trace (…
Marchhill
added a commit
that referenced
this pull request
Aug 19, 2026
* fix(rpc-bench): run benchmarked nodes the way production runs them (#12625)
* diag(rpc-bench): run Nethermind with production-default runtime (no TC=0 pin)
* reword comment for master
* review: document expb divergence, add NODE_ENV_VARS escape hatch
- README: the 'Alignment with expb' section no longer claims the removed
env pins; documents the deliberate code-gen divergence and that JIT
warm-up now lands inside the measured window; dotTrace reports are not
comparable across this change
- start-node.sh: reword comment (no warm-up phase exists yet), add
NODE_ENV_VARS passthrough for deliberate one-off code-gen experiments
* trim comments to one-liners; rationale stays in the PR
* drop the Merge GC flags: inert here and misleading
GCKeeper only runs on Engine API calls; this harness parks the node at a
snapshot head and never sends newPayload, so the three flags changed
nothing while implying the node ran a non-production GC configuration.
* keep the image entrypoint for Nethermind
The override skipped entrypoint.sh, which applies host tuning and enables a
shipped PGO profile. Its comment claimed parity with expb, but expb only
overrides the entrypoint for dotTrace, so normal expb runs (and production)
do run entrypoint.sh - this harness was the outlier.
* Rename EIP-8037 regular gas dimension to execution gas (#12600)
* Auto-update fast sync settings (#12665)
Co-authored-by: rubo <rubo@users.noreply.github.com>
* refactor(net): namespace snap by version (#12606)
* refactor(net): namespace snap messages by version
Prepare the snap subprotocol for a second version by moving the concrete
snap/1 messages, message codes and protocol handler into a versioned
namespace, so a snap/2 handler can be added alongside without touching
snap/1 code.
Snap/Messages/* -> Snap/V1/Messages/*
Snap/SnapMessageCode -> Snap/V1/Snap1MessageCode
Snap/SnapProtocolHandler -> Snap/V1/Snap1ProtocolHandler
P2P/P2PMessageKey.cs -> P2P/VersionedProtocol.cs (file renamed to
match the type it declares)
SnapMessageBase and SnapSerializerBase deliberately stay in
Snap.Messages: they are shared by all protocol versions, not specific to
snap/1.
Introduce SnapVersions constants and ISnapSyncPeer.SnapProtocolVersion so
version checks can replace the magic numbers currently spelled as 1.
PeerInfoExtensions.CanGetSnapData is renamed to CanGetTrieNodes to say
what it actually tests. The rename is nominal - the peer probe is
unchanged, so behaviour is identical.
No functional change.
* refactor(net): remove Snap2 version constant from SnapVersions
* address review comments
* rename
* feat(sync): serve block access lists from the snap server (#12607)
* Refactor SnapServer and SnapStateServer integration
- Renamed SnapServer to SnapStateServer for clarity and consistency.
- Updated WorldStateManager to use SnapStateServer instead of SnapServer.
- Modified StateSyncFeedTestsBase to accommodate changes in SnapServer instantiation.
- Adjusted SnapProviderTests to reflect the new SnapStateServer type.
- Introduced SnapServerTests to validate SnapServer functionality.
- Added SnapStateServerTests to ensure robust testing of state management.
- Implemented new methods in SnapServer for handling bytecode and block access lists.
- Enhanced test coverage for account range retrieval and storage management.
* refactor: change SnapServer field type to interface ISnapServer
* test: enhance SnapServerTests with additional block access list scenarios
* chore: Update Dockerfiles (#12663)
Update Dockerfiles
Co-authored-by: rubo <rubo@users.noreply.github.com>
* test: make prewarmer env-return assertion pool-hit independent (#12616)
PreWarmCaches_ReturnsAddressWarmEnvWhenScopeBuildThrows asserts
Returned == Created, but with maxPoolSize 1 an env returned to the pool
can be rented again without a Create, so a pool hit legitimately
increments Returned twice against a single Create. Whether a hit occurs
depends on worker interleaving: flaky on CI, fails deterministically on
high-core machines (0/30 locally).
ThrowingBuildPolicy.Return now refuses retention, so every rental is a
fresh Create and the invariant holds under any interleaving (30/30
locally). Stack-trace instrumentation confirmed every rental is returned
exactly once - the prewarmer itself is correct.
* Update OP Superchain chains (#12664)
Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com>
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
* fix(receipts): restore the post-merge flag before regeneration (#12641)
* fix(receipts): restore the post-merge flag before regeneration
Stored headers do not carry IsPostMerge and regeneration bypasses the
recovery step that restores it, so post-merge blocks re-executed with
PREVRANDAO evaluating to the zeroed difficulty - any transaction reading
it produced receipts failing the root check (~7% of post-merge blocks
refused on mainnet archives deriving receipts from state).
* test(receipts): dispose buffer, pin logged value
* fix(receipts): classify post-merge via the switcher
A difficulty heuristic misreads chains that repurpose the field -
Taiko stores per-block ZK gas in Difficulty while AlwaysPoS - so ask
IPoSSwitcher instead, mirroring MergeProcessingRecoveryStep. On
mainnet the switcher's TD-null branch is the same difficulty check,
so behavior there is unchanged.
* test(receipts): pin the real switcher's TD-null derivation
The production failure arrived as a mainnet-shaped header with
TotalDifficulty unset; cover PoSSwitcher's TD-null branch end to end,
not only the honour-the-switcher contract.
* test(receipts): cover the switcher registration path
A hand-injected switcher cannot catch a composition regression that
leaves the container-resolved regenerator on the NoPoS default, so
resolve it from a graph whose IPoSSwitcher registration is overridden
the way a merge-enabled node overrides it.
* fix(tests): mark RecoverReceiptsBlockchain.Create as hiding
* Expose the node's ENR in admin_nodeInfo (#12631)
feat(rpc): expose the node's ENR in admin_nodeInfo
Nethermind is the only discv5-capable execution client whose
admin_nodeInfo omits the node record. Tooling that bootstraps
discv5-only networks reads the ENR from this endpoint and has to
special-case Nethermind to the enode instead, which is useless once
discv4 is disabled.
NodeRecordProvider already maintains a signed, sequence-numbered self
record, so surface it as an 'enr' field. Move INodeRecordProvider to
Nethermind.Network so Nethermind.JsonRpc can reference it without a new
dependency on Nethermind.Network.Discovery; the implementation stays in
Discovery. The provider is only registered when discovery is enabled, so
it is resolved optionally and the field is omitted otherwise.
* Validate ABI decode allocation bounds (#12588)
* ci: disable stateless glamsterdam-devnet-7 scheduled run (#12680)
* Fix EIP-7708 tracing with logs (#12577)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Naming
* fix(flat): guard the trie-warmer against a TransientResource recycle race (storage reads as 0x00) (#12429)
* fix(flatdb): warm the trie from persistence only
The trie warmer read the recyclable `_snapshots` and `_transientResource`
while the warm job held only a `ReadOnlySnapshotBundle` lease, which does not
cover them. A concurrent scope reset could recycle those under the running
warmer, so a warm read could return a torn or foreign node.
The warmer only needs to warm from persistence, so restrict its reads to the
trie node cache and the `ReadOnlySnapshotBundle` - exactly what the lease
covers. In-memory nodes are already hot and do not need warming.
Claude-Session: https://claude.ai/code/session_01S3BG321zDG9BNjAgWhZhCX
* fix(flat): warm the transient resource via a per-job lease
The trie warmer now warms nodes into the per-job-pinned TransientResource
(not just the trie node cache and ReadOnlySnapshotBundle), covered by a single
transient lease held for the whole warm traversal. It still never reads the
recyclable _snapshots. A warmer read on a torn-down bundle bails to a
persistence-only read instead of spinning for a lease that will never land.
Claude-Session: https://claude.ai/code/session_01S3BG321zDG9BNjAgWhZhCX
* refactor(flat): drop the warmer transient ThreadStatic capture
Measured TS-on vs always-per-node-lease on x10 (3 runs, heavy-block warmer
load): newPayload 178 vs 178 ms, getProof p99 20.1 vs 20.2 ms, RSS identical.
The ambient capture bought nothing, so keep only the per-node lease + ABA
re-check (the actual recycle-race fix) and drop the ThreadStatic + the
EnterWarmerTransientScope pins in the two warm-job callers.
* fix(flat): register the transient return owner at pool checkout
- ResourcePool.GetCachedResource now calls OnRented, so every checkout
carries a registered return owner; a final ReleaseLease without one
throws instead of silently dropping the resource (which leaked the
BloomFilter allocation on the public AddSnapshot path)
- re-check _isDisposed after the warmer's lease acquire: Dispose releases
the owner lease but leaves _transientResource pointing at the recycled
instance, so the identity re-check alone could latch a resource already
re-rented by another bundle
- document why ReleaseLease is distinct from Dispose: the pool contract
reserves Dispose for destroying an over-capacity resource
- run Nethermind.State.Flat.Test in the nethermind-tests.yml matrix; it
was compiled but not run by any CI job
- FlatDbManagerTests duplicate-snapshot test asserts the resource lands
back in the checkout pool; new ResourcePoolTests cover the final-release
return and the unregistered-release throw; refresh stale warmer test
comments
* fix(flat): pin the transient resource for prewarm dedupe reads
ShouldQueuePrewarm read _transientResource without pinning it. The dedupe
bloom lives on that recyclable resource and the call runs on prewarmer and
BAL threads, so the owner could retire the resource mid-read: the pool
Resets it and, on overflow, Disposes the BloomFilter, whose backing store is
native memory. Route both overloads through the same lease + ABA re-check
the warmer node reads already use, and decline the prewarm on a torn-down
bundle. Rename the helper accordingly, since it is no longer warmer-only.
The FlatWorldStateScopeProvider and FlatOverridableWorldScope test doubles
returned the committed resource to the pool directly instead of releasing
its lease, mirroring neither AddSnapshot implementation. That recycles the
resource while a warmer lease is outstanding and returns it a second time
when that lease is released; the scope provider double also returned the
wrong instance and then returned it again on teardown. Both now release the
lease, which is the single return-to-pool path.
Test changes:
- the persistence-only test now commits the written nodes into the bundle's
recyclable _snapshots before reading, so the warmer's Unknown result is a
genuine miss. Previously the node was still in the transient (SetStateNode
writes both) and was itself Unknown, so the assertion held either way.
- the churn test gives every epoch its own persisted node instance, so a
read served from another epoch's recycled transient is caught by identity
rather than by value, drives both recycle paths (CollectAndApplySnapshot
swap and Dispose), exercises ShouldQueuePrewarm alongside the node reads,
and joins the readers with a bounded wait instead of blocking forever.
- new test: a warmer read and a prewarm check on a disposed bundle fall back
to the leased persistence reader within a bounded wait, covering the
Dispose bail-out deterministically.
* fix(jsonrpc): synchronise SubscriptionManager per-client subscription bag (#12672)
* fix(jsonrpc): synchronise SubscriptionManager per-client bag
The per-client subscription bag is a HashSet mutated and enumerated from
multiple threads: concurrent subscribe requests (socket worker tasks),
unsubscribe, and the Closed handler that fires on connection teardown.
Concurrent HashSet access could corrupt it, dropping a subscription so
its event handlers stayed attached and leaked. Lock on the bag for every
add, remove, and snapshot-before-dispose.
Fixes #12668
* refactor(jsonrpc): dispose client subscriptions under bag lock without snapshot
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(jsonrpc): race unsubscribe path too; drop bag field comment
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* test: eth_createAccessList affordability with omitted fee fields (#12629)
* test(rpc): eth_createAccessList affordability with omitted fee fields (execution-apis #854)
execution-apis PR #854 clarifies that eth_createAccessList must not fail
solely because an unfunded sender cannot afford client-selected default fees
when all gas-fee fields are omitted. Nethermind already conforms (verified in
hive rpc-compat); this adds a regression test mirroring the conformance
fixture: unfunded sender, codeless recipient, zero value, no gas/fee fields
-> {"accessList":[],"gasUsed":"0x5208"}.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(rpc): run affordability test on London chain, assert no in-body error, reuse helper
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
* Fix stale transaction pool snapshots (#12685)
* Encode engine_newPayloadWithWitness witness as an RLP data string (#12635)
* fix
Signed-off-by: jsign <jsign.uy@gmail.com>
* Tighten witness RLP JSON encoding
---------
Signed-off-by: jsign <jsign.uy@gmail.com>
Co-authored-by: jsign <jsign.uy@gmail.com>
* perf(state): skip trie warmup for read-only BAL accounts in flat layout (#12681)
* perf(state): skip trie warmup for read-only BAL accounts in flat layout
With a suggested BAL the block's write set is known upfront, and trie
nodes are only needed at commit for written accounts. Gate address
trie-warm hints (HintBal, HintGet, HintWarmAccount) on the BAL write
set so read-only accounts no longer trigger state-trie path walks.
On BAL blocks dominated by cold account reads this removes up to ~8-10
wasted trie-node DB reads per unique read-only account from the
measured processing window.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Apply suggestions from code review
Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>
* refactor(state): extract QueueStateTrieWarmup and address review findings
- Extract the NeedsStateTrieWarmup + PushAddressJob + increment pattern
into QueueStateTrieWarmup, used by all three address warmup call sites
- Hoist CancelHintBal above the empty-BAL early return so a stale write
set never survives into the next block
- Drop the inaccurate bloom false-positive-rate comment
- Tests: parameterize warm-per-write-kind over balance/nonce/code/storage,
add empty-BAL reset regression test, split the HintWarmAccount test,
wrap scopes in using, use order-insensitive assertions
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Update src/Nethermind/Nethermind.State.Flat/ScopeProvider/FlatWorldStateScope.cs
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
* refactor(test): reuse TestContext for recording-warmer scope construction
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(state): bind BAL warmup gate to HintBal lifecycle; address review findings
- Keep _warmupWriteSet across StartWriteBatch: on the parallel BAL path the
BAL apply commits mid-block, concurrently with tx workers, so clearing the
gate in CancelHintBal collapsed it at a nondeterministic point. The gate is
now replaced only by the next HintBal.
- Drop the token from Task.Run in both HintBal implementations: a task
cancelled before being dequeued never ran the finally that returns the
pooled accountChanges array. The body already observes the token.
- Align TrieStoreScopeProvider.HintBal with the flat scope: a new hint
supersedes the previous one even when it carries no work itself.
- Remove the stale prestate-load mutation mention from ReadOnlyBlockAccessList
docs, state the immutability invariant, and seal the type.
- Tests: pin the gate surviving StartWriteBatch and a second BAL replacing
the previous write set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>
* test: make can-never-fail tests assert what their names claim (#12690)
* test(xdc): drop redundant trailing Assert.Pass in SpecialTransactionsTests
* test(core): bound McsLock re-acquire test instead of passing unconditionally
SingleThreadAcquireRelease asserted nothing and is subsumed by
ReacquireAfterReleaseSucceeds, which now runs on a worker with a timeout
so a broken release fails in seconds instead of hanging the test host.
* test(flat): assert real postconditions instead of Assert.Pass
Add_ConcurrentWithMightContain_ShouldWork now verifies no write is lost
under concurrent readers (a bloom filter never false-negatives).
DisposeAsync test renamed: FlatDbManager does not dispose the repository
(the container owns it); it now asserts bounded drain completion and
idempotent double-dispose. ConstructorAcceptsPersistedRepository removed -
Not.Null on a fresh object, subsumed by every other test in the file.
* test(merge): assert pending-validation cleanup instead of catch-only assertions
The memory-leak test asserted only inside catch blocks and ended with
Assert.Pass, so it passed whenever nothing threw; it now drives the
handler directly and asserts the pending-validation count stays zero
across repeated timed-out payloads. The TrySet double-completion test
is removed: it was a weaker duplicate of the concurrent-calls test
above it (same scenario, but swallowing OperationCanceledException and
keying Assert.Fail on exception message text).
* test(merge): await header-sync test helpers
The helpers were async void and invoked without awaiting - including one
un-awaited call inside the other helper - so their assertions raced the
test body and failures could surface as host crashes or not at all.
* test(network): restore DisconnectsAnalyzer assertions with deterministic flush capture
All four tests had their assertions commented out as CI-flaky, leaving
them assertion-free. The flakiness came from racing the 10ms flush
timer: reports are now recorded while the default 10s interval is in
effect and only then is the interval shortened, so a flush cannot fire
mid-arrangement. Assertions scan captured flush reports, including that
counters aggregate, reset after each flush, and cleared categories do
not resurface (the analyzer double-buffers, so a lost clear shows up as
a stale count in every other flush, not as a doubled count).
* test: address review findings on strengthened tests
Will_clear_after_report no longer issues a second report at all - a
report can race the flush's enumerate-then-clear window (Timer.Stop does
not drain a queued Elapsed callback, so no test-side quiesce is airtight).
A lost clear is observable without it: the analyzer double-buffers, so a
stale count resurfaces in later flushes, and the test asserts the
category appears in exactly one flush across several more (mutation-
verified). Bloom capacity raised to 100k - at 10k the saturated filter's
~34% false-positive rate masked a single lost write - and misses are
collected into one assertion. The repeated-timeouts payload test is
dropped rather than parameterized: the pending dictionary is keyed by
block hash and the test resubmits one block, so the count can never
exceed one and iterations add no coverage. The FlatDbManager dispose
test states its intent with Assert.DoesNotThrowAsync and a corrected
comment (WaitAsync bounds the wait, not the drain). Wait timeouts are
named constants and polling uses Thread.Sleep(1).
* test: simplify comments per ASD-STE100 and drop dead times parameter
Comments now use short, active, single-topic sentences. The times
parameter of ShouldEventuallyReport had no remaining non-default call
site after the second-report removal.
* test: use SpinWait.SpinUntil instead of a custom poll helper
Keep one condition re-check after a timeout: a flush can land in
SpinUntil's final sleep tick.
* Stop parallel transaction execution once BAL validation rejects the block (#12697)
* fix(consensus): stop parallel tx execution once BAL validation rejects
The parallel BAL block validator ran the incremental validator alongside the
transaction workers, but a validator failure faulted neither the worker loop nor
its cancellation token. `ParallelUnbalancedWork` only stops fetching new indices
on caller cancellation or a transaction-worker fault, so a block rejected at an
early transaction index still executed every remaining transaction before the
foreground observed the failure at `GetResult()`. For a block whose invalidity is
decided by a cheap prefix, that turns a sub-second rejection into slot-scale CPU
work.
`IncrementalValidationWorkItem` now owns a cancellation source that is signalled
together with the stored exception, and the worker loop runs under that token, so
workers stop pulling transactions as soon as validation becomes terminal. The
resulting `OperationCanceledException` is translated back into the original
validation failure, keeping the returned error identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(consensus): signal BAL validation failure with a flag, not cancellation
Routing the stop signal through ParallelUnbalancedWork's cancellation token meant
`For` ended by throwing an OperationCanceledException that existed only to be
caught and translated back into the original validation failure. The token also
needed a per-block CancellationTokenSource, whose recycling in Schedule was only
safe because the previous block's validator had already been joined.
Replace all of it with a volatile read of the exception the work item already
stores: workers check `HasFailed` before doing any work, the loop drains its
remaining indices without executing anything, and `GetResult()` reports the
rejection on the normal return path. Same behaviour, no exception used as control
flow, no extra state to keep in sync, and no ordering constraint between Schedule
and the work that follows it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(test): trim comments and simplify the tail-cancellation test
Comments cut back to the non-obvious reasoning only. The regression test now
counts executions instead of collecting indices, so a single Is.InRange assertion
covers both requirements — the decisive prefix ran, the tail did not — and the
prefix array, the index bag and one assertion all go away. Transaction count is a
plain constant rather than derived from the canonical lead, which the test never
depended on: with uniform gas limits the tail sort is stable, so the schedule is
natural order regardless. CreateParallelValidationTransactions takes an optional
gas limit so the block no longer needs patching after construction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(consensus): address review — exempt iteration 0, loosen test bound
Iteration 0 (WaitForBalWarmup + ApplyStateChanges) is now exempt from the
HasFailed guard, so pre-execution keeps its previous semantics instead of being
droppable when validation fails before any worker starts. Skipping it was traced
as benign, but the fix only needs to stop transaction execution, so leaving the
pre-execution step alone keeps the behavioural change narrower at no cost.
The regression test's upper bound leaned on SpinWait outlasting exception
unwinding, because the fake validator releases its gate before the work item
stores the exception. Bound is now txCount / 8, which asserts the tail stopped
without depending on cancellation-propagation timing — a revert still executes
all 2048 and fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* test: pin eth/62-66 serializer wire encodings with hand-derived goldens (#12696)
* test(network): remove duplicate eth serializer tests
ZeroNewBlockMessageSerializerTests.Roundtrip2 and V63
NodeDataMessageSerializerTests.Zero_roundtrip were verbatim copies of
their Roundtrip siblings. Can_deserialize_own_eth_64 shared its body
and one payload with Can_deserialize_eth_64; its unique payload moves
there as another TestCase.
* test(network): pin eth/62-66 wire encodings with hand-derived goldens
Every serializer test file in Eth V62-V66 now asserts an exact wire
encoding somewhere. The goldens are derived from the RLP rules and
verified with an independent encoder (pyrlp + pycryptodome keccak);
values shared across files live in EthSerializerGoldens. ToString
smoke tests now assert the log line names the message type, and the
32-byte-selector test asserts the exact decoded hash.
* test(network): address review feedback on serializer goldens
- BlockBodies: pin the null-body framing (c1c0) and the empty-vs-absent
withdrawals distinction (e9e8...c0c0)
- Status To_string: assert against the independent Protocol.Eth constant
- V63 NodeData: rename Roundtrip_with_nulls to Roundtrip_with_empty_entry
(the data holds an empty array, not null)
* feat: EIP-8037 two-dimensional gas tracing (stateGasTracer + callTracer) (#12628)
* feat(tracing): add EIP-8037 stateGasTracer (execution-apis #852)
Implements the `stateGasTracer` named tracer specified in execution-apis
PR #852, returning the per-transaction two-dimensional gas summary
`{gasUsed, regularGasUsed, stateGasUsed, gasRefund}` (EIP-8037/EIP-7778).
The values are already computed for block-level gas accounting and carried
on `GasConsumed`; the tracer only reads and formats them. Adds the missing
applied EIP-3529 refund (capped) to `GasConsumed.GasRefund`, populated in
the success and top-level-halt refund paths. The native tracer factory is
threaded with `IReleaseSpec` so the fork is determined explicitly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(tracing): add EIP-8037 two-dimensional gas to callTracer (execution-apis #852)
Adds `regularGasUsed`, `stateGasUsed` and `gasRefund` to the callTracer
top-level frame for Amsterdam+ blocks, per execution-apis PR #852. The
fields are gated on `IReleaseSpec.IsEip8037Enabled` (MUST NOT appear before
the fork) and set only on the top frame (omitted on sub-frames). Values are
read from the transaction's `GasConsumed` result, matching the stateGasTracer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): address #12628 review feedback
- Restore native-tracer factory API back-compat: keep the public 4-arg
GethLikeNativeTracerFactoryDelegate/RegisterTracer/CreateTracer so external
plugin registrations stay source- and binary-compatible; built-ins receive
the (nullable) IReleaseSpec via an internal spec-aware factory.
- stateGasTracer: disable IsTracingOpLevelStorage/IsTracingStack so the
terminal-only tracer stops invoking per-opcode storage/stack callbacks.
- Consolidate the callTracer top-frame regularGasUsed/stateGasUsed/gasRefund
into a single TwoDimensionalGas? value, removing the coupled nullables and
the unsafe null-forgiving dereferences in the converter.
- Document that regularGasUsed is floor-clamped (block-accounting value) so
the two-dimensional invariant is exempted under the calldata floor.
- Add an end-to-end stateGasTracer test executing a real Amsterdam tx through
the TransactionProcessor (fresh SSTORE + in-tx reset) that exercises field
selection and the GasConsumed.GasRefund plumbing; assert
regularGasUsed + stateGasUsed == gasUsed + gasRefund.
- Test cleanups: multiple-assert scope and a shared helper for the two
callTracer Amsterdam cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): address #12628 re-review nits
- Document the spec-less CreateTracer overload's pre-fork fallback and cover it
with a factory test (back-compat public contract was untested).
- stateGasTracer hex test now forces the ambient NumberConversion.Raw so it
actually exercises StateGasTraceConverter's hex-quantity override.
- E2E test: use a `using` tracer and correct the refund assertion message
(the slot is reset to its original zero value within the tx, not pre-nonzero).
- Convert the remaining `//` member comments to XML doc.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: remove unused using in NativeStateGasTracerE2ETests
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(tracing): drop excess comments; non-nullable spec in native tracer factory
Address flcl42 review on #12628:
- Remove the newly added explanatory comments that restated the code (kept only a
few essential EIP-referenced DTO docs and the hex-conversion note).
- Make the release spec non-nullable through GethLikeNativeTracerFactory and the
native tracers: drop the unused spec-less 4-arg CreateTracer overload (the only
null source) — RegisterTracer, the actual plugin API, is unchanged. Also drops
the brittle regularGasUsed occurrence-count assertion flcl42 flagged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: collapse double blank line before DeepNesting test
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): skip EIP-3607 on the EIP-7928 BAL path in eth_simulateV1 (#12691)
* fix(simulate): skip EIP-3607 on the EIP-7928 BAL path in eth_simulateV1
eth_simulateV1 relaxes EIP-3607 so a state-overridden contract can be the tx
`from`. That relaxation covered only the main execution (via
SimulateTransactionProcessorAdapter). Under glamsterdam (EIP-7928),
BlockProcessor runs transactions through the BlockAccessListManager's own tx
processors, which bypass the adapter and re-enforce EIP-3607 — so a contract
sender is rejected with `-38024 sender has deployed code` instead of reaching
the normal balance/fee checks (hive rpc-compat divergence on glamsterdam-devnet-8:
ethSimulate-simple-send-from-contract*, ethSimulate-override-address-twice).
Relax EIP-3607 on the block execution context in
SimulateBlockValidationTransactionsExecutor instead:
ParallelBlockValidationTransactionsExecutor sets that context on both the main tx
processor and the BAL manager, so both paths skip the check — while BlockProcessor
still receives the unwrapped spec, preserving chain-specific release-spec
interfaces (Taiko / XDC / Optimism).
Verified end-to-end on a glamsterdam-at-genesis chain (the three hive fixtures
flip -38024 -> -38014/-38012, matching besu/erigon/reth/geth) and by an
integration test through the real EIP-7928 BAL path that fails -38024 without the
fix and passes with it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): preserve PrevRandao in context rebuild; harden 3607 BAL test
Address re-review on #12691:
- SimulateBlockValidationTransactionsExecutor is now the single context-rebuild
funnel for the simulate scope, so preserve the incoming PrevRandao (via
BlockExecutionContext.WithPrevRandao*) instead of re-deriving the default —
a BlockProcessor subclass (e.g. XdcBlockProcessor) may have supplied a
non-default value.
- Make eth_simulateV1_contract_sender_skips_eip3607_on_bal_path validation-enabled
so the -38014 expectation is fork-independent and stable across the #12692 fix
(with validation:false the -38014 relied on the BAL path ignoring NoValidation).
Pin the EIP-7928 premise with an explicit BlockLevelAccessListsEnabled assert.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): forward BlobBaseFee too in the context rebuild
Re-review follow-up on #12691: WithPrevRandao recomputes BlobBaseFee from the
header, so the no-override branch was newly lossy for a processor that forced a
non-derivable value (XdcBlockProcessor sets BlobBaseFee = 0 on a header cloned
with ExcessBlobGas = 0; recomputation yields MinBlobGasPrice = 1). Collapse to a
single WithPrevRandaoAndBlobBaseFee that forwards the incoming BlobBaseFee (or the
block override when present), leaving Spec as the only field the rebuild changes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(simulate): relax EIP-3607 via execution flag, not spec wrapping
The eth_simulateV1 contract-sender fix relaxed EIP-3607 by wrapping the block
execution context spec with WithoutEip3607(), which returns a NoEip3607Spec
decorator implementing only IReleaseSpec. Because the tx processors read their
spec from that context (TransactionProcessor.GetSpec => BlockExecutionContext.Spec),
the decorator reached chain-specific processors that hard-cast it — e.g.
TaikoTransactionProcessor.PayFees does (ITaikoReleaseSpec)spec and
XdcTransactionProcessor casts on every tx — so eth_simulateV1 on Taiko/XDC threw
InvalidCastException/InvalidOperationException instead of returning a result
(Eip3607Transition defaults to 0, so the wrap is always active there).
Replace the spec wrapping with a dedicated ExecutionOptions.SkipSenderCodeCheck
policy flag gated in ValidateSender. The main simulate adapter ORs it into its
Process call; the EIP-7928 BAL path receives it via a small
BlockAccessListTxExecutionOptions injected into BlockAccessListManager and threaded
to its ExecuteTransactionProcessorAdapter. The release spec now keeps its concrete
runtime type on every path, so chain-specific interfaces survive.
Regression test: Nethermind.Taiko.Test exercises a code-bearing sender through
TaikoTransactionProcessor.PayFees — it passes with the flag and throws
InvalidCastException under the old spec-wrapping. eth_simulateV1 BAL-path and full
simulate suite remain green.
Follow-up to #12691; addresses the residual type-erasure raised in its review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore(simulate): trim explanatory comments to essentials
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): carry EIP-3607 relaxation on BlockExecutionContext
Replace the ExecutionOptions.SkipSenderCodeCheck flag (and the machinery to thread
it into the EIP-7928 BAL path) with a SkipSenderCodeCheck field on
BlockExecutionContext. Both the main tx processor and the BAL manager's own
processors already share the context via SetBlockExecutionContext, so the field
reaches every path for free — no threading through BlockAccessListManager /
TxProcessorPool / ExecuteTransactionProcessorAdapter, and no injected policy type.
The context is the same shared channel #12691 originally relaxed on; this just uses
a flag instead of a spec decorator, keeping the spec's concrete runtime type (so
ITaikoReleaseSpec/IXdcReleaseSpec casts survive). Net simpler diff and consistent
with the existing IsGenesis flag on the same struct.
ValidateSender reads VirtualMachine.BlockExecutionContext.SkipSenderCodeCheck; the
simulate executor sets it in its context rebuild. Taiko regression test updated to
set the flag on the context (still throws InvalidCastException under spec-wrapping).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): carry EIP-3607 relaxation on TransactionProcessor
Per review preference (@LukaszRozmej), move the SkipSenderCodeCheck flag from
BlockExecutionContext onto TransactionProcessor. ValidateSender reads the
processor's SkipSenderCodeCheck property. The simulate scope sets it type-
preservingly on both creation paths: Intercept<ITransactionProcessor> for the
main processor, and a factory decorator for the EIP-7928 BAL processors — so each
chain keeps its concrete processor type (no spec wrapping, no processor-type swap).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* style: trim comments to the essential why
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): remove dead WithoutEip3607; address review polish
Follow-up to the SkipSenderCodeCheck switch (review by @claude on #12691):
- Remove the now-dead EIP-3607 spec wrapper — this PR dropped its last production
caller. Deletes WithoutEip3607, NoEip3607Spec, GetNoEip3607Spec (std + zkevm) and
the _noEip3607Specs cache; keeps WithoutEip158 (still live). Fixes two comments
that cited the removed decorator.
- Enforce the relaxation invariant: a shared Apply() throws if the resolved
ITransactionProcessor isn't a TransactionProcessorBase, instead of silently
no-op'ing back to -38024. Used by both the Intercept hook and the factory decorator.
- XML-doc SkipSenderCodeCheck with its set-before-use / unsynchronised-read invariant.
- Make SkipSenderCodeCheckTransactionProcessorFactory internal.
- Revert SimulateBlockValidationTransactionsExecutor to master (the fix no longer
touches it; its PrevRandao tweak was unrelated).
- Assert fee payment in the Taiko regression test (matches its name).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(dns): verify EIP-1459 subtree hashes (#12707)
* fix(dns): verify EIP-1459 subtree hashes
EIP-1459 serves every subtree entry from the subdomain
base32(keccak256(entry)[..16]). The root signature covers only the
enrtree-root entry, so this hash chain is the only binding between the
signed root and the branch and ENR records a client consumes. The
crawler never checked it, so any resolver or poisoned cache could
substitute arbitrary node records and steer the crawl to
attacker-chosen labels.
Verification follows go-ethereum: unpadded standard base32, prefix
comparison against keccak256 of the record, abbreviated labels accepted
down to 12 decoded bytes (minHashLength). The tree root is exempt, as
EIP-1459 serves it from the bare domain with no hash label. A mismatch
logs at Warn and skips the record.
No base32 decoder existed in src/Nethermind, so EnrTreeHash carries a
small one, tested against labels produced by the reference
implementation.
Validated against live trees: all.mainnet.ethdisco.net crawls 3000
records with zero rejections, so the check does not misfire on
anything the reference publisher emits.
* refactor(dns): simplify and harden EnrTreeHash
- Reuse Keccak.Size instead of a local MaxHashLength constant.
- Decode base32 arithmetically instead of scanning the alphabet string.
- Guard the decoded-length check against int overflow on hostile input.
- Hash UTF-8 bytes from a stackalloc/pooled buffer instead of allocating.
* refactor(dns): log hash-mismatch rejections at Debug, clarify base32 mapping
---------
Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
* Selectable dotTrace profiling mode + dotnet-trace EventPipe sidecar for benchmark workflows (#12708)
* benchmarks: selectable dotTrace profiling mode (sampling | tracing | timeline)
The dottrace input on run-rpc-benchmarks and run-expb-reproducible-benchmarks
becomes a choice: false | sampling | tracing | timeline ('true' stays accepted
as a legacy alias for sampling on API dispatches). The mode maps to the
dotTrace CLI's --profiling-type: rpc-bench sets it on the wrapped entrypoint in
start-node.sh; the EXPB workflow passes --dottrace-mode to expb (only when
non-default, so pinned expb versions keep working for sampling runs; requires
execution-payloads-benchmarks feature/dottrace-profiling-modes for the new
modes).
Timeline snapshots cannot be converted to XML by Reporter.exe, so the
generate-dottrace-reports and summary jobs skip that mode - the raw .dtp
artifact is still collected and uploaded for the dotTrace UI. Line-by-line is
deliberately not offered: it needs PDBs the client docker images do not carry.
EXPB additionally rejects trace_blocks with timeline, since per-block
snapshots ride the MeasureProfiler API, which needs a performance session.
* expb workflow: collect a dotnet-trace EventPipe sidecar with every dotTrace run
Whenever dottrace is enabled the run also passes --dotnet-trace to expb: a
host-side EventPipe session records gc/contention/threading/exception events
(no CPU sampler - dotTrace owns the stacks) and the .nettrace ships in the same
dottrace-* artifact. Requires expb feature/dottrace-profiling-modes.
* rpcbench/expb: document the profiling modes and fix two review nits
Docs still described the pre-mode behaviour: the rpc-bench README documented a
dottrace command line with no --profiling-type and stated capture deliberately
runs in default sampling mode, and its input table listed dottrace as a boolean.
AGENTS.md's expb section had the same gap. Both now cover the four choices, when
to reach for each, that timeline yields no XML, and the dotnet-trace sidecar -
that section is what agents read to interpret run artifacts.
Timeline snapshots now save as .dtt rather than .dtp. The report job is gated off
for timeline either way, but the extension is what stops Reporter.exe's .dtp glob
from picking up a snapshot it cannot convert if that gate is ever relaxed.
The expb dottrace flag no longer special-cases sampling: the run already requires
an expb that understands --dotnet-trace, so there is no older-expb compatibility
left to preserve, and the flag string was spelled out twice in each of two
copy-pasted job bodies. The trace_blocks default guard becomes an explicit if -
as the last statement of its if body the AND-list left the block with status 1,
surviving only through errexit's AND-OR exemption.
* docs: scope the EventPipe sidecar to EXPB
The rpc-bench README claimed every profiled run drops a .nettrace into the
dottrace-rpcbench artifact, but --dotnet-trace was only wired into the EXPB
workflow - nothing in run-rpc-benchmarks.yml or scripts/rpc-bench collects one, so
the paragraph sent readers hunting for a file that is never produced, and promised
it precisely for timeline runs, which have no XML either. Say what a timeline
rpc-bench run actually yields: the .dtt snapshot alone.
* fix(jsonrpc): serialize receipt root as full-width DATA (#12706)
* fix(jsonrpc): serialize receipt root as full-width DATA
* test(jsonrpc): parameterize the receipt-root width cases
* test(jsonrpc): pin the whole-byte leading-zero root case
* test: add hand-derived golden tests for eth/71 and snap serializers (#12699)
* test(network): pin eth/71 and snap serializer wire encodings
Every serializer test file in Eth V71 and Snap/V1 now asserts an exact
wire encoding. The goldens are hand-derived from the RLP rules and
verified with an independent encoder (pyrlp + pycryptodome keccak).
Random request ids are pinned only in the goldened tests; ByteCodes
gains a sibling roundtrip that keeps the constructor-assigned id, and
GetAccountRange pins long.MaxValue to keep the eight-byte id path.
* test(network): share repeated snap golden fragments
The request-id, keccak-of-empty-string, and range-hash fragments moved
into SnapSerializerGoldens. Each golden fragment and its test input
come from one hex constant, so the expectation cannot drift from the
input.
* test(network): address review feedback on snap golden tests
- ByteCodes: one parameterized Roundtrip; long.MaxValue pins the
eight-byte request id deterministically instead of a random id
- GetAccountRange: independent field asserts in EnterMultipleScope
- SnapSerializerGoldens: expose RequestId1111 next to its RLP item;
the remarks state which fragments share hex with inputs and that
the keccak("") fragment is an independent literal on purpose
* `debug_trace*`: Fix phantom logs on frame revert (#12621)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Phantom log test
* Remove logs on a reverted frame
* Naming
* Fix leaking `ArrayPooList`
* Code cleanup
* Additional test
* Simplify tests
* Build fix
* Snap sync: reject storage range responses with unmatched slot lists (#12729)
* fix(snap): reject storage range responses with unmatched slot lists
A `StorageRanges` reply carries one slot list per requested account, but
nothing verified that the responder returned no more lists than were asked
for. `AddStorageRange` looped over the response count and indexed the request
account list with it, so a longer-than-requested reply read past the end of
the request.
Reject such a response up front and retry the range, which also settles the
active storage request accounting so that the range download phase can still
reach completion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(snap): pin the slot list count boundary
Cover equal and fewer slot lists than requested accounts so that widening the
guard to `>=` cannot pass, which would reject every well-formed response.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(metrics): durable sync-time and full-pruning duration metrics (#12590)
* feat(metrics): durable sync-time + per-stage breakdown and full-pruning duration
Sync:
- Fix UpdateAndGetSyncTime() so the total is retained after sync completes
instead of resetting to zero (EthSyncingInfo and Taiko override).
- Replace nethermind_sync_time with nethermind_sync_time_seconds (durable).
- Add nethermind_sync_time_in_mode_seconds{sync_mode} per-stage breakdown,
driven by a new SyncTimeInModeTracker on ISyncModeSelector.
Pruning:
- Add nethermind_full_pruning_last_duration_seconds and
nethermind_full_pruning_count, recorded on successful FullPruningDb.PruningFinished.
- Suffix in-memory trie pruning gauges with *Ms and document the unit.
* refactor(metrics): address PR review on sync/pruning time metrics
- Sync time no longer drops to 0 for one scrape when a stopped node
re-syncs: extract shared SyncTimeStopwatch that always returns the
retained total (used by EthSyncingInfo and Taiko override). Add
stop->resume regression tests.
- SyncTimeInModeTracker is now IDisposable (unsubscribes from
ISyncModeSelector.Changed), seeds its labels with TryAdd instead of
clobbering the shared static dictionary, and is owned by the container.
Timestamp seam moved to an internal ctor.
- SyncTimeInModeSeconds is get-only to match the other [KeyIsLabel] metrics.
- FullPruningCount uses the Interlocked backing-field pattern like the
other counters in Db/Metrics.
- Clarify that FullPruningLastDurationSeconds covers the trie copy+commit,
not the wait for a suitable state root.
- Make the full-pruning duration test assert the write actually happened.
* fix(metrics): avoid DI cycle wiring the sync-mode tracker; drop unused using
- Attach SyncTimeInModeTracker via Intercept<ISyncModeSelector> instead of
resolving it during IMonitoringService construction. Resolving it there
created a container cycle (IMonitoringService -> tracker -> ISyncModeSelector
-> ... -> DbTracker -> IMonitoringService) that crashed the node at startup.
This keeps the monitoring module free of outward dependencies, mirroring the
existing IEthSyncingInfo wiring.
- Remove unused `using System;` in SyncTimeInModeTrackerTests (IDE0005).
* Only accept the requested header in FetchHeaderFromPeer (#12730)
* fix(sync): only accept the requested header in FetchHeaderFromPeer
FetchHeaderFromPeer queries every initialized peer in parallel and takes
the first non-null response, but never checked that the returned header
is actually the one that was asked for. A peer that answers with some
other block had its header accepted, so the result depended on which
peer replied first rather than on what was requested.
Compare the returned header's hash against the requested hash on both
response paths. A non-matching response is treated like a missing one,
so the remaining in-flight peers can still supply the header instead of
the whole lookup failing.
The sibling lookups in StartingSyncPivotUpdater and PeerRefresher
already do this; FetchHeaderFromPeer was the one that did not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): cover the allocated-peer fallback and tighten assertions
Add a case where no peer answers the head-header request, so the
GetBlockHeaders fallback is the one that resolves the header. Nothing
pinned its success branch before, so it could have been broken without
a test noticing.
Assert on the header instance rather than its hash, so the negative
case cannot pass for a header that merely has no hash set.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(sync): report a peer that answers with a different block
A mismatched header was discarded silently, leaving it indistinguishable
from a peer that simply does not have the block. Requests are serialised
per peer by MessageQueue and a late response to a timed-out request is
disposed rather than handed to the next caller, so a mismatch is always
a protocol violation rather than a benign response race.
Report it as UnexpectedHeaderHash, matching HeadersSyncFeed and the
other header lookups.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): cover that an honest peer keeps its connection
Reporting a mismatch disconnects the peer, so the costly failure mode is
now an honest peer being dropped rather than a wrong header accepted.
Only the reported direction was asserted.
Cover all three answer shapes, including a peer that does not have the
block, which is the normal answer while a head is unknown and must not
cost a connection.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix doubled revert handling in some tracers (#12715)
* Test for EIP-7708 top frame log
* [WIP] attach log to parent frame
* Attach log to correct frame
* Reuse common code in tests
* More tests
* Phantom log test
* Remove logs on a reverted frame
* Naming
* Fix leaking `ArrayPooList`
* Code cleanup
* Additional test
* Simplify tests
* Showcase test
* Direct fix
* More failing tracing tests
* Remove problematic `ReportActionRevert` > `ReportActionError` delegation from base
* Get rid of virtual-to-virtual calls in report revert/error
* Formatting
* Build fix
* Remove other `ReportActionRevert` -> `ReportActionError` calls
* Move common test codes to base class
* Fix `IsTracingActions` summary
* Small test fix
* Code cleanup
* test(era): anchor accumulator tests to EIP-7643 vectors, remove unused GetProof (#12718)
* test(era): anchor AccumulatorCalculator roots to derived spec vectors
Replaces the two-instances self-oracle, the inequality-only comparisons,
and the DoesNotThrow-only Add tests with parameterized roots derived by
an independent Python SSZ merkleization of the portal-network history
spec. Adds proof fold-up verification against the derived roots.
Deletes the byte-identical duplicate golden test in Era1.Test.
* test(era): assert the accumulator root the readers return
ReadAccumulator and ReadAccumulatorRoot tests discarded the root and
asserted only Throws.Nothing. They now assert the root equals the
accumulator of the written contents. The shared calculator loop moves
into a ComputeAccumulatorRoot helper in each file.
* test(era): apply review round on the accumulator vector tests
Parameterizes the proof verification and adds a three-entry index 2
case, so the upper tree levels exercise the right-hand sibling order.
Moves the proof length assert into the fold-up helper. Rewrites the
provenance comments in active voice and adds the spec link.
* test(era): cite EIP-7643 as the accumulator spec reference
The portal-network history spec was restructured and no longer defines
the accumulator. EIP-7643 defines HeaderRecord and the 8192-entry epoch
record. Also states the exact value of the index 2 proof case: it is
the only case that catches a wrong index shift.
* refactor(era): remove unused AccumulatorCalculator.GetProof
GetProof shipped with the EraE feature but no production code ever
called it; its only callers were its own tests. The removal also drops
the _totalDifficulties list, which only GetProof read, and the proof
tests in Nethermind.EraE.Test.
* docs(era): cite EIP-7643 on AccumulatorCalculator
The linked portal-network document was restructured and no longer
defines the accumulator. EIP-7643 defines HeaderRecord and the
8192-entry epoch record this class computes.
* test(era): apply removal-round polish
Converts the class comment to an XML summary per the documentation
rule. Inlines the two-entry root, which lost its second consumer with
the proof tests.
* test(era): consolidate the accumulator fixtures into Era1.Test
Both fixtures tested Nethermind.Era1.AccumulatorCalculator with
overlapping vectors. The Era1.Test table now carries every
discriminating single-entry case, a pinned empty-accumulator root, and
a Clear reset test against the writer-reuse contract. The EraE.Test
copy is deleted.
* test(era): state only true contracts in the fixture comments
The Clear comment claimed a multi-file writer-reuse path that does not
exist: Add throws after Finalize and each era file gets a fresh writer.
The vector comment claimed a pairwise single-input difference that two
case pairs violate. Both now state only what holds.
* test: strengthen mock-echo and vacuous tests in Abi, Optimism and Xdc test projects (#12720)
* test(abi): pin forwarding and return propagation in encoder extensions
The extension tests asserted only Received on the inner call and
ignored the extension return value. A stub on the exact unpacked
arguments plus an identity assert on the result covers both.
* test(xdc): assert the RocksDb config factory routing
The single test asserted Is.Not.Null on a result that cannot be null.
The factory contract is routing: Xdc databases get a PerTableDbConfig
without consulting the base factory, and every other database delegates
to it. Both branches are now pinned; the delegation branch was
untested.
* test(xdc): pin the routed timeout instance
The vote routing test pins the exact vote, but the timeout test used
Arg.Any, so a handler that routes the wrong timeout passed.
* test: apply the C11 review round
Removes an unused using that fails lint CI. Pins the factory options
against the provided IDbConfig, so a wrong database name in the special
branch turns the test red. Moves the extension rationale to class level
and renames the Abi tests to the project snake_case convention.
* test: state only true mechanisms in the C11 comments
NSubstitute returns an empty array, not null, for an unmatched call on
an array-returning member. The prefixed-options claim holds for only
half of the database prefixes, so the sentence is dropped.
* test(xdc): use a neutral database name in the delegation test
Review feedback on #12720: the delegation test used the literal
"State"/"Code", and "State" is the one name PerTableDbConfig
special-cases (StartsWith("State")). Use nameof(DbNames.Blocks) with a
null column so the test isolates the delegate-to-base contract without
that special-case, matching the sibling FlatRocksDbConfigAdjusterTests
idiom.
* test: anchor crypto and RLP tests to independent expectations (#12712)
* test(core): anchor the keccak span test to an independent vector
* test(core): anchor RLP ulong lengths to the spec
* test(core): compare decoded blocks to the original and drop the ignored file writer
* test(core): compare decoded header fields to the original block
* test(core): anchor the regression block decode to pyrlp-derived fields
* test(core): apply review round - drop dead using, widen anchors, guard indexed asserts
* test(core): apply round-2 review polish
* test(core): cover the header tail fields and sharpen the roundtrip comments
* test(core): apply confirm-round nits
* test(core): compare decoded uncle hashes in the block roundtrip
The body compared uncles by count only. The scenarios build two uncles
with distinct headers, so the hash comparison catches an order or
content error the count cannot see. The count guard moves outside the
multiple-assert scope like the transaction guard.
* Update OP Superchain chains (#12752)
Co-authored-by: emlautarom1 <emlautarom1@users.noreply.github.com>
* Auto-update fast sync settings (#12751)
Co-authored-by: rubo <rubo@users.noreply.github.com>
* test: Db tests assert stored state instead of smoke-calling empty methods (#12693)
* test(db): assert stored state instead of smoke-calling empty methods
MemDb.Flush and Dispose (and the Snapshotable variants) are empty method
bodies, so the six *_does_not_cause_trouble tests could never fail;
removed. The no-assert MemDb creation tests now verify the value round-
trips. ReadOnlyDbProviderTests.Can_clear exercised an empty registry (no
db was ever registered), making ClearTempChanges a no-op; it now
registers a real MemDb-backed read-only db and asserts the overlay is
dropped while the wrapped db stays intact, plus the no-writes contract
when localChanges is off. Can_get_all_on_empty now asserts emptiness
instead of discarding the enumeration.
* test(db): assert overlay drop only where a write could land in the overlay
* test(db): group independent post-condition asserts in Assert.EnterMultipleScope
* Reject invalid fixed-size header RLP (#12579)
* Treat a null header answer as the block being absent (#12741)
* fix(sync): treat a null header answer as the block being absent
An empty list item decodes to a null header and is kept in the response
list, so the allocated-peer fallback could pass one into the hash
comparison and throw. FetchHeaderFromPeer only catches cancellation and
timeouts, so it would surface into engine_forkchoiceUpdated.
Handle it in Validate, which lets the head-header path drop its own null
check too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(sync): name the mock switch after the answer it produces
The peer answers with a null header, not an empty one. Move it next to
the other answer switches so HeaderToReturn keeps its own doc.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Return only the requested header from GetHeadBlockHeader (#12740)
* fix(network): return only the requested header from GetHeadBlockHeader
GetHeadBlockHeader asks for one specific block but returned whatever
single header the peer sent back. Every caller wants the block it asked
for, and two of them re-checked the hash themselves afterwards; the peer
refresh in SyncPeerPool did not, so it would take a substituted header
and record the wrong head number and total difficulty for that peer.
Compare the response against the requested hash where the request is
made, and disconnect a peer that answers with a different block. A peer
that does not have the block answers with an empty list, which is the
normal response while a head is unknown and still yields null.
Drop the now-redundant check in StartingSyncPivotUpdater. The one in
PeerRefresher stays: that method validates its head/parent response
locally anyway, since GetBlockHeaders carries no such guarantee.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(network): treat a null header answer as the block being absent
An empty list item decodes to a null header and is kept in the response
list, so a peer can answer a single-header request with one null entry.
Dereferencing it to compare hashes threw, and two callers do not catch
that: the pivot updater would leave its update loop for good, and
FetchHeaderFromPeer would surface it into engine_forkchoiceUpdated.
Treat it the same as an empty list — the peer does not have the block —
rather than as a breach.
Also type the requested hash as nullable, matching _remoteHeadBlockHash
before the status handshake, and assert the absent cases on the result
itself rather than on its hash.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(network): make the requested-header guarantee unconditional
The hash check was skipped when neither the argument nor the announced
head was known, so the documented contract had a hole. Return early
instead: with no hash to ask for there is no meaningful request to send.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat: add built-in portfolio viewer UI at /portfolio (#12360)
* Handle failed sender recovery (#12757)
* fix(simulate): route the EIP-7928 BAL path through the simulate tx adapter (#12692) (#12721)
* fix(simulate): route EIP-7928 BAL path through the simulate tx adapter
Under EIP-7928, eth_simulateV1 runs transactions through the
BlockAccessListManager's own tx processors, bypassing
SimulateTransactionProcessorAdapter. That lost its GasCap budget clamp, its
TotalGasLeft/BlockGasLeft accounting (block gasUsed reported as 0), and its
validation:false handling (the BAL path always called Execute, never Trace).
Inject the adapter via a new ITransactionProcessorAdapterFactory so the
sequential BAL manager — the only one simulate drives — wraps each tx processor
in the simulate adapter. The parallel manager always uses the default
ExecuteTransactionProcessorAdapter: the stateful simulate adapter is
sequential-only and simulate never triggers the parallel path (it attaches no
BlockAccessList).
The factory is an interface, not a delegate, so Autofac does not
auto-synthesise one on the real block-production scope, where the optional
parameter must stay null and fall back to the default adapter.
The no-gas EIP-8037 execution-dimension default is a separate ordering issue
(the per-tx inclusion check runs before the adapter) and is left as a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(simulate): make the tx-processor-adapter factory a required delegate; add GasCap test
Replace the ITransactionProcessorAdapterFactory interface with a required
TransactionProcessorAdapterFactory delegate, mirroring CodeInfoRepositoryFactory:
the block-processing module registers the default (ExecuteTransactionProcessorAdapter)
and the simulate scope overrides it with the simulate adapter. This drops the
optional/null-fallback and the auto-synthesis hazard an optional delegate would
carry (ITransactionProcessorAdapter is registered on the block-processing scope,
so Autofac would otherwise fill an optional delegate on the real path).
Also add a regression test for the JsonRpc.GasCap budget (#12692 item 2): a
two-call request whose cumulative gas exceeds the cap has its second call clamped
below intrinsic gas and rejected; without the adapter the cap is not enforced and
both calls run unclamped.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: address #12721 feedback (enforce sequential BAL, DI style, docs, test dedup)
- Set ProcessingOptions.ForceSequentialBlockAccessList in the simulate options so the
stateful SimulateTransactionProcessorAdapter can never reach the parallel BAL pool
(previously guaranteed only by simulate not attaching a BlockAccessList); documented
the single-threaded contract on the adapter itself.
- Register the default TransactionProcessorAdapterFactory with AddScoped + a method group
(drops the lambda cast) and the simulate override with the typed-dependency AddScoped
overload (no manual Resolve / captive singleton).
- Reworded the factory <remarks> to describe what is actually wired (default Execute;
other scopes still get the default on the BAL path) and dropped the overstated
"gas defaulting" from the registration comment.
- Extracted BuildAmsterdamBalChain test helper; pinned the EIP-2780 intrinsic in the
GasCap test comment.
Follow-up filed as #12723 (scopes overriding ITransactionProcessorAdapter still get
Execute on the BAL path).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: single-axis tx-processor-adapter registration (step 1)
Addresses LukaszRozmej's review: the TransactionProcessorAdapterFactory was a
second registration axis alongside ITransactionProcessorAdapter, so scopes that
only overrode the latter (block production, trace, proof) drifted to the default
Execute adapter on the EIP-7928 BAL path — the same bug class this PR fixes for
simulate (#12723), left live elsewhere. Notably block production silently
downgraded its intended BuildUp semantics to Execute under Amsterdam.
Make the factory the single source of truth: the root registers the default
(Execute) plus a derivation ITransactionProcessorAdapter = factory(processor), and
each scope overrides only the factory — production BuildUp, trace (…
2 tasks
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
3 participants
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.
Fixes #12692 (items 2, 3 and 4)
Changes
Under EIP-7928,
eth_simulateV1runs transactions through theBlockAccessListManager's own tx processors, which bypassedSimulateTransactionProcessorAdapter. On the Amsterdam (BAL) path this silently dropped three simulate behaviours that the main path has:JsonRpc.GasCapbudget across a request's calls (theTotalGasLeftclamp).TotalGasLeft/BlockGasLeftaccounting — block-levelgasUsedwas reported as0.validation:false— the BAL path always calledExecute, neverTrace, so sender validation (nonce/balance) was never skipped.The fix injects the adapter into the BAL manager via a new required
TransactionProcessorAdapterFactorydelegate (mirroring the existingCodeInfoRepositoryFactorypattern):BlockProcessingModuleregisters the default factory (ExecuteTransactionProcessorAdapter), so real block production/validation is unchanged.eth_simulatescope overrides it with the simulate adapter, so the BAL manager's tx processors get the simulate behaviours.The simulate adapter is only ever driven sequentially: simulate synthesises blocks with no
BlockAccessList, so the BAL manager never takes its parallel path.Not in this PR (follow-up): the implicit no-gas default under EIP-8037. A no-gas call is defaulted to
GasCapat parse time, and the EIP-8037 per-tx inclusion check runs before the adapter can clamp it, so it is an ordering issue independent of the adapter routing rather than something this change can fix.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Three regression tests added in
EthSimulateTestsBlocksAndTransactions, each empirically verified to regress when the simulate-scope factory registration is removed:eth_simulateV1_reports_block_gas_used_on_bal_path— blockgasUsed > 0(was0).eth_simulateV1_honours_validation_flag_on_bal_path—validation:falseskips a stale-nonce rejection;validation:truestill rejects.eth_simulateV1_enforces_gas_cap_across_calls_on_bal_path— a two-call request over theGasCaphas its second call clamped below intrinsic gas and rejected.Local runs: simulate suite 111/111;
BlockAccessList/BlockProcessor/Eip8037/Reorg/Blockhash150/150; AuRa processor 5/5; Proof 41/41; full solution builds clean.Documentation
Requires documentation update
Requires explanation in Release Notes