Fix inaccurate snap sync and abstract state sync operation by asdacap · Pull Request #10469 · NethermindEth/nethermind

asdacap · 2026-02-09T13:12:27Z

Summary

Abstract various operation so that flat specific implementation can be added clearnly later.
- Add ISnapTrieFactory/ISnapTree abstractions enabling pluggable trie implementations for snap sync
- Refactor snap/state sync tests with ISnapServerContext, RemoteDbContext, IStateSyncTestOperation abstractions
Various snap sync fix related to double writes.
- Fix the edge of the snap ranges is inclusive which causes the edge entry to be processed twice. This does not work with flat.
- Harden edge cases where the proof can be saved twice. This works by intercepting the triestore commit not allowing nodes to be persisted when it subtree range exceed the expected limit.
- Change the way incomplete storage range is detected which is by checking if the root is persisted.
The fixes allow storage range split reliably.
- Have a small minimum remaining slots threshold where the storage split is active.
- Enable storage split by default.

Test plan

Full solution builds with 0 errors, 0 warnings
SnapSync tests: 80 passed, 2 skipped (expected - hash db)
StateSyncFeed tests: 826 passed
Core tests (ConcurrencyController, RefCounting, KeccakTests): 1,049 passed
No references to Nethermind.State.Flat in changed files
CI passes

🤖 Generated with Claude Code

Extract non-flat snap sync improvements from flat/snap-sync branch: - Add ISnapTrieFactory/ISnapTree abstractions for pluggable trie implementations - Add PatriciaSnap* implementations for Patricia trie-based snap sync - Add ITreeSyncStore/PatriciaTreeSyncStore for state sync storage - Improve SnapProviderHelper with better range handling and boundary proofs - Add core utilities (ConcurrencyController, ReadWriteLockBox, RefCountingDisposable) - Add TrieLeafIterator for efficient trie traversal - Refactor tests with ISnapServerContext, RemoteDbContext, and IStateSyncTestOperation - Improve test data with proper hash partitioning and parameterized tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove ConcurrencyController additions, ReadWriteLockBox, RefCountingDisposable, IWorldStateManager IDisposable change, TrieStoreScopeProvider refactoring, E2ESyncTests improvements, and most Trie changes. Keep only minimal Trie additions needed by snap sync: GetInlineNodeRlp and ToUpperBoundPath. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors snap/state sync to support pluggable trie/storage backends (e.g., future flat implementations) and introduces multiple fixes to prevent snap sync double-writes and improve range partition correctness (inclusive bounds, proof stitching, and storage range splitting behavior).

Changes:

