Skip to content

Commit

Permalink
Ignore gasPrice if 1559 gas params are specified (#8059)
Browse files Browse the repository at this point in the history
* don't throw if all the gasprice params are specified

Signed-off-by: Sally MacFarlane <[email protected]>

---------

Signed-off-by: Sally MacFarlane <[email protected]>
  • Loading branch information
macfarla authored and siladu committed Dec 21, 2024
1 parent 331cacc commit 4831117
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 25 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
### Additions and Improvements
- 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`, and `eth_estimateGas` [#8059](https://github.com/hyperledger/besu/pull/8059)


### Bug fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

Expand All @@ -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.debug(
"gasPrice will be ignored since 1559 values are defined (maxFeePerGas or maxPriorityFeePerGas). {}",
Arrays.toString(request.getRequest().getParams()));
} catch (Exception e) {
LOG.debug(
"gasPrice will be ignored since 1559 values are defined (maxFeePerGas or maxPriorityFeePerGas)");
}
}
return callParams;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -156,14 +154,16 @@ public void pendingBlockTagEstimateOnPendingBlock() {
}

@Test
public void shouldReturnGasEstimateErrorWhenGasPricePresentForEip1559Transaction() {
public void shouldNotErrorWhenGasPricePresentForEip1559Transaction() {
final Wei gasPrice = Wei.of(1000);
final List<AccessListEntry> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -196,13 +194,15 @@ public void shouldUseGasPriceParameterWhenIsPresent() {
}

@Test
public void shouldReturnGasEstimateErrorWhenGasPricePresentForEip1559Transaction() {
public void shouldNotErrorWhenGasPricePresentForEip1559Transaction() {
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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -578,11 +586,11 @@ private CallParameter eip1559TransactionCallParameter() {
return eip1559TransactionCallParameter(Optional.empty());
}

private JsonCallParameter eip1559TransactionCallParameter(final Optional<Wei> gasPrice) {
private JsonCallParameter eip1559TransactionCallParameter(final Optional<Wei> 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)
Expand All @@ -592,11 +600,15 @@ private JsonCallParameter eip1559TransactionCallParameter(final Optional<Wei> ga
}

private CallParameter modifiedEip1559TransactionCallParameter() {
return modifiedEip1559TransactionCallParameter(Optional.empty());
}

private CallParameter modifiedEip1559TransactionCallParameter(final Optional<Wei> 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,
Expand Down

0 comments on commit 4831117

Please sign in to comment.