Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use remote chain id for pre-fork simulation #567

Merged
merged 2 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
Wodann marked this conversation as resolved.
Show resolved Hide resolved
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;
Loading