fix: use correct hardfork for calls in pre-fork context (#5107)

Author: agostbiro

.changeset/slow-baboons-press.md

@@ -0,0 +1,5 @@
+---
+@nomicfoundation/edr: path
+---
+
+fix: When in fork mode and executing a call in the block immediately preceding the fork, make sure that the hardfork for EVM execution is derived from the block number as opposed to the provider config.

crates/edr_evm/src/block/builder.rs

@@ -90,11 +90,13 @@ where
                 }
                 _ => Self::InvalidTransaction(e),
             },
-            EVMError::Header(
-                InvalidHeader::ExcessBlobGasNotSet | InvalidHeader::PrevrandaoNotSet,
-            ) => unreachable!("error: {error:?}"),
             EVMError::Database(DatabaseComponentError::State(e)) => Self::State(e),
             EVMError::Database(DatabaseComponentError::BlockHash(e)) => Self::BlockHash(e),
+            // This case is a bug in our codebase for local blockchains, but it can happen that the
+            // remote returns incorrect block data in which case we should return a custom error.
+            EVMError::Header(
+                error @ (InvalidHeader::ExcessBlobGasNotSet | InvalidHeader::PrevrandaoNotSet),
+            ) => Self::Custom(error.to_string()),
             EVMError::Custom(error) => Self::Custom(error),
         }
     }

crates/edr_provider/src/data.rs

