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 <[email protected]> Signed-off-by: gconnect <[email protected]>
gconnect · Aug 26, 2024 · badfd2d · badfd2d
1 parent 79f6eda
commit badfd2d
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 30 deletions.
diff --git a/CHANGELOG.md b/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
 

diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/query/BlockchainQueries.java b/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(

diff --git a/...test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGasPriceTest.java b/...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());
   }
 

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/Blockchain.java b/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.

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java b/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

diff --git a/ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/t8n/T8nBlockchain.java b/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.

diff --git a/...s/src/main/java/org/hyperledger/besu/ethereum/referencetests/ReferenceTestBlockchain.java b/...s/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.