backport merge updates to main (#3341)

* backport merge updates to main

Signed-off-by: garyschulte <garyschulte@gmail.com>

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

@@ -17,6 +17,7 @@ package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.ExecutionStatus.INVALID;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.ExecutionStatus.SYNCING;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.ExecutionStatus.VALID;
+import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda;
 
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.datatypes.Hash;
@@ -80,15 +81,7 @@ public class EngineExecutePayload extends ExecutionEngineJsonRpcMethod {
       return respondWith(reqId, null, SYNCING, null);
     }
 
-    // we already have this payload
-    if (protocolContext.getBlockchain().getBlockByHash(blockParam.getBlockHash()).isPresent()) {
-      LOG.debug("block already present");
-      return respondWith(reqId, blockParam.getBlockHash(), VALID, null);
-    }
-
-    if (LOG.isTraceEnabled()) {
-      LOG.trace("blockparam: {}", Json.encodePrettily(blockParam));
-    }
+    traceLambda(LOG, "blockparam: {}", () -> Json.encodePrettily(blockParam));
 
     final List<Transaction> transactions;
     try {
@@ -126,6 +119,25 @@ public class EngineExecutePayload extends ExecutionEngineJsonRpcMethod {
             0,
             headerFunctions);
 
+    String errorMessage = null;
+
+    // ensure the block hash matches the blockParam hash
+    if (!newBlockHeader.getHash().equals(blockParam.getBlockHash())) {
+      errorMessage =
+          String.format(
+              "Computed block hash %s does not match block hash parameter %s",
+              newBlockHeader.getBlockHash(), blockParam.getBlockHash());
+    } else {
+      // do we already have this payload
+      if (protocolContext
+          .getBlockchain()
+          .getBlockByHash(newBlockHeader.getBlockHash())
+          .isPresent()) {
+        LOG.debug("block already present");
+        return respondWith(reqId, blockParam.getBlockHash(), VALID, null);
+      }
+    }
+
     final var block =
         new Block(newBlockHeader, new BlockBody(transactions, Collections.emptyList()));
     final var latestValidAncestor = mergeCoordinator.getLatestValidAncestor(newBlockHeader);
@@ -134,22 +146,8 @@ public class EngineExecutePayload extends ExecutionEngineJsonRpcMethod {
       return respondWith(reqId, null, SYNCING, null);
     }
 
-    boolean execSuccess = false;
-    String errorMessage = null;
-    // ensure the block hash matches the blockParam hash
-    if (newBlockHeader.getHash().equals(blockParam.getBlockHash())) {
-
-      // execute block
-      execSuccess = mergeCoordinator.executeBlock(block);
-    } else {
-      errorMessage =
-          String.format(
-              "Computed block hash %s does not match block hash parameter %s",
-              newBlockHeader.getBlockHash(), blockParam.getBlockHash());
-    }
-
-    // return result response
-    if (execSuccess) {
+    // execute block and return result response
+    if (errorMessage == null && mergeCoordinator.executeBlock(block)) {
       return respondWith(reqId, newBlockHeader.getHash(), VALID, errorMessage);
     } else {
       return respondWith(reqId, latestValidAncestor.get(), INVALID, errorMessage);

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

@@ -20,6 +20,7 @@ import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.Executi
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.ExecutionStatus.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;
 
 import org.hyperledger.besu.consensus.merge.MergeContext;
@@ -102,7 +103,7 @@ public class EngineExecutePayloadTest {
 
   @Test
   public void shouldReturnSuccessOnAlreadyPresent() {
-    BlockHeader mockHeader = new BlockHeaderTestFixture().buildHeader();
+    BlockHeader mockHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
     Block mockBlock =
         new Block(mockHeader, new BlockBody(Collections.emptyList(), Collections.emptyList()));
 
@@ -127,7 +128,7 @@ public class EngineExecutePayloadTest {
             .blockHeaderFunctions(new MainnetBlockHeaderFunctions())
             .buildBlockHeader();
 
-    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
+    //    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.of(mockHash));
 
@@ -144,10 +145,31 @@ public class EngineExecutePayloadTest {
                 realHeader.getBlockHash(), mockHeader.getBlockHash()));
   }
 
+  @Test
+  public void shouldCheckBlockValidityBeforeCheckingByHashForExisting() {
+    BlockHeader realHeader = new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
+    BlockHeader paramHeader = spy(realHeader);
+    when(paramHeader.getHash()).thenReturn(Hash.fromHexStringLenient("0x1337"));
+    when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
+        .thenReturn(Optional.of(mockHash));
+
+    var resp = resp(mockPayload(paramHeader, Collections.emptyList()));
+
+    EngineExecutionResult res = fromSuccessResp(resp);
+    assertThat(res.getLatestValidHash()).isEqualTo(mockHash.toString());
+    assertThat(res.getStatus()).isEqualTo(INVALID.name());
+
+    assertThat(res.getValidationError())
+        .isEqualTo(
+            String.format(
+                "Computed block hash %s does not match block hash parameter %s",
+                realHeader.getBlockHash(), paramHeader.getHash()));
+  }
+
   @Test
   public void shouldReturnInvalidOnMalformedTransactions() {
     BlockHeader mockHeader = new BlockHeaderTestFixture().buildHeader();
-    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
+    //    when(blockchain.getBlockByHash(any())).thenReturn(Optional.empty());
     when(mergeCoordinator.getLatestValidAncestor(any(Hash.class)))
         .thenReturn(Optional.of(mockHash));
 
@@ -203,11 +225,11 @@ public class EngineExecutePayloadTest {
         header.getParentHash(),
         header.getCoinbase(),
         header.getStateRoot(),
-        asUnsingedLongParameter(header.getNumber()),
+        new UnsignedLongParameter(header.getNumber()),
         header.getBaseFee().map(w -> w.toHexString()).orElse("0x0"),
-        asUnsingedLongParameter(header.getGasLimit()),
-        asUnsingedLongParameter(header.getGasUsed()),
-        asUnsingedLongParameter(header.getTimestamp()),
+        new UnsignedLongParameter(header.getGasLimit()),
+        new UnsignedLongParameter(header.getGasUsed()),
+        new UnsignedLongParameter(header.getTimestamp()),
         header.getExtraData().toHexString(),
         header.getReceiptsRoot(),
         header.getLogsBloom(),
@@ -215,10 +237,6 @@ public class EngineExecutePayloadTest {
         txs);
   }
 
-  private UnsignedLongParameter asUnsingedLongParameter(final long val) {
-    return new UnsignedLongParameter(Long.toHexString(val));
-  }
-
   private EngineExecutionResult fromSuccessResp(final JsonRpcResponse resp) {
     assertThat(resp.getType()).isEqualTo(JsonRpcResponseType.SUCCESS);
     return Optional.of(resp)

ethereum/eth/build.gradle

@@ -49,6 +49,7 @@ dependencies {
   implementation project(':services:kvstore')
   implementation project(':services:pipeline')
   implementation project(':services:tasks')
+  implementation project(':util')
 
   implementation 'com.google.guava:guava'
   implementation 'io.vertx:vertx-core'

util/src/main/java/org/hyperledger/besu/util/Log4j2ConfiguratorUtil.java

@@ -48,6 +48,10 @@ public class Log4j2ConfiguratorUtil {
     }
   }
 
+  public static void setLevelDebug(final String loggerName) {
+    setLevel(loggerName, Level.DEBUG);
+  }
+
   public static void setLevel(final String loggerName, final Level level) {
     final LoggerContext loggerContext = getLoggerContext();
     if (Strings.isEmpty(loggerName)) {

util/src/main/java/org/hyperledger/besu/util/Slf4jLambdaHelper.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.util;
+
+import java.util.Arrays;
+import java.util.function.Supplier;
+
+import org.slf4j.Logger;
+
+/**
+ * Static helper class to shim SLF4J with lambda parameter suppliers until the final release of
+ * SLF4J 2.0.
+ */
+public class Slf4jLambdaHelper {
+
+  // sonar code smell
+  private Slf4jLambdaHelper() {}
+
+  public static void debugLambda(
+      final Logger log, final String message, final Supplier<?>... params) {
+    if (log.isDebugEnabled()) {
+      log.debug(message, Arrays.stream(params).map(Supplier::get).toArray());
+    }
+  }
+
+  public static void traceLambda(
+      final Logger log, final String message, final Supplier<?>... params) {
+    if (log.isTraceEnabled()) {
+      log.trace(message, Arrays.stream(params).map(Supplier::get).toArray());
+    }
+  }
+
+  public static void warnLambda(
+      final Logger log, final String message, final Supplier<?>... params) {
+    if (log.isWarnEnabled()) {
+      log.warn(message, Arrays.stream(params).map(Supplier::get).toArray());
+    }
+  }
+}

util/src/test/java/org/hyperledger/besu/util/Slf4jLambdaHelperTest.java

@@ -0,0 +1,94 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.util;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.hyperledger.besu.util.Slf4jLambdaHelper.debugLambda;
+import static org.hyperledger.besu.util.Slf4jLambdaHelper.traceLambda;
+import static org.hyperledger.besu.util.Slf4jLambdaHelper.warnLambda;
+
+import java.util.ArrayDeque;
+import java.util.function.Supplier;
+
+import org.apache.logging.log4j.Level;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class Slf4jLambdaHelperTest {
+  private static final Logger LOG = LoggerFactory.getLogger(Slf4jLambdaHelperTest.class);
+  private static final ArrayDeque<String> paramStack = new ArrayDeque<>();
+
+  @Before
+  public void paramSetup() {
+    paramStack.push("stuff");
+    paramStack.push("more stuff");
+    paramStack.push("last stuff");
+  }
+
+  @Test
+  public void smokeDebugLambda() {
+    Log4j2ConfiguratorUtil.setLevel(LOG.getName(), Level.WARN);
+    debugLambda(
+        LOG,
+        "blah",
+        (Supplier<String>)
+            () -> {
+              throw new RuntimeException("should not evaluate");
+            });
+    Log4j2ConfiguratorUtil.setLevelDebug(LOG.getName());
+    assertThat(paramStack.size()).isEqualTo(3);
+    debugLambda(LOG, "blah {}", paramStack::pop);
+    assertThat(paramStack.size()).isEqualTo(2);
+    debugLambda(LOG, "blah {} {}", paramStack::pop, paramStack::pop);
+    assertThat(paramStack.size()).isZero();
+  }
+
+  @Test
+  public void smokeTraceLambda() {
+    traceLambda(
+        LOG,
+        "blah",
+        (Supplier<String>)
+            () -> {
+              throw new RuntimeException("should not evaluate");
+            });
+    Log4j2ConfiguratorUtil.setLevel(LOG.getName(), Level.TRACE);
+    assertThat(paramStack.size()).isEqualTo(3);
+    traceLambda(LOG, "blah {}", paramStack::pop);
+    assertThat(paramStack.size()).isEqualTo(2);
+    traceLambda(LOG, "blah {} {}", paramStack::pop, paramStack::pop);
+    assertThat(paramStack.size()).isZero();
+  }
+
+  @Test
+  public void smokeWarnLambda() {
+    Log4j2ConfiguratorUtil.setLevel(LOG.getName(), Level.OFF);
+    traceLambda(
+        LOG,
+        "blah",
+        (Supplier<String>)
+            () -> {
+              throw new RuntimeException("should not evaluate");
+            });
+    Log4j2ConfiguratorUtil.setLevel(LOG.getName(), Level.WARN);
+    assertThat(paramStack.size()).isEqualTo(3);
+    warnLambda(LOG, "blah {}", paramStack::pop);
+    assertThat(paramStack.size()).isEqualTo(2);
+    warnLambda(LOG, "blah {} {}", paramStack::pop, paramStack::pop);
+    assertThat(paramStack.size()).isZero();
+  }
+}