Extend error handling of plugin RPC methods (hyperledger#6759)

Signed-off-by: Fabio Di Fabio <[email protected]>
shemnon · Mar 20, 2024 · 86cc6cb · 86cc6cb
1 parent 2bfd510
commit 86cc6cb
Show file tree

Hide file tree

Showing 17 changed files with 369 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@
 - 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)
 
 ### Bug fixes
 - Fix txpool dump/restore race condition [#6665](https://github.com/hyperledger/besu/pull/6665)

diff --git a/besu/src/main/java/org/hyperledger/besu/services/TransactionSimulationServiceImpl.java b/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()));
   }
 }
diff --git a/...ledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthEstimateGasIntegrationTest.java b/...ledger/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);

diff --git a/...um/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcErrorConverter.java b/...um/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;
     }

diff --git a/.../java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AbstractEstimateGas.java b/.../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,

diff --git a/.../java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/PluginJsonRpcMethod.java b/.../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());
+      final JsonRpcError error = new JsonRpcError(ex.getRpcMethodError(), ex.getMessage());
       LOG.error("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));
     }
   }
 }
diff --git a/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java b/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/JsonRpcError.java
@@ -14,14 +14,18 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.response;
 
+import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcErrorConverter;
+import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
+import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
+import org.hyperledger.besu.plugin.services.rpc.RpcMethodError;
+
 import java.util.Objects;
 
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonFormat;
 import com.fasterxml.jackson.annotation.JsonGetter;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
-import org.apache.tuweni.bytes.Bytes;
 
 @JsonInclude(value = JsonInclude.Include.NON_NULL)
 @JsonFormat(shape = JsonFormat.Shape.OBJECT)
@@ -41,23 +45,28 @@ public JsonRpcError(
     this.data = data;
   }
 
-  public JsonRpcError(final RpcErrorType errorType, final String data) {
+  public JsonRpcError(final RpcMethodError errorType, final String data) {
     this(errorType.getCode(), errorType.getMessage(), data);
 
-    // For execution reverted errors decode the data (if present)
-    if (errorType == RpcErrorType.REVERT_ERROR && data != null) {
-      JsonRpcErrorResponse.decodeRevertReason(Bytes.fromHexString(data))
-          .ifPresent(
-              (decodedReason) -> {
-                this.reason = decodedReason;
-              });
+    if (data != null) {
+      errorType.decodeData(data).ifPresent(decodedData -> this.reason = decodedData);
     }
   }
 
   public JsonRpcError(final RpcErrorType errorType) {
     this(errorType, null);
   }
 
+  public static JsonRpcError from(
+      final ValidationResult<TransactionInvalidReason> validationResult) {
+    final var jsonRpcError =
+        new JsonRpcError(
+            JsonRpcErrorConverter.convertTransactionInvalidReason(
+                validationResult.getInvalidReason()));
+    jsonRpcError.reason = validationResult.getErrorMessage();
+    return jsonRpcError;
+  }
+
   @JsonGetter("code")
   public int getCode() {
     return code;
@@ -73,10 +82,6 @@ public String getData() {
     return data;
   }
 
-  public void setReason(final String reason) {
-    this.reason = reason;
-  }
-
   @Override
   public boolean equals(final Object o) {
     if (this == o) {

diff --git a/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java b/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java
@@ -14,7 +14,14 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.response;
 
-public enum RpcErrorType {
+import org.hyperledger.besu.plugin.services.rpc.RpcMethodError;
+
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.tuweni.bytes.Bytes;
+
+public enum RpcErrorType implements RpcMethodError {
   // Standard errors
   PARSE_ERROR(-32700, "Parse error"),
   INVALID_REQUEST(-32600, "Invalid Request"),
@@ -67,7 +74,10 @@ public enum RpcErrorType {
   REPLAY_PROTECTED_SIGNATURES_NOT_SUPPORTED(-32000, "ChainId not supported"),
   REPLAY_PROTECTED_SIGNATURE_REQUIRED(-32000, "ChainId is required"),
   TX_FEECAP_EXCEEDED(-32000, "Transaction fee cap exceeded"),
-  REVERT_ERROR(-32000, "Execution reverted"),
+  REVERT_ERROR(
+      -32000,
+      "Execution reverted",
+      data -> JsonRpcErrorResponse.decodeRevertReason(Bytes.fromHexString(data))),
   TRANSACTION_NOT_FOUND(-32000, "Transaction not found"),
   MAX_PRIORITY_FEE_PER_GAS_EXCEEDS_MAX_FEE_PER_GAS(
       -32000, "Max priority fee per gas exceeds max fee per gas"),
@@ -222,17 +232,31 @@ public enum RpcErrorType {
 
   private final int code;
   private final String message;
+  private final Function<String, Optional<String>> dataDecoder;
 
   RpcErrorType(final int code, final String message) {
+    this(code, message, null);
+  }
+
+  RpcErrorType(
+      final int code, final String message, Function<String, Optional<String>> dataDecoder) {
     this.code = code;
     this.message = message;
+    this.dataDecoder = dataDecoder;
   }
 
+  @Override
   public int getCode() {
     return code;
   }
 
+  @Override
   public String getMessage() {
     return message;
   }
+
+  @Override
+  public Optional<String> decodeData(final String data) {
+    return dataDecoder == null ? Optional.empty() : dataDecoder.apply(data);
+  }
 }
diff --git a/...i/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTestBase.java b/...i/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpServiceTestBase.java
@@ -152,8 +152,7 @@ public static void initServerAndClient() throws Exception {
     baseUrl = service.url();
   }
 
-  protected static JsonRpcHttpService createJsonRpcHttpService(final JsonRpcConfiguration config)
-      throws Exception {
+  protected static JsonRpcHttpService createJsonRpcHttpService(final JsonRpcConfiguration config) {
     return new JsonRpcHttpService(
         vertx,
         folder,
@@ -165,7 +164,7 @@ protected static JsonRpcHttpService createJsonRpcHttpService(final JsonRpcConfig
         HealthService.ALWAYS_HEALTHY);
   }
 
-  protected static JsonRpcHttpService createJsonRpcHttpService() throws Exception {
+  protected static JsonRpcHttpService createJsonRpcHttpService() {
     return new JsonRpcHttpService(
         vertx,
         folder,