Remove hash to append from the queue only if the step succeeds (hyper…

…ledger#4105) * Remove hash to append from the queue only if the step succeeds Signed-off-by: Fabio Di Fabio <[email protected]>
lacchain · Nov 3, 2023 · 2ea6752 · 2ea6752
1 parent ea39842
commit 2ea6752
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 - Add a PoS block header rule to check that the current block is more recent than its parent [#4066](https://github.com/hyperledger/besu/pull/4066)
 - Fixed a trie log layer issue on bonsai during reorg [#4069](https://github.com/hyperledger/besu/pull/4069)
 - Fix transition protocol schedule to return the pre Merge schedule when reorg pre TTD [#4078](https://github.com/hyperledger/besu/pull/4078)
+- Remove hash to sync from the queue only if the sync step succeeds [#4105](https://github.com/hyperledger/besu/pull/4105)
 - The build process runs successfully even though the system language is not English  [#4102](https://github.com/hyperledger/besu/pull/4102)
 
 ## 22.7.0-RC1

diff --git a/.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardChain.java b/.../eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardChain.java
@@ -172,7 +172,13 @@ public synchronized void addNewHash(final Hash newBlockHash) {
   }
 
   public synchronized Optional<Hash> getFirstHashToAppend() {
-    return Optional.ofNullable(hashesToAppend.poll());
+    return Optional.ofNullable(hashesToAppend.peek());
+  }
+
+  public synchronized void removeFromHashToAppend(final Hash hashToRemove) {
+    if (hashesToAppend.contains(hashToRemove)) {
+      hashesToAppend.remove(hashToRemove);
+    }
   }
 
   public void addBadChainToManager(final BadBlockManager badBlocksManager, final Hash hash) {

diff --git a/...main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardsSyncAlgorithm.java b/...main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardsSyncAlgorithm.java
@@ -56,7 +56,13 @@ public CompletableFuture<Void> executeBackwardsSync(final Void unused) {
   public CompletableFuture<Void> pickNextStep() {
     final Optional<Hash> firstHash = context.getBackwardChain().getFirstHashToAppend();
     if (firstHash.isPresent()) {
-      return executeSyncStep(firstHash.get());
+      return executeSyncStep(firstHash.get())
+          .whenComplete(
+              (result, throwable) -> {
+                if (throwable == null) {
+                  context.getBackwardChain().removeFromHashToAppend(firstHash.get());
+                }
+              });
     }
     if (!context.isReady()) {
       return waitForReady();

diff --git a/...t/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/InMemoryBackwardChainTest.java b/...t/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/InMemoryBackwardChainTest.java
@@ -138,12 +138,15 @@ public void shouldAddHeaderToQueue() {
     firstHash = backwardChain.getFirstHashToAppend();
     assertThat(firstHash).isPresent();
     assertThat(firstHash.orElseThrow()).isEqualTo(blocks.get(7).getHash());
+    backwardChain.removeFromHashToAppend(firstHash.get());
     firstHash = backwardChain.getFirstHashToAppend();
     assertThat(firstHash).isPresent();
     assertThat(firstHash.orElseThrow()).isEqualTo(blocks.get(9).getHash());
+    backwardChain.removeFromHashToAppend(firstHash.get());
     firstHash = backwardChain.getFirstHashToAppend();
     assertThat(firstHash).isPresent();
     assertThat(firstHash.orElseThrow()).isEqualTo(blocks.get(11).getHash());
+    backwardChain.removeFromHashToAppend(firstHash.get());
   }
 
   @Test