Refresh snap-sync account storage root via verified GetAccountRange

When a storage root mismatches during snap sync, the account was refreshed
by fetching its storage-trie root node via GetTrieNodes and trusting the
returned node's hash blindly. That refresh path is being removed in snaps/2
and never verified the new storage root against the pivot state root.

Refresh now sends a single-account GetAccountRange (limit = path + 1) and
verifies the returned account against the pivot state root by reconstructing
the account range the same way account-range sync does: fill the boundary
proof nodes, set the returned account(s), then check the recomputed root. The
reconstruction runs in an isolated, empty-backed trie (no writes to the client
state DB, so it cannot race the partition sync), and because the leaf is set
from the response it does not rely on the proof containing it. Outcomes:
verified (adopt storage root, re-queue storage), not found (deleted account,
terminal), expired (stale pivot), or invalid proof (retry and score the peer).
Refreshes are rare, so one account per request is fine.

The shared verification is factored into SnapProviderHelper.BuildAndVerifyRoot,
used by both CommitRange and the new no-commit VerifyAccountRange. The
now-unused GetTrieNodes(AccountsToRefreshRequest) overload is left for the
snaps/2 cleanup.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: asdacap

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

@@ -192,6 +192,87 @@ public void TestGetAccountRange_InvalidRange()
         proofs.Dispose();
     }
 
