Revert "fcu before engine new payload (#10731)" (#10754)

This reverts commit cf7d594.

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

Author: matkt

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java

@@ -727,27 +727,6 @@ public ForkchoiceResult updateForkChoiceWithoutLegacySkip(
     return applyForkChoice(newHead, finalizedBlockHash, safeBlockHash);
   }
 
-  @Override
-  public ForkchoiceResult updateHeadForExecution(final BlockHeader newHead) {
-    final MutableBlockchain blockchain = protocolContext.getBlockchain();
-    final Optional<Hash> latestValid = getLatestValidAncestor(newHead);
-
-    Optional<BlockHeader> parentOfNewHead = blockchain.getBlockHeader(newHead.getParentHash());
-    if (parentOfNewHead.isPresent()
-        && Long.compareUnsigned(newHead.getTimestamp(), parentOfNewHead.get().getTimestamp())
-            <= 0) {
-      return ForkchoiceResult.withFailure(
-          INVALID, "new head timestamp not greater than parent", latestValid);
-    }
-
-    if (!setNewHead(blockchain, newHead)) {
-      LOG.warn("Failed to move world state to new head {}", newHead.toLogString());
-      return ForkchoiceResult.withFailure(INVALID, "Failed to set new head", latestValid);
-    }
-
-    return ForkchoiceResult.withResult(Optional.empty(), Optional.of(newHead));
-  }
-
   private ForkchoiceResult applyForkChoice(
       final BlockHeader newHead, final Hash finalizedBlockHash, final Hash safeBlockHash) {
     final MutableBlockchain blockchain = protocolContext.getBlockchain();

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java

@@ -125,20 +125,6 @@ ForkchoiceResult updateForkChoice(
   ForkchoiceResult updateForkChoiceWithoutLegacySkip(
       final BlockHeader newHead, final Hash finalizedBlockHash, final Hash safeBlockHash);
 
-  /**
-   * Move the canonical chain head without updating forkchoice metadata (finalized or safe).
-   *
-   * <p>Use this for pre-execution head advancement during {@code engine_newPayload}, when the
-   * parent block was previously received via {@code newPayload} but the chain head was not moved
-   * forward yet. Unlike {@link #updateForkChoice}, this does not apply the legacy "ignore update to
-   * old head" optimization. Unlike {@link #updateForkChoice} and {@link
-   * #updateForkChoiceWithoutLegacySkip}, this does not persist finalized or safe block markers.
-   *
-   * @param newHead the new head
-   * @return the forkchoice result
-   */
-  ForkchoiceResult updateHeadForExecution(final BlockHeader newHead);
-
   /**
    * Returns true if the given block hash is a strict ancestor of the currently finalized block
    * (i.e. an older block on the same chain, not finalized itself). Returns false when no finalized

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java

@@ -161,11 +161,6 @@ public ForkchoiceResult updateForkChoiceWithoutLegacySkip(
         newHead, finalizedBlockHash, safeBlockHash);
   }
 
-  @Override
-  public ForkchoiceResult updateHeadForExecution(final BlockHeader newHead) {
-    return mergeCoordinator.updateHeadForExecution(newHead);
-  }
-
   @Override
   public boolean isAncestorOfFinalized(final Hash candidateHeadHash) {
     return mergeCoordinator.isAncestorOfFinalized(candidateHeadHash);

consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java

@@ -1082,55 +1082,6 @@ public void forkchoiceUpdateShouldIgnoreAncestorOfChainHead() {
     verify(blockchain, never()).rewindToBlock(any());
   }
 
-  @Test
-  public void updateHeadForExecutionShouldMoveHeadWithoutUpdatingFinalizedOrSafe() {
-    BlockHeader terminalHeader = terminalPowBlock();
-    sendNewPayloadAndForkchoiceUpdate(
-        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
-
-    BlockHeader parentHeader = nextBlockHeader(terminalHeader);
-    Block parent = new Block(parentHeader, BlockBody.empty());
-    coordinator.rememberBlock(parent);
-
-    BlockHeader childHeader = nextBlockHeader(parentHeader);
-    Block child = new Block(childHeader, BlockBody.empty());
-    sendNewPayloadAndForkchoiceUpdate(child, Optional.empty(), parent.getHash());
-
-    clearInvocations(blockchain, mergeContext);
-
-    ForkchoiceResult result = coordinator.updateHeadForExecution(parentHeader);
-
-    assertThat(result.shouldNotProceedToPayloadBuildProcess()).isFalse();
-    assertThat(result.getNewHead()).contains(parentHeader);
-    assertThat(blockchain.getChainHeadHash()).isEqualTo(parentHeader.getHash());
-
-    verify(blockchain, never()).setFinalized(any());
-    verify(mergeContext, never()).setFinalized(any());
-    verify(blockchain, never()).setSafeBlock(any());
-    verify(mergeContext, never()).setSafeBlock(any());
-  }
-
-  @Test
-  public void updateHeadForExecutionShouldNotIgnoreAncestorOfChainHead() {
-    BlockHeader terminalHeader = terminalPowBlock();
-    sendNewPayloadAndForkchoiceUpdate(
-        new Block(terminalHeader, BlockBody.empty()), Optional.empty(), Hash.ZERO);
-
-    BlockHeader parentHeader = nextBlockHeader(terminalHeader);
-    Block parent = new Block(parentHeader, BlockBody.empty());
-    sendNewPayloadAndForkchoiceUpdate(parent, Optional.empty(), terminalHeader.getHash());
-
-    BlockHeader childHeader = nextBlockHeader(parentHeader);
-    Block child = new Block(childHeader, BlockBody.empty());
-    sendNewPayloadAndForkchoiceUpdate(child, Optional.empty(), parent.getHash());
-
-    ForkchoiceResult result = coordinator.updateHeadForExecution(parentHeader);
-
-    assertThat(result.getStatus()).isEqualTo(ForkchoiceResult.Status.VALID);
-    assertThat(result.getNewHead()).contains(parentHeader);
-    assertThat(blockchain.getChainHeadHash()).isEqualTo(parentHeader.getHash());
-  }
-
   @ParameterizedTest(name = "{index}: {0}")
   @MethodSource("getGasLimits")
   public void shouldSetCorrectTargetGasLimit(final ArgumentsAccessor argumentsAccessor) {

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

@@ -44,7 +44,6 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EnginePayloadStatusResult;
-import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockBody;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
@@ -72,7 +71,6 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -379,16 +377,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
       return respondWith(reqId, blockParam, null, ACCEPTED);
     }
 
-    final MutableBlockchain blockchain = protocolContext.getBlockchain();
-    final Hash chainHeadHash = blockchain.getChainHeadHash();
-    maybeParentHeader
-        .filter(
-            parentHeader ->
-                chainHeadHash != null
-                    && !Objects.equals(newBlockHeader.getParentHash(), chainHeadHash)
-                    && Objects.equals(parentHeader.getParentHash(), chainHeadHash))
-        .ifPresent(mergeCoordinator::updateHeadForExecution);
-
     // execute block and return result response
     final long startTimeNs = System.nanoTime();
     final BlockProcessingResult executionResult =

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

@@ -22,10 +22,8 @@
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -73,7 +71,6 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
@@ -321,82 +318,6 @@ public void shouldRespondWithSyncingDuringBackwardsSync() {
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
-  @Test
-  public void shouldUpdateHeadForExecutionToParentBeforeExecutionWhenParentIsDirectChildOfHead() {
-    BlockHeader mockHeader =
-        setupValidPayload(
-            new BlockProcessingResult(Optional.of(new BlockProcessingOutputs(null, List.of()))),
-            Optional.empty());
-    Hash chainHeadHash =
-        Hash.fromHexStringLenient(
-            "0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
-    BlockHeader parentHeader = mock(BlockHeader.class);
-    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
-        .thenReturn(Optional.of(parentHeader));
-    when(blockchain.getChainHeadHash()).thenReturn(chainHeadHash);
-    when(parentHeader.getParentHash()).thenReturn(chainHeadHash);
-    when(mergeCoordinator.updateHeadForExecution(parentHeader))
-        .thenReturn(
-            MergeMiningCoordinator.ForkchoiceResult.withResult(
-                Optional.empty(), Optional.of(parentHeader)));
-
-    var resp = resp(mockEnginePayload(mockHeader, emptyList()));
-
-    assertValidResponse(mockHeader, resp);
-    InOrder inOrder = inOrder(mergeCoordinator);
-    inOrder.verify(mergeCoordinator).updateHeadForExecution(parentHeader);
-    inOrder.verify(mergeCoordinator).rememberBlock(any(), any());
-    verify(mergeCoordinator, never()).updateForkChoice(any(), any(), any());
-    verify(mergeCoordinator, never()).updateForkChoiceWithoutLegacySkip(any(), any(), any());
-  }
-
-  @Test
-  public void shouldNotUpdateForkChoiceBeforeExecutionWhenParentIsNotDirectChildOfHead() {
-    BlockHeader mockHeader =
-        setupValidPayload(
-            new BlockProcessingResult(Optional.of(new BlockProcessingOutputs(null, List.of()))),
-            Optional.empty());
-    Hash chainHeadHash =
-        Hash.fromHexStringLenient(
-            "0xdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
-    Hash grandparentHash =
-        Hash.fromHexStringLenient(
-            "0xcafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe");
-    BlockHeader parentHeader = mock(BlockHeader.class);
-    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
-        .thenReturn(Optional.of(parentHeader));
-    when(blockchain.getChainHeadHash()).thenReturn(chainHeadHash);
-    when(parentHeader.getParentHash()).thenReturn(grandparentHash);
-
-    var resp = resp(mockEnginePayload(mockHeader, emptyList()));
-
-    assertValidResponse(mockHeader, resp);
-    verify(mergeCoordinator, never()).updateHeadForExecution(any());
-    verify(mergeCoordinator, never()).updateForkChoiceWithoutLegacySkip(any(), any(), any());
-    verify(mergeCoordinator, never()).updateForkChoice(any(), any(), any());
-    verify(mergeCoordinator).rememberBlock(any(), any());
-  }
-
-  @Test
-  public void shouldNotUpdateForkChoiceBeforeExecutionWhenParentIsChainHead() {
-    BlockHeader mockHeader =
-        setupValidPayload(
-            new BlockProcessingResult(Optional.of(new BlockProcessingOutputs(null, List.of()))),
-            Optional.empty());
-    BlockHeader parentHeader = mock(BlockHeader.class);
-    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
-        .thenReturn(Optional.of(parentHeader));
-    when(blockchain.getChainHeadHash()).thenReturn(mockHeader.getParentHash());
-
-    var resp = resp(mockEnginePayload(mockHeader, emptyList()));
-
-    assertValidResponse(mockHeader, resp);
-    verify(mergeCoordinator, never()).updateHeadForExecution(any());
-    verify(mergeCoordinator, never()).updateForkChoiceWithoutLegacySkip(any(), any(), any());
-    verify(mergeCoordinator, never()).updateForkChoice(any(), any(), any());
-    verify(mergeCoordinator).rememberBlock(any(), any());
-  }
-
   @Test
   public void shouldRespondWithInvalidIfExtraDataIsNull() {
     BlockHeader realHeader = createBlockHeader(Optional.empty());