Reinstate removed IGasPolicy methods and apply for eip-8037#10897
Merged
damian-orzechowski merged 14 commits intomasterfrom Mar 24, 2026
Merged
Reinstate removed IGasPolicy methods and apply for eip-8037#10897damian-orzechowski merged 14 commits intomasterfrom
IGasPolicy methods and apply for eip-8037#10897damian-orzechowski merged 14 commits intomasterfrom
Conversation
damian-orzechowski
requested review from
Demuirgos,
LukaszRozmej and
benaadams
as code owners
LukaszRozmej
approved these changes
Mar 20, 2026
Member
|
tests do fail |
Contributor
EVM Opcode Benchmark DiffAggregated runs: base=1, pr=1 No significant regressions or improvements detected. |
…ermindEth/nethermind into refactor/eip8037-gas-policy
Contributor
Author
Should be fixed now |
LukaszRozmej
approved these changes
Mar 20, 2026
benaadams
approved these changes
Mar 20, 2026
This reverts commit d08decc.
svlachakis
approved these changes
Mar 22, 2026
hudem1
approved these changes
Mar 24, 2026
benaadams
added a commit
that referenced
this pull request
Mar 26, 2026
commit f56e0ef Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 19:39:34 2026 +0000 fix: use TryGetValue for account cache hit/miss metrics GetOrAdd + ThreadLocalStateTreeReads counter check misclassified misses as hits because StateProvider.GetState() increments the counter before calling _tree.Get() (ScopeWrapper.Get). The factory GetFromBaseTree doesn't increment any counter, so the delta is always zero. Switch account Get to TryGetValue + Set (on miss) so the TryGetValue result directly drives hit/miss classification. Storage Get retains GetOrAdd since LoadFromTreeStorage increments the counter itself. Remove unused _getFromBaseTree delegate. commit 5fe1d11 Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 18:51:33 2026 +0000 refactor: unify SeqlockCache read path — always GetOrAdd on both prewarmer and main processor Remove the two-mode branching in ScopeWrapper.Get and StorageTreeWrapper.Get. Both prewarmer and main processor now use GetOrAdd (write on miss). On cache hit, HintGet is still called to populate the scope's _loadedAccounts. This means the main processor also populates the cache on miss, giving the next block more carry-forward hits. The populatePreBlockCache flag is retained only for StartWriteBatch (controls carry-forward buffering). Remove populatePreBlockCache from StorageTreeWrapper constructor since the read path no longer branches on it. Disable prewarming in ShouldAnnounceReorgOnDispose pruning test — the test exercises trie pruning infrastructure and was accidentally dependent on cache misses driving trie node loads. With prewarming (or the unified read path), cache hits reduce memory pressure, delaying persistence thresholds. The test was already fragile with prewarming enabled. commit 79ac860 Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 17:42:05 2026 +0000 fix: subscribe to OnAccountUpdated for correct storage roots in carry-forward The base write batch fires OnAccountUpdated during Dispose() with the final storage root. CachePopulatingWriteBatch.Set() captured accounts before storage roots were updated, producing stale StorageRoot values in the carry-forward buffer. Now subscribe to OnAccountUpdated on the base batch to overwrite stale entries with corrected accounts. Fixes E2ESyncTests and AuRa test failures where trie vs flat storage roots diverged due to stale carry-forward data. commit 8c2aa77 Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 17:31:18 2026 +0000 refactor: use List<T> for state carry-forward buffer, keep ConcurrentQueue for storage State writes (StateProvider.FlushToTree) are single-threaded, so List<T> avoids unnecessary interlocked overhead. Storage writes remain in ConcurrentQueue because PersistentStorageProvider.FlushToTree parallelizes across contracts via ParallelUnbalancedWork. commit 9cf509f Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 17:14:42 2026 +0000 fix: buffer carry-forward writes to avoid same-scope cache poisoning The CachePopulatingWriteBatch was writing directly into the SeqlockCache during FlushToTree. Since the same ScopeWrapper reads from that cache, post-commit values were immediately visible to subsequent reads within the same block processing scope (e.g. coinbase creation after tx execution), causing wrong state roots in EF state/VM tests. Fix: buffer carry-forward writes in ConcurrentQueues on PreBlockCaches during FlushToTree, then flush them into the SeqlockCaches from BranchProcessor after WaitAndClear (prewarm task done) and Reset (no more reads from current scope). This eliminates the same-scope feedback loop while preserving cross-block carry-forward. commit 244445a Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 16:21:52 2026 +0000 Tidy commit 313c5cc Author: Ben Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 16:08:37 2026 +0000 feat: cross-block SeqlockCache carry-forward with reorg invalidation Add cache-populating write-batch wrappers in PrewarmerScopeProvider that mirror account and storage writes into PreBlockCaches during FlushToTree. This allows the SeqlockCaches to carry forward block N's post-state as block N+1's pre-state, reducing trie reads on sequential processing. PreBlockCaches tracks committed block number/hash metadata and a legacy storage-clear flag for pre-EIP-6780 SELFDESTRUCT handling. BranchProcessor validates cache continuity before the first prewarm and invalidates on parent mismatch, errors, or legacy storage clears. commit 2275710 Author: Tomass <155266802+zeroprooff@users.noreply.github.com> Date: Thu Mar 26 07:49:30 2026 +0000 Fix incorrect NodeType in TrieNodeTests (#10917) commit 707affd Author: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Date: Thu Mar 26 08:48:19 2026 +0100 Extend `CallResult` constructor visibility (#10949) Extend constructor visibility commit a791dc6 Author: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Date: Thu Mar 26 08:48:00 2026 +0100 New constructor (#10950) commit e14c4f7 Author: Amirul Ashraf <asdacap@gmail.com> Date: Thu Mar 26 08:39:16 2026 +0800 perf: add fast MVCC snapshots for MemDb in FlatDb tests (#10792) * feat: add MVCC snapshot support to MemDb for fast test snapshots Implements SnapshotableMemDb and SnapshotableMemColumnsDb with O(1) snapshot creation using Multi-Version Concurrency Control (MVCC), replacing the O(n) full-copy approach when snapshots are needed. Key features: - O(1) snapshot creation by capturing version numbers - Multiple concurrent snapshots with full isolation - Automatic version garbage collection on snapshot disposal - ISortedKeyValueStore support for sorted iteration - Thread-safe with proper locking This enables efficient snapshot-based testing, particularly for FlatDb tests where snapshots are created frequently. The new classes are drop-in replacements for MemDb/MemColumnsDb when snapshot support is required. All 21 unit tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * chore: migrate FlatDb tests to use SnapshotableMemColumnsDb Replace TestMemColumnsDb with SnapshotableMemColumnsDb in FlatDb test infrastructure to enable fast O(1) MVCC snapshots instead of slow O(n) full database copies. Changes: - PseudoNethermindModule: Register SnapshotableMemColumnsDb for FlatDbColumns - FlatTrieVerifierTests: Update field type and helper method casts to use IDb interface All tests passing (22/22 FlatTrieVerifierTests). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: use primary constructors and optimize write batches - Convert all constructors to primary constructor syntax (C# 12) - Add dedicated MemDbWriteBatch that locks only once during commit - Replace InMemoryWriteBatch with optimized batch that collects operations and commits atomically Performance improvement: - Before: Each Set() in batch acquired lock individually - After: Single lock acquisition for entire batch commit All tests passing (54/54). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * perf: use ArrayPoolList in MemDbWriteBatch to reduce GC pressure Replace List with ArrayPoolList to use pooled arrays instead of allocating new arrays for each write batch. Changes: - Replace List<...> with ArrayPoolList<...> (initial capacity: 16) - Dispose ArrayPoolList to return arrays to pool - Restructure Dispose() to ensure proper cleanup Performance impact: Reduces GC allocations for write batch operations. All tests passing (54/54). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: Change SnapshottedMemDb from Dictionary<byte[], Version> to Dictionary<(byte[], int), byte[]> * fix: Add neverPrune option to SnapshotableMemDb to avoid pruning bug in tests Add neverPrune constructor parameter to SnapshotableMemDb and SnapshotableMemColumnsDb to disable version pruning. Enable this option in PseudoNethermindModule for tests to work around a bug in PruneVersionsOlderThan that removes versions still needed by active snapshots. The pruning bug will be fixed in a separate PR. For tests, memory is not a concern and disabling pruning is the simplest workaround. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Initialize neverPrune before creating column databases The previous constructor chain set _neverPrune AFTER calling GetColumnDb, which meant all SnapshotableMemDb instances were created with neverPrune=false even when neverPrune=true was passed to the constructor. Fixed by creating a private constructor that sets _neverPrune before the GetColumnDb loop, and routing all public constructors through it. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Critical thread-safety bugs in SnapshotableMemDb Fixed three critical race conditions: 1. MemDbSortedView.MoveNext() and CurrentValue accessed _db without lock - Changed GetValueAtVersion() calls to GetAtVersion() which acquires lock - Prevents concurrent modification exceptions when iterating while writing 2. GetViewBetween() read _currentVersion without lock - Now captures version inside lock for consistency These race conditions could cause exceptions or incorrect results when accessing the database from multiple threads concurrently. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Remove Reverse() allocations and fix PruneVersionsOlderThan bug Replace SortedDictionary.Reverse() with forward iteration in GetValueAtVersion and KeepOnlyLatestVersions to avoid O(n) intermediate allocations. Add single-pass FindFirstKeyAtVersion/FindLastKeyAtVersion helpers. Rewrite MemDbSortedView to iterate _db directly instead of buffering all keys. Use ArrayPoolList in pruning methods. Fix PruneVersionsOlderThan which incorrectly removed all entries below minVersion — now keeps the latest pre-minVersion entry per key so active snapshots can still resolve those keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * perf: Switch SnapshotableMemDb from SortedDictionary to SortedSet with GetViewBetween Restructure the internal data store from SortedDictionary<(byte[], int), byte[]?> to SortedSet<(byte[], int, byte[]?)> to leverage GetViewBetween for O(log n) point lookups and efficient range iteration, fixing O(n) linear scan in GetValueAtVersion and O(n²) re-scanning in MemDbSortedView.MoveNext(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * perf: Simplify FindFirst/LastKeyAtVersion using GetValueAtVersion FindFirstKeyAtVersion now delegates to O(log n) GetValueAtVersion per unique key. FindLastKeyAtVersion iterates in reverse to return early instead of scanning the entire set forward. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review comments on SnapshotableMemDb - Fix double-counting reads in bulk key accessor by using GetValueAtVersion directly under a single lock instead of calling Get() which re-increments - Materialize GetAll/GetAllKeys/GetAllValues/GetAllAtVersion results under lock to prevent yield-while-holding-lock deadlock risk - Prune old versions on write when no snapshots are active to prevent unbounded memory growth in the non-snapshot case - Fix StartBefore to return true when key < firstKey so MoveNext correctly yields the first element Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review - use Lock type and Dictionary instead of IDictionary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> commit fbf3aff Author: Ben {chmark} Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 00:17:29 2026 +0000 test: add engine blockchain pyspec fixtures and fix BAL gas budget tracking (#10939) * feat: add engine blockchain test fixtures and fix BAL gas budget tracking - Add PyspecEngineBlockchainTestFixture base class for engine blockchain tests (blockchain_tests_engine directory) - Add engine blockchain test classes for all forks (Frontier through Osaka) - Add Amsterdam engine blockchain test fixtures for all Amsterdam EIPs - Fix BAL validation gas budget: use BlockGasUsed instead of SpentGas (SpentGas includes execution gas, BlockGasUsed is the block-level gas accounting that BAL validation should track) - Add regression test verifying BAL validation budget uses block gas * fix: version-aware ExecutionPayload creation and engine test improvements - Create version-appropriate ExecutionPayload type based on newPayloadVersion (V3 for version >= 3, V4 for version >= 5, base for older) - Only set V3/V4 fields on matching payload types (blob gas, BAL, slots) - Add missing NUnit using in Tests.cs - Use ExecutionPayload base type in BlockchainTestBase for broader compat - Add Convert_engine_payloads_uses_declared_payload_version test - Use Should().Equal instead of BeEquivalentTo for ordered list assertion * fix: restructure CreateExecutionPayload to explicit switch on derived type - Switch on most-derived first (V4 then V3) instead of two separate if-checks that both fire for V4 - makes the mutual exclusivity explicit - Move ParentBeaconBlockRoot to base properties (it lives on ExecutionPayload) - Set ExecutionRequests = [] for V3 payloads too (engine_newPayloadV4 passes executionRequests as a parameter) * fix: only set ExecutionRequests=[] on V4 payloads, not V3 - caused 'Execution requests must be set' failures * refactor: use two if-checks instead of switch to avoid duplicating V3 blob gas fields * refactor: add HexToNumber/HexToNullableNumber helpers, document version mapping Address PR feedback: - Extract HexToNumber<T> and HexToNullableNumber<T> helpers to replace repeated (ulong)Bytes.FromHexString(...).ToUnsignedBigInteger() casts - Add comment documenting the version-to-type mapping source of truth (engine API method signatures in EngineRpcModule.*.cs) * fix: handle both JSON object and RLP hex string formats for withdrawals in engine test fixtures * fix: default ExecutionRequests to empty array for V4 engine tests on ExecutionPayloadV3 * fix: parse executionRequests from params[3] for engine_newPayloadV4/V5 params[3] is executionRequests for V4+ (Prague/Amsterdam), not validationError. The code was deserializing actual consolidation and withdrawal requests as validation errors, causing blocks to be processed with empty requests and producing wrong state roots. For V3: params = [payload, blobHashes, parentBeaconRoot, ?validationError] For V4+: params = [payload, blobHashes, parentBeaconRoot, executionRequests, ?validationError] * fixes * fix * feedback * fix: remove pre-merge engine test fixtures to fix CI timeout The engine blockchain test classes for Frontier through Paris were causing 20-minute CI timeouts. Pre-merge forks don't use the Engine API, and the regular BlockchainTests already cover them. Keep only Cancun+ where newPayload versions differ (blobs, execution requests, BAL). * fix: remove brittle hardcoded engine test name The Engine_from_state_test_on_top_of_genesis_should_not_sync test used .Single() with a hardcoded fixture name from v5.0.0. The name changed in v5.4.0, causing "Sequence contains no matching element". The same test case runs via the generic TestCaseSource fixture. * fix: add timeout to ProcessHelper.RunAndReadOutput WaitForExit with a 5s timeout before ReadToEnd prevents the static constructor of RuntimeInformation from blocking indefinitely when wmic hangs under heavy parallel test load. Returns null on timeout so PhysicalCoreCount falls back to Environment.ProcessorCount. * fix: restrict engine and Amsterdam tests to Linux only Engine blockchain tests and Amsterdam tests are heavy (full DI + Engine API per test, separate fixture download). They exceed the 20-minute CI timeout on Windows/macOS/ARM runners. Restrict to Linux where they complete within budget. * refactor: route engine tests through JsonRpcService instead of reflection Replace custom deserialization + Activator.CreateInstance + MethodInfo reflection with IJsonRpcService.SendRequestAsync. The test fixture params are passed as raw JSON through the real RPC pipeline, which handles method resolution and parameter binding. This removes the need for ParamsExecutionPayload intermediate types and per-version parameter marshalling in the test path. * fix: BAL memory leak and shared reference bug Two bugs in block-level access list handling: 1. TracingEnabled was set to true once and never reset. Every subsequent block (including non-BAL blocks) accumulated AccountChanges, StorageChanges, and Change stack entries without bound. 2. GeneratedBlockAccessList was a singleton reference assigned to Block objects via SetBlockAccessList, then Clear()ed on the next block. Previous blocks lost their BAL data. Fix: reset TracingEnabled per block based on the spec, and allocate a fresh BlockAccessList per block instead of reusing/clearing. * fix: skip heavy engine/Amsterdam tests in checked and no-intrinsics CI Add TEST_SKIP_HEAVY env var to the checked/no-intrinsics workflow. CiRunnerGuard checks this to unconditionally skip engine and Amsterdam blockchain tests in variant builds that don't benefit from running them (the main workflow already covers correctness). * Single spec look up * fix: cancel uncancelled Task.Delays that prevent GC of disposed containers RefCountingPersistenceReader: Task.Delay(60_000) in a loop with no cancellation token kept DB snapshots alive for 60s after scope disposal. With 32 concurrent tests, ~120 zombie snapshots accumulated at steady state. Fix: cancel via CTS on CleanUp. GCKeeper: Task.Delay(postBlockDelayMs) without cancellation held the GCKeeper closure alive after container disposal. Fix: add IDisposable, cancel via CTS on Dispose. * fix: guard GCKeeper.Dispose against double-dispose of CTS CancellationTokenSource.Cancel() throws ObjectDisposedException if the CTS was already disposed by a prior Dispose call (Autofac can call Dispose multiple times). Guard with try/catch. * refactor: address PR review feedback - Use reflection to get newPayload param count from IEngineRpcModule instead of hardcoding version-dependent logic - Use EngineApiVersions.Latest for default version fallbacks - Add Latest constants to EngineApiVersions.Fcu and NewPayload - Move GeneratedBlockAccessList reset into LoadSuggestedBlockAccessList so callers don't need to remember to create a new instance * fix: handle RPC-level errors in engine test negative cases When the Engine API rejects a payload at the RPC layer (e.g. wrong payload version like "ExecutionPayloadV2 expected"), it returns a JsonRpcErrorResponse instead of a PayloadStatusV1 with INVALID status. For negative tests (validationError is set), this is valid — the engine correctly rejected the block. Continue to the next payload instead of failing the assertion. * fix: dispose PeriodicTimer instances to prevent resource leaks DiscoveryApp, DiscoveryPersistenceManager, MultiSyncModeSelector, RetryCache: add using to PeriodicTimer so it is disposed when the loop exits. SessionMonitor: dispose _pingTimer on stop. * fix: cancel MultiSyncModeSelector timer on Dispose MultiSyncModeSelector starts a PeriodicTimer loop (1ms interval in tests) in its constructor. Dispose() only disposed the CTS without cancelling it first, so the timer loop ran forever, holding the entire container graph alive through the async state machine closure. This caused 10k+ zombie containers accumulating over the test run. * fix: skip heavy engine/Amsterdam tests in Flat DB CI jobs * perf: use NullTxPool and disable prewarmer in blockchain tests Blockchain pyspec tests process pre-built blocks from fixtures — they don't need a real TxPool (32 GB allocation per full run) or prewarmer. Use NullTxPool.Instance and set PreWarmStateConcurrency=0. * perf: disable prewarmer and revert ProcessHelper to original Set PreWarmStateOnBlockProcessing=false before module registration (Intercept runs too late). Revert ProcessHelper.RunAndReadOutput to original ReadToEnd-then-WaitForExit pattern — the reversed order caused deadlock on Linux when /proc/cpuinfo output exceeded pipe buffer size. * refactor: address PR review feedback on Task.Delay and Dispose patterns - Add TaskExtensions.DelaySafe helper for cancellable delays that return instead of throwing OperationCanceledException - Use DelaySafe in GCKeeper, TrieStore, RefCountingPersistenceReader, DataFeed - Guard GCKeeper.Dispose and MultiSyncModeSelector.Dispose with Interlocked.Exchange to prevent double-dispose - Add comment on SortedDictionary in BlockAccessList (pre-existing, count check on line 40 handles mismatched entry sets) * chore: remove codex-dotnet.ps1 and AGENTS.md changes * refactor: remove Convert engine payload tests These tests only verified JsonToEthereumTest.Convert which is no longer used in the engine test path (replaced by IJsonRpcService). Remove tests, helper method, and unused imports. * refactor: simplify after code review - Fix missing .Pass.Should().BeTrue() assertion in Amsterdam test fixtures (tests were silently passing without verifying results) - Remove dead Convert(TestEngineNewPayloadsJson[]) and all its helpers (CreateExecutionPayload, HexToNumber, ParseWithdrawal) — engine tests now go through IJsonRpcService directly - Cache reflection param count lookup in static dictionary - Remove empty SetUp() and unreachable _logManager ??= assignment - Remove comments that restate what code does * refactor: DelaySafe returns bool to indicate cancellation Callers can use `if (!await DelaySafe(...)) return;` instead of the separate `IsCancellationRequested` check after the delay. * fix: remove Setup() calls from runner projects after method removal * refactor: use CancelDisposeAndClear for CTS cleanup Replace manual Cancel+Dispose patterns with the existing CancellationTokenExtensions.CancelDisposeAndClear helper which handles thread-safety via Interlocked internally. * refactor: address automated code review feedback - Add descriptive NotSupportedException for missing engine methods instead of null-forgiving NullReferenceException - Improve genesisUsesTargetFork comment to explain why EIP-7928 requires target fork rules at genesis (BlockAccessListHash) commit a075b36 Author: Lukasz Rozmej <lukasz.rozmej@gmail.com> Date: Wed Mar 25 11:50:21 2026 +0100 chore: add fix-nethtest agent skill for EF test debugging (#10903) * chore: add fix-nethtest agent skill for debugging EF test failures Adds a /fix-nethtest slash command that automates the diagnostic workflow for failing Ethereum Foundation tests run with Nethermind.Test.Runner (nethtest). The skill auto-detects test type (state vs blockchain), runs with tracing, classifies the failure, and guides root cause analysis through the EVM/spec/test harness code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update .agents/skills/fix-nethtest/SKILL.md Co-authored-by: Gaurav Dhiman <newmanifold000@gmail.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Gaurav Dhiman <newmanifold000@gmail.com> commit ec00ffc Author: emmmm <155267286+eeemmmmmm@users.noreply.github.com> Date: Wed Mar 25 07:45:26 2026 -0300 fix: remove duplicate NewHeadBlock unsubscription in PoSSwitcher (#10938) commit a429d12 Author: Amirul Ashraf <asdacap@gmail.com> Date: Wed Mar 25 17:14:34 2026 +0800 Migrate TxGossipPolicy to DI using OrderedComponents (#10941) * refactor: migrate TxGossipPolicy to DI using OrderedComponents Move ITxGossipPolicy registration from manual init-step wiring (api.TxGossipPolicy.Policies.Add) to DI modules using AddLast<ITxGossipPolicy> for ordered registration and CompositeTxGossipPolicy as the composite wrapper. Remove IApiWithBlockchain.TxGossipPolicy so any remaining usage fails compilation. HiveModule overrides the composite with an empty instance to preserve hive test behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: address review feedback for TxGossipPolicy DI migration - Add AddLast<T, TImpl> and AddFirst<T, TImpl> overloads to OrderedComponentsContainerBuilderExtensions for auto-resolution (avoids ctx.Resolve anti-pattern) - Make AddDecorator<T, TDecorator> OrderedComponents-aware: when ordered components exist for T, registers the decorator as T taking T[] from the ordered collection - Add OrderedComponents<T>.Clear() and ClearOrderedComponents<T>() DSL for plugins that need to disable all ordered policies (Hive) - Move CompositeTxGossipPolicy to constructor injection in InitializeBlockchain and all subclasses (AuRa, Xdc, Optimism, Taiko) - Remove redundant composite registration from PseudoNetworkModule (NetworkModule already handles it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add anti-pattern section to DI patterns rules Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use AddComposite instead of AddDecorator for OrderedComponents AddComposite is the correct pattern for wrapping multiple ordered policies into a single ITxGossipPolicy. Make AddComposite OrderedComponents-aware: when ordered components exist for T, register TComposite as T via RegisterType (not RegisterComposite) so it receives T[] from the ordered collection. Guard against double registration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: resolve ITxGossipPolicy instead of CompositeTxGossipPolicy Consumers should depend on the interface, not the concrete composite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use separate AddCompositeOrderedComponents DSL - Simplify AddLast<T, TImpl> and AddFirst<T, TImpl> to call the existing factory overload with a closure (avoids duplicating logic) - Add AddCompositeOrderedComponents<T, TComposite> as a separate DSL in OrderedComponentsContainerBuilderExtensions. This is separate from AddComposite because it uses RegisterType (not RegisterComposite) to receive T[] from OrderedComponents, and relaxes the safety check to allow this single T registration. - Revert AddComposite in ContainerBuilderExtensions to its original form - Remove SingleInstance from composite registration - Add unit tests verifying composite is resolved as T even when AddComposite is not the last registration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: test should call AddCompositeOrderedComponents before AddLast Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use Lazy<ITxGossipPolicy> to avoid eager DI resolution in init steps Resolving ITxGossipPolicy eagerly in InitializeBlockchain's constructor triggers the full sync dependency chain (SyncedTxGossipPolicy → MultiSyncModeSelector → ... → IBackgroundTaskScheduler) before the step has run. Use Lazy<ITxGossipPolicy> to defer resolution until CreateTxPool, when all dependencies are available. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add comment explaining why ITxGossipPolicy is Lazy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: add InvalidTxReceived disconnect reason Replace DisconnectReason.Other with a dedicated InvalidTxReceived reason in TxFloodController for peers sending invalid transactions. Maps to EthDisconnectReason.Other to preserve existing behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: make CompositeTxGossipPolicy resolve policies lazily Move laziness into the composite itself: take Lazy<ITxGossipPolicy[]> so the ordered components (and their dependency chains) are only resolved when a gossip check is first made at runtime, not during DI construction. This avoids the eager chain SyncedTxGossipPolicy → ISyncModeSelector → ... → IBackgroundTaskScheduler which isn't available during init step construction. Revert Lazy<ITxGossipPolicy> from InitializeBlockchain and all subclasses — no longer needed with lazy composite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update src/Nethermind/Nethermind.Core.Test/Container/OrderedComponentsTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .agents/rules/di-patterns.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> commit 84e328f Author: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Date: Tue Mar 24 14:19:36 2026 +0100 Reinstate removed `IGasPolicy` methods and apply for eip-8037 (#10897) * Reinstate removed `IGasPolicy` methods and apply for eip-8037 * Simplify ConsumeStorageWrite * Fix ConsumeStorageWrite * Refactor ConsumeStorageWrite * Fix whitespace * bit more readable * Revert "bit more readable" This reverts commit d08decc. * Remove duplicated code --------- Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com> commit 44f65f8 Author: Amirul Ashraf <asdacap@gmail.com> Date: Tue Mar 24 17:45:38 2026 +0800 Connection reset metric (#10935) commit 5a6c779 Author: Amirul Ashraf <asdacap@gmail.com> Date: Tue Mar 24 09:46:36 2026 +0800 fix(flat): periodically clear ReadOnlySnapshotBundle cache (#10922) * fix(flat): periodically clear ReadOnlySnapshotBundle cache to prevent stale readers The snapshot bundle cache was only cleared on compaction/persistence events. If persistence stalled, old entries held RefCountingPersistenceReader leases indefinitely, preventing database compaction. Add a 15-second periodic timer to force-clear stale cache entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add unit test for periodic bundle cache clearing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: iterate ConcurrentDictionary directly instead of copying keys Address PR review feedback: use TryRemove while iterating the ConcurrentDictionary directly, avoiding the temporary key list copy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: remove IsEmpty check as it acquires all bucket locks Address review feedback: ConcurrentDictionary.IsEmpty acquires all bucket locks, making it more expensive than just iterating directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> commit d9e819e Author: Ben {chmark} Adams <thundercat@illyriad.co.uk> Date: Mon Mar 23 22:42:46 2026 +0000 test: update pyspec fixtures to v5.4.0/v5.5.1 and adapt to forked release layout (#10931) * Adapt pyspec fixtures to forked release layout * Fix Amsterdam SSTORE gas ordering * fix: add missing usings for TestItem and InsertCode extension in Eip8037 test * Restore prestate for invalid access-list state tests * fix: add missing usings and fix Ether extension in pre-Berlin access list test * hmm * fix: detect AccessList tx type by field presence, not list emptiness JsonToEthereumTest.Convert set TxType.AccessList only when the built access list was non-empty. Pyspec fixtures with empty accessLists: [[]] were misclassified as legacy txs, so pre-Berlin rejection didn't fire and the post-state root diverged. Check whether the accessLists/accessList JSON field was present rather than whether the parsed list has entries. Rebuild regression test programmatically using the expected hash from pyspec fixture test_eip2930_tx_validity[fork_Istanbul-invalid-state_test]. * test: add Convert regression test for empty accessLists field detection commit 057441c Author: Gaurav Dhiman <newmanifold000@gmail.com> Date: Tue Mar 24 04:02:29 2026 +0530 Fix buffer leak tests to use PooledBufferLeakDetector (#10887) commit ae8e0ee Author: Tomass <155266802+zeroprooff@users.noreply.github.com> Date: Mon Mar 23 22:32:03 2026 +0000 Remove duplicate assertion in SnapshotCompactorTests (#10923) Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> commit d4214dd Author: Marc <Marchhill@users.noreply.github.com> Date: Mon Mar 23 17:59:29 2026 +0000 Gnosis Osaka (#10906) osaka gnosis config Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> commit 4228cb3 Author: Alexey Osipov <me@flcl.me> Date: Mon Mar 23 20:50:00 2026 +0300 Dispose on exception (#10921) * Dispose more * Dispose in rare case * Catch more; cleanup --------- Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> commit 871e9c7 Author: Marc <Marchhill@users.noreply.github.com> Date: Mon Mar 23 16:20:47 2026 +0000 Fix AuRaMergeEngineModuleTests (#10872) # Conflicts: # src/Nethermind/Ethereum.Blockchain.Pyspec.Test/Amsterdam/AmsterdamTestFixture.cs # src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PyspecTestFixture.cs # src/Nethermind/Ethereum.Transaction.Test/TransactionJsonTest.cs # src/Nethermind/Nethermind.Evm/Instructions/EvmInstructions.Storage.cs # src/Nethermind/Nethermind.State.Flat/Persistence/RefCountingPersistenceReader.cs
benaadams
added a commit
that referenced
this pull request
Mar 27, 2026
commit 5cdc84a Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 04:17:44 2026 +0000 Don't throw in shutdown commit 00922cb Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 04:10:29 2026 +0000 fix commit dbe10db Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 03:48:23 2026 +0000 fixes commit 797a4ab Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 03:29:39 2026 +0000 fix: add canonical chain validation to startup repair and promote non-canonical persisted boundary - Startup reconciliation now also checks that the restored block is canonical; a non-canonical block matching the exact persisted state triggers the repair path - Add PromoteStartupBoundaryToMainChain to swap the persisted-state block into the main chain position when its state matches but its chain level markers are stale - Replace FindBlockByStateRoot (scanned all blocks at level) with FindCanonicalBlockByStateRoot (checks canonical block only) to avoid restoring the wrong fork - Remove FindRecoverableCanonicalBoundary backward walk — repair now fails loudly if no canonical match exists at the persisted height - Extract FlatPersistedStateInfoProvider from FlatDbManager into its own class, registered via DI instead of Bind<> - Harden StartupBlockTreeFixer to only suggest blocks descending from the repaired head using eligible parent hash tracking per level - Update tests for canonical validation, promotion, and per-level fixer suggestion filtering commit c9f800f Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 02:35:31 2026 +0000 fix: use named argument for genesisBlockNumber in XdcBlockTree base call The new optional persistedStateInfoProvider parameter on BlockTree's constructor sits before genesisBlockNumber, causing a type mismatch when genesisBlockNumber was passed positionally. commit 24e0f91 Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 02:32:56 2026 +0000 simplify: reduce duplication and fix minor issues in FlatDB stall fix - Extract CreateTreeWithForkAtBlock2 helper to eliminate 7x repeated block tree setup in BlockTreeTests (net -74 lines) - Extract SetupSaturatedCompactorQueue helper for backpressure tests - Replace StubPersistedStateInfoProvider with NSubstitute factory - Expose metadata address constants as internal for test use instead of inline byte array reconstruction - Move BestPersistedState setter batch guard to avoid empty write batch allocation when value is null - Reuse captured persistedStateInfoValue in startup log instead of re-querying the provider - Replace flaky Task.Delay(250) with polling loop in AddressWarmer test commit 7333545 Author: Ben Adams <thundercat@illyriad.co.uk> Date: Fri Mar 27 02:07:46 2026 +0000 fix: prevent FlatDB live stall and make restart recovery exact - Replace unbounded producer-side wait in FlatDbManager.AddSnapshot with inline drain fallback when compactor queue is saturated - Add processing watchdog, startup diagnostics, and backpressure metrics - Harden AddressWarmer.Wait, WaitAndClear, and tx execution cancellation with periodic warnings instead of silent unbounded blocking - Persist exact boundary hash metadata alongside block number for startup - Add startup reconciliation that validates restored head against FlatDB persisted StateId and repairs stale metadata before setting Head - Enrich the max-branch-size exception with persisted-boundary diagnostics - Add IPersistedStateInfoProvider seam for cross-project startup visibility - Add regression tests for backpressure, exact restore, fixer, force-persist, and end-to-end restart recovery commit 2275710 Author: Tomass <155266802+zeroprooff@users.noreply.github.com> Date: Thu Mar 26 07:49:30 2026 +0000 Fix incorrect NodeType in TrieNodeTests (#10917) commit 707affd Author: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Date: Thu Mar 26 08:48:19 2026 +0100 Extend `CallResult` constructor visibility (#10949) Extend constructor visibility commit a791dc6 Author: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Date: Thu Mar 26 08:48:00 2026 +0100 New constructor (#10950) commit e14c4f7 Author: Amirul Ashraf <asdacap@gmail.com> Date: Thu Mar 26 08:39:16 2026 +0800 perf: add fast MVCC snapshots for MemDb in FlatDb tests (#10792) * feat: add MVCC snapshot support to MemDb for fast test snapshots Implements SnapshotableMemDb and SnapshotableMemColumnsDb with O(1) snapshot creation using Multi-Version Concurrency Control (MVCC), replacing the O(n) full-copy approach when snapshots are needed. Key features: - O(1) snapshot creation by capturing version numbers - Multiple concurrent snapshots with full isolation - Automatic version garbage collection on snapshot disposal - ISortedKeyValueStore support for sorted iteration - Thread-safe with proper locking This enables efficient snapshot-based testing, particularly for FlatDb tests where snapshots are created frequently. The new classes are drop-in replacements for MemDb/MemColumnsDb when snapshot support is required. All 21 unit tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * chore: migrate FlatDb tests to use SnapshotableMemColumnsDb Replace TestMemColumnsDb with SnapshotableMemColumnsDb in FlatDb test infrastructure to enable fast O(1) MVCC snapshots instead of slow O(n) full database copies. Changes: - PseudoNethermindModule: Register SnapshotableMemColumnsDb for FlatDbColumns - FlatTrieVerifierTests: Update field type and helper method casts to use IDb interface All tests passing (22/22 FlatTrieVerifierTests). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: use primary constructors and optimize write batches - Convert all constructors to primary constructor syntax (C# 12) - Add dedicated MemDbWriteBatch that locks only once during commit - Replace InMemoryWriteBatch with optimized batch that collects operations and commits atomically Performance improvement: - Before: Each Set() in batch acquired lock individually - After: Single lock acquisition for entire batch commit All tests passing (54/54). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * perf: use ArrayPoolList in MemDbWriteBatch to reduce GC pressure Replace List with ArrayPoolList to use pooled arrays instead of allocating new arrays for each write batch. Changes: - Replace List<...> with ArrayPoolList<...> (initial capacity: 16) - Dispose ArrayPoolList to return arrays to pool - Restructure Dispose() to ensure proper cleanup Performance impact: Reduces GC allocations for write batch operations. All tests passing (54/54). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: Change SnapshottedMemDb from Dictionary<byte[], Version> to Dictionary<(byte[], int), byte[]> * fix: Add neverPrune option to SnapshotableMemDb to avoid pruning bug in tests Add neverPrune constructor parameter to SnapshotableMemDb and SnapshotableMemColumnsDb to disable version pruning. Enable this option in PseudoNethermindModule for tests to work around a bug in PruneVersionsOlderThan that removes versions still needed by active snapshots. The pruning bug will be fixed in a separate PR. For tests, memory is not a concern and disabling pruning is the simplest workaround. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Initialize neverPrune before creating column databases The previous constructor chain set _neverPrune AFTER calling GetColumnDb, which meant all SnapshotableMemDb instances were created with neverPrune=false even when neverPrune=true was passed to the constructor. Fixed by creating a private constructor that sets _neverPrune before the GetColumnDb loop, and routing all public constructors through it. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Critical thread-safety bugs in SnapshotableMemDb Fixed three critical race conditions: 1. MemDbSortedView.MoveNext() and CurrentValue accessed _db without lock - Changed GetValueAtVersion() calls to GetAtVersion() which acquires lock - Prevents concurrent modification exceptions when iterating while writing 2. GetViewBetween() read _currentVersion without lock - Now captures version inside lock for consistency These race conditions could cause exceptions or incorrect results when accessing the database from multiple threads concurrently. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: Remove Reverse() allocations and fix PruneVersionsOlderThan bug Replace SortedDictionary.Reverse() with forward iteration in GetValueAtVersion and KeepOnlyLatestVersions to avoid O(n) intermediate allocations. Add single-pass FindFirstKeyAtVersion/FindLastKeyAtVersion helpers. Rewrite MemDbSortedView to iterate _db directly instead of buffering all keys. Use ArrayPoolList in pruning methods. Fix PruneVersionsOlderThan which incorrectly removed all entries below minVersion — now keeps the latest pre-minVersion entry per key so active snapshots can still resolve those keys. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * perf: Switch SnapshotableMemDb from SortedDictionary to SortedSet with GetViewBetween Restructure the internal data store from SortedDictionary<(byte[], int), byte[]?> to SortedSet<(byte[], int, byte[]?)> to leverage GetViewBetween for O(log n) point lookups and efficient range iteration, fixing O(n) linear scan in GetValueAtVersion and O(n²) re-scanning in MemDbSortedView.MoveNext(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * perf: Simplify FindFirst/LastKeyAtVersion using GetValueAtVersion FindFirstKeyAtVersion now delegates to O(log n) GetValueAtVersion per unique key. FindLastKeyAtVersion iterates in reverse to return early instead of scanning the entire set forward. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review comments on SnapshotableMemDb - Fix double-counting reads in bulk key accessor by using GetValueAtVersion directly under a single lock instead of calling Get() which re-increments - Materialize GetAll/GetAllKeys/GetAllValues/GetAllAtVersion results under lock to prevent yield-while-holding-lock deadlock risk - Prune old versions on write when no snapshots are active to prevent unbounded memory growth in the non-snapshot case - Fix StartBefore to return true when key < firstKey so MoveNext correctly yields the first element Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: Address PR review - use Lock type and Dictionary instead of IDictionary Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> commit fbf3aff Author: Ben {chmark} Adams <thundercat@illyriad.co.uk> Date: Thu Mar 26 00:17:29 2026 +0000 test: add engine blockchain pyspec fixtures and fix BAL gas budget tracking (#10939) * feat: add engine blockchain test fixtures and fix BAL gas budget tracking - Add PyspecEngineBlockchainTestFixture base class for engine blockchain tests (blockchain_tests_engine directory) - Add engine blockchain test classes for all forks (Frontier through Osaka) - Add Amsterdam engine blockchain test fixtures for all Amsterdam EIPs - Fix BAL validation gas budget: use BlockGasUsed instead of SpentGas (SpentGas includes execution gas, BlockGasUsed is the block-level gas accounting that BAL validation should track) - Add regression test verifying BAL validation budget uses block gas * fix: version-aware ExecutionPayload creation and engine test improvements - Create version-appropriate ExecutionPayload type based on newPayloadVersion (V3 for version >= 3, V4 for version >= 5, base for older) - Only set V3/V4 fields on matching payload types (blob gas, BAL, slots) - Add missing NUnit using in Tests.cs - Use ExecutionPayload base type in BlockchainTestBase for broader compat - Add Convert_engine_payloads_uses_declared_payload_version test - Use Should().Equal instead of BeEquivalentTo for ordered list assertion * fix: restructure CreateExecutionPayload to explicit switch on derived type - Switch on most-derived first (V4 then V3) instead of two separate if-checks that both fire for V4 - makes the mutual exclusivity explicit - Move ParentBeaconBlockRoot to base properties (it lives on ExecutionPayload) - Set ExecutionRequests = [] for V3 payloads too (engine_newPayloadV4 passes executionRequests as a parameter) * fix: only set ExecutionRequests=[] on V4 payloads, not V3 - caused 'Execution requests must be set' failures * refactor: use two if-checks instead of switch to avoid duplicating V3 blob gas fields * refactor: add HexToNumber/HexToNullableNumber helpers, document version mapping Address PR feedback: - Extract HexToNumber<T> and HexToNullableNumber<T> helpers to replace repeated (ulong)Bytes.FromHexString(...).ToUnsignedBigInteger() casts - Add comment documenting the version-to-type mapping source of truth (engine API method signatures in EngineRpcModule.*.cs) * fix: handle both JSON object and RLP hex string formats for withdrawals in engine test fixtures * fix: default ExecutionRequests to empty array for V4 engine tests on ExecutionPayloadV3 * fix: parse executionRequests from params[3] for engine_newPayloadV4/V5 params[3] is executionRequests for V4+ (Prague/Amsterdam), not validationError. The code was deserializing actual consolidation and withdrawal requests as validation errors, causing blocks to be processed with empty requests and producing wrong state roots. For V3: params = [payload, blobHashes, parentBeaconRoot, ?validationError] For V4+: params = [payload, blobHashes, parentBeaconRoot, executionRequests, ?validationError] * fixes * fix * feedback * fix: remove pre-merge engine test fixtures to fix CI timeout The engine blockchain test classes for Frontier through Paris were causing 20-minute CI timeouts. Pre-merge forks don't use the Engine API, and the regular BlockchainTests already cover them. Keep only Cancun+ where newPayload versions differ (blobs, execution requests, BAL). * fix: remove brittle hardcoded engine test name The Engine_from_state_test_on_top_of_genesis_should_not_sync test used .Single() with a hardcoded fixture name from v5.0.0. The name changed in v5.4.0, causing "Sequence contains no matching element". The same test case runs via the generic TestCaseSource fixture. * fix: add timeout to ProcessHelper.RunAndReadOutput WaitForExit with a 5s timeout before ReadToEnd prevents the static constructor of RuntimeInformation from blocking indefinitely when wmic hangs under heavy parallel test load. Returns null on timeout so PhysicalCoreCount falls back to Environment.ProcessorCount. * fix: restrict engine and Amsterdam tests to Linux only Engine blockchain tests and Amsterdam tests are heavy (full DI + Engine API per test, separate fixture download). They exceed the 20-minute CI timeout on Windows/macOS/ARM runners. Restrict to Linux where they complete within budget. * refactor: route engine tests through JsonRpcService instead of reflection Replace custom deserialization + Activator.CreateInstance + MethodInfo reflection with IJsonRpcService.SendRequestAsync. The test fixture params are passed as raw JSON through the real RPC pipeline, which handles method resolution and parameter binding. This removes the need for ParamsExecutionPayload intermediate types and per-version parameter marshalling in the test path. * fix: BAL memory leak and shared reference bug Two bugs in block-level access list handling: 1. TracingEnabled was set to true once and never reset. Every subsequent block (including non-BAL blocks) accumulated AccountChanges, StorageChanges, and Change stack entries without bound. 2. GeneratedBlockAccessList was a singleton reference assigned to Block objects via SetBlockAccessList, then Clear()ed on the next block. Previous blocks lost their BAL data. Fix: reset TracingEnabled per block based on the spec, and allocate a fresh BlockAccessList per block instead of reusing/clearing. * fix: skip heavy engine/Amsterdam tests in checked and no-intrinsics CI Add TEST_SKIP_HEAVY env var to the checked/no-intrinsics workflow. CiRunnerGuard checks this to unconditionally skip engine and Amsterdam blockchain tests in variant builds that don't benefit from running them (the main workflow already covers correctness). * Single spec look up * fix: cancel uncancelled Task.Delays that prevent GC of disposed containers RefCountingPersistenceReader: Task.Delay(60_000) in a loop with no cancellation token kept DB snapshots alive for 60s after scope disposal. With 32 concurrent tests, ~120 zombie snapshots accumulated at steady state. Fix: cancel via CTS on CleanUp. GCKeeper: Task.Delay(postBlockDelayMs) without cancellation held the GCKeeper closure alive after container disposal. Fix: add IDisposable, cancel via CTS on Dispose. * fix: guard GCKeeper.Dispose against double-dispose of CTS CancellationTokenSource.Cancel() throws ObjectDisposedException if the CTS was already disposed by a prior Dispose call (Autofac can call Dispose multiple times). Guard with try/catch. * refactor: address PR review feedback - Use reflection to get newPayload param count from IEngineRpcModule instead of hardcoding version-dependent logic - Use EngineApiVersions.Latest for default version fallbacks - Add Latest constants to EngineApiVersions.Fcu and NewPayload - Move GeneratedBlockAccessList reset into LoadSuggestedBlockAccessList so callers don't need to remember to create a new instance * fix: handle RPC-level errors in engine test negative cases When the Engine API rejects a payload at the RPC layer (e.g. wrong payload version like "ExecutionPayloadV2 expected"), it returns a JsonRpcErrorResponse instead of a PayloadStatusV1 with INVALID status. For negative tests (validationError is set), this is valid — the engine correctly rejected the block. Continue to the next payload instead of failing the assertion. * fix: dispose PeriodicTimer instances to prevent resource leaks DiscoveryApp, DiscoveryPersistenceManager, MultiSyncModeSelector, RetryCache: add using to PeriodicTimer so it is disposed when the loop exits. SessionMonitor: dispose _pingTimer on stop. * fix: cancel MultiSyncModeSelector timer on Dispose MultiSyncModeSelector starts a PeriodicTimer loop (1ms interval in tests) in its constructor. Dispose() only disposed the CTS without cancelling it first, so the timer loop ran forever, holding the entire container graph alive through the async state machine closure. This caused 10k+ zombie containers accumulating over the test run. * fix: skip heavy engine/Amsterdam tests in Flat DB CI jobs * perf: use NullTxPool and disable prewarmer in blockchain tests Blockchain pyspec tests process pre-built blocks from fixtures — they don't need a real TxPool (32 GB allocation per full run) or prewarmer. Use NullTxPool.Instance and set PreWarmStateConcurrency=0. * perf: disable prewarmer and revert ProcessHelper to original Set PreWarmStateOnBlockProcessing=false before module registration (Intercept runs too late). Revert ProcessHelper.RunAndReadOutput to original ReadToEnd-then-WaitForExit pattern — the reversed order caused deadlock on Linux when /proc/cpuinfo output exceeded pipe buffer size. * refactor: address PR review feedback on Task.Delay and Dispose patterns - Add TaskExtensions.DelaySafe helper for cancellable delays that return instead of throwing OperationCanceledException - Use DelaySafe in GCKeeper, TrieStore, RefCountingPersistenceReader, DataFeed - Guard GCKeeper.Dispose and MultiSyncModeSelector.Dispose with Interlocked.Exchange to prevent double-dispose - Add comment on SortedDictionary in BlockAccessList (pre-existing, count check on line 40 handles mismatched entry sets) * chore: remove codex-dotnet.ps1 and AGENTS.md changes * refactor: remove Convert engine payload tests These tests only verified JsonToEthereumTest.Convert which is no longer used in the engine test path (replaced by IJsonRpcService). Remove tests, helper method, and unused imports. * refactor: simplify after code review - Fix missing .Pass.Should().BeTrue() assertion in Amsterdam test fixtures (tests were silently passing without verifying results) - Remove dead Convert(TestEngineNewPayloadsJson[]) and all its helpers (CreateExecutionPayload, HexToNumber, ParseWithdrawal) — engine tests now go through IJsonRpcService directly - Cache reflection param count lookup in static dictionary - Remove empty SetUp() and unreachable _logManager ??= assignment - Remove comments that restate what code does * refactor: DelaySafe returns bool to indicate cancellation Callers can use `if (!await DelaySafe(...)) return;` instead of the separate `IsCancellationRequested` check after the delay. * fix: remove Setup() calls from runner projects after method removal * refactor: use CancelDisposeAndClear for CTS cleanup Replace manual Cancel+Dispose patterns with the existing CancellationTokenExtensions.CancelDisposeAndClear helper which handles thread-safety via Interlocked internally. * refactor: address automated code review feedback - Add descriptive NotSupportedException for missing engine methods instead of null-forgiving NullReferenceException - Improve genesisUsesTargetFork comment to explain why EIP-7928 requires target fork rules at genesis (BlockAccessListHash) commit a075b36 Author: Lukasz Rozmej <lukasz.rozmej@gmail.com> Date: Wed Mar 25 11:50:21 2026 +0100 chore: add fix-nethtest agent skill for EF test debugging (#10903) * chore: add fix-nethtest agent skill for debugging EF test failures Adds a /fix-nethtest slash command that automates the diagnostic workflow for failing Ethereum Foundation tests run with Nethermind.Test.Runner (nethtest). The skill auto-detects test type (state vs blockchain), runs with tracing, classifies the failure, and guides root cause analysis through the EVM/spec/test harness code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update .agents/skills/fix-nethtest/SKILL.md Co-authored-by: Gaurav Dhiman <newmanifold000@gmail.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Gaurav Dhiman <newmanifold000@gmail.com> commit ec00ffc Author: emmmm <155267286+eeemmmmmm@users.noreply.github.com> Date: Wed Mar 25 07:45:26 2026 -0300 fix: remove duplicate NewHeadBlock unsubscription in PoSSwitcher (#10938) commit a429d12 Author: Amirul Ashraf <asdacap@gmail.com> Date: Wed Mar 25 17:14:34 2026 +0800 Migrate TxGossipPolicy to DI using OrderedComponents (#10941) * refactor: migrate TxGossipPolicy to DI using OrderedComponents Move ITxGossipPolicy registration from manual init-step wiring (api.TxGossipPolicy.Policies.Add) to DI modules using AddLast<ITxGossipPolicy> for ordered registration and CompositeTxGossipPolicy as the composite wrapper. Remove IApiWithBlockchain.TxGossipPolicy so any remaining usage fails compilation. HiveModule overrides the composite with an empty instance to preserve hive test behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: address review feedback for TxGossipPolicy DI migration - Add AddLast<T, TImpl> and AddFirst<T, TImpl> overloads to OrderedComponentsContainerBuilderExtensions for auto-resolution (avoids ctx.Resolve anti-pattern) - Make AddDecorator<T, TDecorator> OrderedComponents-aware: when ordered components exist for T, registers the decorator as T taking T[] from the ordered collection - Add OrderedComponents<T>.Clear() and ClearOrderedComponents<T>() DSL for plugins that need to disable all ordered policies (Hive) - Move CompositeTxGossipPolicy to constructor injection in InitializeBlockchain and all subclasses (AuRa, Xdc, Optimism, Taiko) - Remove redundant composite registration from PseudoNetworkModule (NetworkModule already handles it) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add anti-pattern section to DI patterns rules Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use AddComposite instead of AddDecorator for OrderedComponents AddComposite is the correct pattern for wrapping multiple ordered policies into a single ITxGossipPolicy. Make AddComposite OrderedComponents-aware: when ordered components exist for T, register TComposite as T via RegisterType (not RegisterComposite) so it receives T[] from the ordered collection. Guard against double registration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: resolve ITxGossipPolicy instead of CompositeTxGossipPolicy Consumers should depend on the interface, not the concrete composite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: use separate AddCompositeOrderedComponents DSL - Simplify AddLast<T, TImpl> and AddFirst<T, TImpl> to call the existing factory overload with a closure (avoids duplicating logic) - Add AddCompositeOrderedComponents<T, TComposite> as a separate DSL in OrderedComponentsContainerBuilderExtensions. This is separate from AddComposite because it uses RegisterType (not RegisterComposite) to receive T[] from OrderedComponents, and relaxes the safety check to allow this single T registration. - Revert AddComposite in ContainerBuilderExtensions to its original form - Remove SingleInstance from composite registration - Add unit tests verifying composite is resolved as T even when AddComposite is not the last registration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: test should call AddCompositeOrderedComponents before AddLast Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use Lazy<ITxGossipPolicy> to avoid eager DI resolution in init steps Resolving ITxGossipPolicy eagerly in InitializeBlockchain's constructor triggers the full sync dependency chain (SyncedTxGossipPolicy → MultiSyncModeSelector → ... → IBackgroundTaskScheduler) before the step has run. Use Lazy<ITxGossipPolicy> to defer resolution until CreateTxPool, when all dependencies are available. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add comment explaining why ITxGossipPolicy is Lazy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: add InvalidTxReceived disconnect reason Replace DisconnectReason.Other with a dedicated InvalidTxReceived reason in TxFloodController for peers sending invalid transactions. Maps to EthDisconnectReason.Other to preserve existing behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: make CompositeTxGossipPolicy resolve policies lazily Move laziness into the composite itself: take Lazy<ITxGossipPolicy[]> so the ordered components (and their dependency chains) are only resolved when a gossip check is first made at runtime, not during DI construction. This avoids the eager chain SyncedTxGossipPolicy → ISyncModeSelector → ... → IBackgroundTaskScheduler which isn't available during init step construction. Revert Lazy<ITxGossipPolicy> from InitializeBlockchain and all subclasses — no longer needed with lazy composite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Update src/Nethermind/Nethermind.Core.Test/Container/OrderedComponentsTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update .agents/rules/di-patterns.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> commit 84e328f Author: Damian Orzechowski <114909782+damian-orzechowski@users.noreply.github.com> Date: Tue Mar 24 14:19:36 2026 +0100 Reinstate removed `IGasPolicy` methods and apply for eip-8037 (#10897) * Reinstate removed `IGasPolicy` methods and apply for eip-8037 * Simplify ConsumeStorageWrite * Fix ConsumeStorageWrite * Refactor ConsumeStorageWrite * Fix whitespace * bit more readable * Revert "bit more readable" This reverts commit d08decc. * Remove duplicated code --------- Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com> commit 44f65f8 Author: Amirul Ashraf <asdacap@gmail.com> Date: Tue Mar 24 17:45:38 2026 +0800 Connection reset metric (#10935) commit 5a6c779 Author: Amirul Ashraf <asdacap@gmail.com> Date: Tue Mar 24 09:46:36 2026 +0800 fix(flat): periodically clear ReadOnlySnapshotBundle cache (#10922) * fix(flat): periodically clear ReadOnlySnapshotBundle cache to prevent stale readers The snapshot bundle cache was only cleared on compaction/persistence events. If persistence stalled, old entries held RefCountingPersistenceReader leases indefinitely, preventing database compaction. Add a 15-second periodic timer to force-clear stale cache entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add unit test for periodic bundle cache clearing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: iterate ConcurrentDictionary directly instead of copying keys Address PR review feedback: use TryRemove while iterating the ConcurrentDictionary directly, avoiding the temporary key list copy. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: remove IsEmpty check as it acquires all bucket locks Address review feedback: ConcurrentDictionary.IsEmpty acquires all bucket locks, making it more expensive than just iterating directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> commit d9e819e Author: Ben {chmark} Adams <thundercat@illyriad.co.uk> Date: Mon Mar 23 22:42:46 2026 +0000 test: update pyspec fixtures to v5.4.0/v5.5.1 and adapt to forked release layout (#10931) * Adapt pyspec fixtures to forked release layout * Fix Amsterdam SSTORE gas ordering * fix: add missing usings for TestItem and InsertCode extension in Eip8037 test * Restore prestate for invalid access-list state tests * fix: add missing usings and fix Ether extension in pre-Berlin access list test * hmm * fix: detect AccessList tx type by field presence, not list emptiness JsonToEthereumTest.Convert set TxType.AccessList only when the built access list was non-empty. Pyspec fixtures with empty accessLists: [[]] were misclassified as legacy txs, so pre-Berlin rejection didn't fire and the post-state root diverged. Check whether the accessLists/accessList JSON field was present rather than whether the parsed list has entries. Rebuild regression test programmatically using the expected hash from pyspec fixture test_eip2930_tx_validity[fork_Istanbul-invalid-state_test]. * test: add Convert regression test for empty accessLists field detection commit 057441c Author: Gaurav Dhiman <newmanifold000@gmail.com> Date: Tue Mar 24 04:02:29 2026 +0530 Fix buffer leak tests to use PooledBufferLeakDetector (#10887) commit ae8e0ee Author: Tomass <155266802+zeroprooff@users.noreply.github.com> Date: Mon Mar 23 22:32:03 2026 +0000 Remove duplicate assertion in SnapshotCompactorTests (#10923) Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> commit d4214dd Author: Marc <Marchhill@users.noreply.github.com> Date: Mon Mar 23 17:59:29 2026 +0000 Gnosis Osaka (#10906) osaka gnosis config Co-authored-by: Marc Harvey-Hill <10379486+Marchhill@users.noreply.github.com> commit 4228cb3 Author: Alexey Osipov <me@flcl.me> Date: Mon Mar 23 20:50:00 2026 +0300 Dispose on exception (#10921) * Dispose more * Dispose in rare case * Catch more; cleanup --------- Co-authored-by: Ben {chmark} Adams <thundercat@illyriad.co.uk> commit 871e9c7 Author: Marc <Marchhill@users.noreply.github.com> Date: Mon Mar 23 16:20:47 2026 +0000 Fix AuRaMergeEngineModuleTests (#10872)
stdevMac
pushed a commit
that referenced
this pull request
Mar 30, 2026
* Reinstate removed `IGasPolicy` methods and apply for eip-8037 * Simplify ConsumeStorageWrite * Fix ConsumeStorageWrite * Refactor ConsumeStorageWrite * Fix whitespace * bit more readable * Revert "bit more readable" This reverts commit d08decc. * Remove duplicated code --------- Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
16 tasks
stdevMac
pushed a commit
that referenced
this pull request
Apr 1, 2026
* Reinstate removed `IGasPolicy` methods and apply for eip-8037 * Simplify ConsumeStorageWrite * Fix ConsumeStorageWrite * Refactor ConsumeStorageWrite * Fix whitespace * bit more readable * Revert "bit more readable" This reverts commit d08decc. * Remove duplicated code --------- Co-authored-by: lukasz.rozmej <lukasz.rozmej@gmail.com>
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
5 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 Closes Resolves #
PR #10697 removed some
IGasPolicymethods which when overriden can be used to apply muli-dimentional gas tracking (e.g. Arbitrum). Replacing them with genericUpdateGasCostmethods removes the context of the gas cost and makes it difficult to attribute correct gas type.Changes
These changes re-add removed method and apply changes for EIP-8037 inside the gas policy:
ConsumeStorageWriteConsumeNewAccountCreationTypes of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Documentation
Requires documentation update
Requires explanation in Release Notes
Remarks
Required for 1.37.0 release of Arbitrum Plugin