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.

Sign up for GitHub

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

Jump to bottom

refactor(rpc): unify system caller invocations #12360

Merged
merged 1 commit into from
Nov 7, 2024
Merged

refactor(rpc): unify system caller invocations #12360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 39 additions & 27 deletions crates/rpc/rpc-eth-api/src/helpers/trace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Loads a pending block from database. Helper trait for `eth_` call and trace RPC methods.

use std::sync::Arc;
use std::{fmt::Display, sync::Arc};

use crate::{FromEvmError, RpcNodeCore};
use alloy_primitives::B256;
Expand All @@ -16,7 +16,9 @@ use reth_rpc_eth_types::{
};
use revm::{db::CacheDB, Database, DatabaseCommit, GetInspector, Inspector};
use revm_inspectors::tracing::{TracingInspector, TracingInspectorConfig};
use revm_primitives::{EnvWithHandlerCfg, EvmState, ExecutionResult, ResultAndState};
use revm_primitives::{
BlockEnv, CfgEnvWithHandlerCfg, EnvWithHandlerCfg, EvmState, ExecutionResult, ResultAndState,
};

use super::{Call, LoadBlock, LoadPendingBlock, LoadState, LoadTransaction};

Expand Down Expand Up @@ -187,26 +189,13 @@ pub trait Trace: LoadState<Evm: ConfigureEvm<Header = Header>> {
// we need to get the state of the parent block because we're essentially replaying the
// block the transaction is included in
let parent_block = block.parent_hash;
let parent_beacon_block_root = block.parent_beacon_block_root;

let this = self.clone();
self.spawn_with_state_at_block(parent_block.into(), move |state| {
let mut db = CacheDB::new(StateProviderDatabase::new(state));
let block_txs = block.transactions_with_sender();

// apply relevant system calls
SystemCaller::new(this.evm_config().clone(), this.provider().chain_spec())
.pre_block_beacon_root_contract_call(
&mut db,
&cfg,
&block_env,
parent_beacon_block_root,
)
.map_err(|_| {
EthApiError::EvmCustom(
"failed to apply 4788 beacon root system call".to_string(),
)
})?;
this.apply_pre_execution_changes(&block, &mut db, &cfg, &block_env)?;

// replay all transactions prior to the targeted transaction
this.replay_transactions_until(
Expand Down Expand Up @@ -333,17 +322,7 @@ pub trait Trace: LoadState<Evm: ConfigureEvm<Header = Header>> {
let mut db =
CacheDB::new(StateProviderDatabase::new(StateProviderTraitObjWrapper(&state)));

// apply relevant system calls
SystemCaller::new(this.evm_config().clone(), this.provider().chain_spec())
.pre_block_beacon_root_contract_call(
&mut db,
&cfg,
&block_env,
block.header().parent_beacon_block_root,
)
.map_err(|_| {
EthApiError::EvmCustom("failed to apply 4788 system call".to_string())
})?;
this.apply_pre_execution_changes(&block, &mut db, &cfg, &block_env)?;

// prepare transactions, we do everything upfront to reduce time spent with open
// state
Expand Down Expand Up @@ -470,4 +449,37 @@ pub trait Trace: LoadState<Evm: ConfigureEvm<Header = Header>> {
{
self.trace_block_until_with_inspector(block_id, block, None, insp_setup, f)
}

/// Applies chain-specific state transitions required before executing a block.
///
/// Note: This should only be called when tracing an entire block vs individual transactions.
/// When tracing transaction on top of an already committed block state, those transitions are
/// already applied.
fn apply_pre_execution_changes<DB: Send + Database<Error: Display> + DatabaseCommit>(
Comment on lines +453 to +458
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense, although this only works as long as the the system calls are the same for eth and op.
but this is a good step

&self,
block: &SealedBlockWithSenders,
db: &mut DB,
cfg: &CfgEnvWithHandlerCfg,
block_env: &BlockEnv,
) -> Result<(), Self::Error> {
let mut system_caller =
SystemCaller::new(self.evm_config().clone(), self.provider().chain_spec());
// apply relevant system calls
system_caller
.pre_block_beacon_root_contract_call(
db,
cfg,
block_env,
block.header.parent_beacon_block_root,
)
.map_err(|_| EthApiError::EvmCustom("failed to apply 4788 system call".to_string()))?;

system_caller
.pre_block_blockhashes_contract_call(db, cfg, block_env, block.header.parent_hash)
.map_err(|_| {
EthApiError::EvmCustom("failed to apply blockhashes system call".to_string())
})?;

Ok(())
}
}
131 changes: 36 additions & 95 deletions crates/rpc/rpc/src/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ use jsonrpsee::core::RpcResult;
use reth_chainspec::EthereumHardforks;
use reth_evm::{
execute::{BlockExecutorProvider, Executor},
system_calls::SystemCaller,
ConfigureEvmEnv,
};
use reth_primitives::{Block, TransactionSignedEcRecovered};
use reth_primitives::{Block, SealedBlockWithSenders};
use reth_provider::{
BlockReaderIdExt, ChainSpecProvider, HeaderProvider, StateProofProvider, StateProviderFactory,
TransactionVariant,
Expand Down Expand Up @@ -93,54 +92,30 @@ where
/// Trace the entire block asynchronously
async fn trace_block(
&self,
at: BlockId,
transactions: Vec<TransactionSignedEcRecovered>,
block: Arc<SealedBlockWithSenders>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these changes are so that we don't need the parent_beacon_block_root argument?

cfg: CfgEnvWithHandlerCfg,
block_env: BlockEnv,
opts: GethDebugTracingOptions,
parent_beacon_block_root: Option<B256>,
) -> Result<Vec<TraceResult>, Eth::Error> {
if transactions.is_empty() {
// nothing to trace
return Ok(Vec::new())
}

// replay all transactions of the block
let this = self.clone();
self.eth_api()
.spawn_with_state_at_block(at, move |state| {
let block_hash = at.as_block_hash();
let mut results = Vec::with_capacity(transactions.len());
.spawn_with_state_at_block(block.parent_hash.into(), move |state| {
let mut results = Vec::with_capacity(block.body.transactions.len());
let mut db = CacheDB::new(StateProviderDatabase::new(state));

// apply relevant system calls
SystemCaller::new(
this.eth_api().evm_config().clone(),
this.eth_api().provider().chain_spec(),
)
.pre_block_beacon_root_contract_call(
&mut db,
&cfg,
&block_env,
parent_beacon_block_root,
)
.map_err(|_| {
EthApiError::EvmCustom(
"failed to apply 4788 beacon root system call".to_string(),
)
})?;
this.eth_api().apply_pre_execution_changes(&block, &mut db, &cfg, &block_env)?;

let mut transactions = transactions.into_iter().enumerate().peekable();
let mut transactions = block.transactions_with_sender().enumerate().peekable();
let mut inspector = None;
while let Some((index, tx)) = transactions.next() {
while let Some((index, (signer, tx))) = transactions.next() {
let tx_hash = tx.hash;

let env = EnvWithHandlerCfg {
env: Env::boxed(
cfg.cfg_env.clone(),
block_env.clone(),
RpcNodeCore::evm_config(this.eth_api())
.tx_env(tx.as_signed(), tx.signer()),
RpcNodeCore::evm_config(this.eth_api()).tx_env(tx, *signer),
),
handler_cfg: cfg.handler_cfg,
};
Expand All @@ -149,7 +124,7 @@ where
env,
&mut db,
Some(TransactionContext {
block_hash,
block_hash: Some(block.hash()),
tx_hash: Some(tx_hash),
tx_index: Some(index),
}),
Expand Down Expand Up @@ -186,45 +161,38 @@ where
.map_err(Eth::Error::from_eth_err)?;

let (cfg, block_env) = self.eth_api().evm_env_for_raw_block(&block.header).await?;
// we trace on top the block's parent block
let parent = block.parent_hash;

// we need the beacon block root for a system call
let parent_beacon_block_root = block.parent_beacon_block_root;

// Depending on EIP-2 we need to recover the transactions differently
let transactions =
if self.inner.provider.chain_spec().is_homestead_active_at_block(block.number) {
block
.body
.transactions
.into_iter()
.map(|tx| {
tx.into_ecrecovered()
.ok_or(EthApiError::InvalidTransactionSignature)
.map_err(Eth::Error::from_eth_err)
})
.collect::<Result<Vec<_>, Eth::Error>>()?
} else {
block
.body
.transactions
.into_iter()
.map(|tx| {
tx.into_ecrecovered_unchecked()
.ok_or(EthApiError::InvalidTransactionSignature)
.map_err(Eth::Error::from_eth_err)
})
.collect::<Result<Vec<_>, Eth::Error>>()?
};
let senders = if self.inner.provider.chain_spec().is_homestead_active_at_block(block.number)
{
block
.body
.transactions
.iter()
.map(|tx| {
tx.recover_signer()
.ok_or(EthApiError::InvalidTransactionSignature)
.map_err(Eth::Error::from_eth_err)
})
.collect::<Result<Vec<_>, Eth::Error>>()?
} else {
block
.body
.transactions
.iter()
.map(|tx| {
tx.recover_signer_unchecked()
.ok_or(EthApiError::InvalidTransactionSignature)
.map_err(Eth::Error::from_eth_err)
})
.collect::<Result<Vec<_>, Eth::Error>>()?
};

self.trace_block(
parent.into(),
transactions,
Arc::new(block.with_senders_unchecked(senders).seal_slow()),
Comment on lines -223 to +192
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we need to seal here because we decoded the block from rlp

cfg,
block_env,
opts,
parent_beacon_block_root,
)
.await
}
Expand All @@ -248,19 +216,8 @@ where
)?;

let block = block.ok_or(EthApiError::HeaderNotFound(block_id))?;
// we need to get the state of the parent block because we're replaying this block on top of
// its parent block's state
let state_at = block.parent_hash;

self.trace_block(
state_at.into(),
(*block).clone().into_transactions_ecrecovered().collect(),
cfg,
block_env,
opts,
block.parent_beacon_block_root,
)
.await
self.trace_block(block, cfg, block_env, opts).await
}

/// Trace the transaction according to the provided options.
Expand All @@ -281,7 +238,6 @@ where
// block the transaction is included in
let state_at: BlockId = block.parent_hash.into();
let block_hash = block.hash();
let parent_beacon_block_root = block.parent_beacon_block_root;

let this = self.clone();
self.eth_api()
Expand All @@ -293,22 +249,7 @@ where

let mut db = CacheDB::new(StateProviderDatabase::new(state));

// apply relevant system calls
SystemCaller::new(
this.eth_api().evm_config().clone(),
this.eth_api().provider().chain_spec(),
)
.pre_block_beacon_root_contract_call(
&mut db,
&cfg,
&block_env,
parent_beacon_block_root,
)
.map_err(|_| {
EthApiError::EvmCustom(
"failed to apply 4788 beacon root system call".to_string(),
)
})?;
this.eth_api().apply_pre_execution_changes(&block, &mut db, &cfg, &block_env)?;

// replay all transactions prior to the targeted transaction
let index = this.eth_api().replay_transactions_until(
Expand Down
Loading