Skip to content

Commit

Permalink
fix: use remote chain id for pre-fork simulation (#567)
Browse files Browse the repository at this point in the history
* fix: use remote chain id for pre-fork simulation

* Fix error message
  • Loading branch information
agostbiro authored Jul 31, 2024
1 parent e02eb22 commit 0bba027
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/fast-kings-search.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@nomicfoundation/edr": patch
---

Fixed a bug in fork mode where the locally overridden chain id was used instead of the remote chain id when simulating transactions in pre-fork blocks.
8 changes: 8 additions & 0 deletions crates/edr_evm/src/blockchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ where
/// Retrieves the instances chain ID.
fn chain_id(&self) -> u64;

/// Retrieves the chain ID of the block at the provided number.
/// The chain ID can be different in fork mode pre- and post-fork block
/// number.
fn chain_id_at_block_number(&self, _block_number: u64) -> Result<u64, Self::BlockchainError> {
// Chain id only depends on the block number in fork mode
Ok(self.chain_id())
}

/// Retrieves the last block in the blockchain.
fn last_block(
&self,
Expand Down
12 changes: 12 additions & 0 deletions crates/edr_evm/src/blockchain/forked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,18 @@ where
self.chain_id
}

fn chain_id_at_block_number(&self, block_number: u64) -> Result<u64, Self::BlockchainError> {
if block_number > self.last_block_number() {
return Err(BlockchainError::UnknownBlockNumber);
}

if block_number <= self.fork_block_number {
Ok(self.remote_chain_id())
} else {
Ok(self.chain_id())
}
}

#[cfg_attr(feature = "tracing", tracing::instrument(skip_all))]
fn last_block(
&self,
Expand Down
110 changes: 88 additions & 22 deletions crates/edr_provider/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,20 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
self.blockchain.chain_id()
}

pub fn chain_id_at_block_spec(
&self,
block_spec: &BlockSpec,
) -> Result<u64, ProviderError<LoggerErrorT>> {
let block_number = self.block_number_by_block_spec(block_spec)?;

let chain_id = if let Some(block_number) = block_number {
self.chain_id_at_block_number(block_number, block_spec)?
} else {
self.blockchain.chain_id()
};

Ok(chain_id)
}
pub fn coinbase(&self) -> Address {
self.beneficiary
}
Expand All @@ -607,9 +621,8 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
.ok_or_else(|| ProviderError::InvalidTransactionHash(*transaction_hash))?;

let header = block.header();
let block_spec = Some(BlockSpec::Number(header.number));

let cfg_env = self.create_evm_config(block_spec.as_ref())?;
let cfg_env = self.create_evm_config_at_block_spec(&BlockSpec::Number(header.number))?;

let transactions = block.transactions().to_vec();

Expand Down Expand Up @@ -659,7 +672,7 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
block_spec: &BlockSpec,
trace_config: DebugTraceConfig,
) -> Result<DebugTraceResultWithTraces, ProviderError<LoggerErrorT>> {
let cfg_env = self.create_evm_config(Some(block_spec))?;
let cfg_env = self.create_evm_config_at_block_spec(block_spec)?;

let tx_env: TxEnv = transaction.into();

Expand Down Expand Up @@ -691,7 +704,7 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
transaction: transaction::Signed,
block_spec: &BlockSpec,
) -> Result<EstimateGasResult, ProviderError<LoggerErrorT>> {
let cfg_env = self.create_evm_config(Some(block_spec))?;
let cfg_env = self.create_evm_config_at_block_spec(block_spec)?;
// Minimum gas cost that is required for transaction to be included in
// a block
let minimum_cost = transaction::initial_cost(&transaction, self.spec_id());
Expand Down Expand Up @@ -1422,7 +1435,7 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
block_spec: &BlockSpec,
state_overrides: &StateOverrides,
) -> Result<CallResult, ProviderError<LoggerErrorT>> {
let cfg_env = self.create_evm_config(Some(block_spec))?;
let cfg_env = self.create_evm_config_at_block_spec(block_spec)?;
let tx_env = transaction.into();

let mut debugger = Debugger::with_mocker(
Expand Down Expand Up @@ -1909,26 +1922,32 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
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.
/// Wrapper over `Blockchain::chain_id_at_block_number` that handles error
/// conversion.
fn chain_id_at_block_number(
&self,
block_number: u64,
block_spec: &BlockSpec,
) -> Result<u64, ProviderError<LoggerErrorT>> {
self.blockchain
.chain_id_at_block_number(block_number)
.map_err(|err| match err {
BlockchainError::UnknownBlockNumber => ProviderError::InvalidBlockNumberOrHash {
block_spec: block_spec.clone(),
latest_block_number: self.blockchain.last_block_number(),
},
_ => ProviderError::Blockchain(err),
})
}

/// Creates an EVM configuration with the provided hardfork and chain id
fn create_evm_config(
&self,
block_spec: Option<&BlockSpec>,
spec_id: SpecId,
chain_id: u64,
) -> Result<CfgEnvWithHandlerCfg, ProviderError<LoggerErrorT>> {
let block_number = block_spec
.map(|block_spec| self.block_number_by_block_spec(block_spec))
.transpose()?
.flatten();

let spec_id = if let Some(block_number) = block_number {
self.blockchain.spec_at_block_number(block_number)?
} else {
self.blockchain.spec_id()
};

let mut cfg_env = CfgEnv::default();
cfg_env.chain_id = self.blockchain.chain_id();
cfg_env.chain_id = chain_id;
cfg_env.limit_contract_code_size = if self.allow_unlimited_contract_size {
Some(usize::MAX)
} else {
Expand All @@ -1939,6 +1958,29 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
Ok(CfgEnvWithHandlerCfg::new_with_spec_id(cfg_env, spec_id))
}

/// Creates a configuration, taking into the hardfork and chain id at the
/// provided `BlockSpec`.
fn create_evm_config_at_block_spec(
&self,
block_spec: &BlockSpec,
) -> Result<CfgEnvWithHandlerCfg, ProviderError<LoggerErrorT>> {
let block_number = self.block_number_by_block_spec(block_spec)?;

let spec_id = if let Some(block_number) = block_number {
self.spec_at_block_number(block_number, block_spec)?
} else {
self.blockchain.spec_id()
};

let chain_id = if let Some(block_number) = block_number {
self.chain_id_at_block_number(block_number, block_spec)?
} else {
self.blockchain.chain_id()
};

self.create_evm_config(spec_id, chain_id)
}

fn execute_in_block_context<T>(
&mut self,
block_spec: Option<&BlockSpec>,
Expand Down Expand Up @@ -1995,7 +2037,8 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
options.beneficiary = Some(options.beneficiary.unwrap_or(self.beneficiary));
options.gas_limit = Some(options.gas_limit.unwrap_or_else(|| self.block_gas_limit()));

let evm_config = self.create_evm_config(None)?;
let evm_config =
self.create_evm_config(self.blockchain.spec_id(), self.blockchain.chain_id())?;

if options.mix_hash.is_none() && evm_config.handler_cfg.spec_id >= SpecId::MERGE {
options.mix_hash = Some(self.prev_randao_generator.next_value());
Expand Down Expand Up @@ -2228,6 +2271,24 @@ impl<LoggerErrorT: Debug, TimerT: Clone + TimeSinceEpoch> ProviderData<LoggerErr
}
}

/// Wrapper over `Blockchain::spec_at_block_number` that handles error
/// conversion.
fn spec_at_block_number(
&self,
block_number: u64,
block_spec: &BlockSpec,
) -> Result<SpecId, ProviderError<LoggerErrorT>> {
self.blockchain
.spec_at_block_number(block_number)
.map_err(|err| match err {
BlockchainError::UnknownBlockNumber => ProviderError::InvalidBlockNumberOrHash {
block_spec: block_spec.clone(),
latest_block_number: self.blockchain.last_block_number(),
},
_ => ProviderError::Blockchain(err),
})
}

pub fn sign_transaction_request(
&self,
transaction_request: TransactionRequestAndSender,
Expand Down Expand Up @@ -2979,6 +3040,11 @@ mod tests {
let chain_id = fixture.provider_data.chain_id();
assert_eq!(chain_id, fixture.config.chain_id);

let chain_id_at_block = fixture
.provider_data
.chain_id_at_block_spec(&BlockSpec::Number(1))?;
assert_eq!(chain_id_at_block, 1);

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/edr_provider/src/requests/eth/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub(crate) fn resolve_call_request_inner<LoggerErrorT: Debug, TimerT: Clone + Ti
..
} = request;

let chain_id = data.chain_id();
let chain_id = data.chain_id_at_block_spec(block_spec)?;
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);
Expand Down
44 changes: 44 additions & 0 deletions crates/edr_provider/tests/issues/issue_533.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use std::str::FromStr as _;

use edr_eth::B256;
use edr_provider::{
hardhat_rpc_types::ForkConfig, test_utils::create_test_config_with_fork, time::CurrentTime,
MethodInvocation, NoopLogger, Provider, ProviderRequest,
};
use edr_test_utils::env::get_alchemy_url;
use tokio::runtime;

// https://github.com/NomicFoundation/edr/issues/533
#[tokio::test(flavor = "multi_thread")]
async fn issue_533() -> anyhow::Result<()> {
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(),
block_number: Some(20_384_300),
http_headers: None,
}));

// The default chain id set by Hardhat
config.chain_id = 31337;

let provider = Provider::new(
runtime::Handle::current(),
logger,
subscriber,
config,
CurrentTime,
)?;

let transaction_hash =
B256::from_str("0x0537316f37627655b7fe5e50e23f71cd835b377d1cde4226443c94723d036e32")?;

let result = provider.handle_request(ProviderRequest::Single(
MethodInvocation::DebugTraceTransaction(transaction_hash, None),
))?;

assert!(!result.traces.is_empty());

Ok(())
}
1 change: 1 addition & 0 deletions crates/edr_provider/tests/issues/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ mod issue_361;
mod issue_384;
mod issue_407;
mod issue_503;
mod issue_533;

0 comments on commit 0bba027

Please sign in to comment.