Remove duplicate calls to FindHeader in eth_getLogs (#6421)

* Remove duplicate calls to FindHeader in eth_getLogs * Check cancellation token * Fix tests due to params change
NethermindEth · Dec 26, 2023 · 1df2cf4 · 1df2cf4
1 parent 3fa5ba5
commit 1df2cf4
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 11 deletions.
diff --git a/src/Nethermind/Nethermind.Facade/BlockchainBridge.cs b/src/Nethermind/Nethermind.Facade/BlockchainBridge.cs
@@ -302,10 +302,28 @@ public IEnumerable<FilterLog> GetLogs(
             IEnumerable<object>? topics = null,
             CancellationToken cancellationToken = default)
         {
-            LogFilter filter = _filterStore.CreateLogFilter(fromBlock, toBlock, address, topics, false);
+            LogFilter filter = GetFilter(fromBlock, toBlock, address, topics);
             return _logFinder.FindLogs(filter, cancellationToken);
         }
 
+        public LogFilter GetFilter(
+            BlockParameter fromBlock,
+            BlockParameter toBlock,
+            object? address = null,
+            IEnumerable<object>? topics = null)
+        {
+            return _filterStore.CreateLogFilter(fromBlock, toBlock, address, topics, false);
+        }
+
+        public IEnumerable<FilterLog> GetLogs(
+            LogFilter filter,
+            BlockHeader fromBlock,
+            BlockHeader toBlock,
+            CancellationToken cancellationToken = default)
+        {
+            return _logFinder.FindLogs(filter, fromBlock, toBlock, cancellationToken);
+        }
+
         public bool TryGetLogs(int filterId, out IEnumerable<FilterLog> filterLogs, CancellationToken cancellationToken = default)
         {
             LogFilter? filter;
@@ -379,6 +397,11 @@ public bool HasStateForRoot(Hash256 stateRoot)
             return _processingEnv.StateReader.HasStateForRoot(stateRoot);
         }
 
+        public IEnumerable<FilterLog> FindLogs(LogFilter filter, BlockHeader fromBlock, BlockHeader toBlock, CancellationToken cancellationToken = default)
+        {
+            return _logFinder.FindLogs(filter, fromBlock, toBlock, cancellationToken);
+        }
+
         public IEnumerable<FilterLog> FindLogs(LogFilter filter, CancellationToken cancellationToken = default)
         {
             return _logFinder.FindLogs(filter, cancellationToken);

diff --git a/src/Nethermind/Nethermind.Facade/Filters/ILogFinder.cs b/src/Nethermind/Nethermind.Facade/Filters/ILogFinder.cs
@@ -4,12 +4,14 @@
 using System.Collections.Generic;
 using System.Threading;
 using Nethermind.Blockchain.Filters;
+using Nethermind.Core;
 using Nethermind.Facade.Filters;
 
 namespace Nethermind.Blockchain.Find
 {
     public interface ILogFinder
     {
         IEnumerable<FilterLog> FindLogs(LogFilter filter, CancellationToken cancellationToken = default);
+        IEnumerable<FilterLog> FindLogs(LogFilter filter, BlockHeader fromBlock, BlockHeader toBlock, CancellationToken cancellationToken = default);
     }
 }
diff --git a/src/Nethermind/Nethermind.Facade/Filters/LogFinder.cs b/src/Nethermind/Nethermind.Facade/Filters/LogFinder.cs
@@ -54,9 +54,18 @@ BlockHeader FindHeader(BlockParameter blockParameter, string name, bool headLimi
                 _blockFinder.FindHeader(blockParameter, headLimit) ?? throw new ResourceNotFoundException($"Block not found: {name} {blockParameter}");
 
             cancellationToken.ThrowIfCancellationRequested();
-            var toBlock = FindHeader(filter.ToBlock, nameof(filter.ToBlock), false);
+            BlockHeader toBlock = FindHeader(filter.ToBlock, nameof(filter.ToBlock), false);
+            cancellationToken.ThrowIfCancellationRequested();
+            BlockHeader fromBlock = filter.ToBlock == filter.FromBlock ?
+                toBlock :
+                FindHeader(filter.FromBlock, nameof(filter.FromBlock), false);
+
+            return FindLogs(filter, fromBlock, toBlock, cancellationToken);
+        }
+
+        public IEnumerable<FilterLog> FindLogs(LogFilter filter, BlockHeader fromBlock, BlockHeader toBlock, CancellationToken cancellationToken = default)
+        {
             cancellationToken.ThrowIfCancellationRequested();
-            var fromBlock = FindHeader(filter.FromBlock, nameof(filter.FromBlock), false);
 
             if (fromBlock.Number > toBlock.Number && toBlock.Number != 0)
             {

diff --git a/src/Nethermind/Nethermind.Facade/IBlockchainBridge.cs b/src/Nethermind/Nethermind.Facade/IBlockchainBridge.cs
@@ -41,7 +41,10 @@ public interface IBlockchainBridge : ILogFinder
         FilterType GetFilterType(int filterId);
         FilterLog[] GetFilterLogs(int filterId);
 
+        LogFilter GetFilter(BlockParameter fromBlock, BlockParameter toBlock, object? address = null, IEnumerable<object>? topics = null);
+        IEnumerable<FilterLog> GetLogs(LogFilter filter, BlockHeader fromBlock, BlockHeader toBlock, CancellationToken cancellationToken = default);
         IEnumerable<FilterLog> GetLogs(BlockParameter fromBlock, BlockParameter toBlock, object? address = null, IEnumerable<object>? topics = null, CancellationToken cancellationToken = default);
+
         bool TryGetLogs(int filterId, out IEnumerable<FilterLog> filterLogs, CancellationToken cancellationToken = default);
         void RunTreeVisitor(ITreeVisitor treeVisitor, Hash256 stateRoot);
         bool HasStateForRoot(Hash256 stateRoot);

diff --git a/src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs b/src/Nethermind/Nethermind.JsonRpc.Test/Modules/Eth/EthRpcModuleTests.cs
@@ -9,6 +9,7 @@
 using System.Threading.Tasks;
 using FluentAssertions;
 using Nethermind.Blockchain;
+using Nethermind.Blockchain.Filters;
 using Nethermind.Blockchain.Find;
 using Nethermind.Blockchain.Receipts;
 using Nethermind.Core;
@@ -523,7 +524,7 @@ public async Task Eth_get_logs(string parameter, string expected)
     {
         using Context ctx = await Context.Create();
         IBlockchainBridge bridge = Substitute.For<IBlockchainBridge>();
-        bridge.GetLogs(Arg.Any<BlockParameter>(), Arg.Any<BlockParameter>(), Arg.Any<object>(), Arg.Any<IEnumerable<object>>(), Arg.Any<CancellationToken>())
+        bridge.GetLogs(Arg.Any<LogFilter>(), Arg.Any<BlockHeader>(), Arg.Any<BlockHeader>(), Arg.Any<CancellationToken>())
             .Returns(new[] { new FilterLog(1, 0, 1, TestItem.KeccakA, 1, TestItem.KeccakB, TestItem.AddressA, new byte[] { 1, 2, 3 }, new[] { TestItem.KeccakC, TestItem.KeccakD }) });
         bridge.FilterExists(1).Returns(true);
 
@@ -538,10 +539,10 @@ public async Task Eth_get_logs_cancellation()
     {
         using Context ctx = await Context.Create();
         IBlockchainBridge bridge = Substitute.For<IBlockchainBridge>();
-        bridge.GetLogs(Arg.Any<BlockParameter>(), Arg.Any<BlockParameter>(), Arg.Any<object>(), Arg.Any<IEnumerable<object>>(), Arg.Any<CancellationToken>())
+        bridge.GetLogs(Arg.Any<LogFilter>(), Arg.Any<BlockHeader>(), Arg.Any<BlockHeader>(), Arg.Any<CancellationToken>())
             .Returns(c =>
             {
-                return GetLogs(c.ArgAt<CancellationToken>(4));
+                return GetLogs(c.ArgAt<CancellationToken>(3));
 
                 [DoesNotReturn]
                 IEnumerable<FilterLog> GetLogs(CancellationToken ct)
@@ -564,7 +565,7 @@ public async Task Eth_get_logs_with_resourceNotFound(string parameter, string ex
     {
         using Context ctx = await Context.Create();
         IBlockchainBridge bridge = Substitute.For<IBlockchainBridge>();
-        bridge.GetLogs(Arg.Any<BlockParameter>(), Arg.Any<BlockParameter>(), Arg.Any<object>(), Arg.Any<IEnumerable<object>>(), Arg.Any<CancellationToken>())
+        bridge.GetLogs(Arg.Any<LogFilter>(), Arg.Any<BlockHeader>(), Arg.Any<BlockHeader>(), Arg.Any<CancellationToken>())
             .Throws(new ResourceNotFoundException("resource not found message"));
         bridge.FilterExists(1).Returns(true);
 

diff --git a/src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs b/src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs
@@ -602,7 +602,9 @@ public ResultWrapper<IEnumerable<FilterLog>> eth_getLogs(Filter filter)
         SearchResult<BlockHeader> toBlockResult;
 
         if (filter.FromBlock == filter.ToBlock)
+        {
             fromBlockResult = toBlockResult = _blockFinder.SearchForHeader(filter.ToBlock);
+        }
         else
         {
             toBlockResult = _blockFinder.SearchForHeader(filter.ToBlock);
@@ -625,8 +627,11 @@ public ResultWrapper<IEnumerable<FilterLog>> eth_getLogs(Filter filter)
 
         cancellationToken.ThrowIfCancellationRequested();
 
-        long fromBlockNumber = fromBlockResult.Object!.Number;
-        long toBlockNumber = toBlockResult.Object!.Number;
+        BlockHeader fromBlock = fromBlockResult.Object!;
+        BlockHeader toBlock = toBlockResult.Object!;
+
+        long fromBlockNumber = fromBlock.Number;
+        long toBlockNumber = toBlock.Number;
 
         if (fromBlockNumber > toBlockNumber && toBlockNumber != 0)
         {
@@ -637,8 +642,9 @@ public ResultWrapper<IEnumerable<FilterLog>> eth_getLogs(Filter filter)
 
         try
         {
-            IEnumerable<FilterLog> filterLogs = _blockchainBridge.GetLogs(filter.FromBlock!, filter.ToBlock!,
-                filter.Address, filter.Topics, cancellationToken);
+            LogFilter logFilter = _blockchainBridge.GetFilter(filter.FromBlock, filter.ToBlock,
+                filter.Address, filter.Topics);
+            IEnumerable<FilterLog> filterLogs = _blockchainBridge.GetLogs(logFilter, fromBlock, toBlock, cancellationToken);
 
             return ResultWrapper<IEnumerable<FilterLog>>.Success(GetLogs(filterLogs, cancellationTokenSource));
         }