Fix EthFeeHistory reward calculation (#6168)

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

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

@@ -47,6 +47,7 @@ import java.util.stream.Stream;
 
 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;
 
 public class EthFeeHistory implements JsonRpcMethod {
@@ -187,7 +188,8 @@ public class EthFeeHistory implements JsonRpcMethod {
                 .build()));
   }
 
-  private List<Wei> computeRewards(final List<Double> rewardPercentiles, final Block block) {
+  @VisibleForTesting
+  public List<Wei> computeRewards(final List<Double> rewardPercentiles, final Block block) {
     final List<Transaction> transactions = block.getBody().getTransactions();
     if (transactions.isEmpty()) {
       // all 0's for empty block
@@ -201,13 +203,11 @@ public class EthFeeHistory implements JsonRpcMethod {
     // we need to get the gas used for the individual transactions and can't use the cumulative gas
     // used because we're going to be reordering the transactions
     final List<Long> transactionsGasUsed = new ArrayList<>();
+    long cumulativeGasUsed = 0L;
     for (final TransactionReceipt transactionReceipt :
         blockchain.getTxReceipts(block.getHash()).get()) {
-      transactionsGasUsed.add(
-          transactionsGasUsed.isEmpty()
-              ? transactionReceipt.getCumulativeGasUsed()
-              : transactionReceipt.getCumulativeGasUsed()
-                  - transactionsGasUsed.get(transactionsGasUsed.size() - 1));
+      transactionsGasUsed.add(transactionReceipt.getCumulativeGasUsed() - cumulativeGasUsed);
+      cumulativeGasUsed = transactionReceipt.getCumulativeGasUsed();
     }
 
     record TransactionInfo(Transaction transaction, Long gasUsed, Wei effectivePriorityFeePerGas) {}
@@ -226,23 +226,26 @@ public class EthFeeHistory implements JsonRpcMethod {
             .sorted(Comparator.comparing(TransactionInfo::effectivePriorityFeePerGas))
             .collect(toUnmodifiableList());
 
-    // We need to weight the percentile of rewards by the gas used in the transaction.
-    // That's why we're keeping track of the cumulative gas used and checking to see which
-    // percentile markers we've passed
     final ArrayList<Wei> rewards = new ArrayList<>();
-    int rewardPercentileIndex = 0;
-    long gasUsed = 0;
-    for (final TransactionInfo transactionAndGasUsed :
-        transactionsAndGasUsedAscendingEffectiveGasFee) {
-
-      gasUsed += transactionAndGasUsed.gasUsed();
-
-      while (rewardPercentileIndex < rewardPercentiles.size()
-          && 100.0 * gasUsed / block.getHeader().getGasUsed()
-              >= rewardPercentiles.get(rewardPercentileIndex)) {
-        rewards.add(transactionAndGasUsed.effectivePriorityFeePerGas);
-        rewardPercentileIndex++;
+    // Start with the gas used by the first transaction
+    double totalGasUsed = transactionsAndGasUsedAscendingEffectiveGasFee.get(0).gasUsed();
+    var transactionIndex = 0;
+    for (var rewardPercentile : rewardPercentiles) {
+      // Calculate the threshold gas used for the current reward percentile. This is the amount of
+      // gas that needs to be used to reach this percentile
+      var thresholdGasUsed = rewardPercentile * block.getHeader().getGasUsed() / 100;
+
+      // Stop when totalGasUsed reaches the threshold or there are no more transactions
+      while (totalGasUsed < thresholdGasUsed
+          && transactionIndex < transactionsAndGasUsedAscendingEffectiveGasFee.size() - 1) {
+        transactionIndex++;
+        totalGasUsed +=
+            transactionsAndGasUsedAscendingEffectiveGasFee.get(transactionIndex).gasUsed();
       }
+      // Add the effective priority fee per gas of the transaction that reached the percentile value
+      rewards.add(
+          transactionsAndGasUsedAscendingEffectiveGasFee.get(transactionIndex)
+              .effectivePriorityFeePerGas);
     }
     // Put the computed rewards in the cache
     cache.put(new RewardCacheKey(block.getHeader().getBlockHash(), rewardPercentiles), rewards);

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistoryTest.java

@@ -23,6 +23,7 @@ import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
@@ -34,15 +35,27 @@ 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.chain.Blockchain;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.Block;
+import org.hyperledger.besu.ethereum.core.BlockBody;
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.Transaction;
+import org.hyperledger.besu.ethereum.core.TransactionReceipt;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
 import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
+import java.util.stream.Stream;
 
+import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.bytes.Bytes32;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -104,6 +117,68 @@ public class EthFeeHistoryTest {
                     .build()));
   }
 
+  @Test
+  public void shouldComputeRewardsCorrectly() {
+    // Define the percentiles of rewards we want to compute
+    List<Double> rewardPercentiles =
+        Arrays.asList(0.0, 5.0, 10.0, 30.0, 31.0, 59.0, 60.0, 61.0, 100.0);
+
+    // Define the expected rewards for each percentile
+    // The expected rewards match the fees of the transactions at each percentile in the
+    // rewardPercentiles list
+    List<Wei> expectedRewards = Stream.of(1, 1, 2, 5, 5, 6, 6, 7, 7).map(Wei::of).toList();
+
+    // Define a list of gas used and fee pairs. Each pair represents a transaction in the block.
+    // The first number is the gas used by the transaction, and the second number the fee.
+    // The comments indicate the cumulative gas used up as a percentage of the total gas limit.
+    List<Object[]> gasUsedAndFee = new ArrayList<>();
+    gasUsedAndFee.add(new Object[] {100, 1L}); // 5%
+    gasUsedAndFee.add(new Object[] {150, 2L}); // 12.5%
+    gasUsedAndFee.add(new Object[] {200, 3L}); // 22.5%
+    gasUsedAndFee.add(new Object[] {100, 4L}); // 27.5%
+    gasUsedAndFee.add(new Object[] {200, 5L}); // 37.5%
+    gasUsedAndFee.add(new Object[] {450, 6L}); // 60.0%
+    gasUsedAndFee.add(new Object[] {800, 7L}); // 100.0%
+    Collections.shuffle(gasUsedAndFee);
+
+    Block block = mock(Block.class);
+    Blockchain blockchain = mockBlockchainTransactionsWithPriorityFee(gasUsedAndFee, block);
+    EthFeeHistory ethFeeHistory = new EthFeeHistory(null, blockchain);
+
+    List<Wei> rewards = ethFeeHistory.computeRewards(rewardPercentiles, block);
+
+    // Check that the number of computed rewards is equal to the number of requested percentiles
+    assertThat(rewards.size()).isEqualTo(rewardPercentiles.size());
+    assertThat(rewards).isEqualTo(expectedRewards);
+  }
+
+  private Blockchain mockBlockchainTransactionsWithPriorityFee(
+      final List<Object[]> gasUsedAndFee, final Block block) {
+    final Blockchain blockchain = mock(Blockchain.class);
+
+    when(block.getHash()).thenReturn(Hash.wrap(Bytes32.wrap(Bytes.random(32))));
+    BlockBody body = mock(BlockBody.class);
+    BlockHeader blockHeader = mock(BlockHeader.class);
+    when(block.getHeader()).thenReturn(blockHeader);
+    when(block.getBody()).thenReturn(body);
+    long cumulativeGasUsed = 0;
+    List<Transaction> transactions = new ArrayList<>();
+    List<TransactionReceipt> receipts = new ArrayList<>();
+    for (Object[] objects : gasUsedAndFee) {
+      Transaction transaction = mock(Transaction.class);
+      when(transaction.getEffectivePriorityFeePerGas(any())).thenReturn(Wei.of((Long) objects[1]));
+      cumulativeGasUsed += (int) objects[0];
+      transactions.add(transaction);
+      TransactionReceipt receipt = mock(TransactionReceipt.class);
+      when(receipt.getCumulativeGasUsed()).thenReturn(cumulativeGasUsed);
+      receipts.add(receipt);
+    }
+    when(blockHeader.getGasUsed()).thenReturn(cumulativeGasUsed);
+    when(blockchain.getTxReceipts(any())).thenReturn(Optional.of(receipts));
+    when(body.getTransactions()).thenReturn(transactions);
+    return blockchain;
+  }
+
   @Test
   public void cantGetBlockHigherThanChainHead() {
     assertThat(