@@ -584,16 +584,16 @@ impl<LoggerErrorT: Debug> ProviderData<LoggerErrorT> {
     pub fn debug_trace_call(
         &mut self,
         transaction: ExecutableTransaction,
-        block_spec: Option<&BlockSpec>,
+        block_spec: &BlockSpec,
         trace_config: DebugTraceConfig,
     ) -> Result<DebugTraceResult, ProviderError<LoggerErrorT>> {
-        let cfg_env = self.create_evm_config(block_spec)?;
+        let cfg_env = self.create_evm_config(Some(block_spec))?;
 
         let tx_env: TxEnv = transaction.into();
 
         let mut tracer = TracerEip3155::new(trace_config);
 
-        self.execute_in_block_context(block_spec, |blockchain, block, state| {
+        self.execute_in_block_context(Some(block_spec), |blockchain, block, state| {
             let result = run_call(RunCallArgs {
                 blockchain,
                 header: block.header(),
@@ -1346,15 +1346,15 @@ impl<LoggerErrorT: Debug> ProviderData<LoggerErrorT> {
     pub fn run_call(
         &mut self,
         transaction: ExecutableTransaction,
-        block_spec: Option<&BlockSpec>,
+        block_spec: &BlockSpec,
         state_overrides: &StateOverrides,
     ) -> Result<CallResult, ProviderError<LoggerErrorT>> {
-        let cfg_env = self.create_evm_config(block_spec)?;
+        let cfg_env = self.create_evm_config(Some(block_spec))?;
         let tx_env = transaction.into();
 
         let mut debugger = Debugger::with_mocker(Mocker::new(self.call_override.clone()));
 
-        self.execute_in_block_context(block_spec, |blockchain, block, state| {
+        self.execute_in_block_context(Some(block_spec), |blockchain, block, state| {
             let execution_result = call::run_call(RunCallArgs {
                 blockchain,
                 header: block.header(),
@@ -1819,6 +1819,9 @@ impl<LoggerErrorT: Debug> ProviderData<LoggerErrorT> {
         Ok(transaction_hash)
     }
 
+    /// Creates a configuration, taking into the hardfork at the provided
+    /// `BlockSpec`. If none is provided, assumes the hardfork for newly
+    /// mined blocks.
     fn create_evm_config(
         &self,
         block_spec: Option<&BlockSpec>,
@@ -2817,7 +2820,7 @@ mod tests {
 
         let result = fixture.provider_data.run_call(
             pending_transaction,
-            None,
+            &BlockSpec::latest(),
             &StateOverrides::default(),
         )?;
 
@@ -3549,10 +3552,9 @@ mod tests {
         ) -> Result<CallResult, ProviderError<Infallible>> {
             let state_overrides = StateOverrides::default();
 
-            let transaction =
-                resolve_call_request(data, request, Some(&block_spec), &state_overrides)?;
+            let transaction = resolve_call_request(data, request, &block_spec, &state_overrides)?;
 
-            data.run_call(transaction, Some(&block_spec), &state_overrides)
+            data.run_call(transaction, &block_spec, &state_overrides)
         }
 
         const EIP_1559_ACTIVATION_BLOCK: u64 = 12_965_000;

crates/edr_provider/src/requests/debug.rs

@@ -9,7 +9,10 @@ use serde::{Deserialize, Deserializer};
 
 use crate::{
     data::ProviderData,
-    requests::{eth::resolve_call_request, validation::validate_call_request},
+    requests::{
+        eth::{resolve_block_spec_for_call_request, resolve_call_request},
+        validation::validate_call_request,
+    },
     ProviderError,
 };
 
@@ -36,17 +39,14 @@ pub fn handle_debug_trace_call<LoggerErrorT: Debug>(
     block_spec: Option<BlockSpec>,
     config: Option<DebugTraceConfig>,
 ) -> Result<DebugTraceResult, ProviderError<LoggerErrorT>> {
+    let block_spec = resolve_block_spec_for_call_request(block_spec);
     validate_call_request(data.spec_id(), &call_request, &block_spec)?;
 
-    let transaction = resolve_call_request(
-        data,
-        call_request,
-        block_spec.as_ref(),
-        &StateOverrides::default(),
-    )?;
+    let transaction =
+        resolve_call_request(data, call_request, &block_spec, &StateOverrides::default())?;
     data.debug_trace_call(
         transaction,
-        block_spec.as_ref(),
+        &block_spec,
         config.map(Into::into).unwrap_or_default(),
     )
 }

crates/edr_provider/src/requests/eth/call.rs

@@ -21,13 +21,14 @@ pub fn handle_call_request<LoggerErrorT: Debug>(
     block_spec: Option<BlockSpec>,
     state_overrides: Option<StateOverrideOptions>,
 ) -> Result<(Bytes, Trace), ProviderError<LoggerErrorT>> {
+    let block_spec = resolve_block_spec_for_call_request(block_spec);
     validate_call_request(data.spec_id(), &request, &block_spec)?;
 
     let state_overrides =
         state_overrides.map_or(Ok(StateOverrides::default()), StateOverrides::try_from)?;
 
-    let transaction = resolve_call_request(data, request, block_spec.as_ref(), &state_overrides)?;
-    let result = data.run_call(transaction.clone(), block_spec.as_ref(), &state_overrides)?;
+    let transaction = resolve_call_request(data, request, &block_spec, &state_overrides)?;
+    let result = data.run_call(transaction.clone(), &block_spec, &state_overrides)?;
 
     let spec_id = data.spec_id();
     data.logger_mut()
@@ -51,10 +52,14 @@ pub fn handle_call_request<LoggerErrorT: Debug>(
     Ok((output, result.trace))
 }
 
+pub(crate) fn resolve_block_spec_for_call_request(block_spec: Option<BlockSpec>) -> BlockSpec {
+    block_spec.unwrap_or_else(BlockSpec::latest)
+}
+
 pub(crate) fn resolve_call_request<LoggerErrorT: Debug>(
     data: &mut ProviderData<LoggerErrorT>,
     request: CallRequest,
-    block_spec: Option<&BlockSpec>,
+    block_spec: &BlockSpec,
     state_overrides: &StateOverrides,
 ) -> Result<ExecutableTransaction, ProviderError<LoggerErrorT>> {
     resolve_call_request_inner(
@@ -78,7 +83,7 @@ pub(crate) fn resolve_call_request<LoggerErrorT: Debug>(
 pub(crate) fn resolve_call_request_inner<LoggerErrorT: Debug>(
     data: &mut ProviderData<LoggerErrorT>,
     request: CallRequest,
-    block_spec: Option<&BlockSpec>,
+    block_spec: &BlockSpec,
     state_overrides: &StateOverrides,
     default_gas_price_fn: impl FnOnce(
         &ProviderData<LoggerErrorT>,
@@ -108,7 +113,7 @@ pub(crate) fn resolve_call_request_inner<LoggerErrorT: Debug>(
     let from = from.unwrap_or_else(|| data.default_caller());
     let gas_limit = gas.unwrap_or_else(|| data.block_gas_limit());
     let input = input.map_or(Bytes::new(), Bytes::from);
-    let nonce = data.nonce(&from, block_spec, state_overrides)?;
+    let nonce = data.nonce(&from, Some(block_spec), state_overrides)?;
     let value = value.unwrap_or(U256::ZERO);
 
     let transaction = if data.spec_id() < SpecId::LONDON || gas_price.is_some() {
@@ -179,7 +184,7 @@ mod tests {
         let resolved = resolve_call_request_inner(
             &mut fixture.provider_data,
             request,
-            Some(&BlockSpec::pending()),
+            &BlockSpec::pending(),
             &StateOverrides::default(),
             |_data| unreachable!("gas_price is set"),
             |_, _, _| unreachable!("gas_price is set"),
@@ -208,7 +213,7 @@ mod tests {
         let resolved = resolve_call_request_inner(
             &mut fixture.provider_data,
             request,
-            Some(&BlockSpec::pending()),
+            &BlockSpec::pending(),
             &StateOverrides::default(),
             |_data| unreachable!("max fees are set"),
             |_, max_fee_per_gas, max_priority_fee_per_gas| {

crates/edr_provider/src/requests/eth/gas.rs

@@ -22,12 +22,12 @@ pub fn handle_estimate_gas<LoggerErrorT: Debug>(
     call_request: CallRequest,
     block_spec: Option<BlockSpec>,
 ) -> Result<(U64, Vec<Trace>), ProviderError<LoggerErrorT>> {
-    validate_call_request(data.spec_id(), &call_request, &block_spec)?;
-
     // Matching Hardhat behavior in defaulting to "pending" instead of "latest" for
     // estimate gas.
     let block_spec = block_spec.unwrap_or_else(BlockSpec::pending);
 
+    validate_call_request(data.spec_id(), &call_request, &block_spec)?;
+
     let transaction =
         resolve_estimate_gas_request(data, call_request, &block_spec, &StateOverrides::default())?;
 
@@ -109,7 +109,7 @@ fn resolve_estimate_gas_request<LoggerErrorT: Debug>(
     resolve_call_request_inner(
         data,
         request,
-        Some(block_spec),
+        block_spec,
         state_overrides,
         ProviderData::gas_price,
         |data, max_fee_per_gas, max_priority_fee_per_gas| {

crates/edr_provider/src/requests/validation.rs

@@ -155,11 +155,9 @@ pub fn validate_transaction_spec<LoggerErrorT: Debug>(
 pub fn validate_call_request<LoggerErrorT: Debug>(
     spec_id: SpecId,
     call_request: &CallRequest,
-    block_spec: &Option<BlockSpec>,
+    block_spec: &BlockSpec,
 ) -> Result<(), ProviderError<LoggerErrorT>> {
-    if let Some(ref block_spec) = block_spec {
-        validate_post_merge_block_tags(spec_id, block_spec)?;
-    }
+    validate_post_merge_block_tags(spec_id, block_spec)?;
 
     validate_transaction_and_call_request(
         spec_id,

crates/edr_provider/tests/issue_356.rs

@@ -0,0 +1,58 @@
+use std::str::FromStr;
+
+use anyhow::Context;
+use edr_eth::{remote::eth::CallRequest, Address, Bytes, SpecId};
+use edr_provider::{
+    hardhat_rpc_types::ForkConfig, test_utils::create_test_config_with_fork, MethodInvocation,
+    NoopLogger, Provider, ProviderRequest,
+};
+use edr_test_utils::env::get_alchemy_url;
+use sha3::{Digest, Keccak256};
+use tokio::runtime;
+
+// Check that there is no panic when calling a forked blockchain where the
+// hardfork is specified as Cancun, but the block number is before the Cancun
+// hardfork. https://github.com/NomicFoundation/edr/issues/356
+#[tokio::test(flavor = "multi_thread")]
+async fn issue_324() -> anyhow::Result<()> {
+    // ERC-20 contract
+    const TEST_CONTRACT_ADDRESS: &str = "0xaa8e23fb1079ea71e0a56f48a2aa51851d8433d0";
+
+    let contract_address = Address::from_str(TEST_CONTRACT_ADDRESS).context("Invalid address")?;
+
+    let logger = Box::new(NoopLogger);
+    let subscriber = Box::new(|_event| {});
+
+    let mut config = create_test_config_with_fork(Some(ForkConfig {
+        json_rpc_url: get_alchemy_url().replace("mainnet", "sepolia"),
+        // Pre-cancun Sepolia block
+        block_number: Some(4243456),
+        http_headers: None,
+    }));
+    config.hardfork = SpecId::CANCUN;
+
+    let provider = Provider::new(runtime::Handle::current(), logger, subscriber, config)?;
+
+    let selector = Bytes::copy_from_slice(
+        &Keccak256::new_with_prefix("decimals()")
+            .finalize()
+            .as_slice()[..4],
+    );
+
+    let response = provider.handle_request(ProviderRequest::Single(MethodInvocation::Call(
+        CallRequest {
+            to: Some(contract_address),
+            data: Some(selector),
+            ..CallRequest::default()
+        },
+        None,
+        None,
+    )))?;
+
+    assert_eq!(
+        response.result,
+        "0x0000000000000000000000000000000000000000000000000000000000000006"
+    );
+
+    Ok(())
+}