+    // Refresh re-fetches a single account via GetAccountRange and verifies its storage root against the
+    // state root from the proof. A correct root must verify and propagate the real storage root; a wrong
+    // root must be rejected.
+    [TestCase(true, AddRangeResult.OK)]
+    [TestCase(false, AddRangeResult.DifferentRootHash)]
+    public void TestRefreshAccount(bool useCorrectRoot, AddRangeResult expectedResult)
+    {
+        using ISnapServerContext context = CreateContext();
+        FillWithTestAccounts(context);
+        Hash256 storageRoot = FillAccountWithDefaultStorage(context);
+
+        ValueHash256 path = TestItem.Tree.AccountsWithPaths[0].Path;
+        (IOwnedReadOnlyList<PathWithAccount> accounts, IByteArrayList proofs) =
+            context.Server.GetAccountRanges(context.RootHash, path, path.IncrementPath(), 4000, CancellationToken.None);
+        using AccountsAndProofs response = new() { PathAndAccounts = accounts, Proofs = proofs };
+
+        PathWithAccount stale = new(path, Build.An.Account.WithBalance(2).TestObject);
+        using AccountsToRefreshRequest request = new()
+        {
+            RootHash = useCorrectRoot ? context.RootHash : TestItem.KeccakA,
+            Paths = new ArrayPoolList<AccountWithStorageStartingHash>(1)
+            {
+                new() { PathAndAccount = stale, StorageStartingHash = ValueKeccak.Zero, StorageHashLimit = Keccak.MaxValue }
+            }
+        };
+
+        AddRangeResult result = context.SnapProvider.RefreshAccounts(request, response);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(result, Is.EqualTo(expectedResult));
+            // On success the stale empty storage root must be replaced by the verified one.
+            Assert.That(stale.Account.StorageRoot, Is.EqualTo(useCorrectRoot ? storageRoot : Keccak.EmptyTreeHash));
+        }
+    }
+
+    // The range proof can omit the leaf node. Verification must still succeed by setting the account from the
+    // returned accounts (not by hashing a proof node), then checking the reconstructed root.
+    [Test]
+    public void TestRefreshAccount_LeafMissingFromProof()
+    {
+        using ISnapServerContext context = CreateContext();
+        FillWithTestAccounts(context);
+        Hash256 storageRoot = FillAccountWithDefaultStorage(context);
+
+        ValueHash256 path = TestItem.Tree.AccountsWithPaths[0].Path;
+        (IOwnedReadOnlyList<PathWithAccount> accounts, IByteArrayList proofs) =
+            context.Server.GetAccountRanges(context.RootHash, path, path.IncrementPath(), 4000, CancellationToken.None);
+
+        // Drop the leaf nodes from the proof; the leaves are still present in the returned accounts.
+        ArrayPoolList<byte[]> trimmedProofs = new(proofs.Count);
+        for (int i = 0; i < proofs.Count; i++)
+        {
+            byte[] proof = proofs[i].ToArray();
+            TrieNode node = new(NodeType.Unknown, proof);
+            node.ResolveNode(null!, TreePath.Empty);
+            if (!node.IsLeaf) trimmedProofs.Add(proof);
+        }
+        Assert.That(trimmedProofs.Count, Is.LessThan(proofs.Count), "test setup: expected at least one leaf in the proof");
+        proofs.Dispose();
+
+        using AccountsAndProofs response = new() { PathAndAccounts = accounts, Proofs = new ByteArrayListAdapter(trimmedProofs) };
+        PathWithAccount stale = new(path, Build.An.Account.WithBalance(2).TestObject);
+        using AccountsToRefreshRequest request = new()
+        {
+            RootHash = context.RootHash,
+            Paths = new ArrayPoolList<AccountWithStorageStartingHash>(1)
+            {
+                new() { PathAndAccount = stale, StorageStartingHash = ValueKeccak.Zero, StorageHashLimit = Keccak.MaxValue }
+            }
+        };
+
+        AddRangeResult result = context.SnapProvider.RefreshAccounts(request, response);
+
+        using (Assert.EnterMultipleScope())
+        {
+            Assert.That(result, Is.EqualTo(AddRangeResult.OK));
+            Assert.That(stale.Account.StorageRoot, Is.EqualTo(storageRoot));
+        }
+    }
+
     [Test]
     public void TestGetTrieNode_Root()
     {

src/Nethermind/Nethermind.Synchronization/SnapSync/ISnapProvider.cs

@@ -20,7 +20,7 @@ public interface ISnapProvider
 
         void AddCodes(IReadOnlyList<ValueHash256> requestedHashes, IByteArrayList codes);
 
-        void RefreshAccounts(AccountsToRefreshRequest request, IByteArrayList response);
+        AddRangeResult RefreshAccounts(AccountsToRefreshRequest request, AccountsAndProofs response);
 
         void RetryRequest(SnapSyncBatch batch);
 

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

@@ -269,19 +269,17 @@ private SnapSyncBatch CreateAccountRangeRequest(Hash256 rootHash, AccountRangePa
             return new SnapSyncBatch { AccountRangeRequest = range };
         }
 
-        private SnapSyncBatch DequeAccountToRefresh(Hash256 rootHash)
+        private SnapSyncBatch? DequeAccountToRefresh(Hash256 rootHash)
         {
+            // One account per request: each refresh is served by a single GetAccountRange.
+            if (!AccountsToRefresh.TryDequeue(out AccountWithStorageStartingHash acc))
+                return null;
+
             Interlocked.Increment(ref _activeAccRefreshRequests);
 
             LogRequest($"AccountsToRefresh: {AccountsToRefresh.Count}");
 
-            int queueLength = AccountsToRefresh.Count;
-            ArrayPoolList<AccountWithStorageStartingHash> paths = new(queueLength);
-
-            for (int i = 0; i < queueLength && AccountsToRefresh.TryDequeue(out AccountWithStorageStartingHash acc); i++)
-            {
-                paths.Add(acc);
-            }
+            ArrayPoolList<AccountWithStorageStartingHash> paths = new(1) { acc };
 
             return new SnapSyncBatch { AccountsToRefreshRequest = new AccountsToRefreshRequest { RootHash = rootHash, Paths = paths } };
         }

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

@@ -15,7 +15,9 @@
 using Nethermind.Core.Extensions;
 using Nethermind.Db;
 using Nethermind.Logging;
+using Nethermind.Serialization.Rlp;
 using Nethermind.State.Snap;
+using Nethermind.Trie;
 
 namespace Nethermind.Synchronization.SnapSync
 {
@@ -243,50 +245,98 @@ public AddRangeResult AddStorageRangeForAccount(StorageRange request, int accoun
             }
         }
 
-        public void RefreshAccounts(AccountsToRefreshRequest request, IByteArrayList response)
+        public AddRangeResult RefreshAccounts(AccountsToRefreshRequest request, AccountsAndProofs response)
         {
-            int respLength = response.Count;
-            ReadOnlySpan<AccountWithStorageStartingHash> paths = request.Paths.AsSpan();
-            for (int reqIndex = 0; reqIndex < paths.Length; reqIndex++)
-            {
-                AccountWithStorageStartingHash requestedPath = paths[reqIndex];
-
-                if (reqIndex < respLength)
-                {
-                    ReadOnlySpan<byte> nodeData = response[reqIndex];
-
-                    if (nodeData.Length == 0)
-                    {
-                        RetryAccountRefresh(requestedPath);
-                        _logger.Trace($"SNAP - Empty Account Refresh: {requestedPath.PathAndAccount.Path}");
-                        continue;
-                    }
+            AccountWithStorageStartingHash requestedPath = request.Paths[0];
+            ValueHash256 path = requestedPath.PathAndAccount.Path;
 
-                    requestedPath.PathAndAccount.Account = requestedPath.PathAndAccount.Account.WithChangedStorageRoot(Keccak.Compute(nodeData));
+            AddRangeResult result;
+            switch (VerifyRefreshedAccount(response, request.RootHash, path, out Account? account))
+            {
+                case RefreshVerifyResult.Verified:
+                    result = AddRangeResult.OK;
+                    requestedPath.PathAndAccount.Account = requestedPath.PathAndAccount.Account.WithChangedStorageRoot(account!.StorageRoot);
 
                     if (requestedPath.StorageStartingHash > ValueKeccak.Zero)
                     {
-                        StorageRange range = new()
+                        _progressTracker.EnqueueNextSlot(new StorageRange
                         {
                             Accounts = new ArrayPoolList<PathWithAccount>(1) { requestedPath.PathAndAccount },
                             StartingHash = requestedPath.StorageStartingHash,
                             LimitHash = requestedPath.StorageHashLimit
-                        };
-
-                        _progressTracker.EnqueueNextSlot(range);
+                        });
                     }
                     else
                     {
                         _progressTracker.EnqueueAccountStorage(requestedPath.PathAndAccount);
                     }
-                }
-                else
-                {
+                    break;
+
+                case RefreshVerifyResult.NotFound:
+                    // The account no longer exists at the pivot, so there is no storage to retrieve. It remains
+                    // tracked for healing. Terminal success - must not retry or the refresh would loop forever.
+                    result = AddRangeResult.OK;
+                    break;
+
+                case RefreshVerifyResult.Expired:
+                    // The peer does not have the state for this root (stale pivot).
+                    result = AddRangeResult.ExpiredRootHash;
                     RetryAccountRefresh(requestedPath);
-                }
+                    break;
+
+                default: // InvalidProof - the proof does not reconstruct the state root.
+                    result = AddRangeResult.DifferentRootHash;
+                    RetryAccountRefresh(requestedPath);
+                    break;
             }
 
+            Metrics.SnapRangeResult.Increment(new SnapRangeResult(isStorage: false, result: result));
+            // Must be the last statement, after any enqueue, so IsSnapGetRangesFinished cannot observe
+            // an empty queue with a zeroed active count while work is still being scheduled.
             _progressTracker.ReportAccountRefreshFinished();
+            return result;
+        }
+
+        private enum RefreshVerifyResult { Verified, NotFound, Expired, InvalidProof }
+
+        /// <summary>
+        /// Reconstructs the returned account range and verifies it against <paramref name="stateRoot"/> in an
+        /// isolated, empty-backed trie - the account leaf is set from the response, not assumed to be in the proof -
+        /// then extracts the verified account at <paramref name="path"/>. Nothing is written to the client state DB.
+        /// </summary>
+        private RefreshVerifyResult VerifyRefreshedAccount(AccountsAndProofs response, Hash256 stateRoot, in ValueHash256 path, out Account? account)
+        {
+            account = null;
+            IReadOnlyList<PathWithAccount> accounts = response.PathAndAccounts;
+            if (accounts.Count == 0)
+                return response.Proofs.Count == 0 ? RefreshVerifyResult.Expired : RefreshVerifyResult.NotFound;
+
+            AddRangeResult result;
+            try
+            {
+                // Empty-backed isolated factory: a proof node that cannot be resolved from the proof itself fails
+                // verification instead of being completed from (or racing) the live client state DB.
+                ISnapTrieFactory factory = new PatriciaSnapTrieFactory(new NodeStorage(new MemDb()), logManager);
+                result = SnapProviderHelper.VerifyAccountRange(factory, stateRoot, path, path.IncrementPath(), accounts, response.Proofs);
+            }
+            catch (Exception e) when (e is TrieException or TrieNodeException or RlpException)
+            {
+                return RefreshVerifyResult.InvalidProof;
+            }
+
+            if (result != AddRangeResult.OK)
+                return RefreshVerifyResult.InvalidProof;
+
+            // Filter to the requested account; the response also carries at least one account past the limit.
+            for (int i = 0; i < accounts.Count; i++)
+            {
+                if (accounts[i].Path == path)
+                {
+                    account = accounts[i].Account;
+                    return RefreshVerifyResult.Verified;
+                }
+            }
+            return RefreshVerifyResult.NotFound;
         }
 
         private void RetryAccountRefresh(AccountWithStorageStartingHash requestedPath) => _progressTracker.EnqueueAccountRefresh(requestedPath.PathAndAccount, requestedPath.StorageStartingHash, requestedPath.StorageHashLimit);

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

@@ -70,6 +70,25 @@ public static (AddRangeResult result, bool moreChildrenToRight, Hash256 actualRo
             return (AddRangeResult.OK, moreChildrenToRight, tree.RootHash, isRootPersisted);
         }
 
+        /// <summary>
+        /// Verifies that <paramref name="accounts"/> and <paramref name="proofs"/> reconstruct
+        /// <paramref name="expectedRootHash"/> for the range starting at <paramref name="startingHash"/>, without
+        /// committing anything. Intended for single-account refresh against an isolated, empty-backed factory:
+        /// the boundary nodes and the returned accounts are assembled in memory only to recompute the root, so an
+        /// incomplete proof fails verification rather than being completed from (or racing) the live state DB.
+        /// </summary>
+        public static AddRangeResult VerifyAccountRange(
+            ISnapTrieFactory factory,
+            in ValueHash256 expectedRootHash,
+            in ValueHash256 startingHash,
+            in ValueHash256 limitHash,
+            IReadOnlyList<PathWithAccount> accounts,
+            IByteArrayList? proofs)
+        {
+            using ISnapTree<PathWithAccount> tree = factory.CreateStateTree();
+            return BuildAndVerifyRoot(tree, accounts, startingHash, limitHash, expectedRootHash, proofs).result;
+        }
+
         private static (AddRangeResult result, bool moreChildrenToRight, bool isRootPersisted) CommitRange<TEntry>(
             ISnapTree<TEntry> tree,
             IReadOnlyList<TEntry> entries,
@@ -78,30 +97,16 @@ private static (AddRangeResult result, bool moreChildrenToRight, bool isRootPers
             in ValueHash256 expectedRootHash,
             IByteArrayList? proofs) where TEntry : ISnapEntry
         {
-            if (entries.Count == 0)
-                return (AddRangeResult.EmptyRange, true, false);
-
-            // Validate sorting order
-            for (int i = 1; i < entries.Count; i++)
-            {
-                if (entries[i - 1].Path.CompareTo(entries[i].Path) >= 0)
-                    return (AddRangeResult.InvalidOrder, true, false);
-            }
-
-            if (entries[0].Path < startingHash)
-                return (AddRangeResult.InvalidOrder, true, false);
-
-            ValueHash256 lastPath = entries[entries.Count - 1].Path;
-
-            (AddRangeResult result, List<(TrieNode, TreePath)> sortedBoundaryList, bool moreChildrenToRight) =
-                FillBoundaryTree(tree, startingHash, lastPath, limitHash, expectedRootHash, proofs);
+            (AddRangeResult result, List<(TrieNode, TreePath)>? sortedBoundaryList, bool moreChildrenToRight) =
+                BuildAndVerifyRoot(tree, entries, startingHash, limitHash, expectedRootHash, proofs);
 
             if (result != AddRangeResult.OK)
                 return (result, true, false);
 
             // The upper bound is used to prevent proof nodes that covers next range from being persisted, except if
             // this is the last range. This prevent double node writes per path which break flat. It also prevent leaf o
             // that is after the range from being persisted, which prevent double write again.
+            ValueHash256 lastPath = entries[entries.Count - 1].Path;
             ValueHash256 upperBound = lastPath;
             if (upperBound > limitHash)
             {
@@ -112,11 +117,6 @@ private static (AddRangeResult result, bool moreChildrenToRight, bool isRootPers
                 if (!moreChildrenToRight) upperBound = ValueKeccak.MaxValue;
             }
 
-            tree.BulkSetAndUpdateRootHash(entries);
-
-            if (tree.RootHash.ValueHash256 != expectedRootHash)
-                return (AddRangeResult.DifferentRootHash, true, false);
-
             StitchBoundaries(sortedBoundaryList, tree, startingHash);
 
             tree.Commit(upperBound);
@@ -125,6 +125,47 @@ private static (AddRangeResult result, bool moreChildrenToRight, bool isRootPers
             return (AddRangeResult.OK, moreChildrenToRight, isRootPersisted);
         }
 
+        /// <summary>
+        /// Fills the boundary proof nodes and bulk-sets the range entries into <paramref name="tree"/> in memory,
+        /// then checks that the recomputed root equals <paramref name="expectedRootHash"/>. Nothing is committed.
+        /// </summary>
+        private static (AddRangeResult result, List<(TrieNode, TreePath)>? sortedBoundaryList, bool moreChildrenToRight) BuildAndVerifyRoot<TEntry>(
+            ISnapTree<TEntry> tree,
+            IReadOnlyList<TEntry> entries,
+            in ValueHash256 startingHash,
+            in ValueHash256 limitHash,
+            in ValueHash256 expectedRootHash,
+            IByteArrayList? proofs) where TEntry : ISnapEntry
+        {
+            if (entries.Count == 0)
+                return (AddRangeResult.EmptyRange, null, false);
+
+            // Validate sorting order
+            for (int i = 1; i < entries.Count; i++)
+            {
+                if (entries[i - 1].Path.CompareTo(entries[i].Path) >= 0)
+                    return (AddRangeResult.InvalidOrder, null, false);
+            }
+
+            if (entries[0].Path < startingHash)
+                return (AddRangeResult.InvalidOrder, null, false);
+
+            ValueHash256 lastPath = entries[entries.Count - 1].Path;
+
+            (AddRangeResult result, List<(TrieNode, TreePath)> sortedBoundaryList, bool moreChildrenToRight) =
+                FillBoundaryTree(tree, startingHash, lastPath, limitHash, expectedRootHash, proofs);
+
+            if (result != AddRangeResult.OK)
+                return (result, null, false);
+
+            tree.BulkSetAndUpdateRootHash(entries);
+
+            if (tree.RootHash.ValueHash256 != expectedRootHash)
+                return (AddRangeResult.DifferentRootHash, null, false);
+
+            return (AddRangeResult.OK, sortedBoundaryList, moreChildrenToRight);
+        }
+
         [SkipLocalsInit]
         private static (AddRangeResult result, List<(TrieNode, TreePath)> sortedBoundaryList, bool moreChildrenToRight) FillBoundaryTree<TEntry>(
             ISnapTree<TEntry> tree,

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapSyncBatch.cs

@@ -20,7 +20,7 @@ public class SnapSyncBatch : IDisposable
         public IByteArrayList? CodesResponse { get; set; }
 
         public AccountsToRefreshRequest? AccountsToRefreshRequest { get; set; }
-        public IByteArrayList? AccountsToRefreshResponse { get; set; }
+        public AccountsAndProofs? AccountsToRefreshResponse { get; set; }
 
         public override string ToString()
         {