Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RPC] Use apiConfiguration to limit gasPrice in eth_getGasPrice #6243

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Expand Up @@ -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"},
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
}
Expand Down
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
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Loading
Loading