Add new abstractions for snap sync (ISnapTrieFactory, ISnapTree) and state sync storage (ITreeSyncStore) with Patricia-backed implementations.
Fix snap sync range boundary handling to avoid double-processing/double-writes (inclusive limits, upper-bound commit filtering, proof stitching adjustments).
Refactor and expand sync test infrastructure to use the new abstractions and updated range semantics.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/Nethermind/Nethermind.Trie/TrieNode.cs	Adds helper to extract inline child node RLP for proof handling.
src/Nethermind/Nethermind.Trie/Pruning/TreePath.cs	Adds subtree upper-bound path helper for commit limiting.
src/Nethermind/Nethermind.Synchronization/Synchronizer.cs	Registers new snap/state sync abstractions in DI.
src/Nethermind/Nethermind.Synchronization/SnapSync/UnknownNodeResolver.cs	Adds resolver for proof-only decoding without DB lookups.
src/Nethermind/Nethermind.Synchronization/SnapSync/SnapUpperBoundAdapter.cs	Wraps trie store commits to enforce an upper bound and skip boundary proof nodes.
src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProviderHelper.cs	Refactors range commit/stitching logic; applies inclusive bound handling.
src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProvider.cs	Switches provider to factory-based trees and updated range semantics.
src/Nethermind/Nethermind.Synchronization/SnapSync/ProgressTracker.cs	Adjusts partition limits to be inclusive; updates storage split behavior & healing tracking.
src/Nethermind/Nethermind.Synchronization/SnapSync/PatriciaSnapTrieFactory.cs	Patricia implementation of `ISnapTrieFactory`.
src/Nethermind/Nethermind.Synchronization/SnapSync/PatriciaSnapStorageTree.cs	Patricia implementation of `ISnapTree` for storage tries.
src/Nethermind/Nethermind.Synchronization/SnapSync/PatriciaSnapStateTree.cs	Patricia implementation of `ISnapTree` for the state trie.
src/Nethermind/Nethermind.Synchronization/SnapSync/ISnapTrieFactory.cs	New interface for creating snap trees.
src/Nethermind/Nethermind.Synchronization/SnapSync/ISnapTree.cs	New interface for snap tree operations used during proof fill/commit.
src/Nethermind/Nethermind.Synchronization/SnapSync/AddRangeResult.cs	Renames empty slots result to `EmptyRange`.
src/Nethermind/Nethermind.Synchronization/FastSync/TreeSync.cs	Abstracts storage ops via `ITreeSyncStore`; changes finalization and verification flow.
src/Nethermind/Nethermind.Synchronization/FastSync/PatriciaTreeSyncStore.cs	Patricia implementation of the new `ITreeSyncStore`.
src/Nethermind/Nethermind.Synchronization/FastSync/ITreeSyncStore.cs	New abstraction for state sync storage + verification context.
src/Nethermind/Nethermind.Synchronization.Test/TestSynchronizerModule.cs	Updates test module wiring for new snap abstractions.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/TestSnapTrieFactory.cs	Test factory implementation for `ISnapTrieFactory`.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/SnapServerTest.cs	Refactors snap server tests to use an abstract server context + batching.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/SnapProviderTests.cs	Updates tests for new APIs and adds persistence coverage for storage ranges.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/RecreateStateFromStorageRangesTests.cs	Refactors to container-based setup and new result semantics.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/RecreateStateFromAccountRangesTests.cs	Refactors to container-based setup and helper-based assertions.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/ProgressTrackerTests.cs	Updates expected partition start/limit (inclusive) and slot splitting semantics.
src/Nethermind/Nethermind.Synchronization.Test/SnapSync/ISnapTestHelper.cs	Adds helper abstraction for trie-node DB assertions in tests.
src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedTestsBase.cs	Refactors remote/local DB setup into `RemoteDbContext` and test operations abstraction.
src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedTests.cs	Updates state sync feed tests to use new contexts and abstractions.
src/Nethermind/Nethermind.Synchronization.Test/FastSync/StateSyncFeedHealingTests.cs	Updates healing tests to use new snap factory + test operations abstraction.
src/Nethermind/Nethermind.Synchronization.Test/FastSync/LocalDbContext.cs	Implements local DB test operations used by state sync feed tests.
src/Nethermind/Nethermind.Synchronization.Test/FastSync/IStateSyncTestOperation.cs	Introduces abstraction for local test DB operations and comparisons.
src/Nethermind/Nethermind.State/StateTree.cs	Adjusts bulk setter to accept nullable accounts.
src/Nethermind/Nethermind.Core/Crypto/Hash256.cs	Adds increment/decrement helpers for inclusive/exclusive path progression.
src/Nethermind/Nethermind.Core/Collections/CollectionExtensions.cs	Adds dictionary add-or-update range helper.
src/Nethermind/Nethermind.Core.Test/TestMemDb.cs	Extends TestMemDb with sorted-view capabilities for tests.
src/Nethermind/Nethermind.Core.Test/TestMemColumnDb.cs	Tightens generic constraints and adds snapshot support in tests.
src/Nethermind/Nethermind.Core.Test/KeccakTests.cs	Adds tests for increment/decrement path helpers.
src/Nethermind/Nethermind.Core.Test/Builders/TestItem.Tree.cs	Updates storage slot test data encoding.
src/Nethermind/Nethermind.Blockchain/Synchronization/SyncConfig.cs	Enables storage range splitting by default.
src/Nethermind/Nethermind.Blockchain/Synchronization/ISyncConfig.cs	Updates default config metadata to match new default.

