Use synchronized call to access the chain head block in `eth_estimate…

…Gas` (hyperledger#6345) * Use synchronized call to access the chain head block in estimateGas() Signed-off-by: Matthew Whitehead <[email protected]> * Add error log entries when throwing internal error from estimateGas() Signed-off-by: Matthew Whitehead <[email protected]> * Update unit tests Signed-off-by: Matthew Whitehead <[email protected]> * Update changelog Signed-off-by: Matthew Whitehead <[email protected]> --------- Signed-off-by: Matthew Whitehead <[email protected]>
daniellehrner · Jan 8, 2024 · 428177f · 428177f
1 parent 15bdd95
commit 428177f
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@
 - Optimize RocksDB WAL files, allows for faster restart and a more linear disk space utilization [#6328](https://github.com/hyperledger/besu/pull/6328)
 
 ### Bug fixes
+- INTERNAL_ERROR from `eth_estimateGas` JSON/RPC calls [#6344](https://github.com/hyperledger/besu/issues/6344)
 
 ## 23.10.3
 

diff --git a/.../java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AbstractEstimateGas.java b/.../java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AbstractEstimateGas.java
@@ -23,6 +23,7 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
+import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.mainnet.ValidationResult;
 import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
@@ -48,8 +49,16 @@ public AbstractEstimateGas(
   }
 
   protected BlockHeader blockHeader() {
-    final long headBlockNumber = blockchainQueries.headBlockNumber();
-    return blockchainQueries.getBlockchain().getBlockHeader(headBlockNumber).orElse(null);
+    final Blockchain theChain = blockchainQueries.getBlockchain();
+
+    // Optimistically get the block header for the chain head without taking a lock,
+    // but revert to the safe implementation if it returns an empty optional. (It's
+    // possible the chain head has been updated but the block is still being persisted
+    // to storage/cache under the lock).
+    return theChain
+        .getBlockHeader(theChain.getChainHeadHash())
+        .or(() -> theChain.getBlockHeaderSafe(theChain.getChainHeadHash()))
+        .orElse(null);
   }
 
   protected CallParameter overrideGasLimitAndPrice(

diff --git a/.../main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGas.java b/.../main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGas.java
@@ -32,8 +32,13 @@
 
 import java.util.Optional;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class EthEstimateGas extends AbstractEstimateGas {
 
+  private static final Logger LOG = LoggerFactory.getLogger(EthEstimateGas.class);
+
   public EthEstimateGas(
       final BlockchainQueries blockchainQueries, final TransactionSimulator transactionSimulator) {
     super(blockchainQueries, transactionSimulator);
@@ -50,6 +55,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
 
     final BlockHeader blockHeader = blockHeader();
     if (blockHeader == null) {
+      LOG.error("Chain head block not found");
       return errorResponse(requestContext, RpcErrorType.INTERNAL_ERROR);
     }
     if (!blockchainQueries
@@ -70,6 +76,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
             blockHeader, modifiedCallParams, operationTracer, isAllowExceedingBalance);
 
     if (gasUsed.isEmpty()) {
+      LOG.error("gasUsed is empty after simulating transaction.");
       return errorResponse(requestContext, RpcErrorType.INTERNAL_ERROR);
     }
 

diff --git a/...a/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessListTest.java b/...a/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessListTest.java
@@ -17,14 +17,14 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.datatypes.AccessListEntry;
 import org.hyperledger.besu.datatypes.Address;
+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;
@@ -78,10 +78,16 @@ public class EthCreateAccessListTest {
 
   @BeforeEach
   public void setUp() {
-    when(blockchainQueries.headBlockNumber()).thenReturn(1L);
     when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
     when(blockchainQueries.getWorldStateArchive()).thenReturn(worldStateArchive);
-    when(blockchain.getBlockHeader(eq(1L))).thenReturn(Optional.of(blockHeader));
+    when(blockchain.getChainHeadHash())
+        .thenReturn(
+            Hash.fromHexString(
+                "0x3f07a9c83155594c000642e7d60e8a8a00038d03e9849171a05ed0e2d47acbb3"));
+    when(blockchain.getBlockHeader(
+            Hash.fromHexString(
+                "0x3f07a9c83155594c000642e7d60e8a8a00038d03e9849171a05ed0e2d47acbb3")))
+        .thenReturn(Optional.of(blockHeader));
     when(blockHeader.getGasLimit()).thenReturn(Long.MAX_VALUE);
     when(blockHeader.getNumber()).thenReturn(1L);
     when(worldStateArchive.isWorldStateAvailable(any(), any())).thenReturn(true);

diff --git a/...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java b/...t/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthEstimateGasTest.java
@@ -22,6 +22,7 @@
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.datatypes.Address;
+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;
@@ -73,10 +74,16 @@ public class EthEstimateGasTest {
 
   @BeforeEach
   public void setUp() {
-    when(blockchainQueries.headBlockNumber()).thenReturn(1L);
     when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
     when(blockchainQueries.getWorldStateArchive()).thenReturn(worldStateArchive);
-    when(blockchain.getBlockHeader(eq(1L))).thenReturn(Optional.of(blockHeader));
+    when(blockchain.getChainHeadHash())
+        .thenReturn(
+            Hash.fromHexString(
+                "0x3f07a9c83155594c000642e7d60e8a8a00038d03e9849171a05ed0e2d47acbb3"));
+    when(blockchain.getBlockHeader(
+            Hash.fromHexString(
+                "0x3f07a9c83155594c000642e7d60e8a8a00038d03e9849171a05ed0e2d47acbb3")))
+        .thenReturn(Optional.of(blockHeader));
     when(blockHeader.getGasLimit()).thenReturn(Long.MAX_VALUE);
     when(blockHeader.getNumber()).thenReturn(1L);
     when(worldStateArchive.isWorldStateAvailable(any(), any())).thenReturn(true);