Revert "Improve Block related caching (#9571)"

src/Nethermind/Nethermind.Blockchain.Test/BlockhashProviderTests.cs

@@ -10,11 +10,12 @@
 using Nethermind.Core.Specs;
 using Nethermind.Core.Test;
 using Nethermind.Core.Test.Builders;
-using Nethermind.Evm.State;
 using Nethermind.Logging;
 using Nethermind.Specs;
 using Nethermind.Specs.Forks;
 using Nethermind.Specs.Test;
+using Nethermind.Evm.State;
+using Nethermind.State;
 using NUnit.Framework;
 
 namespace Nethermind.Blockchain.Test;
@@ -80,21 +81,20 @@ public void Can_lookup_correctly_on_disconnected_main_chain()
         BlockhashProvider provider = CreateBlockHashProvider(tree, Frontier.Instance);
 
         BlockHeader notCanonParent = tree.FindHeader(chainLength - 4, BlockTreeLookupOptions.None)!;
-        Block expected = tree.FindBlock(chainLength - 3, BlockTreeLookupOptions.None)!;
-
+        BlockHeader expectedHeader = tree.FindHeader(chainLength - 3, BlockTreeLookupOptions.None)!;
         Block headParent = tree.FindBlock(chainLength - 2, BlockTreeLookupOptions.None)!;
         Block head = tree.FindBlock(chainLength - 1, BlockTreeLookupOptions.None)!;
 
         Block branch = Build.A.Block.WithParent(notCanonParent).WithTransactions(Build.A.Transaction.TestObject).TestObject;
         tree.Insert(branch, BlockTreeInsertBlockOptions.SaveHeader).Should().Be(AddBlockResult.Added);
         tree.UpdateMainChain(branch); // Update branch
 
-        tree.UpdateMainChain([expected, headParent, head], true); // Update back to original again, but skipping the branch block.
+        tree.UpdateMainChain([headParent, head], true); // Update back to original again, but skipping the branch block.
 
         Block current = Build.A.Block.WithParent(head).TestObject; // At chainLength
 
         Hash256? result = provider.GetBlockhash(current.Header, chainLength - 3);
-        Assert.That(result, Is.EqualTo(expected.Header.Hash));
+        Assert.That(result, Is.EqualTo(expectedHeader.Hash));
     }
 
     [Test, MaxTime(Timeout.MaxTestTime)]

src/Nethermind/Nethermind.Blockchain.Test/Blocks/HeaderStoreTests.cs

