Merge branch 'main' into refactor-picknextstep

Signed-off-by: Simon Dudley <[email protected]>

.github/pull_request_template.md

@@ -12,10 +12,10 @@
 - [ ] Considered the changelog and included an [update if required](https://wiki.hyperledger.org/display/BESU/Changelog).
 - [ ] For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests
 
-### Most advanced CI tests are deferred until PR approval, but you could:
+### Locally, you can run these tests to catch failures early:
 
-- [ ] locally run all unit tests via: `./gradlew build`
-- [ ] locally run all acceptance tests via: `./gradlew acceptanceTest`
-- [ ] locally run all integration tests via: `./gradlew integrationTest`
-- [ ] locally run all reference tests via: `./gradlew ethereum:referenceTests:referenceTests`
+- [ ] unit tests: `./gradlew build`
+- [ ] acceptance tests: `./gradlew acceptanceTest`
+- [ ] integration tests: `./gradlew integrationTest`
+- [ ] reference tests: `./gradlew ethereum:referenceTests:referenceTests`
 

.github/workflows/artifacts.yml

@@ -48,13 +48,13 @@ jobs:
           compression-level: 0
       - name: Artifactory Publish
         env:
-          ARTIFACTORY_USER: ${{ secrets.BESU_ARTIFACTORY }}
+          ARTIFACTORY_USER: ${{ secrets.BESU_ARTIFACTORY_USER }}
+          ARTIFACTORY_KEY: ${{ secrets.BESU_ARTIFACTORY_TOKEN }}
         run: ./gradlew -Prelease.releaseVersion=${{ github.ref_name }} -Pversion=${{github.ref_name}} artifactoryPublish
   testWindows:
     runs-on: windows-2022
     needs: artifacts
     timeout-minutes: 10
-    if: ${{ github.actor != 'dependabot[bot]' }}
     steps:
       - name: Set up Java
         uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93

.github/workflows/develop.yml

@@ -36,10 +36,15 @@ jobs:
           echo "zipSha=$(shasum -a 256 besu*.zip)" >> $GITHUB_OUTPUT
           echo "tarSha=$(shasum -a 256 besu*.tar.gz)" >> $GITHUB_OUTPUT
           cd ../..
+      - name: delete current develop release
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: gh release delete develop -y --cleanup-tag
       - name: Upload Release assets
         uses: softprops/action-gh-release@de2c0eb89ae2a093876385947365aca7b0e5f844
         with:
           prerelease: true
+          draft: false
           name: develop
           tag_name: develop
           fail_on_unmatched_files: true

.github/workflows/pre-review.yml

@@ -35,7 +35,6 @@ jobs:
       - uses: gradle/wrapper-validation-action@56b90f209b02bf6d1deae490e9ef18b21a389cd4
   spotless:
     runs-on: ubuntu-22.04
-    if: ${{ github.actor != 'dependabot[bot]' }}
     steps:
       - name: Checkout Repo
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11

.github/workflows/sonarcloud.yml

@@ -36,6 +36,6 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}  # Needed to get PR information, if any
           SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
-          SONAR_ORGANIZATION: ${{ env.SONAR_ORGANIZATION }}
-          SONAR_PROJECT_KEY: $ {{ env.SONAR_PROJECT_KEY }}
+          SONAR_ORGANIZATION: ${{ vars.SONAR_ORGANIZATION }}
+          SONAR_PROJECT_KEY: ${{ vars.SONAR_PROJECT_KEY }}
         run: ./gradlew build sonarqube --continue --info -Dorg.gradle.parallel=true -Dorg.gradle.caching=true

CHANGELOG.md

