Return the correct latest valid hash in case of bad block (#4056)

* Return of lastest valid hash in case of bad block

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Add CHANGELOG entry

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Remove now unused method

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Revert changes to backward sync, since not needed for this fix

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Adapt new test coming from the rebase with main

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

* Unit tests

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

CHANGELOG.md

@@ -20,6 +20,7 @@
 - Support free gas networks in the London fee market [#4003](https://github.com/hyperledger/besu/pull/4003)
 - Limit the size of outgoing eth subprotocol messages.  [#4034](https://github.com/hyperledger/besu/pull/4034)
 - Fixed a state root mismatch issue on bonsai that may appear occasionally [#4041](https://github.com/hyperledger/besu/pull/4041)
+- Return the correct latest valid hash in case of bad block when calling engine methods [#4056](https://github.com/hyperledger/besu/pull/4056)
 
 ## 22.4.3
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java

@@ -85,22 +85,17 @@ public class EngineForkchoiceUpdated extends ExecutionEngineJsonRpcMethod {
           requestId,
           new EngineUpdateForkchoiceResult(
               INVALID,
-              Hash.ZERO,
+              mergeCoordinator
+                  .getLatestValidAncestor(forkChoice.getHeadBlockHash())
+                  .orElse(Hash.ZERO),
               null,
               Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
     }
 
     Optional<BlockHeader> newHead =
-        protocolContext.getBlockchain().getBlockHeader(forkChoice.getHeadBlockHash());
+        mergeCoordinator.getOrSyncHeaderByHash(forkChoice.getHeadBlockHash());
 
     if (newHead.isEmpty()) {
-      Optional.ofNullable(forkChoice.getHeadBlockHash())
-          .filter(hash -> !hash.equals(Hash.ZERO))
-          .ifPresent(
-              blockHash ->
-                  mergeCoordinator.getOrSyncHeaderByHash(
-                      blockHash, forkChoice.getFinalizedBlockHash()));
-
       return syncingResponse(requestId);
     }
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java

@@ -142,7 +142,11 @@ public class EngineNewPayload extends ExecutionEngineJsonRpcMethod {
       return respondWith(reqId, blockParam, blockParam.getBlockHash(), VALID);
     }
     if (mergeCoordinator.isBadBlock(blockParam.getParentHash())) {
-      return respondWith(reqId, blockParam, Hash.ZERO, INVALID);
+      return respondWith(
+          reqId,
+          blockParam,
+          mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(Hash.ZERO),
+          INVALID);
     }
 
     Optional<BlockHeader> parentHeader =
@@ -160,8 +164,7 @@ public class EngineNewPayload extends ExecutionEngineJsonRpcMethod {
     final var block =
         new Block(newBlockHeader, new BlockBody(transactions, Collections.emptyList()));
 
-    if (mergeContext.isSyncing()
-        || mergeCoordinator.getOrSyncHeaderByHash(newBlockHeader.getParentHash()).isEmpty()) {
+    if (mergeContext.isSyncing() || parentHeader.isEmpty()) {
       mergeCoordinator.appendNewPayloadToSync(block);
       return respondWith(reqId, blockParam, null, SYNCING);
     }

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedTest.java

@@ -66,6 +66,9 @@ public class EngineForkchoiceUpdatedTest {
   private static final Vertx vertx = Vertx.vertx();
   private static final Hash mockHash = Hash.hash(Bytes32.fromHexStringLenient("0x1337deadbeef"));
 
+  private static final EngineForkchoiceUpdatedParameter mockFcuParam =
+      new EngineForkchoiceUpdatedParameter(mockHash, mockHash, mockHash);
+
   @Mock private ProtocolContext protocolContext;
 
   @Mock private MergeContext mergeContext;
@@ -91,40 +94,62 @@ public class EngineForkchoiceUpdatedTest {
   public void shouldReturnSyncingIfForwardSync() {
     when(mergeContext.isSyncing()).thenReturn(true);
     assertSuccessWithPayloadForForkchoiceResult(
-        Optional.empty(), mock(ForkchoiceResult.class), SYNCING);
+        mockFcuParam, Optional.empty(), mock(ForkchoiceResult.class), SYNCING);
   }
 
   @Test
   public void shouldReturnSyncingIfMissingNewHead() {
     assertSuccessWithPayloadForForkchoiceResult(
-        Optional.empty(), mock(ForkchoiceResult.class), SYNCING);
+        mockFcuParam, Optional.empty(), mock(ForkchoiceResult.class), SYNCING);
   }
 
   @Test
   public void shouldReturnInvalidOnBadTerminalBlock() {
     BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
 
-    when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader));
+    when(mergeCoordinator.getOrSyncHeaderByHash(mockHeader.getHash()))
+        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(false);
-    when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
     assertSuccessWithPayloadForForkchoiceResult(
-        Optional.empty(), mock(ForkchoiceResult.class), INVALID, Optional.of(Hash.ZERO));
+        new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO),
+        Optional.empty(),
+        mock(ForkchoiceResult.class),
+        INVALID,
+        Optional.of(Hash.ZERO));
+  }
+
+  @Test
+  public void shouldReturnInvalidWithLatestValidHashOnBadBlock() {
+    BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
+    Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe"));
+    when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true);
+    when(mergeCoordinator.getLatestValidAncestor(mockHeader.getHash()))
+        .thenReturn(Optional.of(latestValidHash));
+
+    assertSuccessWithPayloadForForkchoiceResult(
+        new EngineForkchoiceUpdatedParameter(
+            mockHeader.getHash(), Hash.ZERO, mockHeader.getParentHash()),
+        Optional.empty(),
+        mock(ForkchoiceResult.class),
+        INVALID,
+        Optional.of(latestValidHash));
   }
 
   @Test
   public void shouldReturnSyncingOnHeadNotFound() {
     assertSuccessWithPayloadForForkchoiceResult(
-        Optional.empty(), mock(ForkchoiceResult.class), SYNCING);
+        mockFcuParam, Optional.empty(), mock(ForkchoiceResult.class), SYNCING);
   }
 
   @Test
   public void shouldReturnValidWithoutFinalizedOrPayload() {
     BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-    when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader));
+    when(mergeCoordinator.getOrSyncHeaderByHash(mockHeader.getHash()))
+        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
-    when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
 
     assertSuccessWithPayloadForForkchoiceResult(
+        new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO),
         Optional.empty(),
         ForkchoiceResult.withResult(Optional.empty(), Optional.of(mockHeader)),
         VALID);
