Fix BlockchainQueries::gasPrice when chain head block body still not …

…fully persisted (hyperledger#7482)

* Fix BlockchainQueries::gasPrice when chain head block body still not fully persisted

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

CHANGELOG.md

@@ -13,6 +13,7 @@
 ### Bug fixes
 - Fix tracing in precompiled contracts when halting for out of gas [#7318](https://github.com/hyperledger/besu/issues/7318)
 - Correctly release txpool save and restore lock in case of exceptions [#7473](https://github.com/hyperledger/besu/pull/7473)
+- Fix for `eth_gasPrice` could not retrieve block error [#7482](https://github.com/hyperledger/besu/pull/7482)
 
 ## 24.8.0
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java

@@ -60,6 +60,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.LongStream;
+import java.util.stream.Stream;
 
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.units.bigints.UInt256;
@@ -976,27 +977,35 @@ public <U> Optional<U> getAndMapWorldState(
  }
 
  public Wei gasPrice() {
- final long blockHeight = headBlockNumber();
- final var chainHeadHeader = blockchain.getChainHeadHeader();
+ final Block chainHeadBlock = blockchain.getChainHeadBlock();
+ final var chainHeadHeader = chainHeadBlock.getHeader();
+ final long blockHeight = chainHeadHeader.getNumber();
+
  final var nextBlockProtocolSpec =
  protocolSchedule.getForNextBlockHeader(chainHeadHeader, System.currentTimeMillis());
  final var nextBlockFeeMarket = nextBlockProtocolSpec.getFeeMarket();
+
  final Wei[] gasCollection =
- LongStream.rangeClosed(
- Math.max(0, blockHeight - apiConfig.getGasPriceBlocks() + 1), blockHeight)
- .mapToObj(
- l ->
- blockchain
- .getBlockByNumber(l)
- .map(Block::getBody)
- .map(BlockBody::getTransactions)
- .orElseThrow(
- () -> new IllegalStateException("Could not retrieve block #" + l)))
+ 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.getGasPrice().isPresent())
  .map(t -> t.getGasPrice().get())
  .sorted()
  .toArray(Wei[]::new);
+
  return (gasCollection == null || gasCollection.length == 0)
  ? gasPriceLowerBound(chainHeadHeader, nextBlockFeeMarket)
  : UInt256s.max(

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

@@ -17,7 +17,9 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
@@ -61,7 +63,6 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.mockito.Mock;
-import org.mockito.internal.verification.VerificationModeFactory;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
@@ -106,8 +107,8 @@ public void shouldReturnMinValueWhenNoTransactionsExist() {
  final JsonRpcResponse actualResponse = method.response(request);
  assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
 
- verify(blockchain).getChainHeadBlockNumber();
- verify(blockchain, VerificationModeFactory.times(100)).getBlockByNumber(anyLong());
+ verify(blockchain).getChainHeadBlock();
+ verify(blockchain, times(99)).getBlockByNumber(anyLong());
  verifyNoMoreInteractions(blockchain);
  }
 
@@ -127,8 +128,7 @@ public void shouldReturnBaseFeeAsMinValueOnGenesis() {
  final JsonRpcResponse actualResponse = method.response(request);
  assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
 
- verify(blockchain).getChainHeadBlockNumber();
- verify(blockchain, VerificationModeFactory.times(1)).getBlockByNumber(anyLong());
+ verify(blockchain).getChainHeadBlock();
  verifyNoMoreInteractions(blockchain);
  }
 
@@ -146,8 +146,8 @@ public void shouldReturnMedianWhenTransactionsExist() {
  final JsonRpcResponse actualResponse = method.response(request);
  assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
 
- verify(blockchain).getChainHeadBlockNumber();
- verify(blockchain, VerificationModeFactory.times(100)).getBlockByNumber(anyLong());
+ verify(blockchain).getChainHeadBlock();
+ verify(blockchain, times(99)).getBlockByNumber(anyLong());
  verifyNoMoreInteractions(blockchain);
  }
 
@@ -165,8 +165,8 @@ public void shortChainQueriesAllBlocks() {
  final JsonRpcResponse actualResponse = method.response(request);
  assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
 
- verify(blockchain).getChainHeadBlockNumber();
- verify(blockchain, VerificationModeFactory.times(81)).getBlockByNumber(anyLong());
+ verify(blockchain).getChainHeadBlock();
+ verify(blockchain, times(80)).getBlockByNumber(anyLong());
  verifyNoMoreInteractions(blockchain);
  }
 
@@ -252,8 +252,7 @@ public void ethGasPriceAtGenesis(
  final JsonRpcResponse actualResponse = method.response(request);
  assertThat(actualResponse).usingRecursiveComparison().isEqualTo(expectedResponse);
 
- verify(blockchain).getChainHeadBlockNumber();
- verify(blockchain, VerificationModeFactory.times(1)).getBlockByNumber(anyLong());
+ verify(blockchain).getChainHeadBlock();
  verifyNoMoreInteractions(blockchain);
  }
 
@@ -328,12 +327,14 @@ private void mockBlockchain(
  blocksByNumber.put(i, createFakeBlock(i, txsNum, baseFee));
  }
 
- when(blockchain.getChainHeadBlockNumber()).thenReturn(chainHeadBlockNumber);
- when(blockchain.getBlockByNumber(anyLong()))
- .thenAnswer(
- invocation -> Optional.of(blocksByNumber.get(invocation.getArgument(0, Long.class))));
-
- when(blockchain.getChainHeadHeader())
+ when(blockchain.getChainHeadBlock()).thenReturn(blocksByNumber.get(chainHeadBlockNumber));
+ if (chainHeadBlockNumber > 1) {
+ when(blockchain.getBlockByNumber(anyLong()))
+ .thenAnswer(
+ invocation -> Optional.of(blocksByNumber.get(invocation.getArgument(0, Long.class))));
+ }
+ lenient()
+ .when(blockchain.getChainHeadHeader())
  .thenReturn(blocksByNumber.get(chainHeadBlockNumber).getHeader());
  }
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java

@@ -167,6 +167,15 @@ default boolean contains(final Hash blockHash) {
  */
  Optional<BlockBody> getBlockBody(Hash blockHeaderHash);
 
+ /**
+ * Safe version of {@code getBlockBody} (it should take any locks necessary to ensure any block
+ * updates that might be taking place have been completed first)
+ *
+ * @param blockHeaderHash The hash of the block whose header we want to retrieve.
+ * @return The block body corresponding to this block hash.
+ */
+ Optional<BlockBody> getBlockBodySafe(Hash blockHeaderHash);
+
  /**
  * Given a block's hash, returns the list of transaction receipts associated with this block's
  * transactions. Associated block is not necessarily on the canonical chain.

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java

@@ -299,7 +299,10 @@ public BlockHeader getChainHeadHeader() {
 
  @Override
  public Block getChainHeadBlock() {
- return new Block(chainHeader, blockchainStorage.getBlockBody(chainHeader.getHash()).get());
+ return new Block(
+ chainHeader,
+ getBlockBody(chainHeader.getHash())
+ .orElseGet(() -> getBlockBodySafe(chainHeader.getHash()).get()));
  }
 
  @Override
@@ -337,6 +340,11 @@ public Optional<BlockBody> getBlockBody(final Hash blockHeaderHash) {
  .orElseGet(() -> blockchainStorage.getBlockBody(blockHeaderHash));
  }
 
+ @Override
+ public synchronized Optional<BlockBody> getBlockBodySafe(final Hash blockHeaderHash) {
+ return getBlockBody(blockHeaderHash);
+ }
+
  @Override
  public Optional<List<TransactionReceipt>> getTxReceipts(final Hash blockHeaderHash) {
  return transactionReceiptsCache

ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/t8n/T8nBlockchain.java

@@ -145,6 +145,11 @@ public Optional<BlockBody> getBlockBody(final Hash blockHeaderHash) {
  throw new UnsupportedOperationException();
  }
 
+ @Override
+ public synchronized Optional<BlockBody> getBlockBodySafe(final Hash blockHeaderHash) {
+ return getBlockBody(blockHeaderHash);
+ }
+
  @Override
  public Optional<List<TransactionReceipt>> getTxReceipts(final Hash blockHeaderHash) {
  // Deterministic, but just not implemented.

ethereum/referencetests/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java

@@ -142,6 +142,11 @@ public Optional<BlockBody> getBlockBody(final Hash blockHeaderHash) {
  throw new UnsupportedOperationException();
  }
 
+ @Override
+ public synchronized Optional<BlockBody> getBlockBodySafe(final Hash blockHeaderHash) {
+ return getBlockBody(blockHeaderHash);
+ }
+
  @Override
  public Optional<List<TransactionReceipt>> getTxReceipts(final Hash blockHeaderHash) {
  // Deterministic, but just not implemented.