@@ -23,6 +23,8 @@
 - Introduce `TransactionSimulationService` [#6686](https://github.com/hyperledger/besu/pull/6686)
 - Transaction call object to accept both `input` and `data` field simultaneously if they are set to equal values [#6702](https://github.com/hyperledger/besu/pull/6702)
 - `eth_call` for blob tx allows for empty `maxFeePerBlobGas` [#6731](https://github.com/hyperledger/besu/pull/6731)
+- Extend error handling of plugin RPC methods [#6759](https://github.com/hyperledger/besu/pull/6759)
+- Added engine_newPayloadV4 and engine_getPayloadV4 methods [#6783](https://github.com/hyperledger/besu/pull/6783)
 
 ### Bug fixes
 - Fix txpool dump/restore race condition [#6665](https://github.com/hyperledger/besu/pull/6665)
@@ -441,7 +443,7 @@ https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/23.4.4/besu-23.4.4.z
 - Early access - layered transaction pool implementation [#5290](https://github.com/hyperledger/besu/pull/5290)
 - New RPC method `debug_getRawReceipts` [#5476](https://github.com/hyperledger/besu/pull/5476)
 - Add TrieLogFactory plugin support [#5440](https://github.com/hyperledger/besu/pull/5440)
-- Ignore `min-block-occupancy-ratio` option when on PoS networks, since in some cases, it prevents to have full blocks even if enough transactions are present [#5491](https://github.com/hyperledger/besu/pull/5491) 
+- Ignore `min-block-occupancy-ratio` option when on PoS networks, since in some cases, it prevents to have full blocks even if enough transactions are present [#5491](https://github.com/hyperledger/besu/pull/5491)
 
 ### Bug Fixes
 - Fix eth_feeHistory response for the case in which blockCount is higher than highestBlock requested. [#5397](https://github.com/hyperledger/besu/pull/5397)

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/jsonrpc/ExecutionEngineEip6110AcceptanceTest.java → acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/jsonrpc/ExecutionEnginePragueAcceptanceTest.java

@@ -22,13 +22,13 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.params.provider.Arguments;
 
-public class ExecutionEngineEip6110AcceptanceTest extends AbstractJsonRpcTest {
-  private static final String GENESIS_FILE = "/jsonrpc/engine/eip6110/genesis.json";
-  private static final String TEST_CASE_PATH = "/jsonrpc/engine/eip6110/test-cases/";
+public class ExecutionEnginePragueAcceptanceTest extends AbstractJsonRpcTest {
+  private static final String GENESIS_FILE = "/jsonrpc/engine/prague/genesis.json";
+  private static final String TEST_CASE_PATH = "/jsonrpc/engine/prague/test-cases/";
 
   private static JsonRpcTestsContext testsContext;
 
-  public ExecutionEngineEip6110AcceptanceTest() {
+  public ExecutionEnginePragueAcceptanceTest() {
     super(testsContext);
   }
 

acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/permissioning/NodeSmartContractPermissioningV2AcceptanceTest.java

@@ -47,10 +47,7 @@ public void setUp() {
     permissionedNode.execute(allowNode(permissionedNode));
     permissionedNode.verify(connectionIsAllowed(permissionedNode));
 
-    allowedNode.verify(eth.syncingStatus(false));
-    bootnode.verify(eth.syncingStatus(false));
-    permissionedNode.verify(eth.syncingStatus(false));
-    forbiddenNode.verify(eth.syncingStatus(false));
+    verifyAllNodesHaveFinishedSyncing();
   }
 
   @Test
@@ -92,6 +89,8 @@ public void permissioningUpdatesPropagateThroughNetwork() {
     permissionedNode.verify(admin.addPeer(allowedNode));
     permissionedNode.verify(net.awaitPeerCount(2));
 
+    verifyAllNodesHaveFinishedSyncing();
+
     // permissioning changes in peer should propagate to permissioned node
     allowedNode.execute(allowNode(forbiddenNode));
     allowedNode.verify(connectionIsAllowed(forbiddenNode));
@@ -101,6 +100,13 @@ public void permissioningUpdatesPropagateThroughNetwork() {
     permissionedNode.verify(net.awaitPeerCount(3));
   }
 
+  private void verifyAllNodesHaveFinishedSyncing() {
+    allowedNode.verify(eth.syncingStatus(false));
+    bootnode.verify(eth.syncingStatus(false));
+    permissionedNode.verify(eth.syncingStatus(false));
+    forbiddenNode.verify(eth.syncingStatus(false));
+  }
+
   @Test
   public void onchainPermissioningAllowlistShouldPersistAcrossRestarts() {
     permissionedCluster.stop();

acceptance-tests/tests/src/test/resources/jsonrpc/engine/eip6110/genesis.json → acceptance-tests/tests/src/test/resources/jsonrpc/engine/prague/genesis.json

@@ -14,7 +14,7 @@
     "londonBlock":0,
     "terminalTotalDifficulty":0,
     "cancunTime":0,
-    "experimentalEipsTime":20,
+    "pragueTime":20,
     "clique": {
       "period": 5,
       "epoch": 30000

acceptance-tests/tests/src/test/resources/jsonrpc/engine/eip6110/test-cases/06_eip6110_getPayloadV6110.json → acceptance-tests/tests/src/test/resources/jsonrpc/engine/prague/test-cases/06_prague_getPayloadV4.json

@@ -1,7 +1,7 @@
 {
   "request": {
     "jsonrpc": "2.0",
-    "method": "engine_getPayloadV6110",
+    "method": "engine_getPayloadV4",
     "params": [
       "0x282643b909febddf"
     ],

acceptance-tests/tests/src/test/resources/jsonrpc/engine/eip6110/test-cases/08_eip6110_invalid_null_deposits_execute_payload.json → acceptance-tests/tests/src/test/resources/jsonrpc/engine/prague/test-cases/08_prague_invalid_null_deposits_execute_payload.json

@@ -1,7 +1,7 @@
 {
   "request": {
     "jsonrpc": "2.0",
-    "method": "engine_newPayloadV6110",
+    "method": "engine_newPayloadV4",
     "params": [
       {
         "parentHash": "0x45811fa27a100ce9035e5e086b9669275041a4ec0ebbd920be028fd7b0aa2356",

acceptance-tests/tests/src/test/resources/jsonrpc/engine/eip6110/test-cases/09_eip6110_newPayloadV6110.json → acceptance-tests/tests/src/test/resources/jsonrpc/engine/prague/test-cases/09_prague_newPayloadV4.json

@@ -1,7 +1,7 @@
 {
   "request": {
     "jsonrpc": "2.0",
-    "method": "engine_newPayloadV6110",
+    "method": "engine_newPayloadV4",
     "params": [
       {
         "parentHash": "0x1dd4f141551d53ce393845e2873754e43396101a8ebc0fd0eeb2e6798a591315",

acceptance-tests/tests/src/test/resources/jsonrpc/engine/eip6110/test-cases/11_eip6110_getPayloadV6110.json → acceptance-tests/tests/src/test/resources/jsonrpc/engine/prague/test-cases/11_prague_getPayloadV4.json

@@ -1,7 +1,7 @@
 {
   "request": {
     "jsonrpc": "2.0",
-    "method": "engine_getPayloadV6110",
+    "method": "engine_getPayloadV4",
     "params": [
       "0x282643db882670cf"
     ],

acceptance-tests/tests/src/test/resources/jsonrpc/engine/shanghai/test-cases/05_shanghai_prepare_payload_invalid_null_withdrawals.json

@@ -20,8 +20,8 @@
     "jsonrpc" : "2.0",
     "id" : 67,
     "error" : {
-      "code" : -38003,
-      "message" : "Invalid payload attributes"
+      "code" : -32602,
+      "message" : "Invalid params"
     }
   },
   "statusCode" : 200

besu/src/main/java/org/hyperledger/besu/services/TransactionSimulationServiceImpl.java

@@ -19,7 +19,10 @@
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.mainnet.ImmutableTransactionValidationParams;
 import org.hyperledger.besu.ethereum.mainnet.TransactionValidationParams;
+import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
+import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
 import org.hyperledger.besu.ethereum.transaction.CallParameter;
+import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 import org.hyperledger.besu.ethereum.transaction.TransactionSimulator;
 import org.hyperledger.besu.evm.tracing.OperationTracer;
 import org.hyperledger.besu.plugin.Unstable;
@@ -62,14 +65,16 @@ public Optional<TransactionSimulationResult> simulate(
 
     final CallParameter callParameter = CallParameter.fromTransaction(transaction);
 
-    final var blockHeader =
-        blockchain
-            .getBlockHeader(blockHash)
-            .or(() -> blockchain.getBlockHeaderSafe(blockHash))
-            .orElseThrow(
-                () ->
-                    new IllegalStateException(
-                        "Block header not yet present for chain head hash: " + blockHash));
+    final var maybeBlockHeader =
+        blockchain.getBlockHeader(blockHash).or(() -> blockchain.getBlockHeaderSafe(blockHash));
+
+    if (maybeBlockHeader.isEmpty()) {
+      return Optional.of(
+          new TransactionSimulationResult(
+              transaction,
+              TransactionProcessingResult.invalid(
+                  ValidationResult.invalid(TransactionInvalidReason.BLOCK_NOT_FOUND))));
+    }
 
     return transactionSimulator
         .process(
@@ -78,7 +83,7 @@ public Optional<TransactionSimulationResult> simulate(
                 ? SIMULATOR_ALLOWING_EXCEEDING_BALANCE
                 : TransactionValidationParams.transactionSimulator(),
             operationTracer,
-            blockHeader)
+            maybeBlockHeader.get())
         .map(res -> new TransactionSimulationResult(transaction, res.result()));
   }
 }

ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthEstimateGasIntegrationTest.java

@@ -28,7 +28,8 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 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.mainnet.ValidationResult;
+import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 import org.hyperledger.besu.testutil.BlockTestUtil;
 
 import java.util.Map;
@@ -171,11 +172,11 @@ public void shouldNotIgnoreSenderBalanceAccountWhenStrictModeDisabledAndThrowErr
             null,
             null);
     final JsonRpcRequestContext request = requestWithParams(callParameter);
-
-    final RpcErrorType rpcErrorType = RpcErrorType.TRANSACTION_UPFRONT_COST_EXCEEDS_BALANCE;
-    final JsonRpcError rpcError = new JsonRpcError(rpcErrorType);
-    rpcError.setReason(
-        "transaction up-front cost 0x1cc31b3333167018 exceeds transaction sender account balance 0x140");
+    final ValidationResult<TransactionInvalidReason> validationResult =
+        ValidationResult.invalid(
+            TransactionInvalidReason.UPFRONT_COST_EXCEEDS_BALANCE,
+            "transaction up-front cost 0x1cc31b3333167018 exceeds transaction sender account balance 0x140");
+    final JsonRpcError rpcError = JsonRpcError.from(validationResult);
     final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(null, rpcError);
 
     final JsonRpcResponse response = method.response(request);

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/internal/Scalars.java

@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.api.graphql.internal;
 
 import org.hyperledger.besu.datatypes.Address;
+import org.hyperledger.besu.datatypes.VersionedHash;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity;
 
 import java.math.BigInteger;
@@ -256,6 +257,8 @@ Bytes32 convertImpl(final Object input) {
                 return null;
               }
             }
+          } else if (input instanceof VersionedHash versionedHash) {
+            return versionedHash.toBytes();
           } else {
             return null;
           }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java

@@ -83,6 +83,8 @@ public static RpcErrorType convertTransactionInvalidReason(
         return RpcErrorType.BLOB_GAS_PRICE_BELOW_CURRENT_BLOB_BASE_FEE;
       case EXECUTION_HALTED:
         return RpcErrorType.EXECUTION_HALTED;
+      case BLOCK_NOT_FOUND:
+        return RpcErrorType.BLOCK_NOT_FOUND;
       default:
         return RpcErrorType.INTERNAL_ERROR;
     }

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/RpcMethod.java

@@ -54,11 +54,11 @@ public enum RpcMethod {
   ENGINE_GET_PAYLOAD_V1("engine_getPayloadV1"),
   ENGINE_GET_PAYLOAD_V2("engine_getPayloadV2"),
   ENGINE_GET_PAYLOAD_V3("engine_getPayloadV3"),
-  ENGINE_GET_PAYLOAD_V6110("engine_getPayloadV6110"),
+  ENGINE_GET_PAYLOAD_V4("engine_getPayloadV4"),
   ENGINE_NEW_PAYLOAD_V1("engine_newPayloadV1"),
   ENGINE_NEW_PAYLOAD_V2("engine_newPayloadV2"),
   ENGINE_NEW_PAYLOAD_V3("engine_newPayloadV3"),
-  ENGINE_NEW_PAYLOAD_V6110("engine_newPayloadV6110"),
+  ENGINE_NEW_PAYLOAD_V4("engine_newPayloadV4"),
   ENGINE_FORKCHOICE_UPDATED_V1("engine_forkchoiceUpdatedV1"),
   ENGINE_FORKCHOICE_UPDATED_V2("engine_forkchoiceUpdatedV2"),
   ENGINE_FORKCHOICE_UPDATED_V3("engine_forkchoiceUpdatedV3"),

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

@@ -111,12 +111,7 @@ protected JsonRpcErrorResponse errorResponse(
         result.getValidationResult();
     if (validationResult != null && !validationResult.isValid()) {
       if (validationResult.getErrorMessage().length() > 0) {
-        final RpcErrorType rpcErrorType =
-            JsonRpcErrorConverter.convertTransactionInvalidReason(
-                validationResult.getInvalidReason());
-        final JsonRpcError rpcError = new JsonRpcError(rpcErrorType);
-        rpcError.setReason(validationResult.getErrorMessage());
-        return errorResponse(request, rpcError);
+        return errorResponse(request, JsonRpcError.from(validationResult));
       }
       return errorResponse(
           request,

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

@@ -14,9 +14,6 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;
 
-import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INTERNAL_ERROR;
-import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.PLUGIN_INTERNAL_ERROR;
-
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
@@ -53,13 +50,9 @@ public JsonRpcResponse response(final JsonRpcRequestContext request) {
       final Object result = function.apply(() -> request.getRequest().getParams());
       return new JsonRpcSuccessResponse(request.getRequest().getId(), result);
     } catch (final PluginRpcEndpointException ex) {
-      final JsonRpcError error = new JsonRpcError(PLUGIN_INTERNAL_ERROR, ex.getMessage());
-      LOG.error("Error calling plugin JSON-RPC endpoint", ex);
+      final JsonRpcError error = new JsonRpcError(ex.getRpcMethodError(), ex.getData());
+      LOG.debug("Error calling plugin JSON-RPC endpoint", ex);
       return new JsonRpcErrorResponse(request.getRequest().getId(), error);
-    } catch (final Exception ex) {
-      LOG.error("Error calling plugin JSON-RPC endpoint", ex);
-      return new JsonRpcErrorResponse(
-          request.getRequest().getId(), new JsonRpcError(INTERNAL_ERROR));
     }
   }
 }

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

@@ -172,7 +172,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
       if (!getWithdrawalsValidator(
               protocolSchedule.get(), newHead, maybePayloadAttributes.get().getTimestamp())
           .validateWithdrawals(withdrawals)) {
-        return new JsonRpcErrorResponse(requestId, getInvalidPayloadError());
+        return new JsonRpcErrorResponse(requestId, getInvalidParametersError());
       }
     }
 
@@ -241,7 +241,7 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributeRelevantToNewHead(
     if (payloadAttributes.getTimestamp() <= headBlockHeader.getTimestamp()) {
       LOG.warn(
           "Payload attributes timestamp is smaller than timestamp of header in fork choice update");
-      return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadError()));
+      return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadAttributesError()));
     }
 
     return Optional.empty();
@@ -364,10 +364,14 @@ protected boolean requireTerminalPoWBlockValidation() {
     return false;
   }
 
-  protected RpcErrorType getInvalidPayloadError() {
+  protected RpcErrorType getInvalidParametersError() {
     return RpcErrorType.INVALID_PARAMS;
   }
 
+  protected RpcErrorType getInvalidPayloadAttributesError() {
+    return RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES;
+  }
+
   // fcU calls are synchronous, no need to make volatile
   private long lastFcuInfoLog = System.currentTimeMillis();
   private static final String logMessage =

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

@@ -320,7 +320,13 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
 
     if (executionResult.isSuccessful()) {
       logImportedBlockInfo(
-          block, blobTransactions.size(), (System.currentTimeMillis() - startTimeMs) / 1000.0);
+          block,
+          blobTransactions.stream()
+              .map(Transaction::getVersionedHashes)
+              .flatMap(Optional::stream)
+              .mapToInt(List::size)
+              .sum(),
+          (System.currentTimeMillis() - startTimeMs) / 1000.0);
       return respondWith(reqId, blockParam, newBlockHeader.getHash(), VALID);
     } else {
       if (executionResult.causedBy().isPresent()) {