@@ -139,12 +164,12 @@ public class EngineForkchoiceUpdatedTest {
             .parentHash(parent.getHash())
             .timestamp(parent.getTimestamp())
             .buildHeader();
-    when(blockchain.getBlockHeader(mockHeader.getHash())).thenReturn(Optional.of(mockHeader));
     when(blockchain.getBlockHeader(parent.getHash())).thenReturn(Optional.of(parent));
-    //    when(blockchain.getChainHeadHeader()).thenReturn(parent);
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(mockHeader.getHash()))
+        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.updateForkChoice(
             mockHeader, parent.getHash(), parent.getHash(), Optional.empty()))
         .thenReturn(
@@ -172,10 +197,13 @@ public class EngineForkchoiceUpdatedTest {
     BlockHeader mockParent = builder.number(9L).buildHeader();
     BlockHeader mockHeader = builder.number(10L).parentHash(mockParent.getHash()).buildHeader();
     when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader));
+    when(mergeCoordinator.getOrSyncHeaderByHash(mockHeader.getHash()))
+        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
 
     assertSuccessWithPayloadForForkchoiceResult(
+        new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, mockParent.getHash()),
         Optional.empty(),
         ForkchoiceResult.withResult(Optional.of(mockParent), Optional.of(mockHeader)),
         VALID);
@@ -184,9 +212,9 @@ public class EngineForkchoiceUpdatedTest {
   @Test
   public void shouldReturnValidWithoutFinalizedWithPayload() {
     BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-    when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader));
