Fix get logs patch 2 (#1381)

LogBloom could be invalid in the cache after a reorg. With this PR, there is an update of the cache every time a reorg takes place

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>

CHANGELOG.md

@@ -7,7 +7,7 @@
 
 ### Bug Fixes
 * Added `debug_getBadBlocks` JSON-RPC API to analyze and detect consensus flaws. Even if a block is rejected it will be returned by this method [\#1378](https://github.com/hyperledger/besu/pull/1378)
-* Fix logs queries missing results against chain head [\#1351](https://github.com/hyperledger/besu/pull/1351)
+* Fix logs queries missing results against chain head [\#1351](https://github.com/hyperledger/besu/pull/1351) and [\#1381](https://github.com/hyperledger/besu/pull/1381) 
 
 #### Previously identified known issues
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/AutoTransactionLogBloomCachingService.java

@@ -17,6 +17,7 @@ package org.hyperledger.besu.ethereum.api.query.cache;
 import static org.hyperledger.besu.ethereum.api.query.cache.LogBloomCacheMetadata.DEFAULT_VERSION;
 
 import org.hyperledger.besu.ethereum.chain.Blockchain;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
 
 import java.io.File;
 import java.io.IOException;
@@ -51,7 +52,7 @@ public class AutoTransactionLogBloomCachingService {
       }
       final LogBloomCacheMetadata logBloomCacheMetadata =
           LogBloomCacheMetadata.lookUpFrom(cacheDir);
-      if (logBloomCacheMetadata.getVersion() == 0) {
+      if (logBloomCacheMetadata.getVersion() < DEFAULT_VERSION) {
         try (Stream<Path> walk = Files.walk(cacheDir)) {
           walk.filter(Files::isRegularFile).map(Path::toFile).forEach(File::delete);
         } catch (Exception e) {
@@ -65,10 +66,14 @@ public class AutoTransactionLogBloomCachingService {
               blockchain.observeBlockAdded(
                   event -> {
                     if (event.isNewCanonicalHead()) {
+                      final BlockHeader eventBlockHeader = event.getBlock().getHeader();
+                      final Optional<BlockHeader> commonAncestorBlockHeader =
+                          blockchain.getBlockHeader(event.getCommonAncestorHash());
                       transactionLogBloomCacher.cacheLogsBloomForBlockHeader(
-                          event.getBlock().getHeader(), Optional.empty());
+                          eventBlockHeader, commonAncestorBlockHeader, Optional.empty());
                     }
                   }));
+
       transactionLogBloomCacher
           .getScheduler()
           .scheduleFutureTask(transactionLogBloomCacher::cacheAll, Duration.ofMinutes(1));

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/LogBloomCacheMetadata.java

@@ -29,7 +29,7 @@ import org.apache.logging.log4j.Logger;
 public class LogBloomCacheMetadata {
   private static final Logger LOG = LogManager.getLogger();
 
-  public static final int DEFAULT_VERSION = 1;
+  public static final int DEFAULT_VERSION = 2;
 
   private static final String METADATA_FILENAME = "CACHE_METADATA.json";
   private static final ObjectMapper MAPPER = new ObjectMapper();

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/cache/TransactionLogBloomCacher.java

@@ -22,6 +22,7 @@ import static com.google.common.base.Preconditions.checkState;
 
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.ProcessableBlockHeader;
 import org.hyperledger.besu.ethereum.eth.manager.EthScheduler;
 
 import java.io.File;
@@ -110,7 +111,9 @@ public class TransactionLogBloomCacher {
           blockchain
               .getBlockHeader(blockNum)
               .ifPresent(
-                  blockHeader -> cacheLogsBloomForBlockHeader(blockHeader, Optional.of(cacheFile)));
+                  blockHeader ->
+                      cacheLogsBloomForBlockHeader(
+                          blockHeader, Optional.empty(), Optional.of(cacheFile)));
           fillCacheFile(blockNum, blockNum + BLOCKS_PER_BLOOM_CACHE, cacheFile);
         }
       } catch (final Exception e) {
@@ -141,7 +144,9 @@ public class TransactionLogBloomCacher {
   }
 
   void cacheLogsBloomForBlockHeader(
-      final BlockHeader blockHeader, final Optional<File> reusedCacheFile) {
+      final BlockHeader blockHeader,
+      final Optional<BlockHeader> commonAncestorBlockHeader,
+      final Optional<File> reusedCacheFile) {
     try {
       if (cachingStatus.cachingCount.incrementAndGet() != 1) {
         return;
@@ -151,6 +156,20 @@ public class TransactionLogBloomCacher {
       final File cacheFile = reusedCacheFile.orElse(calculateCacheFileName(blockNumber, cacheDir));
       if (cacheFile.exists()) {
         try {
+          final Optional<Long> ancestorBlockNumber =
+              commonAncestorBlockHeader.map(ProcessableBlockHeader::getNumber);
+          if (ancestorBlockNumber.isPresent()) {
+            // walk through the blocks from the common ancestor to the received block in order to
+            // reload the cache in case of reorg
+            for (long number = ancestorBlockNumber.get() + 1;
+                number < blockHeader.getNumber();
+                number++) {
+              Optional<BlockHeader> ancestorBlockHeader = blockchain.getBlockHeader(number);
+              if (ancestorBlockHeader.isPresent()) {
+                cacheSingleBlock(ancestorBlockHeader.get(), cacheFile);
+              }
+            }
+          }
           cacheSingleBlock(blockHeader, cacheFile);
         } catch (final InvalidCacheException e) {
           populateLatestSegment(blockNumber);

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/query/cache/TransactionLogBloomCacherTest.java

@@ -133,7 +133,7 @@ public class TransactionLogBloomCacherTest {
     assertThat(logBloom.length()).isEqualTo(BLOOM_BITS_LENGTH * 3);
 
     transactionLogBloomCacher.cacheLogsBloomForBlockHeader(
-        blockchain.getBlockHeader(3).get(), Optional.of(logBloom));
+        blockchain.getBlockHeader(3).get(), Optional.empty(), Optional.of(logBloom));
 
     assertThat(logBloom.length()).isEqualTo(BLOOM_BITS_LENGTH * 4);
     assertThat(cacheDir.getRoot().list().length).isEqualTo(1);
@@ -152,7 +152,7 @@ public class TransactionLogBloomCacherTest {
     }
 
     transactionLogBloomCacher.cacheLogsBloomForBlockHeader(
-        blockchain.getBlockHeader(4).get(), Optional.of(logBloom));
+        blockchain.getBlockHeader(4).get(), Optional.empty(), Optional.of(logBloom));
 
     for (int i = 0; i < 5; i++) {
       assertThat(blockHeaders.get(i).getLogsBloom().toArray())
@@ -186,16 +186,37 @@ public class TransactionLogBloomCacherTest {
   public void shouldUpdateCacheWhenChainReorgFired() throws IOException {
     final File logBloom = cacheDir.newFile("logBloom-0.cache");
 
+    final List<BlockHeader> firstBranch = new ArrayList<>();
+
     for (int i = 0; i < 5; i++) {
-      createBlock(i);
+      firstBranch.add(createBlock(i));
     }
 
     transactionLogBloomCacher.cacheLogsBloomForBlockHeader(
-        blockchain.getBlockHeader(4).get(), Optional.of(logBloom));
+        blockchain.getBlockHeader(4).get(), Optional.empty(), Optional.of(logBloom));
     assertThat(logBloom.length()).isEqualTo(BLOOM_BITS_LENGTH * 5);
+    for (int i = 0; i < 5; i++) {
+      assertThat(firstBranch.get(i).getLogsBloom().toArray())
+          .containsExactly(readLogBloomCache(logBloom, i));
+    }
+
+    final List<BlockHeader> forkBranch = new ArrayList<>();
+    forkBranch.add(firstBranch.get(0));
+    forkBranch.add(firstBranch.get(1));
+    for (int i = 2; i < 5; i++) {
+      forkBranch.add(createBlock(i, Optional.of("111111111111111111111111")));
+    }
 
     transactionLogBloomCacher.cacheLogsBloomForBlockHeader(
-        blockchain.getBlockHeader(1).get(), Optional.of(logBloom));
+        blockchain.getBlockHeader(4).get(), blockchain.getBlockHeader(1), Optional.of(logBloom));
+    assertThat(logBloom.length()).isEqualTo(BLOOM_BITS_LENGTH * 5);
+    for (int i = 0; i < 5; i++) {
+      assertThat(forkBranch.get(i).getLogsBloom().toArray())
+          .containsExactly(readLogBloomCache(logBloom, i));
+    }
+
+    transactionLogBloomCacher.cacheLogsBloomForBlockHeader(
+        blockchain.getBlockHeader(1).get(), Optional.empty(), Optional.of(logBloom));
     assertThat(logBloom.length()).isEqualTo(BLOOM_BITS_LENGTH * 2);
 
     assertThat(cacheDir.getRoot().list().length).isEqualTo(1);
@@ -217,7 +238,12 @@ public class TransactionLogBloomCacherTest {
   }
 
   private BlockHeader createBlock(final long number) {
-    final Address testAddress = Address.fromHexString(String.format("%02X", number));
+    return createBlock(number, Optional.empty());
+  }
+
+  private BlockHeader createBlock(final long number, final Optional<String> message) {
+    final Address testAddress =
+        Address.fromHexString(message.orElse(String.format("%02X", number)));
     final Bytes testMessage = Bytes.fromHexString(String.format("%02X", number));
     final Log testLog = new Log(testAddress, testMessage, List.of());
     final BlockHeader fakeHeader =
@@ -241,7 +267,6 @@ public class TransactionLogBloomCacherTest {
             new MainnetBlockHeaderFunctions());
     testHash = fakeHeader.getHash();
     when(blockchain.getBlockHeader(number)).thenReturn(Optional.of(fakeHeader));
-
     return fakeHeader;
   }
 }