Fix eth_feeHistory rewards when bounded by configuration (hyperledger…

…#7750)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>

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
 

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.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.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 @@ record RewardCacheKey(Hash blockHash, List<Double> rewardPercentiles) {}
 
   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 JsonRpcResponse response(final JsonRpcRequestContext request) {
     final List<Double> gasUsedRatios = getGasUsedRatios(blockHeaderRange);
     final List<Double> blobGasUsedRatios = getBlobGasUsedRatios(blockHeaderRange);
     final Optional<List<List<Wei>>> maybeRewards =
-        maybeRewardPercentiles.map(rewards -> getRewards(rewards, blockHeaderRange));
+        maybeRewardPercentiles.map(
+            percentiles -> getRewards(percentiles, blockHeaderRange, nextBaseFee));
     return new JsonRpcSuccessResponse(
         requestId,
         createFeeHistoryResult(
@@ -203,17 +207,19 @@ private Wei computeNextBaseFee(
   }
 
   private List<List<Wei>> getRewards(
-      final List<Double> rewardPercentiles, final List<BlockHeader> blockHeaders) {
+      final List<Double> rewardPercentiles,
+      final List<BlockHeader> 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<List<Wei>> calculateBlockHeaderReward(
-      final List<Double> sortedPercentiles, final BlockHeader blockHeader) {
+      final List<Double> 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 @@ private Optional<List<Wei>> calculateBlockHeaderReward(
               Optional<Block> block = blockchain.getBlockByHash(blockHeader.getBlockHash());
               return block.map(
                   b -> {
-                    List<Wei> rewards = computeRewards(sortedPercentiles, b);
+                    List<Wei> 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 @@ private Optional<List<Wei>> calculateBlockHeaderReward(
   record TransactionInfo(Transaction transaction, Long gasUsed, Wei effectivePriorityFeePerGas) {}
 
   @VisibleForTesting
-  public List<Wei> computeRewards(final List<Double> rewardPercentiles, final Block block) {
+  public List<Wei> computeRewards(
+      final List<Double> rewardPercentiles, final Block block, final Wei nextBaseFee) {
     final List<Transaction> transactions = block.getBody().getTransactions();
     if (transactions.isEmpty()) {
       // all 0's for empty block
@@ -253,7 +260,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.isGasAndPriorityFeeLimitingEnabled()) {
-      return boundRewards(realRewards);
+      return boundRewards(realRewards, nextBaseFee);
     } else {
       return realRewards;
     }
@@ -292,16 +299,20 @@ private List<Wei> calculateRewards(
    * 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<Wei> boundRewards(final List<Wei> rewards) {
-    Wei minPriorityFee = miningCoordinator.getMinPriorityFeePerGas();
-    Wei lowerBound =
-        minPriorityFee
+  private List<Wei> boundRewards(final List<Wei> 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 @@ private FeeHistory.FeeHistoryResult createFeeHistoryResult(
             .oldestBlock(oldestBlock)
             .baseFeePerGas(
                 Stream.concat(explicitlyRequestedBaseFees.stream(), Stream.of(nextBaseFee))
-                    .collect(toUnmodifiableList()))
+                    .toList())
             .baseFeePerBlobGas(requestedBlobBaseFees)
             .gasUsedRatio(gasUsedRatios)
             .blobGasUsedRatio(blobGasUsedRatio)

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthJsonRpcMethods.java

@@ -132,11 +132,7 @@ protected Map<String, JsonRpcMethod> create() {
                 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),

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.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 @@
 
 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 void setUp() {
     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 void shouldComputeRewardsCorrectly() {
     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<Wei> rewards = ethFeeHistory.computeRewards(rewardPercentiles, block);
+    List<Wei> 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 void shouldBoundRewardsCorrectly() {
             .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<Wei> rewards = ethFeeHistory.computeRewards(rewardPercentiles, block);
+    List<Wei> rewards = ethFeeHistory.computeRewards(rewardPercentiles, block, Wei.of(7));
 
     // Define the expected bounded rewards for each percentile
     List<Wei> 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<Double> 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<Wei> rewards = ethFeeHistory.computeRewards(rewardPercentiles, block, Wei.of(8));
+
+    // Define the expected bounded rewards for each percentile
+    List<Wei> 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 @@ private JsonRpcResponse feeHistoryRequest(final Object... params) {
     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;
+  }
 }