+    when(mergeCoordinator.getOrSyncHeaderByHash(mockHeader.getHash()))
+        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
-    when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
 
     var payloadParams =
         new EnginePayloadAttributesParameter(
@@ -202,6 +230,7 @@ public class EngineForkchoiceUpdatedTest {
 
     var res =
         assertSuccessWithPayloadForForkchoiceResult(
+            new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO),
             Optional.of(payloadParams),
             ForkchoiceResult.withResult(Optional.empty(), Optional.of(mockHeader)),
             VALID);
@@ -214,9 +243,10 @@ public class EngineForkchoiceUpdatedTest {
     BlockHeader newHead = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
     Hash finalizedBlockHash = Hash.hash(Bytes32.fromHexStringLenient("0x424abcdef"));
 
-    when(blockchain.getBlockHeader(newHead.getHash())).thenReturn(Optional.of(newHead));
     when(blockchain.getBlockHeader(finalizedBlockHash)).thenReturn(Optional.empty());
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(newHead.getHash()))
+        .thenReturn(Optional.of(newHead));
 
     var resp =
         resp(
@@ -235,6 +265,8 @@ public class EngineForkchoiceUpdatedTest {
     when(blockchain.getBlockHeader(newHead.getHash())).thenReturn(Optional.of(newHead));
     when(blockchain.getBlockHeader(finalized.getHash())).thenReturn(Optional.of(finalized));
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(newHead.getHash()))
+        .thenReturn(Optional.of(newHead));
     when(mergeCoordinator.isDescendantOf(finalized, newHead)).thenReturn(false);
 
     var resp =
@@ -256,9 +288,10 @@ public class EngineForkchoiceUpdatedTest {
             .timestamp(parent.getTimestamp())
             .buildHeader();
 
-    when(blockchain.getBlockHeader(newHead.getHash())).thenReturn(Optional.of(newHead));
     when(blockchain.getBlockHeader(parent.getHash())).thenReturn(Optional.of(parent));
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(newHead.getHash()))
+        .thenReturn(Optional.of(newHead));
 
     var resp =
         resp(
@@ -280,10 +313,11 @@ public class EngineForkchoiceUpdatedTest {
             .buildHeader();
     Hash safeBlockBlockHash = Hash.hash(Bytes32.fromHexStringLenient("0x424abcdef"));
 
-    when(blockchain.getBlockHeader(newHead.getHash())).thenReturn(Optional.of(newHead));
     when(blockchain.getBlockHeader(finalized.getHash())).thenReturn(Optional.of(finalized));
     when(blockchain.getBlockHeader(safeBlockBlockHash)).thenReturn(Optional.empty());
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(newHead.getHash()))
+        .thenReturn(Optional.of(newHead));
     when(mergeCoordinator.isDescendantOf(finalized, newHead)).thenReturn(true);
 
     var resp =
@@ -305,6 +339,8 @@ public class EngineForkchoiceUpdatedTest {
     when(blockchain.getBlockHeader(finalized.getHash())).thenReturn(Optional.of(finalized));
     when(blockchain.getBlockHeader(safeBlock.getHash())).thenReturn(Optional.of(safeBlock));
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(newHead.getHash()))
+        .thenReturn(Optional.of(newHead));
     when(mergeCoordinator.isDescendantOf(finalized, newHead)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(finalized, safeBlock)).thenReturn(false);
 
@@ -327,6 +363,8 @@ public class EngineForkchoiceUpdatedTest {
     when(blockchain.getBlockHeader(finalized.getHash())).thenReturn(Optional.of(finalized));
     when(blockchain.getBlockHeader(safeBlock.getHash())).thenReturn(Optional.of(safeBlock));
     when(mergeContext.isSyncing()).thenReturn(false);
+    when(mergeCoordinator.getOrSyncHeaderByHash(newHead.getHash()))
+        .thenReturn(Optional.of(newHead));
     when(mergeCoordinator.isDescendantOf(finalized, newHead)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(finalized, safeBlock)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(safeBlock, newHead)).thenReturn(false);
@@ -344,7 +382,8 @@ public class EngineForkchoiceUpdatedTest {
   public void shouldIgnoreUpdateToOldHeadAndNotPreparePayload() {
     BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
 
-    when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader));
+    when(mergeCoordinator.getOrSyncHeaderByHash(mockHeader.getHash()))
+        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
 
     var ignoreOldHeadUpdateRes = ForkchoiceResult.withIgnoreUpdateToOldHead(mockHeader);
@@ -376,14 +415,16 @@ public class EngineForkchoiceUpdatedTest {
   }
 
   private EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult(
+      final EngineForkchoiceUpdatedParameter fcuParam,
       final Optional<EnginePayloadAttributesParameter> payloadParam,
       final ForkchoiceResult forkchoiceResult,
       final EngineStatus expectedStatus) {
     return assertSuccessWithPayloadForForkchoiceResult(
-        payloadParam, forkchoiceResult, expectedStatus, Optional.empty());
+        fcuParam, payloadParam, forkchoiceResult, expectedStatus, Optional.empty());
   }
 
   private EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult(
+      final EngineForkchoiceUpdatedParameter fcuParam,
       final Optional<EnginePayloadAttributesParameter> payloadParam,
       final ForkchoiceResult forkchoiceResult,
       final EngineStatus expectedStatus,
@@ -393,8 +434,7 @@ public class EngineForkchoiceUpdatedTest {
     when(mergeCoordinator.updateForkChoice(
             any(BlockHeader.class), any(Hash.class), any(Hash.class), any()))
         .thenReturn(forkchoiceResult);
-    var resp =
-        resp(new EngineForkchoiceUpdatedParameter(mockHash, mockHash, mockHash), payloadParam);
+    var resp = resp(fcuParam, payloadParam);
     var res = fromSuccessResp(resp);
 
     assertThat(res.getPayloadStatus().getStatusAsString()).isEqualTo(expectedStatus.name());
@@ -417,7 +457,12 @@ public class EngineForkchoiceUpdatedTest {
 
     // assert that listeners are always notified
     verify(mergeContext)
-        .fireNewUnverifiedForkchoiceMessageEvent(mockHash, Optional.of(mockHash), mockHash);
+        .fireNewUnverifiedForkchoiceMessageEvent(
+            fcuParam.getHeadBlockHash(),
+            fcuParam.getFinalizedBlockHash().isZero()
+                ? Optional.empty()
+                : Optional.of(fcuParam.getFinalizedBlockHash()),
+            fcuParam.getSafeBlockHash());
 
     return res;
   }

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadTest.java

@@ -21,6 +21,7 @@ import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.Executi
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.SYNCING;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
@@ -88,14 +89,14 @@ public class EngineNewPayloadTest {
 
   @Test
   public void shouldReturnValid() {
-    BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
+    BlockHeader mockHeader = createBlockHeader();
+    when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty());
+    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
+        .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.of(mockHash));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
         .thenReturn(true);
-    when(mergeCoordinator.getOrSyncHeaderByHash(any(Hash.class)))
-        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.rememberBlock(any()))
         .thenReturn(new Result(new BlockProcessingOutputs(null, List.of())));
 
@@ -109,14 +110,14 @@ public class EngineNewPayloadTest {
 
   @Test
   public void shouldReturnInvalidOnBlockExecutionError() {
-    BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
+    BlockHeader mockHeader = createBlockHeader();
+    when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty());
+    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
+        .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.of(mockHash));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
         .thenReturn(true);
-    when(mergeCoordinator.getOrSyncHeaderByHash(any(Hash.class)))
-        .thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.rememberBlock(any())).thenReturn(new Result("error 42"));
 
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
@@ -129,14 +130,14 @@ public class EngineNewPayloadTest {
 
   @Test
   public void shouldReturnAcceptedOnLatestValidAncestorEmpty() {
-    BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
+    BlockHeader mockHeader = createBlockHeader();
+    when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty());
+    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
+        .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.empty());
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
         .thenReturn(true);
-    when(mergeCoordinator.getOrSyncHeaderByHash(any(Hash.class)))
-        .thenReturn(Optional.of(mockHeader));
 
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
 
@@ -164,14 +165,12 @@ public class EngineNewPayloadTest {
 
   @Test
   public void shouldReturnInvalidOnBadTerminalBlock() {
-    BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-
-    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
-
+    BlockHeader mockHeader = createBlockHeader();
+    when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty());
+    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
+        .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
         .thenReturn(false);
-    when(mergeCoordinator.getOrSyncHeaderByHash(any(Hash.class)))
-        .thenReturn(Optional.of(mockHeader));
 
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
 
@@ -180,6 +179,23 @@ public class EngineNewPayloadTest {
     assertThat(res.getStatusAsString()).isEqualTo(INVALID.name());
   }
 
+  @Test
+  public void shouldReturnInvalidWithLatestValidHashIfDescendingFromBadBlock() {
+    BlockHeader mockHeader = createBlockHeader();
+    Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe"));
+
+    when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty());
+    when(mergeCoordinator.isBadBlock(mockHeader.getParentHash())).thenReturn(true);
+    when(mergeCoordinator.getLatestValidAncestor(mockHeader.getParentHash()))
+        .thenReturn(Optional.of(latestValidHash));
+
+    var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
+
+    EnginePayloadStatusResult res = fromSuccessResp(resp);
+    assertThat(res.getLatestValidHash()).isEqualTo(Optional.of(latestValidHash));
+    assertThat(res.getStatusAsString()).isEqualTo(INVALID.name());
+  }
+
   @Test
   public void shouldReturnInvalidBlockHashOnBadHashParameter() {
     BlockHeader mockHeader = new BlockHeaderTestFixture().buildHeader();
@@ -255,7 +271,6 @@ public class EngineNewPayloadTest {
 
   @Test
   public void shouldRespondWithSyncingDuringBackwardsSync() {
-    when(mergeCoordinator.getOrSyncHeaderByHash(any(Hash.class))).thenReturn(Optional.empty());
     BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
 
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
@@ -333,4 +348,17 @@ public class EngineNewPayloadTest {
         .map(EnginePayloadStatusResult.class::cast)
         .get();
   }
+
+  private BlockHeader createBlockHeader() {
+    BlockHeader parentBlockHeader =
+        new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
+    BlockHeader mockHeader =
+        new BlockHeaderTestFixture()
+            .baseFeePerGas(Wei.ONE)
+            .parentHash(parentBlockHeader.getParentHash())
+            .number(parentBlockHeader.getNumber() + 1)
+            .timestamp(parentBlockHeader.getTimestamp() + 1)
+            .buildHeader();
+    return mockHeader;
+  }
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardChain.java

@@ -144,6 +144,7 @@ public class BackwardChain {
   }
 
   public synchronized void appendTrustedBlock(final Block newPivot) {
+    debugLambda(LOG, "appending trusted block {}", newPivot::toLogString);
     headers.put(newPivot.getHash(), newPivot.getHeader());
     blocks.put(newPivot.getHash(), newPivot);
     if (lastStoredPivot.isEmpty()) {