From d5ee9b7bb1e852ce2c367d130d818591c1771770 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Fri, 11 Oct 2024 10:28:39 +0200 Subject: [PATCH] Fix eth_feeHistory rewards when bounded by configuration (#7750) Signed-off-by: Fabio Di Fabio --- CHANGELOG.md | 1 + .../internal/methods/EthFeeHistory.java | 45 ++++++++----- .../jsonrpc/methods/EthJsonRpcMethods.java | 6 +- .../internal/methods/EthFeeHistoryTest.java | 67 +++++++++++++++++-- 4 files changed, 90 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64b6844330..5c26ef6e77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ ### Additions and Improvements ### Bug fixes +- Fix eth_feeHistory rewards when bounded by configuration [#7750](https://github.com/hyperledger/besu/pull/7750) ## 24.10.0 diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java index b95746dad9..2c5c6afd05 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods; -import static java.util.stream.Collectors.toUnmodifiableList; import static org.hyperledger.besu.ethereum.mainnet.feemarket.ExcessBlobGasCalculator.calculateExcessBlobGasForParent; import org.hyperledger.besu.datatypes.Hash; @@ -32,6 +31,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSucces import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.FeeHistory; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.ImmutableFeeHistory; +import org.hyperledger.besu.ethereum.api.query.BlockchainQueries; import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Block; @@ -57,9 +57,11 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Streams; +import org.apache.tuweni.units.bigints.UInt256s; public class EthFeeHistory implements JsonRpcMethod { private final ProtocolSchedule protocolSchedule; + private final BlockchainQueries blockchainQueries; private final Blockchain blockchain; private final MiningCoordinator miningCoordinator; private final ApiConfiguration apiConfiguration; @@ -70,13 +72,14 @@ public class EthFeeHistory implements JsonRpcMethod { public EthFeeHistory( final ProtocolSchedule protocolSchedule, - final Blockchain blockchain, + final BlockchainQueries blockchainQueries, final MiningCoordinator miningCoordinator, final ApiConfiguration apiConfiguration) { this.protocolSchedule = protocolSchedule; - this.blockchain = blockchain; + this.blockchainQueries = blockchainQueries; this.miningCoordinator = miningCoordinator; this.apiConfiguration = apiConfiguration; + this.blockchain = blockchainQueries.getBlockchain(); this.cache = Caffeine.newBuilder().maximumSize(MAXIMUM_CACHE_SIZE).build(); } @@ -136,7 +139,8 @@ public class EthFeeHistory implements JsonRpcMethod { final List gasUsedRatios = getGasUsedRatios(blockHeaderRange); final List blobGasUsedRatios = getBlobGasUsedRatios(blockHeaderRange); final Optional>> maybeRewards = - maybeRewardPercentiles.map(rewards -> getRewards(rewards, blockHeaderRange)); + maybeRewardPercentiles.map( + percentiles -> getRewards(percentiles, blockHeaderRange, nextBaseFee)); return new JsonRpcSuccessResponse( requestId, createFeeHistoryResult( @@ -203,17 +207,19 @@ public class EthFeeHistory implements JsonRpcMethod { } private List> getRewards( - final List rewardPercentiles, final List blockHeaders) { + final List rewardPercentiles, + final List blockHeaders, + final Wei nextBaseFee) { var sortedPercentiles = rewardPercentiles.stream().sorted().toList(); return blockHeaders.stream() .parallel() - .map(blockHeader -> calculateBlockHeaderReward(sortedPercentiles, blockHeader)) + .map(blockHeader -> calculateBlockHeaderReward(sortedPercentiles, blockHeader, nextBaseFee)) .flatMap(Optional::stream) .toList(); } private Optional> calculateBlockHeaderReward( - final List sortedPercentiles, final BlockHeader blockHeader) { + final List sortedPercentiles, final BlockHeader blockHeader, final Wei nextBaseFee) { // Create a new key for the reward cache final RewardCacheKey key = new RewardCacheKey(blockHeader.getBlockHash(), sortedPercentiles); @@ -226,7 +232,7 @@ public class EthFeeHistory implements JsonRpcMethod { Optional block = blockchain.getBlockByHash(blockHeader.getBlockHash()); return block.map( b -> { - List rewards = computeRewards(sortedPercentiles, b); + List rewards = computeRewards(sortedPercentiles, b, nextBaseFee); // Put the computed rewards in the cache for future use cache.put(key, rewards); return rewards; @@ -237,7 +243,8 @@ public class EthFeeHistory implements JsonRpcMethod { record TransactionInfo(Transaction transaction, Long gasUsed, Wei effectivePriorityFeePerGas) {} @VisibleForTesting - public List computeRewards(final List rewardPercentiles, final Block block) { + public List computeRewards( + final List rewardPercentiles, final Block block, final Wei nextBaseFee) { final List transactions = block.getBody().getTransactions(); if (transactions.isEmpty()) { // all 0's for empty block @@ -253,7 +260,7 @@ public class EthFeeHistory implements JsonRpcMethod { // If the priority fee boundary is set, return the bounded rewards. Otherwise, return the real // rewards. if (apiConfiguration.isGasAndPriorityFeeLimitingEnabled()) { - return boundRewards(realRewards); + return boundRewards(realRewards, nextBaseFee); } else { return realRewards; } @@ -292,16 +299,20 @@ public class EthFeeHistory implements JsonRpcMethod { * This method returns a list of bounded rewards. * * @param rewards The list of rewards to be bounded. + * @param nextBaseFee The base fee of the next block. * @return The list of bounded rewards. */ - private List boundRewards(final List rewards) { - Wei minPriorityFee = miningCoordinator.getMinPriorityFeePerGas(); - Wei lowerBound = - minPriorityFee + private List boundRewards(final List rewards, final Wei nextBaseFee) { + final Wei lowerBoundGasPrice = blockchainQueries.gasPriceLowerBound(); + final Wei lowerBoundPriorityFee = lowerBoundGasPrice.subtract(nextBaseFee); + final Wei minPriorityFee = miningCoordinator.getMinPriorityFeePerGas(); + final Wei forcedMinPriorityFee = UInt256s.max(minPriorityFee, lowerBoundPriorityFee); + final Wei lowerBound = + forcedMinPriorityFee .multiply(apiConfiguration.getLowerBoundGasAndPriorityFeeCoefficient()) .divide(100); - Wei upperBound = - minPriorityFee + final Wei upperBound = + forcedMinPriorityFee .multiply(apiConfiguration.getUpperBoundGasAndPriorityFeeCoefficient()) .divide(100); @@ -438,7 +449,7 @@ public class EthFeeHistory implements JsonRpcMethod { .oldestBlock(oldestBlock) .baseFeePerGas( Stream.concat(explicitlyRequestedBaseFees.stream(), Stream.of(nextBaseFee)) - .collect(toUnmodifiableList())) + .toList()) .baseFeePerBlobGas(requestedBlobBaseFees) .gasUsedRatio(gasUsedRatios) .blobGasUsedRatio(blobGasUsedRatio) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthJsonRpcMethods.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthJsonRpcMethods.java index 5baa110646..de5662692f 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthJsonRpcMethods.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthJsonRpcMethods.java @@ -132,11 +132,7 @@ public class EthJsonRpcMethods extends ApiGroupJsonRpcMethods { blockchainQueries.getWorldStateArchive(), protocolSchedule, apiConfiguration.getGasCap())), - new EthFeeHistory( - protocolSchedule, - blockchainQueries.getBlockchain(), - miningCoordinator, - apiConfiguration), + new EthFeeHistory(protocolSchedule, blockchainQueries, miningCoordinator, apiConfiguration), new EthGetCode(blockchainQueries), new EthGetLogs(blockchainQueries, apiConfiguration.getMaxLogsRange()), new EthGetProof(blockchainQueries), diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistoryTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistoryTest.java index a04cb06c67..f34f336e03 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistoryTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistoryTest.java @@ -39,6 +39,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.FeeHistory; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.ImmutableFeeHistory; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.ImmutableFeeHistoryResult; +import org.hyperledger.besu.ethereum.api.query.BlockchainQueries; import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator; import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; @@ -69,6 +70,7 @@ import org.junit.jupiter.api.Test; public class EthFeeHistoryTest { final BlockDataGenerator gen = new BlockDataGenerator(); + private BlockchainQueries blockchainQueries; private MutableBlockchain blockchain; private EthFeeHistory method; private ProtocolSchedule protocolSchedule; @@ -82,14 +84,15 @@ public class EthFeeHistoryTest { gen.blockSequence(genesisBlock, 10) .forEach(block -> blockchain.appendBlock(block, gen.receipts(block))); miningCoordinator = mock(MergeCoordinator.class); - when(miningCoordinator.getMinPriorityFeePerGas()).thenReturn(Wei.ONE); + + blockchainQueries = mockBlockchainQueries(blockchain, Wei.of(7)); mockFork(); method = new EthFeeHistory( protocolSchedule, - blockchain, + blockchainQueries, miningCoordinator, ImmutableApiConfiguration.builder().build()); } @@ -139,11 +142,16 @@ public class EthFeeHistoryTest { Block block = mock(Block.class); Blockchain blockchain = mockBlockchainTransactionsWithPriorityFee(block); + final var blockchainQueries = mockBlockchainQueries(blockchain, Wei.of(7)); + EthFeeHistory ethFeeHistory = new EthFeeHistory( - null, blockchain, miningCoordinator, ImmutableApiConfiguration.builder().build()); + null, + blockchainQueries, + miningCoordinator, + ImmutableApiConfiguration.builder().build()); - List rewards = ethFeeHistory.computeRewards(rewardPercentiles, block); + List rewards = ethFeeHistory.computeRewards(rewardPercentiles, block, Wei.of(7)); // Define the expected rewards for each percentile // The expected rewards match the fees of the transactions at each percentile in the @@ -179,14 +187,51 @@ public class EthFeeHistoryTest { .upperBoundGasAndPriorityFeeCoefficient(500L) .build(); // Max reward = Wei.One * 500L / 100 = 5.0 + final var blockchainQueries = mockBlockchainQueries(blockchain, Wei.of(7)); + when(miningCoordinator.getMinPriorityFeePerGas()).thenReturn(Wei.ONE); + EthFeeHistory ethFeeHistory = - new EthFeeHistory(null, blockchain, miningCoordinator, apiConfiguration); + new EthFeeHistory(null, blockchainQueries, miningCoordinator, apiConfiguration); - List rewards = ethFeeHistory.computeRewards(rewardPercentiles, block); + List rewards = ethFeeHistory.computeRewards(rewardPercentiles, block, Wei.of(7)); // Define the expected bounded rewards for each percentile List expectedBoundedRewards = Stream.of(2, 2, 2, 4, 5, 5, 5, 5, 5).map(Wei::of).toList(); - assertThat(expectedBoundedRewards).isEqualTo(rewards); + assertThat(rewards).isEqualTo(expectedBoundedRewards); + } + + @Test + public void shouldApplyLowerBoundRewardsCorrectly() { + // This test checks that the rewards are correctly bounded by the lower and upper limits, + // when the calculated lower bound for the priority fee is greater than the configured one. + // Configured minPriorityFeePerGas is 0 wei, minGasPrice is 10 wei and baseFee is 8 wei, + // so for a tx to be mined the minPriorityFeePerGas is raised to 2 wei before applying the + // coefficients. + + List rewardPercentiles = + Arrays.asList(0.0, 5.0, 10.0, 27.50, 31.0, 59.0, 60.0, 61.0, 100.0); + + Block block = mock(Block.class); + Blockchain blockchain = mockBlockchainTransactionsWithPriorityFee(block); + + ApiConfiguration apiConfiguration = + ImmutableApiConfiguration.builder() + .isGasAndPriorityFeeLimitingEnabled(true) + .lowerBoundGasAndPriorityFeeCoefficient(200L) // Min reward = 2 * 200L / 100 = 4.0 + .upperBoundGasAndPriorityFeeCoefficient(300L) + .build(); // Max reward = 2 * 300L / 100 = 6.0 + + final var blockchainQueries = mockBlockchainQueries(blockchain, Wei.of(10)); + when(miningCoordinator.getMinPriorityFeePerGas()).thenReturn(Wei.ZERO); + + EthFeeHistory ethFeeHistory = + new EthFeeHistory(null, blockchainQueries, miningCoordinator, apiConfiguration); + + List rewards = ethFeeHistory.computeRewards(rewardPercentiles, block, Wei.of(8)); + + // Define the expected bounded rewards for each percentile + List expectedBoundedRewards = Stream.of(4, 4, 4, 4, 5, 6, 6, 6, 6).map(Wei::of).toList(); + assertThat(rewards).isEqualTo(expectedBoundedRewards); } private Blockchain mockBlockchainTransactionsWithPriorityFee(final Block block) { @@ -399,4 +444,12 @@ public class EthFeeHistoryTest { return method.response( new JsonRpcRequestContext(new JsonRpcRequest("2.0", "eth_feeHistory", params))); } + + private BlockchainQueries mockBlockchainQueries( + final Blockchain blockchain, final Wei gasPriceLowerBound) { + final var blockchainQueries = mock(BlockchainQueries.class); + when(blockchainQueries.getBlockchain()).thenReturn(blockchain); + when(blockchainQueries.gasPriceLowerBound()).thenReturn(gasPriceLowerBound); + return blockchainQueries; + } }