Comments suppressed due to low confidence (1)

src/Nethermind/Nethermind.Synchronization.Test/SnapSync/RecreateStateFromStorageRangesTests.cs:200

This assignment to rootFinished is useless, since its value is never read.

            (AddRangeResult result, bool moreChildrenToRight, Hash256 _, bool rootFinished) = SnapProviderHelper.AddStorageRange(
                factory,
                pathWithAccount,
                [
                    new PathWithStorageSlot(new ValueHash256("0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563"), Bytes.FromHexString("94654f75e491acf8c380d2a6906e67e2e56813665e")),
                ],
                Keccak.Zero,
                null,
                proofs: [
                    Bytes.FromHexString("f838a120290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e5639594654f75e491acf8c380d2a6906e67e2e56813665e"),
                    Bytes.FromHexString("f838a120290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e5639594654f75e491acf8c380d2a6906e67e2e56813665e"),
                ]);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProviderHelper.cs

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapUpperBoundAdapter.cs

src/Nethermind/Nethermind.Synchronization/FastSync/TreeSync.cs

src/Nethermind/Nethermind.Core.Test/TestMemDb.cs

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProviderHelper.cs

src/Nethermind/Nethermind.Synchronization/SnapSync/ProgressTracker.cs

src/Nethermind/Nethermind.Synchronization.Test/SnapSync/RecreateStateFromStorageRangesTests.cs

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProvider.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ix/inaccurate-snap-sync

LukaszRozmej · 2026-02-12T07:24:26Z

src/Nethermind/Nethermind.Core/Crypto/Hash256.cs

+        public ValueHash256 IncrementPath()
+        {
+            ValueHash256 result = this;
+            Span<byte> bytes = result.BytesAsSpan;
+
+            for (int i = 31; i >= 0; i--)
+            {
+                if (bytes[i] < 0xFF)
+                {
+                    bytes[i]++;
+                    return result;
+                }
+                bytes[i] = 0x00;
+            }
+
+            // Overflow - return max (shouldn't happen in practice)
+            result = ValueKeccak.Zero;
+            result.BytesAsSpan.Fill(0xFF);
+            return result;
+        }
+
+        public ValueHash256 DecrementPath()
+        {
+            ValueHash256 result = this;
+            Span<byte> bytes = result.BytesAsSpan;
+
+            for (int i = 31; i >= 0; i--)
+            {
+                if (bytes[i] > 0)
+                {
+                    bytes[i]--;
+                    return result;
+                }
+                bytes[i] = 0xFF;
+            }
+
+            // Underflow - return zero (shouldn't happen in practice)
+            return ValueKeccak.Zero;
+        }


Kind of belong better as extension methods

src/Nethermind/Nethermind.Synchronization.Test/FastSync/LocalDbContext.cs

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProvider.cs

LukaszRozmej · 2026-02-12T07:35:47Z

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProvider.cs

+                if (result == AddRangeResult.MissingRootHashInProofs)
+                {
+                    _logger.Trace(
+                        $"SNAP - AddStorageRange failed, missing root hash {actualRootHash} in the proofs, startingHash:{request.StartingHash}");
                }
