Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[RPC] Use apiConfiguration to limit gasPrice in eth_getGasPrice (hype…
Browse files Browse the repository at this point in the history
…rledger#6243)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
Signed-off-by: jflo <justin+github@florentine.us>
Gabriel-Trintinalia authored and jflo committed Dec 12, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent d981269 commit ce148f6
Showing 8 changed files with 196 additions and 80 deletions.
42 changes: 21 additions & 21 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
@@ -1218,27 +1218,27 @@ static class PermissionsOptionGroup {
private final Long apiGasPriceMax = 500_000_000_000L;

@CommandLine.Option(
names = {"--api-priority-fee-limiting-enabled"},
names = {"--api-gas-and-priority-fee-limiting-enabled"},
hidden = true,
description =
"Set to enable priority fee limit in eth_feeHistory (default: ${DEFAULT-VALUE})")
private final Boolean apiPriorityFeeLimitingEnabled = false;
"Set to enable gas price and minimum priority fee limit in eth_getGasPrice and eth_feeHistory (default: ${DEFAULT-VALUE})")
private final Boolean apiGasAndPriorityFeeLimitingEnabled = false;

@CommandLine.Option(
names = {"--api-priority-fee-lower-bound-coefficient"},
names = {"--api-gas-and-priority-fee-lower-bound-coefficient"},
hidden = true,
description =
"Coefficient for setting the lower limit of minimum priority fee in eth_feeHistory (default: ${DEFAULT-VALUE})")
private final Long apiPriorityFeeLowerBoundCoefficient =
ApiConfiguration.DEFAULT_LOWER_BOUND_PRIORITY_FEE_COEFFICIENT;
"Coefficient for setting the lower limit of gas price and minimum priority fee in eth_getGasPrice and eth_feeHistory (default: ${DEFAULT-VALUE})")
private final Long apiGasAndPriorityFeeLowerBoundCoefficient =
ApiConfiguration.DEFAULT_LOWER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT;

@CommandLine.Option(
names = {"--api-priority-fee-upper-bound-coefficient"},
names = {"--api-gas-and-priority-fee-upper-bound-coefficient"},
hidden = true,
description =
"Coefficient for setting the upper limit of minimum priority fee in eth_feeHistory (default: ${DEFAULT-VALUE})")
private final Long apiPriorityFeeUpperBoundCoefficient =
ApiConfiguration.DEFAULT_UPPER_BOUND_PRIORITY_FEE_COEFFICIENT;
"Coefficient for setting the upper limit of gas price and minimum priority fee in eth_getGasPrice and eth_feeHistory (default: ${DEFAULT-VALUE})")
private final Long apiGasAndPriorityFeeUpperBoundCoefficient =
ApiConfiguration.DEFAULT_UPPER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT;

@CommandLine.Option(
names = {"--static-nodes-file"},
@@ -1902,11 +1902,11 @@ private void checkApiOptionsDependencies() {
CommandLineUtils.checkOptionDependencies(
logger,
commandLine,
"--api-priority-fee-limiting-enabled",
!apiPriorityFeeLimitingEnabled,
"--api-gas-and-priority-fee-limiting-enabled",
!apiGasAndPriorityFeeLimitingEnabled,
asList(
"--api-priority-fee-upper-bound-coefficient",
"--api-priority-fee-lower-bound-coefficient"));
"--api-gas-and-priority-fee-upper-bound-coefficient",
"--api-gas-and-priority-fee-lower-bound-coefficient"));
}

