In forkchoiceUpdated, log VALID instead of INVALID when ignoring update for old head (#5101)

This is misleading when debugging because we return VALID in the API and there's not actually an INVALID issue to debug despite what the logs were saying.

Also rename and flip the isValid boolean to be shouldNotProceedToPayloadBuildProcess for clarity.

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>

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

@@ -305,8 +305,8 @@ public interface MergeMiningCoordinator extends MiningCoordinator {
      *
      * @return the boolean
      */
-    public boolean isValid() {
-      return status == VALID;
+    public boolean shouldNotProceedToPayloadBuildProcess() {
+      return status != VALID;
     }
   }
 }

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

@@ -633,7 +633,7 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
         coordinator.updateForkChoice(
             childHeader, terminalHeader.getHash(), terminalHeader.getHash());
 
-    assertThat(result.isValid()).isFalse();
+    assertThat(result.shouldNotProceedToPayloadBuildProcess()).isTrue();
     assertThat(result.getErrorMessage()).isPresent();
     assertThat(result.getErrorMessage().get())
         .isEqualTo("new head timestamp not greater than parent");
@@ -982,6 +982,7 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
         coordinator.updateForkChoice(parentHeader, Hash.ZERO, terminalHeader.getHash());
 
     assertThat(res.getStatus()).isEqualTo(ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD);
+    assertThat(res.shouldNotProceedToPayloadBuildProcess()).isTrue();
     assertThat(res.getNewHead().isEmpty()).isTrue();
     assertThat(res.getLatestValid().isPresent()).isTrue();
     assertThat(res.getLatestValid().get()).isEqualTo(parentHeader.getHash());
@@ -1000,8 +1001,8 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
                     block.getHeader(),
                     finalizedHeader.map(BlockHeader::getHash).orElse(Hash.ZERO),
                     safeHash)
-                .isValid())
-        .isTrue();
+                .shouldNotProceedToPayloadBuildProcess())
+        .isFalse();
 
     when(mergeContext.getFinalized()).thenReturn(finalizedHeader);
   }

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

@@ -159,8 +159,12 @@ public abstract class AbstractEngineForkchoiceUpdated extends ExecutionEngineJso
       return new JsonRpcErrorResponse(requestId, getInvalidPayloadError());
     }
 
-    if (!result.isValid()) {
-      logForkchoiceUpdatedCall(INVALID, forkChoice);
+    if (result.shouldNotProceedToPayloadBuildProcess()) {
+      if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) {
+        logForkchoiceUpdatedCall(VALID, forkChoice);
+      } else {
+        logForkchoiceUpdatedCall(INVALID, forkChoice);
+      }
       return handleNonValidForkchoiceUpdate(requestId, result);
     }