-                else if (accountIndex == 0 && request.Accounts.Count == 1)
+                else if (result == AddRangeResult.DifferentRootHash)
                {
-                    _progressTracker.OnCompletedLargeStorage(pathWithAccount);
+                    _logger.Trace(
+                        $"SNAP - AddStorageRange failed, expected storage root hash:{pathWithAccount.Account.StorageRoot} but was {actualRootHash}, startingHash:{request.StartingHash}");
+                }
+                else if (result == AddRangeResult.InvalidOrder)
+                {
+                    if (_logger.IsTrace)
+                        _logger.Trace(
+                            $"SNAP - AddStorageRange failed, slots are not in sorted order, startingHash:{request.StartingHash}");
+                }
+                else if (result == AddRangeResult.OutOfBounds)
+                {
+                    if (_logger.IsTrace)
+                        _logger.Trace(
+                            $"SNAP - AddStorageRange failed, slots are out of bounds, startingHash:{request.StartingHash}");
+                }
+                else if (result == AddRangeResult.EmptyRange)
+                {
+                    if (_logger.IsTrace)
+                        _logger.Trace(
+                            $"SNAP - AddStorageRange failed, slots list is empty, startingHash:{request.StartingHash}");


make it a switch something like (pseudocode)

Suggested change

if (result == AddRangeResult.MissingRootHashInProofs)

{

_logger.Trace(

$"SNAP - AddStorageRange failed, missing root hash {actualRootHash} in the proofs, startingHash:{request.StartingHash}");

}

else if (accountIndex == 0 && request.Accounts.Count == 1)

else if (result == AddRangeResult.DifferentRootHash)

{

_progressTracker.OnCompletedLargeStorage(pathWithAccount);

_logger.Trace(

$"SNAP - AddStorageRange failed, expected storage root hash:{pathWithAccount.Account.StorageRoot} but was {actualRootHash}, startingHash:{request.StartingHash}");

}

else if (result == AddRangeResult.InvalidOrder)

{

if (_logger.IsTrace)

_logger.Trace(

$"SNAP - AddStorageRange failed, slots are not in sorted order, startingHash:{request.StartingHash}");

}

else if (result == AddRangeResult.OutOfBounds)

{

if (_logger.IsTrace)

_logger.Trace(

$"SNAP - AddStorageRange failed, slots are out of bounds, startingHash:{request.StartingHash}");

}

else if (result == AddRangeResult.EmptyRange)

{

if (_logger.IsTrace)

_logger.Trace(

$"SNAP - AddStorageRange failed, slots list is empty, startingHash:{request.StartingHash}");

if (_logger.IsTrace) _logger.Trace(result switch

{

AddRangeResult.MissingRootHashInProofs => $"SNAP - AddStorageRange failed, missing root hash {actualRootHash} in the proofs, startingHash:{request.StartingHash},

...

}

LukaszRozmej · 2026-02-12T07:39:50Z

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProviderHelper.cs


-        public static (AddRangeResult result, bool moreChildrenToRight, List<PathWithAccount> storageRoots, List<ValueHash256> codeHashes) AddAccountRange(
-            StateTree tree,
+        public static (AddRangeResult result, bool moreChildrenToRight, List<PathWithAccount> storageRoots, List<ValueHash256> codeHashes, Hash256 actualRootHash) AddAccountRange(


those tuples are getting ridiculous in this type (in multiple methods)
create custom types with better naming and some conversions and/or composition

LukaszRozmej · 2026-02-12T07:40:43Z

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProviderHelper.cs

+            List<PathWithAccount> accountsWithStorage = new();
+            List<ValueHash256> codeHashes = new();


could they be ArrayPoolList or ArrayPoolListRef if the containing type would be ref struct?

LukaszRozmej · 2026-02-12T07:43:44Z

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapUpperBoundAdapter.cs

+
+    public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => baseTrieStore.TryLoadRlp(in path, hash, flags);
+
+    public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) => throw new NotSupportedException("Get storage trie node resolver not supported");


could this be avoided? Multiple interfaces perhaps?

Yes, I'm thinking so to. But that would cut through other critical class. So definitely not here.

LukaszRozmej · 2026-02-12T07:47:15Z

src/Nethermind/Nethermind.Trie/TrieNode.cs

+            else
+            {
+                int length = rlpStream.PeekNextRlpLength();
+                return rlpStream.Read(length).ToArray();


you could read from original _rlp by using Position and length and keep the result as Span<byte>. Doesn't make sense if you still need to allocate an array later.

…bContext.cs Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>

…r.cs Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>

asdacap · 2026-02-12T08:29:07Z

@copilot address comment

Copilot · 2026-02-12T08:29:21Z

@asdacap I've opened a new pull request, #10497, to work on those changes. Once the pull request is ready, I'll request review from you.

…tor logging (#10497) * Initial plan * Move IncrementPath/DecrementPath to extension methods Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> * Convert logging if-else chains to switch expressions Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> * Add missing using directive for Hash256Extensions Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> * Improve switch expressions to avoid logging null Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: asdacap <1841324+asdacap@users.noreply.github.com>

…ix/inaccurate-snap-sync

asdacap requested review from LukaszRozmej, benaadams, damian-orzechowski, flcl42 and marcindsobczak as code owners February 9, 2026 13:12

asdacap changed the title ~~Fix inaccurate snap sync with abstracted trie factory~~ Fix inaccurate snap sync and abstract state sync operation Feb 9, 2026

Enable EnableSnapSyncStorageRangeSplit by default

8a04b1f

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

asdacap requested a review from MarekM25 as a code owner February 9, 2026 13:15

asdacap marked this pull request as draft February 9, 2026 13:21

asdacap and others added 3 commits February 9, 2026 21:32

Cleanup

7548f06

Whitespace

1f45846

asdacap marked this pull request as ready for review February 9, 2026 13:50

LukaszRozmej requested a review from Copilot February 9, 2026 14:44

Copilot started reviewing on behalf of LukaszRozmej February 9, 2026 14:44 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

asdacap and others added 3 commits February 10, 2026 07:52

Update src/Nethermind/Nethermind.Core.Test/TestMemDb.cs

8703a14

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Address comment

2fc0dcd

Merge remote-tracking branch 'origin/fix/inaccurate-snap-sync' into f…

47fe009

…ix/inaccurate-snap-sync

benaadams approved these changes Feb 10, 2026

View reviewed changes

LukaszRozmej approved these changes Feb 12, 2026

View reviewed changes

asdacap and others added 2 commits February 12, 2026 16:26

Update src/Nethermind/Nethermind.Synchronization.Test/FastSync/LocalD…

9ebb702

…bContext.cs Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>

Update src/Nethermind/Nethermind.Synchronization/SnapSync/SnapProvide…

f997f05

…r.cs Co-authored-by: Lukasz Rozmej <lukasz.rozmej@gmail.com>

Copilot AI mentioned this pull request Feb 12, 2026

Address review comments: move Hash256 methods to extensions and refactor logging #10497

Merged

16 tasks

Copilot AI and others added 3 commits February 12, 2026 17:03

Disable storage range split

f8ad9f3

Merge remote-tracking branch 'origin/fix/inaccurate-snap-sync' into f…

e2c42e8

…ix/inaccurate-snap-sync

benaadams added the bug fix + reliability label Feb 13, 2026

asdacap added 4 commits February 13, 2026 09:04

Add back account refresh tracking

d900b9e

Persisted check is probably wrong

bd4bb24

Should check for extension also

e9a4e3c

Testing disabled

c085ad6

github-actions bot added snap sync state+storage sync trie labels Feb 13, 2026

		List<PathWithAccount> accountsWithStorage = new();
		List<ValueHash256> codeHashes = new();


		public byte[]? TryLoadRlp(in TreePath path, Hash256 hash, ReadFlags flags = ReadFlags.None) => baseTrieStore.TryLoadRlp(in path, hash, flags);

		public ITrieNodeResolver GetStorageTrieNodeResolver(Hash256? address) => throw new NotSupportedException("Get storage trie node resolver not supported");

Conversation

asdacap commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LukaszRozmej Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LukaszRozmej Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

asdacap Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

LukaszRozmej Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

asdacap commented Feb 12, 2026

Uh oh!

Copilot AI commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asdacap commented Feb 9, 2026 •

edited

Loading