private void ensureValidPeerBoundParams() {
@@ -2534,16 +2534,16 @@ private ApiConfiguration apiConfiguration() {
.gasPriceMax(apiGasPriceMax)
.maxLogsRange(rpcMaxLogsRange)
.gasCap(rpcGasCap)
.isPriorityFeeLimitingEnabled(apiPriorityFeeLimitingEnabled);
if (apiPriorityFeeLimitingEnabled) {
if (apiPriorityFeeLowerBoundCoefficient > apiPriorityFeeUpperBoundCoefficient) {
.isGasAndPriorityFeeLimitingEnabled(apiGasAndPriorityFeeLimitingEnabled);
if (apiGasAndPriorityFeeLimitingEnabled) {
if (apiGasAndPriorityFeeLowerBoundCoefficient > apiGasAndPriorityFeeUpperBoundCoefficient) {
throw new ParameterException(
this.commandLine,
"--api-priority-fee-lower-bound-coefficient cannot be greater than the value of --api-priority-fee-upper-bound-coefficient");
"--api-gas-and-priority-fee-lower-bound-coefficient cannot be greater than the value of --api-gas-and-priority-fee-upper-bound-coefficient");
}
builder
.lowerBoundPriorityFeeCoefficient(apiPriorityFeeLowerBoundCoefficient)
.upperBoundPriorityFeeCoefficient(apiPriorityFeeUpperBoundCoefficient);
.lowerBoundGasAndPriorityFeeCoefficient(apiGasAndPriorityFeeLowerBoundCoefficient)
.upperBoundGasAndPriorityFeeCoefficient(apiGasAndPriorityFeeUpperBoundCoefficient);
}
return builder.build();
}
29 changes: 15 additions & 14 deletions besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
Original file line number Diff line number Diff line change
@@ -1611,11 +1611,12 @@ public void rpcGasCapOptionMustBeUsed() {

@Test
public void apiPriorityFeeLimitingEnabledOptionMustBeUsed() {
parseCommand("--api-priority-fee-limiting-enabled");
parseCommand("--api-gas-and-priority-fee-limiting-enabled");
verify(mockRunnerBuilder).apiConfiguration(apiConfigurationCaptor.capture());
verify(mockRunnerBuilder).build();
assertThat(apiConfigurationCaptor.getValue())
.isEqualTo(ImmutableApiConfiguration.builder().isPriorityFeeLimitingEnabled(true).build());
.isEqualTo(
ImmutableApiConfiguration.builder().isGasAndPriorityFeeLimitingEnabled(true).build());

assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
@@ -1625,16 +1626,16 @@ public void apiPriorityFeeLimitingEnabledOptionMustBeUsed() {
public void apiPriorityFeeLowerBoundCoefficientOptionMustBeUsed() {
final long lowerBound = 150L;
parseCommand(
"--api-priority-fee-lower-bound-coefficient",
"--api-gas-and-priority-fee-lower-bound-coefficient",
Long.toString(lowerBound),
"--api-priority-fee-limiting-enabled");
"--api-gas-and-priority-fee-limiting-enabled");
verify(mockRunnerBuilder).apiConfiguration(apiConfigurationCaptor.capture());
verify(mockRunnerBuilder).build();
assertThat(apiConfigurationCaptor.getValue())
.isEqualTo(
ImmutableApiConfiguration.builder()
.lowerBoundPriorityFeeCoefficient(lowerBound)
.isPriorityFeeLimitingEnabled(true)
.lowerBoundGasAndPriorityFeeCoefficient(lowerBound)
.isGasAndPriorityFeeLimitingEnabled(true)
.build());

assertThat(commandOutput.toString(UTF_8)).isEmpty();
@@ -1648,32 +1649,32 @@ public void apiPriorityFeeLowerBoundCoefficientOptionMustBeUsed() {
final long upperBound = 100L;

parseCommand(
"--api-priority-fee-limiting-enabled",
"--api-priority-fee-lower-bound-coefficient",
"--api-gas-and-priority-fee-limiting-enabled",
"--api-gas-and-priority-fee-lower-bound-coefficient",
Long.toString(lowerBound),
"--api-priority-fee-upper-bound-coefficient",
"--api-gas-and-priority-fee-upper-bound-coefficient",
Long.toString(upperBound));
Mockito.verifyNoInteractions(mockRunnerBuilder);
assertThat(commandOutput.toString(UTF_8)).isEmpty();
assertThat(commandErrorOutput.toString(UTF_8))
.contains(
"--api-priority-fee-lower-bound-coefficient cannot be greater than the value of --api-priority-fee-upper-bound-coefficient");
"--api-gas-and-priority-fee-lower-bound-coefficient cannot be greater than the value of --api-gas-and-priority-fee-upper-bound-coefficient");
}

@Test
public void apiPriorityFeeUpperBoundCoefficientsOptionMustBeUsed() {
final long upperBound = 200L;
parseCommand(
"--api-priority-fee-upper-bound-coefficient",
"--api-gas-and-priority-fee-upper-bound-coefficient",
Long.toString(upperBound),
"--api-priority-fee-limiting-enabled");
"--api-gas-and-priority-fee-limiting-enabled");
verify(mockRunnerBuilder).apiConfiguration(apiConfigurationCaptor.capture());
verify(mockRunnerBuilder).build();
assertThat(apiConfigurationCaptor.getValue())
.isEqualTo(
ImmutableApiConfiguration.builder()
.upperBoundPriorityFeeCoefficient(upperBound)
.isPriorityFeeLimitingEnabled(true)
.upperBoundGasAndPriorityFeeCoefficient(upperBound)
.isGasAndPriorityFeeLimitingEnabled(true)
.build());

assertThat(commandOutput.toString(UTF_8)).isEmpty();
Original file line number Diff line number Diff line change
@@ -24,8 +24,8 @@
@Value.Style(allParameters = true)
public abstract class ApiConfiguration {

public static final long DEFAULT_LOWER_BOUND_PRIORITY_FEE_COEFFICIENT = 0L;
public static final long DEFAULT_UPPER_BOUND_PRIORITY_FEE_COEFFICIENT = Long.MAX_VALUE;
public static final long DEFAULT_LOWER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT = 0L;
public static final long DEFAULT_UPPER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT = Long.MAX_VALUE;

@Value.Default
public long getGasPriceBlocks() {
@@ -64,17 +64,17 @@ public Long getGasCap() {
}

@Value.Default
public boolean isPriorityFeeLimitingEnabled() {
public boolean isGasAndPriorityFeeLimitingEnabled() {
return false;
}

@Value.Default
public Long getLowerBoundPriorityFeeCoefficient() {
return DEFAULT_LOWER_BOUND_PRIORITY_FEE_COEFFICIENT;
public Long getLowerBoundGasAndPriorityFeeCoefficient() {
return DEFAULT_LOWER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT;
}

@Value.Default
public Long getUpperBoundPriorityFeeCoefficient() {
return DEFAULT_UPPER_BOUND_PRIORITY_FEE_COEFFICIENT;
public Long getUpperBoundGasAndPriorityFeeCoefficient() {
return DEFAULT_UPPER_BOUND_GAS_AND_PRIORITY_FEE_COEFFICIENT;
}
}
Original file line number Diff line number Diff line change
@@ -218,7 +218,7 @@ public List<Wei> computeRewards(final List<Double> rewardPercentiles, final Bloc

// If the priority fee boundary is set, return the bounded rewards. Otherwise, return the real
// rewards.
if (apiConfiguration.isPriorityFeeLimitingEnabled()) {
if (apiConfiguration.isGasAndPriorityFeeLimitingEnabled()) {
return boundRewards(realRewards);
} else {
return realRewards;
@@ -263,9 +263,13 @@ private List<Wei> calculateRewards(
private List<Wei> boundRewards(final List<Wei> rewards) {
Wei minPriorityFee = miningCoordinator.getMinPriorityFeePerGas();
Wei lowerBound =
minPriorityFee.multiply(apiConfiguration.getLowerBoundPriorityFeeCoefficient()).divide(100);
minPriorityFee
.multiply(apiConfiguration.getLowerBoundGasAndPriorityFeeCoefficient())
.divide(100);
Wei upperBound =
minPriorityFee.multiply(apiConfiguration.getUpperBoundPriorityFeeCoefficient()).divide(100);
minPriorityFee
.multiply(apiConfiguration.getUpperBoundGasAndPriorityFeeCoefficient())
.divide(100);

return rewards.stream().map(reward -> boundReward(reward, lowerBound, upperBound)).toList();
}
@@ -279,19 +283,9 @@ private List<Wei> boundRewards(final List<Wei> rewards) {
* @return The bounded reward.
*/
private Wei boundReward(final Wei reward, final Wei lowerBound, final Wei upperBound) {

// If the reward is less than the lower bound, return the lower bound.
if (reward.compareTo(lowerBound) <= 0) {
return lowerBound;
}

// If the reward is greater than the upper bound, return the upper bound.
if (reward.compareTo(upperBound) > 0) {
return upperBound;
}

// If the reward is within the bounds, return the reward as is.
return reward;
return reward.compareTo(lowerBound) <= 0
? lowerBound
: reward.compareTo(upperBound) >= 0 ? upperBound : reward;
}

private List<Long> calculateTransactionsGasUsed(final Block block) {
Original file line number Diff line number Diff line change
@@ -14,6 +14,8 @@
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;

import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.api.ApiConfiguration;
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.response.JsonRpcResponse;
@@ -22,22 +24,29 @@
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator;

import java.util.Optional;
import java.util.function.Supplier;

public class EthGasPrice implements JsonRpcMethod {

private final Supplier<BlockchainQueries> blockchain;
private final MiningCoordinator miningCoordinator;
private final ApiConfiguration apiConfiguration;

public EthGasPrice(
final BlockchainQueries blockchain, final MiningCoordinator miningCoordinator) {
this(() -> blockchain, miningCoordinator);
final BlockchainQueries blockchain,
final MiningCoordinator miningCoordinator,
final ApiConfiguration apiConfiguration) {
this(() -> blockchain, miningCoordinator, apiConfiguration);
}

public EthGasPrice(
final Supplier<BlockchainQueries> blockchain, final MiningCoordinator miningCoordinator) {
final Supplier<BlockchainQueries> blockchain,
final MiningCoordinator miningCoordinator,
final ApiConfiguration apiConfiguration) {
this.blockchain = blockchain;
this.miningCoordinator = miningCoordinator;
this.apiConfiguration = apiConfiguration;
}

@Override
@@ -48,11 +57,37 @@ public String getName() {
@Override
public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
return new JsonRpcSuccessResponse(
requestContext.getRequest().getId(),
blockchain
.get()
.gasPrice()
.map(Quantity::create)
.orElseGet(() -> Quantity.create(miningCoordinator.getMinTransactionGasPrice())));
requestContext.getRequest().getId(), Quantity.create(calculateGasPrice()));
}

private Wei calculateGasPrice() {
Wei gasPrice = getGasPrice().orElseGet(miningCoordinator::getMinTransactionGasPrice);
return isGasPriceLimitingEnabled() ? limitGasPrice(gasPrice) : gasPrice;
}

private Optional<Wei> getGasPrice() {
return blockchain.get().gasPrice().map(Wei::of);
}

private boolean isGasPriceLimitingEnabled() {
return apiConfiguration.isGasAndPriorityFeeLimitingEnabled();
}

private Wei limitGasPrice(final Wei gasPrice) {
Wei minTransactionGasPrice = miningCoordinator.getMinTransactionGasPrice();
Wei lowerBound =
calculateBound(
minTransactionGasPrice, apiConfiguration.getLowerBoundGasAndPriorityFeeCoefficient());
Wei upperBound =
calculateBound(
minTransactionGasPrice, apiConfiguration.getUpperBoundGasAndPriorityFeeCoefficient());

return gasPrice.compareTo(lowerBound) <= 0
? lowerBound
: gasPrice.compareTo(upperBound) >= 0 ? upperBound : gasPrice;
}

private Wei calculateBound(final Wei price, final long coefficient) {
return price.multiply(coefficient).divide(100);
}
}
Original file line number Diff line number Diff line change
@@ -174,7 +174,7 @@ protected Map<String, JsonRpcMethod> create() {
new EthMining(miningCoordinator),
new EthCoinbase(miningCoordinator),
new EthProtocolVersion(supportedCapabilities),
new EthGasPrice(blockchainQueries, miningCoordinator),
new EthGasPrice(blockchainQueries, miningCoordinator, apiConfiguration),
new EthGetWork(miningCoordinator),
new EthSubmitWork(miningCoordinator),
new EthHashrate(miningCoordinator),
Original file line number Diff line number Diff line change
@@ -174,9 +174,9 @@ public void shouldBoundRewardsCorrectly() {

ApiConfiguration apiConfiguration =
ImmutableApiConfiguration.builder()
.isPriorityFeeLimitingEnabled(true)
.lowerBoundPriorityFeeCoefficient(200L) // Min reward = Wei.One * 200L / 100 = 2.0
.upperBoundPriorityFeeCoefficient(500L)
.isGasAndPriorityFeeLimitingEnabled(true)
.lowerBoundGasAndPriorityFeeCoefficient(200L) // Min reward = Wei.One * 200L / 100 = 2.0
.upperBoundGasAndPriorityFeeCoefficient(500L)
.build(); // Max reward = Wei.One * 500L / 100 = 5.0

EthFeeHistory ethFeeHistory =
Original file line number Diff line number Diff line change
@@ -23,6 +23,7 @@
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.api.ApiConfiguration;
import org.hyperledger.besu.ethereum.api.ImmutableApiConfiguration;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
@@ -60,15 +61,8 @@ public class EthGasPriceTest {

@BeforeEach
public void setUp() {
method =
new EthGasPrice(
new BlockchainQueries(
blockchain,
null,
Optional.empty(),
Optional.empty(),
ImmutableApiConfiguration.builder().gasPriceMinSupplier(() -> 100).build()),
miningCoordinator);
ApiConfiguration apiConfig = createDefaultApiConfiguration();
method = createEthGasPriceMethod(apiConfig);
}

@Test
@@ -141,6 +135,68 @@ public void shortChainQueriesAllBlocks() {
verifyNoMoreInteractions(blockchain);
}

/**
* Test to verify that the method returns the lower bound gas price when the lower bound parameter
* is present.
*/
@Test
public void shouldReturnLimitedPriceWhenLowerBoundIsPresent() {
long gasPrice = 5000000;
long lowerBoundGasPrice = gasPrice + 1;
verifyGasPriceLimit(lowerBoundGasPrice, null, lowerBoundGasPrice);
}

/**
* Test to verify that the method returns the upper bound gas price when the upper bound parameter
* is present.
*/
@Test
public void shouldReturnLimitedPriceWhenUpperBoundIsPresent() {
long gasPrice = 5000000;
long upperBoundGasPrice = gasPrice - 1;
verifyGasPriceLimit(null, upperBoundGasPrice, upperBoundGasPrice);
}

/**
* Test to verify that the method returns the actual gas price when the gas price is within the
* bound range.
*/
@Test
public void shouldReturnActualGasPriceWhenWithinBoundRange() {
long gasPrice = 5000000;
long lowerBoundGasPrice = gasPrice - 1;
long upperBoundGasPrice = gasPrice + 1;
verifyGasPriceLimit(lowerBoundGasPrice, upperBoundGasPrice, gasPrice);
}

/**
* Helper method to verify the gas price limit.
*
* @param lowerBound The lower bound of the gas price.
* @param upperBound The upper bound of the gas price.
* @param expectedGasPrice The expected gas price.
*/
private void verifyGasPriceLimit(
final Long lowerBound, final Long upperBound, final long expectedGasPrice) {
when(miningCoordinator.getMinTransactionGasPrice()).thenReturn(Wei.of(100));

var apiConfig =
createApiConfiguration(Optional.ofNullable(lowerBound), Optional.ofNullable(upperBound));
method = createEthGasPriceMethod(apiConfig);

final JsonRpcRequestContext request = requestWithParams();
final JsonRpcResponse expectedResponse =
new JsonRpcSuccessResponse(
request.getRequest().getId(), Wei.of(expectedGasPrice).toShortHexString());

when(blockchain.getChainHeadBlockNumber()).thenReturn(10L);
when(blockchain.getBlockByNumber(anyLong()))
.thenAnswer(invocation -> createFakeBlock(invocation.getArgument(0, Long.class)));

final JsonRpcResponse actualResponse = method.response(request);
assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
}

private Object createFakeBlock(final Long height) {
return Optional.of(
new Block(
@@ -210,4 +266,34 @@ private Object createEmptyBlock(final Long height) {
private JsonRpcRequestContext requestWithParams(final Object... params) {
return new JsonRpcRequestContext(new JsonRpcRequest(JSON_RPC_VERSION, ETH_METHOD, params));
}

private ApiConfiguration createDefaultApiConfiguration() {
return createApiConfiguration(Optional.empty(), Optional.empty());
}

private ApiConfiguration createApiConfiguration(
final Optional<Long> lowerBound, final Optional<Long> upperBound) {
ImmutableApiConfiguration.Builder builder =
ImmutableApiConfiguration.builder().gasPriceMinSupplier(() -> 100);

lowerBound.ifPresent(
value ->
builder
.isGasAndPriorityFeeLimitingEnabled(true)
.lowerBoundGasAndPriorityFeeCoefficient(value));
upperBound.ifPresent(
value ->
builder
.isGasAndPriorityFeeLimitingEnabled(true)
.upperBoundGasAndPriorityFeeCoefficient(value));

return builder.build();
}

private EthGasPrice createEthGasPriceMethod(final ApiConfiguration apiConfig) {
return new EthGasPrice(
new BlockchainQueries(blockchain, null, Optional.empty(), Optional.empty(), apiConfig),
miningCoordinator,
apiConfig);
}
}

0 comments on commit ce148f6

Please sign in to comment.