Refresh snap-sync account storage root via verified GetAccountRange

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

@@ -192,6 +192,121 @@ 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));
+        }
+    }
+
+    // A non-existent account is a terminal no-op only when its absence is *proven* by the verified range -
+    // the storage root must be left untouched and the refresh must not retry.
+    [Test]
+    public void TestRefreshAccount_VerifiedNotFound()
+    {
+        using ISnapServerContext context = CreateContext();
+        FillWithTestAccounts(context);
+
+        // A path immediately before an existing account: the account is absent, but the next account proves it.
+        ValueHash256 path = TestItem.Tree.AccountsWithPaths[1].Path.DecrementPath();
+        (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 = 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));
+            // Absent account: nothing adopted, storage root unchanged.
+            Assert.That(stale.Account.StorageRoot, Is.EqualTo(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

@@ -16,6 +16,7 @@
 using Nethermind.Db;
 using Nethermind.Logging;
 using Nethermind.State.Snap;
+using Nethermind.Trie;
 
 namespace Nethermind.Synchronization.SnapSync
 {
@@ -243,50 +244,103 @@ 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;
+            // An empty response carries no range to verify, so the account's absence cannot be proven here -
+            // retry rather than concluding (unproven) that it was deleted.
+            if (accounts.Count == 0)
+                return RefreshVerifyResult.Expired;
+
+            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)
+            {
+                // The proof is untrusted P2P data: any failure assembling or decoding it (trie, RLP, bounds, etc.)
+                // means the proof is invalid, never a crash of the sync task.
+                return RefreshVerifyResult.InvalidProof;
+            }
+
+            if (result != AddRangeResult.OK)
+                return RefreshVerifyResult.InvalidProof;
+
+            // The verified range proves which accounts exist around [path, path + 1]; filter to the requested one.
+            // Its absence here is therefore a proven non-existence (deleted account), not an unverified guess.
+            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);