setMinPriorityFee - Return Invalid Param when invalid and use hexadec…

…imal instead of Long (#6099) Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
hyperledger · Nov 1, 2023 · 7acdd87 · 7acdd87
1 parent 094c841
commit 7acdd87
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 18 deletions.
diff --git a/.../hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinPriorityFee.java b/.../hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinPriorityFee.java
@@ -19,6 +19,7 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod;
 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.results.Quantity;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
 
 public class MinerGetMinPriorityFee implements JsonRpcMethod {
@@ -36,6 +37,7 @@ public String getName() {
   @Override
   public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
     return new JsonRpcSuccessResponse(
-        requestContext.getRequest().getId(), miningParameters.getMinPriorityFeePerGas().getValue());
+        requestContext.getRequest().getId(),
+        Quantity.create(miningParameters.getMinPriorityFeePerGas()));
   }
 }
diff --git a/.../hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinPriorityFee.java b/.../hyperledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinPriorityFee.java
@@ -18,8 +18,11 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
+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.core.MiningParameters;
 
 import org.slf4j.Logger;
@@ -42,12 +45,16 @@ public String getName() {
   @Override
   public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
     try {
-      final Wei minPriorityFeePerGas = Wei.of(requestContext.getRequiredParameter(0, Long.class));
+      final Wei minPriorityFeePerGas =
+          Wei.fromHexString(requestContext.getRequiredParameter(0, String.class));
       miningParameters.setMinPriorityFeePerGas(minPriorityFeePerGas);
-      LOG.debug("min priority fee per gas changed to {}", minPriorityFeePerGas);
+      LOG.debug(
+          "min priority fee per gas changed to {}", minPriorityFeePerGas.toHumanReadableString());
       return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true);
     } catch (final IllegalArgumentException invalidJsonRpcParameters) {
-      return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), false);
+      return new JsonRpcErrorResponse(
+          requestContext.getRequest().getId(),
+          new JsonRpcError(RpcErrorType.INVALID_PARAMS, invalidJsonRpcParameters.getMessage()));
     }
   }
 }
diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinPriorityFeeTest.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerGetMinPriorityFeeTest.java
@@ -39,14 +39,15 @@ public void setUp() {
 
   @Test
   public void shouldReturnMinPriorityFee() {
-    Wei minPriorityFee = Wei.of(70);
+    String minPriorityFee = "0x46";
+
     final JsonRpcRequestContext request =
         new JsonRpcRequestContext(new JsonRpcRequest("2.0", method.getName(), new Object[] {}));
 
-    when(miningParameters.getMinPriorityFeePerGas()).thenReturn(minPriorityFee);
+    when(miningParameters.getMinPriorityFeePerGas()).thenReturn(Wei.fromHexString(minPriorityFee));
 
     final JsonRpcResponse expected =
-        new JsonRpcSuccessResponse(request.getRequest().getId(), minPriorityFee.getValue());
+        new JsonRpcSuccessResponse(request.getRequest().getId(), minPriorityFee);
 
     final JsonRpcResponse actual = method.response(request);
     assertThat(actual).usingRecursiveComparison().isEqualTo(expected);

diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinPriorityFeeTest.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/miner/MinerSetMinPriorityFeeTest.java
@@ -19,8 +19,11 @@
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
 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;
 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.core.MiningParameters;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -36,32 +39,52 @@ public void setUp() {
   }
 
   @Test
-  public void shouldReturnFalseWhenParameterIsInvalid() {
-    final long newMinPriorityFee = -1;
-    final var request = request(newMinPriorityFee);
-
+  public void shouldReturnInvalidParamsWhenParameterIsInvalid() {
+    final String invalidMinPriorityFee =
+        "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff";
+    final var request = request(invalidMinPriorityFee);
     method.response(request);
+
     final JsonRpcResponse expected =
-        new JsonRpcSuccessResponse(request.getRequest().getId(), false);
+        new JsonRpcErrorResponse(
+            request.getRequest().getId(),
+            new JsonRpcError(
+                RpcErrorType.INVALID_PARAMS,
+                "Hex value is too large: expected at most 32 bytes but got 33"));
+
+    final JsonRpcResponse actual = method.response(request);
+    assertThat(actual).usingRecursiveComparison().isEqualTo(expected);
+  }
 
+  @Test
+  public void shouldReturnInvalidParamsWhenParameterIsMissing() {
+    final var request =
+        new JsonRpcRequestContext(new JsonRpcRequest("2.0", method.getName(), new Object[] {}));
+    method.response(request);
+    final JsonRpcResponse expected =
+        new JsonRpcErrorResponse(
+            request.getRequest().getId(),
+            new JsonRpcError(
+                RpcErrorType.INVALID_PARAMS, "Missing required json rpc parameter at index 0"));
     final JsonRpcResponse actual = method.response(request);
     assertThat(actual).usingRecursiveComparison().isEqualTo(expected);
   }
 
   @Test
-  public void shouldChangeMinPriorityFee() {
-    final long newMinPriorityFee = 10;
+  public void shouldReturnTrueWhenChangeMinPriorityFee() {
+    final String newMinPriorityFee = "0x10";
     final var request = request(newMinPriorityFee);
     method.response(request);
-    final JsonRpcResponse expected = new JsonRpcSuccessResponse(request.getRequest().getId(), true);
 
+    final JsonRpcResponse expected = new JsonRpcSuccessResponse(request.getRequest().getId(), true);
     final JsonRpcResponse actual = method.response(request);
     assertThat(actual).usingRecursiveComparison().isEqualTo(expected);
-    assertThat(miningParameters.getMinPriorityFeePerGas()).isEqualTo(Wei.of(newMinPriorityFee));
+    assertThat(miningParameters.getMinPriorityFeePerGas())
+        .isEqualTo(Wei.fromHexString(newMinPriorityFee));
   }
 
-  private JsonRpcRequestContext request(final long longParam) {
+  private JsonRpcRequestContext request(final String newMinPriorityFee) {
     return new JsonRpcRequestContext(
-        new JsonRpcRequest("2.0", method.getName(), new Object[] {String.valueOf(longParam)}));
+        new JsonRpcRequest("2.0", method.getName(), new Object[] {newMinPriorityFee}));
   }
 }