@@ -17,7 +17,7 @@ public class HeaderStoreTests
     [Test]
     public void TestCanStoreAndGetHeader()
     {
-        IHeaderStore store = new HeaderStore(new MemDb(), new MemDb());
+        HeaderStore store = new(new MemDb(), new MemDb());
 
         BlockHeader header = Build.A.BlockHeader.WithNumber(100).TestObject;
         BlockHeader header2 = Build.A.BlockHeader.WithNumber(102).TestObject;
@@ -39,7 +39,7 @@ public void TestCanStoreAndGetHeader()
     public void TestCanReadHeaderStoredWithHash()
     {
         IDb headerDb = new MemDb();
-        IHeaderStore store = new HeaderStore(headerDb, new MemDb());
+        HeaderStore store = new(headerDb, new MemDb());
 
         BlockHeader header = Build.A.BlockHeader.WithNumber(100).TestObject;
         headerDb.Set(header.Hash!, new HeaderDecoder().Encode(header).Bytes);
@@ -50,7 +50,7 @@ public void TestCanReadHeaderStoredWithHash()
     [Test]
     public void TestCanReadCacheHeader()
     {
-        IHeaderStore store = new HeaderStore(new MemDb(), new MemDb());
+        HeaderStore store = new(new MemDb(), new MemDb());
 
         BlockHeader header = Build.A.BlockHeader.WithNumber(100).TestObject;
         store.Cache(header);
@@ -60,7 +60,7 @@ public void TestCanReadCacheHeader()
     [Test]
     public void TestCanDeleteHeader()
     {
-        IHeaderStore store = new HeaderStore(new MemDb(), new MemDb());
+        HeaderStore store = new(new MemDb(), new MemDb());
         BlockHeader header = Build.A.BlockHeader.WithNumber(100).TestObject;
         store.Insert(header);
         store.Delete(header.Hash!);
@@ -72,7 +72,7 @@ public void TestCanDeleteHeader()
     public void TestCanDeleteHeaderStoredWithHash()
     {
         IDb headerDb = new MemDb();
-        IHeaderStore store = new HeaderStore(headerDb, new MemDb());
+        HeaderStore store = new(headerDb, new MemDb());
 
         BlockHeader header = Build.A.BlockHeader.WithNumber(100).TestObject;
         headerDb.Set(header.Hash!, new HeaderDecoder().Encode(header).Bytes);

src/Nethermind/Nethermind.Blockchain/BlockTree.cs

@@ -559,15 +559,7 @@ public AddBlockResult SuggestBlock(Block block, BlockTreeSuggestOptions options
             return blockHash is null ? null : FindHeader(blockHash, options, blockNumber: number);
         }
 
-        public Hash256? FindBlockHash(long blockNumber)
-        {
-            Hash256? blockHash = _headerStore.GetBlockHash(blockNumber);
-            if (blockHash is not null)
-            {
-                return blockHash;
-            }
-            return GetBlockHashOnMainOrBestDifficultyHash(blockNumber);
-        }
+        public Hash256? FindBlockHash(long blockNumber) => GetBlockHashOnMainOrBestDifficultyHash(blockNumber);
 
         public bool HasBlock(long blockNumber, Hash256 blockHash) => _blockStore.HasBlock(blockNumber, blockHash);
 
@@ -579,17 +571,7 @@ public AddBlockResult SuggestBlock(Block block, BlockTreeSuggestOptions options
                 return null;
             }
 
-            BlockHeader? header = null;
-            if ((options & BlockTreeLookupOptions.RequireCanonical) == 0)
-            {
-                header = _headerStore.GetFromCache(blockHash);
-                if (header is not null && (!blockNumber.HasValue || header.Number == blockNumber.Value))
-                {
-                    return header;
-                }
-            }
-
-            header = _headerStore.Get(blockHash, out bool fromCache, shouldCache: false, blockNumber: blockNumber);
+            BlockHeader? header = _headerStore.Get(blockHash, shouldCache: false, blockNumber: blockNumber);
             if (header is null)
             {
                 bool allowInvalid = (options & BlockTreeLookupOptions.AllowInvalid) == BlockTreeLookupOptions.AllowInvalid;
@@ -606,7 +588,6 @@ public AddBlockResult SuggestBlock(Block block, BlockTreeSuggestOptions options
             bool createLevelIfMissing = (options & BlockTreeLookupOptions.DoNotCreateLevelIfMissing) == BlockTreeLookupOptions.None;
             bool requiresCanonical = (options & BlockTreeLookupOptions.RequireCanonical) == BlockTreeLookupOptions.RequireCanonical;
 
-            bool isMainChain = false;
             if ((totalDifficultyNeeded && header.TotalDifficulty is null) || requiresCanonical)
             {
                 (BlockInfo blockInfo, ChainLevelInfo level) = LoadInfo(header.Number, header.Hash, true);
@@ -633,16 +614,16 @@ public AddBlockResult SuggestBlock(Block block, BlockTreeSuggestOptions options
                     SetTotalDifficultyFromBlockInfo(header, blockInfo);
                 }
 
-                isMainChain = level?.MainChainBlock?.BlockHash?.Equals(blockHash) == true;
                 if (requiresCanonical)
                 {
-                    header = isMainChain ? header : null;
+                    bool isMain = level.MainChainBlock?.BlockHash?.Equals(blockHash) == true;
+                    header = isMain ? header : null;
                 }
             }
 
-            if (header is not null && !fromCache && ShouldCache(header.Number))
+            if (header is not null && ShouldCache(header.Number))
             {
-                _headerStore.Cache(header, isMainChain);
+                _headerStore.Cache(header);
             }
 
             return header;
@@ -669,6 +650,11 @@ public AddBlockResult SuggestBlock(Block block, BlockTreeSuggestOptions options
             return null;
         }
 
+        public Hash256? FindHash(long number)
+        {
+            return GetBlockHashOnMainOrBestDifficultyHash(number);
+        }
+
         public IOwnedReadOnlyList<BlockHeader> FindHeaders(Hash256? blockHash, int numberOfBlocks, int skip, bool reverse)
         {
             if (numberOfBlocks == 0)
@@ -774,9 +760,7 @@ as it does not require the step of resolving number -> hash */
 
             if (level.HasBlockOnMainChain)
             {
-                Hash256 blockHash = level.BlockInfos[0].BlockHash;
-                _headerStore.CacheBlockHash(blockNumber, blockHash);
-                return blockHash;
+                return level.BlockInfos[0].BlockHash;
             }
 
             UInt256 bestDifficultySoFar = UInt256.Zero;
@@ -960,9 +944,11 @@ public void MarkChainAsProcessed(IReadOnlyList<Block> blocks)
             for (int i = 0; i < blocks.Count; i++)
             {
                 Block block = blocks[i];
-                // Header also updates the number <-> hash mapping so always update
-                _blockStore.Cache(block);
-                _headerStore.Cache(block.Header, isMainChain: true);
+                if (ShouldCache(block.Number))
+                {
+                    _blockStore.Cache(block);
+                    _headerStore.Cache(block.Header);
+                }
 
                 ChainLevelInfo? level = LoadLevel(block.Number);
                 int? index = (level?.FindIndex(block.Hash)) ?? throw new InvalidOperationException($"Cannot mark unknown block {block.ToString(Block.Format.FullHashAndNumber)} as processed");
@@ -1027,9 +1013,12 @@ public void UpdateMainChain(IReadOnlyList<Block> blocks, bool wereProcessed, boo
             for (int i = 0; i < blocks.Count; i++)
             {
                 Block block = blocks[i];
-                _blockStore.Cache(block);
-                // Header also updates the number <-> hash mapping so always update
-                _headerStore.Cache(block.Header, wereProcessed);
+                if (ShouldCache(block.Number))
+                {
+                    _blockStore.Cache(block);
+                    _headerStore.Cache(block.Header);
+                }
+
                 // we only force update head block for last block in processed blocks
                 bool lastProcessedBlock = i == blocks.Count - 1;
 
@@ -1446,23 +1435,12 @@ private bool ShouldCache(long number)
             }
 
             Block? block = null;
-            if ((options & BlockTreeLookupOptions.RequireCanonical) == 0)
-            {
-                block = _blockStore.GetFromCache(blockHash);
-                if (block is not null && block.TotalDifficulty.HasValue && (!blockNumber.HasValue || block.Number == blockNumber.Value))
-                {
-                    return block;
-                }
-            }
-
-            bool fromCache = false;
             blockNumber ??= _headerStore.GetBlockNumber(blockHash);
             if (blockNumber is not null)
             {
                 block = _blockStore.Get(
                     blockNumber.Value,
                     blockHash,
-                    out fromCache,
                     (options & BlockTreeLookupOptions.ExcludeTxHashes) != 0 ? RlpBehaviors.ExcludeHashes : RlpBehaviors.None,
                     shouldCache: false);
             }
@@ -1485,7 +1463,6 @@ private bool ShouldCache(long number)
             bool requiresCanonical = (options & BlockTreeLookupOptions.RequireCanonical) ==
                                      BlockTreeLookupOptions.RequireCanonical;
 
-            bool isMainChain = false;
             if ((totalDifficultyNeeded && block.TotalDifficulty is null) || requiresCanonical)
             {
                 (BlockInfo blockInfo, ChainLevelInfo level) = LoadInfo(block.Number, block.Hash, true);
@@ -1512,17 +1489,17 @@ private bool ShouldCache(long number)
                     SetTotalDifficultyFromBlockInfo(block.Header, blockInfo);
                 }
 
-                isMainChain = level?.MainChainBlock?.BlockHash.Equals(blockHash) == true;
                 if (requiresCanonical)
                 {
-                    block = isMainChain ? block : null;
+                    bool isMain = level.MainChainBlock?.BlockHash.Equals(blockHash) == true;
+                    block = isMain ? block : null;
                 }
             }
 
-            if (block is not null && !fromCache && ShouldCache(block.Number))
+            if (block is not null && ShouldCache(block.Number))
             {
                 _blockStore.Cache(block);
-                _headerStore.Cache(block.Header, isMainChain);
+                _headerStore.Cache(block.Header);
             }
 
             return block;

src/Nethermind/Nethermind.Blockchain/BlockTreeOverlay.cs

@@ -108,7 +108,7 @@ public void UpdateMainChain(IReadOnlyList<Block> blocks, bool wereProcessed, boo
 
     public BlockInfo FindCanonicalBlockInfo(long blockNumber) => _overlayTree.FindCanonicalBlockInfo(blockNumber) ?? _baseTree.FindCanonicalBlockInfo(blockNumber);
 
-    public Hash256 FindBlockHash(long blockNumber) => _overlayTree.FindBlockHash(blockNumber) ?? _baseTree.FindBlockHash(blockNumber);
+    public Hash256 FindHash(long blockNumber) => _overlayTree.FindHash(blockNumber) ?? _baseTree.FindHash(blockNumber);
 
     public IOwnedReadOnlyList<BlockHeader> FindHeaders(Hash256 hash, int numberOfBlocks, int skip, bool reverse)
     {
@@ -268,6 +268,9 @@ public bool HasBlock(long blockNumber, Hash256 blockHash) =>
     public BlockHeader? FindHeader(long blockNumber, BlockTreeLookupOptions options) =>
         _overlayTree.FindHeader(blockNumber, options) ?? _baseTree.FindHeader(blockNumber, options);
 
+    public Hash256? FindBlockHash(long blockNumber) =>
+        _overlayTree.FindBlockHash(blockNumber) ?? _baseTree.FindBlockHash(blockNumber);
+
     public bool IsMainChain(BlockHeader blockHeader) =>
         _baseTree.IsMainChain(blockHeader) || _overlayTree.IsMainChain(blockHeader);
 

src/Nethermind/Nethermind.Blockchain/BlockhashProvider.cs

@@ -46,13 +46,6 @@ public BlockhashProvider(IBlockFinder blockTree, ISpecProvider specProvider, IWo
                 return null;
             }
 
-            return (currentBlock.ParentHash == _blockTree.HeadHash || currentBlock.ParentHash == _blockTree.Head?.ParentHash) ?
-                _blockTree.FindBlockHash(number) :
-                GetBlockHashFromNonHeadParent(currentBlock, number);
-        }
-
-        private Hash256 GetBlockHashFromNonHeadParent(BlockHeader currentBlock, long number)
-        {
             BlockHeader header = _blockTree.FindParentHeader(currentBlock, BlockTreeLookupOptions.TotalDifficultyNotNeeded) ??
                 throw new InvalidDataException("Parent header cannot be found when executing BLOCKHASH operation");
 

src/Nethermind/Nethermind.Blockchain/Blocks/BlockStore.cs

@@ -18,7 +18,8 @@ public class BlockStore([KeyFilter(DbNames.Blocks)] IDb blockDb, IHeaderDecoder
     private readonly BlockDecoder _blockDecoder = new(headerDecoder ?? new HeaderDecoder());
     public const int CacheSize = 128 + 32;
 
-    private readonly ClockCache<Hash256AsKey, Block> _blockCache = new(CacheSize);
+    private readonly ClockCache<ValueHash256, Block>
+        _blockCache = new(CacheSize);
 
     public void SetMetadata(byte[] key, byte[] value)
     {
@@ -66,16 +67,9 @@ public void Delete(long blockNumber, Hash256 blockHash)
 
     public Block? Get(long blockNumber, Hash256 blockHash, RlpBehaviors rlpBehaviors = RlpBehaviors.None, bool shouldCache = false)
     {
-        Block? b = blockDb.Get(blockNumber, blockHash, _blockDecoder, out _, _blockCache, rlpBehaviors, shouldCache);
+        Block? b = blockDb.Get(blockNumber, blockHash, _blockDecoder, _blockCache, rlpBehaviors, shouldCache);
         if (b is not null) return b;
-        return blockDb.Get(blockHash, _blockDecoder, out _, _blockCache, rlpBehaviors, shouldCache);
-    }
-
-    public Block? Get(long blockNumber, Hash256 blockHash, out bool fromCache, RlpBehaviors rlpBehaviors = RlpBehaviors.None, bool shouldCache = false)
-    {
-        Block? b = blockDb.Get(blockNumber, blockHash, _blockDecoder, out fromCache, _blockCache, rlpBehaviors, shouldCache);
-        if (b is not null) return b;
-        return blockDb.Get(blockHash, _blockDecoder, out fromCache, _blockCache, rlpBehaviors, shouldCache);
+        return blockDb.Get(blockHash, _blockDecoder, _blockCache, rlpBehaviors, shouldCache);
     }
 
     public byte[]? GetRlp(long blockNumber, Hash256 blockHash)
@@ -102,7 +96,4 @@ public void Cache(Block block)
     {
         _blockCache.Set(block.Hash, block);
     }
-
-    public Block? GetFromCache(Hash256 blockHash)
-        => _blockCache.Get(blockHash);
 }

src/Nethermind/Nethermind.Blockchain/Blocks/IBlockStore.cs

@@ -15,12 +15,9 @@ public interface IBlockStore
 {
     void Insert(Block block, WriteFlags writeFlags = WriteFlags.None);
     void Delete(long blockNumber, Hash256 blockHash);
-    Block? Get(long blockNumber, Hash256 blockHash, RlpBehaviors rlpBehaviors = RlpBehaviors.None, bool shouldCache = true)
-        => Get(blockNumber, blockHash, out _, rlpBehaviors, shouldCache);
-    Block? Get(long blockNumber, Hash256 blockHash, out bool fromCache, RlpBehaviors rlpBehaviors = RlpBehaviors.None, bool shouldCache = true);
+    Block? Get(long blockNumber, Hash256 blockHash, RlpBehaviors rlpBehaviors = RlpBehaviors.None, bool shouldCache = true);
     byte[]? GetRlp(long blockNumber, Hash256 blockHash);
     ReceiptRecoveryBlock? GetReceiptRecoveryBlock(long blockNumber, Hash256 blockHash);
     void Cache(Block block);
-    Block? GetFromCache(Hash256 blockHash);
     bool HasBlock(long blockNumber, Hash256 blockHash);
 }