Honor block number or tag parameter in eth_estimateGas and eth_createAccessList

CHANGELOG.md

@@ -10,6 +10,7 @@
 - Include current chain head block when computing `eth_maxPriorityFeePerGas` [#7485](https://github.com/hyperledger/besu/pull/7485)
 
 ### Bug fixes
+- Honor block number or tag parameter in eth_estimateGas and eth_createAccessList [#7502](https://github.com/hyperledger/besu/pull/7502)
 
 ## 24.9.0
 

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/AbstractEstimateGas.java

@@ -14,15 +14,19 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;
 
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonCallParameterUtil.validateAndGetCallParams;
+
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcErrorConverter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.BlockParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonRpcParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
 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;
@@ -34,32 +38,68 @@
 
 import java.util.Optional;
 
-public abstract class AbstractEstimateGas implements JsonRpcMethod {
+public abstract class AbstractEstimateGas extends AbstractBlockParameterMethod {
 
   private static final double SUB_CALL_REMAINING_GAS_RATIO = 65D / 64D;
 
-  protected final BlockchainQueries blockchainQueries;
   protected final TransactionSimulator transactionSimulator;
 
   public AbstractEstimateGas(
       final BlockchainQueries blockchainQueries, final TransactionSimulator transactionSimulator) {
-    this.blockchainQueries = blockchainQueries;
+    super(blockchainQueries);
     this.transactionSimulator = transactionSimulator;
   }
 
-  protected BlockHeader blockHeader() {
-    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);
+  @Override
+  protected BlockParameter blockParameter(final JsonRpcRequestContext request) {
+    try {
+      return request.getOptionalParameter(1, BlockParameter.class).orElse(BlockParameter.LATEST);
+    } catch (JsonRpcParameter.JsonRpcParameterException e) {
+      throw new InvalidJsonRpcParameters(
+          "Invalid block parameter (index 1)", RpcErrorType.INVALID_BLOCK_PARAMS, e);
+    }
+  }
+
+  protected Optional<BlockHeader> blockHeader(final long blockNumber) {
+    if (getBlockchainQueries().headBlockNumber() == blockNumber) {
+      // chain head header if cached, and we can return it form memory
+      return Optional.of(getBlockchainQueries().getBlockchain().getChainHeadHeader());
+    }
+    return getBlockchainQueries().getBlockHeaderByNumber(blockNumber);
+  }
+
+  protected Optional<RpcErrorType> validateBlockHeader(
+      final Optional<BlockHeader> maybeBlockHeader) {
+    if (maybeBlockHeader.isEmpty()) {
+      return Optional.of(RpcErrorType.BLOCK_NOT_FOUND);
+    }
+
+    final var blockHeader = maybeBlockHeader.get();
+    if (!getBlockchainQueries()
+        .getWorldStateArchive()
+        .isWorldStateAvailable(blockHeader.getStateRoot(), blockHeader.getHash())) {
+      return Optional.of(RpcErrorType.WORLD_STATE_UNAVAILABLE);
+    }
+    return Optional.empty();
   }
 
+  @Override
+  protected Object resultByBlockNumber(
+      final JsonRpcRequestContext requestContext, final long blockNumber) {
+    final JsonCallParameter jsonCallParameter = validateAndGetCallParams(requestContext);
+    final Optional<BlockHeader> maybeBlockHeader = blockHeader(blockNumber);
+    final Optional<RpcErrorType> jsonRpcError = validateBlockHeader(maybeBlockHeader);
+    if (jsonRpcError.isPresent()) {
+      return errorResponse(requestContext, jsonRpcError.get());
+    }
+    return resultByBlockHeader(requestContext, jsonCallParameter, maybeBlockHeader.get());
+  }
+
+  protected abstract Object resultByBlockHeader(
+      final JsonRpcRequestContext requestContext,
+      final JsonCallParameter jsonCallParameter,
+      final BlockHeader blockHeader);
+
   protected CallParameter overrideGasLimitAndPrice(
       final JsonCallParameter callParams, final long gasLimit) {
     return new CallParameter(

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthCreateAccessList.java

@@ -14,15 +14,11 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods;
 
-import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonCallParameterUtil.validateAndGetCallParams;
-
 import org.hyperledger.besu.datatypes.AccessListEntry;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.JsonCallParameter;
-import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
-import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.CreateAccessListResult;
 import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
@@ -52,44 +48,29 @@ public String getName() {
   }
 
   @Override
-  public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
-    final JsonCallParameter jsonCallParameter = validateAndGetCallParams(requestContext);
-    final BlockHeader blockHeader = blockHeader();
-    final Optional<RpcErrorType> jsonRpcError = validateBlockHeader(blockHeader);
-    if (jsonRpcError.isPresent()) {
-      return errorResponse(requestContext, jsonRpcError.get());
-    }
+  protected Object resultByBlockHeader(
+      final JsonRpcRequestContext requestContext,
+      final JsonCallParameter jsonCallParameter,
+      final BlockHeader blockHeader) {
     final AccessListSimulatorResult maybeResult =
         processTransaction(jsonCallParameter, blockHeader);
     // if the call accessList is different from the simulation result, calculate gas and return
-    if (shouldProcessWithAccessListOverride(jsonCallParameter, maybeResult.getTracer())) {
+    if (shouldProcessWithAccessListOverride(jsonCallParameter, maybeResult.tracer())) {
       final AccessListSimulatorResult result =
           processTransactionWithAccessListOverride(
-              jsonCallParameter, blockHeader, maybeResult.getTracer().getAccessList());
+              jsonCallParameter, blockHeader, maybeResult.tracer().getAccessList());
       return createResponse(requestContext, result);
     } else {
       return createResponse(requestContext, maybeResult);
     }
   }
 
-  private Optional<RpcErrorType> validateBlockHeader(final BlockHeader blockHeader) {
-    if (blockHeader == null) {
-      return Optional.of(RpcErrorType.INTERNAL_ERROR);
-    }
-    if (!blockchainQueries
-        .getWorldStateArchive()
-        .isWorldStateAvailable(blockHeader.getStateRoot(), blockHeader.getHash())) {
-      return Optional.of(RpcErrorType.WORLD_STATE_UNAVAILABLE);
-    }
-    return Optional.empty();
-  }
-
-  private JsonRpcResponse createResponse(
+  private Object createResponse(
       final JsonRpcRequestContext requestContext, final AccessListSimulatorResult result) {
     return result
-        .getResult()
-        .map(createResponse(requestContext, result.getTracer()))
-        .orElse(errorResponse(requestContext, RpcErrorType.INTERNAL_ERROR));
+        .result()
+        .map(createResponse(requestContext, result.tracer()))
+        .orElseGet(() -> errorResponse(requestContext, RpcErrorType.INTERNAL_ERROR));
   }
 
   private TransactionValidationParams transactionValidationParams(
@@ -117,14 +98,12 @@ private boolean shouldProcessWithAccessListOverride(
     return !Objects.equals(tracer.getAccessList(), parameters.getAccessList().get());
   }
 
-  private Function<TransactionSimulatorResult, JsonRpcResponse> createResponse(
+  private Function<TransactionSimulatorResult, Object> createResponse(
       final JsonRpcRequestContext request, final AccessListOperationTracer operationTracer) {
     return result ->
         result.isSuccessful()
-            ? new JsonRpcSuccessResponse(
-                request.getRequest().getId(),
-                new CreateAccessListResult(
-                    operationTracer.getAccessList(), processEstimateGas(result, operationTracer)))
+            ? new CreateAccessListResult(
+                operationTracer.getAccessList(), processEstimateGas(result, operationTracer))
             : errorResponse(request, result);
   }
 
@@ -138,8 +117,7 @@ private AccessListSimulatorResult processTransaction(
 
     final AccessListOperationTracer tracer = AccessListOperationTracer.create();
     final Optional<TransactionSimulatorResult> result =
-        transactionSimulator.process(
-            callParams, transactionValidationParams, tracer, blockHeader.getNumber());
+        transactionSimulator.process(callParams, transactionValidationParams, tracer, blockHeader);
     return new AccessListSimulatorResult(result, tracer);
   }
 
@@ -156,7 +134,7 @@ private AccessListSimulatorResult processTransactionWithAccessListOverride(
 
     final Optional<TransactionSimulatorResult> result =
         transactionSimulator.process(
-            callParameter, transactionValidationParams, tracer, blockHeader.getNumber());
+            callParameter, transactionValidationParams, tracer, blockHeader);
     return new AccessListSimulatorResult(result, tracer);
   }
 
@@ -176,22 +154,6 @@ protected CallParameter overrideAccessList(
         Optional.ofNullable(accessListEntries));
   }
 
-  private static class AccessListSimulatorResult {
-    final Optional<TransactionSimulatorResult> result;
-    final AccessListOperationTracer tracer;
-
-    public AccessListSimulatorResult(
-        final Optional<TransactionSimulatorResult> result, final AccessListOperationTracer tracer) {
-      this.result = result;
-      this.tracer = tracer;
-    }
-
-    public Optional<TransactionSimulatorResult> getResult() {
-      return result;
-    }
-
-    public AccessListOperationTracer getTracer() {
-      return tracer;
-    }
-  }
+  private record AccessListSimulatorResult(
+      Optional<TransactionSimulatorResult> result, AccessListOperationTracer tracer) {}
 }