Skip to content

Commit

Permalink
Include current chain head block when computing eth_maxPriorityFeePer…
Browse files Browse the repository at this point in the history 
…Gas (hyperledger#7485)

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
  • Loading branch information
@fab-10
fab-10 authored Sep 2, 2024
1 parent ac5e9af commit e1dd400
Show file tree
Hide file tree
Showing 9 changed files with 207 additions and 92 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
### Breaking Changes

### Additions and Improvements
- Include current chain head block when computing `eth_maxPriorityFeePerGas` [#7485](https://github.com/hyperledger/besu/pull/7485)

### Bug fixes

Expand Down
4 changes: 2 additions & 2 deletions ...reum/api/src/main/java/org/hyperledger/besu/ethereum/api/graphql/GraphQLDataFetchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,15 @@ public DataFetcher<Optional<BigInteger>> getChainIdDataFetcher() {
*
* <p>The DataFetcher is a functional interface. It has a single method that takes a
* DataFetchingEnvironment object as input and returns the maximum priority fee per gas as a Wei
* object. If the maximum priority fee per gas is not available, it returns Wei.ZERO.
* object.
*
* @return a DataFetcher that fetches the maximum priority fee per gas of the Ethereum node
*/
public DataFetcher<Wei> getMaxPriorityFeePerGasDataFetcher() {
return dataFetchingEnvironment -> {
final BlockchainQueries blockchainQuery =
dataFetchingEnvironment.getGraphQlContext().get(GraphQLContextType.BLOCKCHAIN_QUERIES);
return blockchainQuery.gasPriorityFee().orElse(Wei.ZERO);
return blockchainQuery.gasPriorityFee();
};
}

Expand Down
11 changes: 2 additions & 9 deletions ...a/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthMaxPriorityFeePerGas.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,13 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity;
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.blockcreation.MiningCoordinator;

import java.util.Optional;

public class EthMaxPriorityFeePerGas implements JsonRpcMethod {

private final BlockchainQueries blockchainQueries;
private final MiningCoordinator miningCoordinator;

public EthMaxPriorityFeePerGas(
final BlockchainQueries blockchainQueries, final MiningCoordinator miningCoordinator) {
public EthMaxPriorityFeePerGas(final BlockchainQueries blockchainQueries) {
this.blockchainQueries = blockchainQueries;
this.miningCoordinator = miningCoordinator;
}

@Override
Expand All @@ -48,7 +42,6 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
}

private Wei fetchAndLimitPriorityFeePerGas() {
final Optional<Wei> gasPrice = blockchainQueries.gasPriorityFee();
return gasPrice.orElseGet(miningCoordinator::getMinPriorityFeePerGas);
return blockchainQueries.gasPriorityFee();
}
}
2 changes: 1 addition & 1 deletion ...pi/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthJsonRpcMethods.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,6 @@ protected Map<String, JsonRpcMethod> create() {
new EthGetMinerDataByBlockHash(blockchainQueries, protocolSchedule),
new EthGetMinerDataByBlockNumber(blockchainQueries, protocolSchedule),
new EthBlobBaseFee(blockchainQueries.getBlockchain(), protocolSchedule),
new EthMaxPriorityFeePerGas(blockchainQueries, miningCoordinator));
new EthMaxPriorityFeePerGas(blockchainQueries));
}
}
57 changes: 32 additions & 25 deletions ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import java.io.EOFException;
import java.io.IOException;
import java.io.RandomAccessFile;
import java.math.BigInteger;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -1006,7 +1005,7 @@ public Wei gasPrice() {
.sorted()
.toArray(Wei[]::new);

return (gasCollection == null || gasCollection.length == 0)
return gasCollection.length == 0
? gasPriceLowerBound(chainHeadHeader, nextBlockFeeMarket)
: UInt256s.max(
gasPriceLowerBound(chainHeadHeader, nextBlockFeeMarket),
Expand Down Expand Up @@ -1045,31 +1044,39 @@ private Wei gasPriceLowerBound(
return minGasPrice;
}

public Optional<Wei> gasPriorityFee() {
final long blockHeight = headBlockNumber();
final BigInteger[] gasCollection =
LongStream.range(Math.max(0, blockHeight - apiConfig.getGasPriceBlocks()), blockHeight)
.mapToObj(
l ->
blockchain
.getBlockByNumber(l)
.map(Block::getBody)
.map(BlockBody::getTransactions)
.orElseThrow(
() -> new IllegalStateException("Could not retrieve block #" + l)))
public Wei gasPriorityFee() {
final Block chainHeadBlock = blockchain.getChainHeadBlock();
final long blockHeight = chainHeadBlock.getHeader().getNumber();

final Wei[] gasCollection =
Stream.concat(
LongStream.range(
Math.max(0, blockHeight - apiConfig.getGasPriceBlocks() + 1), blockHeight)
.mapToObj(
l ->
blockchain
.getBlockByNumber(l)
.orElseThrow(
() ->
new IllegalStateException(
"Could not retrieve block #" + l))),
Stream.of(chainHeadBlock))
.map(Block::getBody)
.map(BlockBody::getTransactions)
.flatMap(Collection::stream)
.filter(t -> t.getMaxPriorityFeePerGas().isPresent())
.map(t -> t.getMaxPriorityFeePerGas().get().toBigInteger())
.sorted(BigInteger::compareTo)
.toArray(BigInteger[]::new);
return (gasCollection.length == 0)
? Optional.empty()
: Optional.of(
Wei.of(
gasCollection[
Math.min(
gasCollection.length - 1,
(int) ((gasCollection.length) * apiConfig.getGasPriceFraction()))]));
.map(t -> t.getMaxPriorityFeePerGas().get())
.sorted()
.toArray(Wei[]::new);

return gasCollection.length == 0
? miningParameters.getMinPriorityFeePerGas()
: UInt256s.max(
miningParameters.getMinPriorityFeePerGas(),
gasCollection[
Math.min(
gasCollection.length - 1,
(int) ((gasCollection.length) * apiConfig.getGasPriceFraction()))]);
}

/**
Expand Down
16 changes: 10 additions & 6 deletions ...i/src/test/java/org/hyperledger/besu/ethereum/api/graphql/GraphQLHttpServiceCorsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
*/
package org.hyperledger.besu.ethereum.api.graphql;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.blockcreation.PoWMiningCoordinator;
import org.hyperledger.besu.ethereum.core.Synchronizer;
Expand All @@ -38,7 +42,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mockito;

public class GraphQLHttpServiceCorsTest {
@TempDir private Path folder;
Expand Down Expand Up @@ -208,18 +211,19 @@ private GraphQLHttpService createGraphQLHttpServiceWithAllowedDomains(
config.setCorsAllowedDomains(Lists.newArrayList(corsAllowedDomains));
}

final BlockchainQueries blockchainQueries = Mockito.mock(BlockchainQueries.class);
final Synchronizer synchronizer = Mockito.mock(Synchronizer.class);
final BlockchainQueries blockchainQueries = mock(BlockchainQueries.class);
when(blockchainQueries.gasPriorityFee()).thenReturn(Wei.ONE);
final Synchronizer synchronizer = mock(Synchronizer.class);

final PoWMiningCoordinator miningCoordinatorMock = Mockito.mock(PoWMiningCoordinator.class);
final PoWMiningCoordinator miningCoordinatorMock = mock(PoWMiningCoordinator.class);

// mock graphql context
final Map<GraphQLContextType, Object> graphQLContextMap =
Map.of(
GraphQLContextType.BLOCKCHAIN_QUERIES,
blockchainQueries,
GraphQLContextType.TRANSACTION_POOL,
Mockito.mock(TransactionPool.class),
mock(TransactionPool.class),
GraphQLContextType.MINING_COORDINATOR,
miningCoordinatorMock,
GraphQLContextType.SYNCHRONIZER,
Expand All @@ -233,7 +237,7 @@ private GraphQLHttpService createGraphQLHttpServiceWithAllowedDomains(

final GraphQLHttpService graphQLHttpService =
new GraphQLHttpService(
vertx, folder, config, graphQL, graphQLContextMap, Mockito.mock(EthScheduler.class));
vertx, folder, config, graphQL, graphQLContextMap, mock(EthScheduler.class));
graphQLHttpService.start().join();

return graphQLHttpService;
Expand Down
16 changes: 10 additions & 6 deletions ...t/java/org/hyperledger/besu/ethereum/api/graphql/GraphQLHttpServiceHostWhitelistTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
*/
package org.hyperledger.besu.ethereum.api.graphql;

import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
import org.hyperledger.besu.ethereum.blockcreation.PoWMiningCoordinator;
import org.hyperledger.besu.ethereum.core.Synchronizer;
Expand Down Expand Up @@ -42,7 +46,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mockito;

public class GraphQLHttpServiceHostWhitelistTest {

Expand All @@ -69,17 +72,18 @@ public void initServerAndClient() throws Exception {
}

private GraphQLHttpService createGraphQLHttpService() throws Exception {
final BlockchainQueries blockchainQueries = Mockito.mock(BlockchainQueries.class);
final Synchronizer synchronizer = Mockito.mock(Synchronizer.class);
final BlockchainQueries blockchainQueries = mock(BlockchainQueries.class);
when(blockchainQueries.gasPriorityFee()).thenReturn(Wei.ONE);
final Synchronizer synchronizer = mock(Synchronizer.class);

final PoWMiningCoordinator miningCoordinatorMock = Mockito.mock(PoWMiningCoordinator.class);
final PoWMiningCoordinator miningCoordinatorMock = mock(PoWMiningCoordinator.class);

final Map<GraphQLContextType, Object> graphQLContextMap =
Map.of(
GraphQLContextType.BLOCKCHAIN_QUERIES,
blockchainQueries,
GraphQLContextType.TRANSACTION_POOL,
Mockito.mock(TransactionPool.class),
mock(TransactionPool.class),
GraphQLContextType.MINING_COORDINATOR,
miningCoordinatorMock,
GraphQLContextType.SYNCHRONIZER,
Expand All @@ -92,7 +96,7 @@ private GraphQLHttpService createGraphQLHttpService() throws Exception {
final GraphQL graphQL = GraphQLProvider.buildGraphQL(dataFetchers);

return new GraphQLHttpService(
vertx, folder, graphQLConfig, graphQL, graphQLContextMap, Mockito.mock(EthScheduler.class));
vertx, folder, graphQLConfig, graphQL, graphQLContextMap, mock(EthScheduler.class));
}

private static GraphQLConfiguration createGraphQLConfig() {
Expand Down
2 changes: 1 addition & 1 deletion ...test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGasPriceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ private void mockBlockchain(
}

when(blockchain.getChainHeadBlock()).thenReturn(blocksByNumber.get(chainHeadBlockNumber));
if (chainHeadBlockNumber > 1) {
if (chainHeadBlockNumber > 0) {
when(blockchain.getBlockByNumber(anyLong()))
.thenAnswer(
invocation -> Optional.of(blocksByNumber.get(invocation.getArgument(0, Long.class))));
Expand Down
Loading

0 comments on commit e1dd400

Please sign in to comment.