From d98bdd568f354213f667874825245ce46783b39c Mon Sep 17 00:00:00 2001 From: Gabriel-Trintinalia Date: Wed, 15 Nov 2023 13:11:31 +1100 Subject: [PATCH] Fix rewards calculation Signed-off-by: Gabriel-Trintinalia --- .../internal/methods/EthFeeHistory.java | 45 ++++++----- .../internal/methods/EthFeeHistoryTest.java | 76 +++++++++++++++++++ 2 files changed, 103 insertions(+), 18 deletions(-) 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 ffe3b96b05a..b9d0bc161c8 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 @@ -47,6 +47,7 @@ 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 JsonRpcResponse response(final JsonRpcRequestContext request) { .build())); } - private List computeRewards(final List rewardPercentiles, final Block block) { + @VisibleForTesting + public List computeRewards(final List rewardPercentiles, final Block block) { final List transactions = block.getBody().getTransactions(); if (transactions.isEmpty()) { // all 0's for empty block @@ -201,13 +203,11 @@ private List computeRewards(final List rewardPercentiles, final Blo // 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 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) {} @@ -230,19 +230,28 @@ record TransactionInfo(Transaction transaction, Long gasUsed, Wei effectivePrior // That's why we're keeping track of the cumulative gas used and checking to see which // percentile markers we've passed final ArrayList 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 + cumulativeGasUsed = transactionsAndGasUsedAscendingEffectiveGasFee.get(0).gasUsed(); + var transactionIndex = 0; + // Iterate over each reward percentile + 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; + + // Update cumulativeGasUsed by adding the gas used by each transaction + // Stop when cumulativeGasUsed reaches the threshold or there are no more transactions + while (cumulativeGasUsed < thresholdGasUsed + && transactionIndex < transactionsAndGasUsedAscendingEffectiveGasFee.size() - 1) { + transactionIndex++; + cumulativeGasUsed += + transactionsAndGasUsedAscendingEffectiveGasFee.get(transactionIndex).gasUsed(); } + // Add the effective priority fee per gas of the transaction that reached the percentile to + // the rewards list + rewards.add( + transactionsAndGasUsedAscendingEffectiveGasFee.get(transactionIndex) + .effectivePriorityFeePerGas); } // Put the computed rewards in the cache cache.put(new RewardCacheKey(block.getHeader().getBlockHash(), rewardPercentiles), rewards); 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 9005c36d16b..c225de2d1d2 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 @@ -23,6 +23,7 @@ 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.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,69 @@ public void allFieldsPresentForLatestBlock() { .build())); } + @Test + public void shouldComputeRewardsCorrectly() { + // Define the percentiles of rewards we want to compute + List 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 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 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 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 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 transactions = new ArrayList<>(); + List 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(