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
hash blindly. That path is being removed in snaps/2 and never verified the
result against the state root.

Refresh now sends a single-account GetAccountRange (limit = path + 1) and
verifies the returned account against the pivot state root in an isolated,
in-memory, content-addressed trie (throwaway MemDb + Hash-scheme NodeStorage
+ PatriciaTree.Get) — no writes to the client state DB, so it cannot race the
partition account-range sync. Outcomes: verified (adopt storage root, re-queue
storage), not found (deleted account, terminal), or invalid proof (retry and
score the peer). Refreshes are rare, so one account per request is fine.

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,42 @@ 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));
+        }
+    }
+
     [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,10 @@
 using Nethermind.Core.Extensions;
 using Nethermind.Db;
 using Nethermind.Logging;
+using Nethermind.Serialization.Rlp;
 using Nethermind.State.Snap;
+using Nethermind.Trie;
+using Nethermind.Trie.Pruning;
 
 namespace Nethermind.Synchronization.SnapSync
 {
@@ -30,6 +33,8 @@ public class SnapProvider(ProgressTracker progressTracker, [KeyFilter(DbNames.Co
         // This is actually close to 97% effective.
         private readonly AssociativeKeyCache<ValueHash256> _codeExistKeyCache = new(1024 * 16);
 
+        private static readonly AccountDecoder _accountDecoder = new();
+
         public bool CanSync() => _progressTracker.CanSync();
 
         public bool IsFinished(out SnapSyncBatch? nextBatch) => _progressTracker.IsFinished(out nextBatch);
@@ -243,50 +248,96 @@ 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];
+            AccountWithStorageStartingHash requestedPath = request.Paths[0];
+            ValueHash256 path = requestedPath.PathAndAccount.Path;
 
-                if (reqIndex < respLength)
+            AddRangeResult result;
+            if (response.PathAndAccounts.Count == 0 && response.Proofs.Count == 0)
+            {
+                // The peer does not have the state for this root (stale pivot).
+                result = AddRangeResult.ExpiredRootHash;
+                RetryAccountRefresh(requestedPath);
+            }
+            else
+            {
+                switch (VerifyRefreshedAccount(response.Proofs, request.RootHash, path, out Account? account))
                 {
-                    ReadOnlySpan<byte> nodeData = response[reqIndex];
+                    case RefreshVerifyResult.Verified:
+                        result = AddRangeResult.OK;
+                        requestedPath.PathAndAccount.Account = requestedPath.PathAndAccount.Account.WithChangedStorageRoot(account!.StorageRoot);
 
-                    if (nodeData.Length == 0)
-                    {
+                        if (requestedPath.StorageStartingHash > ValueKeccak.Zero)
+                        {
+                            _progressTracker.EnqueueNextSlot(new StorageRange
+                            {
+                                Accounts = new ArrayPoolList<PathWithAccount>(1) { requestedPath.PathAndAccount },
+                                StartingHash = requestedPath.StorageStartingHash,
+                                LimitHash = requestedPath.StorageHashLimit
+                            });
+                        }
+                        else
+                        {
+                            _progressTracker.EnqueueAccountStorage(requestedPath.PathAndAccount);
+                        }
+                        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;
+
+                    default: // InvalidProof - the peer returned a missing or tampered proof.
+                        result = AddRangeResult.DifferentRootHash;
                         RetryAccountRefresh(requestedPath);
-                        _logger.Trace($"SNAP - Empty Account Refresh: {requestedPath.PathAndAccount.Path}");
-                        continue;
-                    }
+                        break;
+                }
+            }
 
-                    requestedPath.PathAndAccount.Account = requestedPath.PathAndAccount.Account.WithChangedStorageRoot(Keccak.Compute(nodeData));
+            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;
+        }
 
-                    if (requestedPath.StorageStartingHash > ValueKeccak.Zero)
-                    {
-                        StorageRange range = new()
-                        {
-                            Accounts = new ArrayPoolList<PathWithAccount>(1) { requestedPath.PathAndAccount },
-                            StartingHash = requestedPath.StorageStartingHash,
-                            LimitHash = requestedPath.StorageHashLimit
-                        };
+        private enum RefreshVerifyResult { Verified, NotFound, InvalidProof }
 
-                        _progressTracker.EnqueueNextSlot(range);
-                    }
-                    else
-                    {
-                        _progressTracker.EnqueueAccountStorage(requestedPath.PathAndAccount);
-                    }
-                }
-                else
-                {
-                    RetryAccountRefresh(requestedPath);
-                }
+        /// <summary>
+        /// Verifies that <paramref name="path"/> resolves to an account under <paramref name="stateRoot"/>
+        /// using only <paramref name="proofs"/>, reconstructed in an isolated in-memory content-addressed
+        /// trie. Nothing is written to the client state DB, so this cannot race the partition account-range sync.
+        /// </summary>
+        private RefreshVerifyResult VerifyRefreshedAccount(IByteArrayList proofs, Hash256 stateRoot, in ValueHash256 path, out Account? account)
+        {
+            account = null;
+            if (proofs.Count == 0) return RefreshVerifyResult.InvalidProof;
+
+            using MemDb memDb = new();
+            NodeStorage nodeStorage = new(memDb, INodeStorage.KeyScheme.Hash);
+            for (int i = 0; i < proofs.Count; i++)
+            {
+                ReadOnlySpan<byte> rlp = proofs[i];
+                nodeStorage.Set(null, TreePath.Empty, ValueKeccak.Compute(rlp), rlp);
             }
 
-            _progressTracker.ReportAccountRefreshFinished();
+            try
+            {
+                PatriciaTree tree = new(new RawScopedTrieStore(nodeStorage), logManager);
+                ReadOnlySpan<byte> accountRlp = tree.Get(path.Bytes, stateRoot);
+                if (accountRlp.IsEmpty) return RefreshVerifyResult.NotFound;
+
+                Rlp.ValueDecoderContext ctx = new(accountRlp);
+                account = _accountDecoder.Decode(ref ctx);
+                return account is null ? RefreshVerifyResult.NotFound : RefreshVerifyResult.Verified;
+            }
+            catch (Exception e) when (e is TrieException or TrieNodeException or RlpException)
+            {
+                return RefreshVerifyResult.InvalidProof;
+            }
         }
 
         private void RetryAccountRefresh(AccountWithStorageStartingHash requestedPath) => _progressTracker.EnqueueAccountRefresh(requestedPath.PathAndAccount, requestedPath.StorageStartingHash, requestedPath.StorageHashLimit);

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()
         {

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapSyncDownloader.cs

@@ -5,8 +5,11 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Nethermind.Blockchain.Synchronization;
+using Nethermind.Core.Crypto;
+using Nethermind.Core.Extensions;
 using Nethermind.Logging;
 using Nethermind.Network.Contract.P2P;
+using Nethermind.State.Snap;
 using Nethermind.Synchronization.ParallelSync;
 using Nethermind.Synchronization.Peers;
 
@@ -36,9 +39,15 @@ public async Task Dispatch(PeerInfo peerInfo, SnapSyncBatch batch, CancellationT
                     {
                         batch.CodesResponse = await handler.GetByteCodes(batch.CodesRequest, cancellationToken);
                     }
-                    else if (batch.AccountsToRefreshRequest is not null)
+                    else if (batch.AccountsToRefreshRequest is { Paths.Count: > 0 })
                     {
-                        batch.AccountsToRefreshResponse = await handler.GetTrieNodes(batch.AccountsToRefreshRequest, cancellationToken);
+                        // Refresh a single account via GetAccountRange so its storage root is verified
+                        // against the state root. Limit = path + 1 (the protocol returns at least one
+                        // account past the limit, so start == limit must be avoided).
+                        AccountWithStorageStartingHash account = batch.AccountsToRefreshRequest.Paths[0];
+                        ValueHash256 path = account.PathAndAccount.Path;
+                        AccountRange range = new(batch.AccountsToRefreshRequest.RootHash, path, path.IncrementPath());
+                        batch.AccountsToRefreshResponse = await handler.GetAccountRange(range, cancellationToken);
                     }
                 }
                 catch (OperationCanceledException)

src/Nethermind/Nethermind.Synchronization/SnapSync/SnapSyncFeed.cs

@@ -84,7 +84,7 @@ public SyncResponseHandlingResult HandleResponse(SnapSyncBatch batch, PeerInfo?
                 }
                 else if (batch.AccountsToRefreshResponse is not null)
                 {
-                    _snapProvider.RefreshAccounts(batch.AccountsToRefreshRequest, batch.AccountsToRefreshResponse);
+                    result = _snapProvider.RefreshAccounts(batch.AccountsToRefreshRequest, batch.AccountsToRefreshResponse);
                 }
                 else
                 {