From d1bdb12dc02f553f3c4e8a13059c3eeee476670c Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 20 Dec 2024 14:16:38 +1000 Subject: [PATCH 1/6] don't throw if all the gasprice params are specified Signed-off-by: Sally MacFarlane --- .../methods/JsonCallParameterUtil.java | 17 +++++++++-- .../internal/methods/EthEstimateGasTest.java | 28 +++++++++++++------ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java index df593f9fbc3..152a62cd1c8 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java @@ -20,7 +20,13 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonRpcParameter.JsonRpcParameterException; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; +import java.util.Arrays; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class JsonCallParameterUtil { + private static final Logger LOG = LoggerFactory.getLogger(JsonCallParameterUtil.class); private JsonCallParameterUtil() {} @@ -36,9 +42,14 @@ public static JsonCallParameter validateAndGetCallParams(final JsonRpcRequestCon if (callParams.getGasPrice() != null && (callParams.getMaxFeePerGas().isPresent() || callParams.getMaxPriorityFeePerGas().isPresent())) { - throw new InvalidJsonRpcParameters( - "gasPrice cannot be used with maxFeePerGas or maxPriorityFeePerGas", - RpcErrorType.INVALID_GAS_PRICE_PARAMS); + try { + LOG.warn( + "gasPrice will be ignored since 1559 values are defined (maxFeePerGas or maxPriorityFeePerGas). {}", + Arrays.toString(request.getRequest().getParams())); + } catch (Exception e) { + LOG.warn( + "gasPrice will be ignored since 1559 values are defined (maxFeePerGas or maxPriorityFeePerGas)"); + } } return callParams; } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java index de6b74ceb0b..38a55858920 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java @@ -28,7 +28,6 @@ import org.hyperledger.besu.datatypes.parameters.UnsignedLongParameter; 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.exception.InvalidJsonRpcParameters; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; @@ -52,7 +51,6 @@ import java.util.Optional; import org.apache.tuweni.bytes.Bytes; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -197,12 +195,14 @@ public void shouldUseGasPriceParameterWhenIsPresent() { @Test public void shouldReturnGasEstimateErrorWhenGasPricePresentForEip1559Transaction() { + final Wei gasPrice = Wei.of(1000); final JsonRpcRequestContext request = - ethEstimateGasRequest(eip1559TransactionCallParameter(Optional.of(Wei.of(10)))); - mockTransientProcessorResultGasEstimate(1L, false, false, latestBlockHeader); - Assertions.assertThatThrownBy(() -> method.response(request)) - .isInstanceOf(InvalidJsonRpcParameters.class) - .hasMessageContaining("gasPrice cannot be used with maxFeePerGas or maxPriorityFeePerGas"); + ethEstimateGasRequest(eip1559TransactionCallParameter(Optional.of(gasPrice))); + mockTransientProcessorResultGasEstimate( + 1L, true, gasPrice, Optional.empty(), latestBlockHeader); + + final JsonRpcResponse expectedResponse = new JsonRpcSuccessResponse(null, Quantity.create(1L)); + assertThat(method.response(request)).usingRecursiveComparison().isEqualTo(expectedResponse); } @Test @@ -534,6 +534,14 @@ private TransactionSimulatorResult getMockTransactionSimulatorResult( any(OperationTracer.class), eq(blockHeader))) .thenReturn(Optional.of(mockTxSimResult)); + // for testing different combination of gasPrice params + when(transactionSimulator.process( + eq(modifiedEip1559TransactionCallParameter(Optional.of(gasPrice))), + eq(Optional.empty()), // no account overrides + any(TransactionValidationParams.class), + any(OperationTracer.class), + eq(blockHeader))) + .thenReturn(Optional.of(mockTxSimResult)); } final TransactionProcessingResult mockResult = mock(TransactionProcessingResult.class); when(mockResult.getEstimateGasUsedByTransaction()).thenReturn(estimateGas); @@ -592,11 +600,15 @@ private JsonCallParameter eip1559TransactionCallParameter(final Optional ga } private CallParameter modifiedEip1559TransactionCallParameter() { + return modifiedEip1559TransactionCallParameter(Optional.empty()); + } + + private CallParameter modifiedEip1559TransactionCallParameter(final Optional gasPrice) { return new CallParameter( Address.fromHexString("0x0"), Address.fromHexString("0x0"), Long.MAX_VALUE, - Wei.ZERO, + gasPrice.orElse(Wei.ZERO), Optional.of(Wei.fromHexString("0x10")), Optional.of(Wei.fromHexString("0x10")), Wei.ZERO, From 2e1fe7b08a5fd7d34c0ea634bf6642e195a47820 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 20 Dec 2024 14:22:05 +1000 Subject: [PATCH 2/6] remove unused error code Signed-off-by: Sally MacFarlane --- .../ethereum/api/jsonrpc/execution/TracedJsonRpcProcessor.java | 1 - .../ethereum/api/jsonrpc/internal/response/RpcErrorType.java | 1 - 2 files changed, 2 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/TracedJsonRpcProcessor.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/TracedJsonRpcProcessor.java index 4ac790617fd..f86918b9b97 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/TracedJsonRpcProcessor.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/TracedJsonRpcProcessor.java @@ -82,7 +82,6 @@ public JsonRpcResponse process( case INVALID_EXECUTION_REQUESTS_PARAMS: case INVALID_EXTRA_DATA_PARAMS: case INVALID_FILTER_PARAMS: - case INVALID_GAS_PRICE_PARAMS: case INVALID_HASH_RATE_PARAMS: case INVALID_ID_PARAMS: case INVALID_RETURN_COMPLETE_TRANSACTION_PARAMS: diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java index fa67fce15bc..57e09ed5ffb 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java @@ -65,7 +65,6 @@ public enum RpcErrorType implements RpcMethodError { INVALID_EXECUTION_REQUESTS_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid execution requests params"), INVALID_EXTRA_DATA_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid extra data params"), INVALID_FILTER_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid filter params"), - INVALID_GAS_PRICE_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid gas price params"), INVALID_HASH_RATE_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid hash rate params"), INVALID_ID_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid ID params"), INVALID_RETURN_COMPLETE_TRANSACTION_PARAMS( From edd4819558d5607e666eecd1b33489300fe0e61f Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 20 Dec 2024 14:52:02 +1000 Subject: [PATCH 3/6] fixed createAccessList unit test Signed-off-by: Sally MacFarlane --- .../methods/EthCreateAccessListTest.java | 16 ++++++++-------- .../internal/methods/EthEstimateGasTest.java | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessListTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessListTest.java index 5046152a6fb..7cb68b79dea 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessListTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessListTest.java @@ -27,7 +27,6 @@ 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.exception.InvalidJsonRpcParameters; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; @@ -51,7 +50,6 @@ import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -156,14 +154,16 @@ public void pendingBlockTagEstimateOnPendingBlock() { } @Test - public void shouldReturnGasEstimateErrorWhenGasPricePresentForEip1559Transaction() { + public void shouldNotErrorWhenGasPricePresentForEip1559Transaction() { + final Wei gasPrice = Wei.of(1000); + final List expectedAccessList = new ArrayList<>(); final JsonRpcRequestContext request = - ethCreateAccessListRequest(eip1559TransactionCallParameter(Optional.of(Wei.of(10)))); - mockTransactionSimulatorResult(false, false, 1L, latestBlockHeader); + ethCreateAccessListRequest(eip1559TransactionCallParameter(Optional.of(gasPrice))); + mockTransactionSimulatorResult(true, false, 1L, latestBlockHeader); - Assertions.assertThatThrownBy(() -> method.response(request)) - .isInstanceOf(InvalidJsonRpcParameters.class) - .hasMessageContaining("gasPrice cannot be used with maxFeePerGas or maxPriorityFeePerGas"); + final JsonRpcResponse expectedResponse = + new JsonRpcSuccessResponse(null, new CreateAccessListResult(expectedAccessList, 1L)); + assertThat(method.response(request)).usingRecursiveComparison().isEqualTo(expectedResponse); } @Test diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java index 38a55858920..13237ecc145 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java @@ -194,7 +194,7 @@ public void shouldUseGasPriceParameterWhenIsPresent() { } @Test - public void shouldReturnGasEstimateErrorWhenGasPricePresentForEip1559Transaction() { + public void shouldNotErrorWhenGasPricePresentForEip1559Transaction() { final Wei gasPrice = Wei.of(1000); final JsonRpcRequestContext request = ethEstimateGasRequest(eip1559TransactionCallParameter(Optional.of(gasPrice))); @@ -586,11 +586,11 @@ private CallParameter eip1559TransactionCallParameter() { return eip1559TransactionCallParameter(Optional.empty()); } - private JsonCallParameter eip1559TransactionCallParameter(final Optional gasPrice) { + private JsonCallParameter eip1559TransactionCallParameter(final Optional maybeGasPrice) { return new JsonCallParameter.JsonCallParameterBuilder() .withFrom(Address.fromHexString("0x0")) .withTo(Address.fromHexString("0x0")) - .withGasPrice(gasPrice.orElse(null)) + .withGasPrice(maybeGasPrice.orElse(null)) .withMaxPriorityFeePerGas(Wei.fromHexString("0x10")) .withMaxFeePerGas(Wei.fromHexString("0x10")) .withValue(Wei.ZERO) From e82e6f5a9ea568a48f1b0c0f5738fca66a6a16d7 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 20 Dec 2024 14:58:18 +1000 Subject: [PATCH 4/6] changelog Signed-off-by: Sally MacFarlane --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bad5945a611..bc62eec499e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Add RPC HTTP options to specify custom truststore and its password [#7978](https://github.com/hyperledger/besu/pull/7978) - Retrieve all transaction receipts for a block in one request [#6646](https://github.com/hyperledger/besu/pull/6646) - Implement EIP-7840: Add blob schedule to config files [#8042](https://github.com/hyperledger/besu/pull/8042) +- Allow gasPrice (legacy) and 1559 gasPrice params to be specified simultaneously for `eth_call`, `eth_createAccessList`, `eth_estimateGas` ### Bug fixes From bca3d8ca9b67f442c110f94e5871044c6e22d666 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 20 Dec 2024 15:01:18 +1000 Subject: [PATCH 5/6] changelog Signed-off-by: Sally MacFarlane --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc62eec499e..efaca489248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ - Add RPC HTTP options to specify custom truststore and its password [#7978](https://github.com/hyperledger/besu/pull/7978) - Retrieve all transaction receipts for a block in one request [#6646](https://github.com/hyperledger/besu/pull/6646) - Implement EIP-7840: Add blob schedule to config files [#8042](https://github.com/hyperledger/besu/pull/8042) -- Allow gasPrice (legacy) and 1559 gasPrice params to be specified simultaneously for `eth_call`, `eth_createAccessList`, `eth_estimateGas` +- Allow gasPrice (legacy) and 1559 gasPrice params to be specified simultaneously for `eth_call`, `eth_createAccessList`, and `eth_estimateGas` [#8059](https://github.com/hyperledger/besu/pull/8059) ### Bug fixes From 30d193ec076a96111919d0acc5a634b782811507 Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Fri, 20 Dec 2024 15:19:27 +1000 Subject: [PATCH 6/6] log params at debug if invalid params exception Signed-off-by: Sally MacFarlane --- .../ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java | 5 ++++- .../api/jsonrpc/internal/methods/JsonCallParameterUtil.java | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java index db6481b46aa..a8225f9bfb2 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/execution/BaseJsonRpcProcessor.java @@ -24,6 +24,8 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.ethereum.privacy.MultiTenancyValidationException; +import java.util.Arrays; + import io.opentelemetry.api.trace.Span; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; @@ -44,7 +46,8 @@ public JsonRpcResponse process( return method.response(request); } catch (final InvalidJsonRpcParameters e) { LOG.debug( - "Invalid Params for method: {}, error: {}", + "Invalid Params {} for method: {}, error: {}", + Arrays.toString(request.getRequest().getParams()), method.getName(), e.getRpcErrorType().getMessage(), e); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java index 152a62cd1c8..6c73e99a9c9 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/JsonCallParameterUtil.java @@ -43,11 +43,11 @@ public static JsonCallParameter validateAndGetCallParams(final JsonRpcRequestCon && (callParams.getMaxFeePerGas().isPresent() || callParams.getMaxPriorityFeePerGas().isPresent())) { try { - LOG.warn( + LOG.debug( "gasPrice will be ignored since 1559 values are defined (maxFeePerGas or maxPriorityFeePerGas). {}", Arrays.toString(request.getRequest().getParams())); } catch (Exception e) { - LOG.warn( + LOG.debug( "gasPrice will be ignored since 1559 values are defined (maxFeePerGas or maxPriorityFeePerGas)"); } }