jleony/nanoreth
1
0
Fork 0
mirror of https://github.com/hl-archive-node/nanoreth.git synced 2025-12-06 10:59:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: improve parity statediff (#5037)

Browse Source
This commit is contained in:
Matthias Seitz
2023-10-16 15:19:11 +02:00
committed by GitHub
parent 1468563f2f
commit 931065d71c
4 changed files with 64 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files

60
crates/revm/revm-inspectors/src/tracing/builder/parity.rs
View File

@ -146,22 +146,17 @@ impl ParityTraceBuilder {
/// Consumes the inspector and returns the trace results according to the configured trace
/// types.
///
/// Warning: If `trace_types` contains [TraceType::StateDiff] the returned [StateDiff] will only
/// contain accounts with changed state, not including their balance changes because this is not
/// tracked during inspection and requires the State map returned after inspection. Use
/// [ParityTraceBuilder::into_trace_results_with_state] to populate the balance and nonce
/// changes for the [StateDiff] using the [DatabaseRef].
/// Warning: If `trace_types` contains [TraceType::StateDiff] the returned [StateDiff] will not
/// be filled. Use [ParityTraceBuilder::into_trace_results_with_state] or
/// [populate_state_diff] to populate the balance and nonce changes for the [StateDiff]
/// using the [DatabaseRef].
pub fn into_trace_results(
self,
res: ExecutionResult,
res: &ExecutionResult,
trace_types: &HashSet<TraceType>,
) -> TraceResults {
let gas_used = res.gas_used();
let output = match res {
ExecutionResult::Success { output, .. } => output.into_data(),
ExecutionResult::Revert { output, .. } => output,
ExecutionResult::Halt { .. } => Default::default(),
};
let output = res.output().cloned().unwrap_or_default();
let (trace, vm_trace, state_diff) = self.into_trace_type_traces(trace_types);
@ -186,14 +181,14 @@ impl ParityTraceBuilder {
/// with the [TracingInspector](crate::tracing::TracingInspector).
pub fn into_trace_results_with_state<DB>(
self,
res: ResultAndState,
res: &ResultAndState,
trace_types: &HashSet<TraceType>,
db: DB,
) -> Result<TraceResults, DB::Error>
where
DB: DatabaseRef,
{
let ResultAndState { result, state } = res;
let ResultAndState { ref result, ref state } = res;
let breadth_first_addresses = if trace_types.contains(&TraceType::VmTrace) {
CallTraceNodeWalkerBF::new(&self.nodes)
@ -207,7 +202,7 @@ impl ParityTraceBuilder {
// check the state diff case
if let Some(ref mut state_diff) = trace_res.state_diff {
populate_account_balance_nonce_diffs(state_diff, &db, state.iter())?;
populate_state_diff(state_diff, &db, state.iter())?;
}
// check the vm trace case
@ -218,7 +213,12 @@ impl ParityTraceBuilder {
Ok(trace_res)
}
/// Returns the tracing types that are configured in the set
/// Returns the tracing types that are configured in the set.
///
/// Warning: if [TraceType::StateDiff] is provided this does __not__ fill the state diff, since
/// this requires access to the account diffs.
///
/// See [Self::into_trace_results_with_state] and [populate_state_diff].
pub fn into_trace_type_traces(
self,
trace_types: &HashSet<TraceType>,
@ -234,7 +234,6 @@ impl ParityTraceBuilder {
if trace_types.contains(&TraceType::VmTrace) { Some(self.vm_trace()) } else { None };
let mut traces = Vec::with_capacity(if with_traces { self.nodes.len() } else { 0 });
let mut diff = StateDiff::default();
for node in self.iter_traceable_nodes() {
let trace_address = self.trace_address(node.idx);
@ -262,13 +261,10 @@ impl ParityTraceBuilder {
}
}
}
if with_diff {
node.parity_update_state_diff(&mut diff);
}
}
let traces = with_traces.then_some(traces);
let diff = with_diff.then_some(diff);
let diff = with_diff.then_some(StateDiff::default());
(traces, vm_trace, diff)
}
@ -565,9 +561,7 @@ where
///
/// It's expected that `DB` is a revm [Database](revm::db::Database) which at this point already
/// contains all the accounts that are in the state map and never has to fetch them from disk.
///
/// This is intended to be called after inspecting a transaction with the returned state.
pub fn populate_account_balance_nonce_diffs<'a, DB, I>(
pub fn populate_state_diff<'a, DB, I>(
state_diff: &mut StateDiff,
db: DB,
account_diffs: I,
@ -586,7 +580,7 @@ where
let entry = state_diff.entry(addr).or_default();
// we check if this account was created during the transaction
if changed_acc.is_created() {
if changed_acc.is_created() || changed_acc.is_loaded_as_not_existing() {
entry.balance = Delta::Added(changed_acc.info.balance);
entry.nonce = Delta::Added(U64::from(changed_acc.info.nonce));
if changed_acc.info.code_hash == KECCAK_EMPTY {
@ -594,13 +588,27 @@ where
// marked as added
entry.code = Delta::Added(Default::default());
}
// new storage values
for (key, slot) in changed_acc.storage.iter() {
entry.storage.insert((*key).into(), Delta::Added(slot.present_value.into()));
}
} else {
// account already exists, we need to fetch the account from the db
let db_acc = db.basic(addr)?.unwrap_or_default();
// update _changed_ storage values
for (key, slot) in changed_acc.storage.iter().filter(|(_, slot)| slot.is_changed()) {
entry.storage.insert(
(*key).into(),
Delta::changed(
slot.previous_or_original_value.into(),
slot.present_value.into(),
),
);
}
// check if the account was changed at all
// NOTE: changed storage values are set by the the
// `CallTraceNode::parity_update_state_diff`
if entry.storage.is_empty() &&
db_acc == changed_acc.info &&
!changed_acc.is_selfdestructed()

74
crates/revm/revm-inspectors/src/tracing/types.rs
View File

@ -8,15 +8,15 @@ use reth_rpc_types::trace::{
AccountState, CallFrame, CallLogFrame, DiffStateKind, GethDefaultTracingOptions, StructLog,
},
parity::{
Action, ActionType, CallAction, CallOutput, CallType, ChangedType, CreateAction,
CreateOutput, Delta, SelfdestructAction, StateDiff, TraceOutput, TransactionTrace,
Action, ActionType, CallAction, CallOutput, CallType, CreateAction, CreateOutput,
SelfdestructAction, TraceOutput, TransactionTrace,
},
};
use revm::interpreter::{
opcode, CallContext, CallScheme, CreateScheme, InstructionResult, Memory, OpCode, Stack,
};
use serde::{Deserialize, Serialize};
use std::collections::{btree_map::Entry, BTreeMap, VecDeque};
use std::collections::{BTreeMap, VecDeque};
/// A unified representation of a call
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
@ -308,74 +308,6 @@ impl CallTraceNode {
self.status() == InstructionResult::SelfDestruct
}
/// Updates the values of the state diff
pub(crate) fn parity_update_state_diff(&self, diff: &mut StateDiff) {
let addr = self.trace.address;
let acc = diff.entry(addr).or_default();
if self.kind().is_any_create() {
let code = self.trace.output.clone();
if acc.code == Delta::Unchanged {
acc.code = Delta::Added(code)
}
}
// iterate over all storage diffs
for change in self.trace.steps.iter().filter_map(|s| s.storage_change) {
let StorageChange { key, value, had_value, .. } = change;
let b256_value = B256::from(value);
match acc.storage.entry(key.into()) {
Entry::Vacant(entry) => {
if let Some(had_value) = had_value {
if value != had_value {
entry.insert(Delta::Changed(ChangedType {
from: had_value.into(),
to: b256_value,
}));
}
} else {
entry.insert(Delta::Added(b256_value));
}
}
Entry::Occupied(mut entry) => {
let value = match entry.get() {
Delta::Unchanged => {
if let Some(had_value) = had_value {
if value != had_value {
Delta::Changed(ChangedType {
from: had_value.into(),
to: b256_value,
})
} else {
Delta::Unchanged
}
} else {
Delta::Added(b256_value)
}
}
Delta::Added(added) => {
if added == &b256_value {
Delta::Added(*added)
} else {
Delta::Changed(ChangedType { from: *added, to: b256_value })
}
}
Delta::Removed(_) => Delta::Added(b256_value),
Delta::Changed(c) => {
if c.from == b256_value {
// remains unchanged if the value is the same
Delta::Unchanged
} else {
Delta::Changed(ChangedType { from: c.from, to: b256_value })
}
}
};
entry.insert(value);
}
}
}
}
/// Converts this node into a parity `TransactionTrace`
pub(crate) fn parity_transaction_trace(&self, trace_address: Vec<usize>) -> TransactionTrace {
let action = self.parity_action();

5
crates/rpc/rpc-types/src/eth/trace/parity.rs
View File

@ -85,6 +85,11 @@ pub enum Delta<T> {
// === impl Delta ===
impl<T> Delta<T> {
/// Creates a new [Delta::Changed] variant
pub fn changed(from: T, to: T) -> Self {
Self::Changed(ChangedType { from, to })
}
/// Returns true if the value is unchanged
pub fn is_unchanged(&self) -> bool {
matches!(self, Delta::Unchanged)

47
crates/rpc/rpc/src/trace.rs
View File

@ -15,9 +15,7 @@ use reth_provider::{BlockReader, ChainSpecProvider, EvmEnvProvider, StateProvide
use reth_revm::{
database::StateProviderDatabase,
env::tx_env_with_recovered,
tracing::{
parity::populate_account_balance_nonce_diffs, TracingInspector, TracingInspectorConfig,
},
tracing::{parity::populate_state_diff, TracingInspector, TracingInspectorConfig},
};
use reth_rpc_api::TraceApiServer;
use reth_rpc_types::{
@ -26,7 +24,7 @@ use reth_rpc_types::{
BlockError, BlockOverrides, CallRequest, Index,
};
use revm::{db::CacheDB, primitives::Env};
use revm_primitives::{db::DatabaseCommit, ResultAndState};
use revm_primitives::db::DatabaseCommit;
use std::{collections::HashSet, sync::Arc};
use tokio::sync::{AcquireError, OwnedSemaphorePermit};
@ -84,7 +82,7 @@ where
.spawn_with_call_at(call, at, overrides, move |db, env| {
let (res, _, db) = inspect_and_return_db(db, env, &mut inspector)?;
let trace_res = inspector.into_parity_builder().into_trace_results_with_state(
res,
&res,
&trace_types,
&db,
)?;
@ -116,7 +114,7 @@ where
.eth_api
.spawn_trace_at_with_state(env, config, at, move |inspector, res, db| {
Ok(inspector.into_parity_builder().into_trace_results_with_state(
res,
&res,
&trace_types,
&db,
)?)
@ -158,16 +156,12 @@ where
let config = tracing_config(&trace_types);
let mut inspector = TracingInspector::new(config);
let (res, _) = inspect(&mut db, env, &mut inspector)?;
let ResultAndState { result, state } = res;
let mut trace_res =
inspector.into_parity_builder().into_trace_results(result, &trace_types);
// If statediffs were requested, populate them with the account balance and
// nonce from pre-state
if let Some(ref mut state_diff) = trace_res.state_diff {
populate_account_balance_nonce_diffs(state_diff, &db, state.iter())?;
}
let trace_res = inspector.into_parity_builder().into_trace_results_with_state(
&res,
&trace_types,
&db,
)?;
results.push(trace_res);
@ -176,7 +170,7 @@ where
if calls.peek().is_some() {
// need to apply the state changes of this call before executing
// the next call
db.commit(state)
db.commit(res.state)
}
}
@ -196,7 +190,7 @@ where
.eth_api
.spawn_trace_transaction_in_block(hash, config, move |_, inspector, res, db| {
let trace_res = inspector.into_parity_builder().into_trace_results_with_state(
res,
&res,
&trace_types,
&db,
)?;
@ -417,12 +411,12 @@ where
tracing_config(&trace_types),
move |tx_info, inspector, res, state, db| {
let mut full_trace =
inspector.into_parity_builder().into_trace_results(res, &trace_types);
inspector.into_parity_builder().into_trace_results(&res, &trace_types);
// If statediffs were requested, populate them with the account balance and
// nonce from pre-state
if let Some(ref mut state_diff) = full_trace.state_diff {
populate_account_balance_nonce_diffs(state_diff, db, state.iter())?;
populate_state_diff(state_diff, db, state.iter())?;
}
let trace = TraceResultsWithTransactionHash {
@ -567,12 +561,13 @@ struct TraceApiInner<Provider, Eth> {
}
/// Returns the [TracingInspectorConfig] depending on the enabled [TraceType]s
///
/// Note: the parity statediffs can be populated entirely via the execution result, so we don't need
/// statediff recording
#[inline]
fn tracing_config(trace_types: &HashSet<TraceType>) -> TracingInspectorConfig {
let needs_diff = trace_types.contains(&TraceType::StateDiff);
let needs_vm_trace = trace_types.contains(&TraceType::VmTrace);
let needs_steps = needs_vm_trace || needs_diff;
TracingInspectorConfig::default_parity().set_steps(needs_steps).set_state_diffs(needs_diff)
TracingInspectorConfig::default_parity().set_steps(needs_vm_trace)
}
/// Helper to construct a [`LocalizedTransactionTrace`] that describes a reward to the block
@ -602,8 +597,9 @@ mod tests {
let mut s = HashSet::new();
s.insert(TraceType::StateDiff);
let config = tracing_config(&s);
assert!(config.record_steps);
assert!(config.record_state_diff);
// not required
assert!(!config.record_steps);
assert!(!config.record_state_diff);
let mut s = HashSet::new();
s.insert(TraceType::VmTrace);
@ -616,6 +612,7 @@ mod tests {
s.insert(TraceType::StateDiff);
let config = tracing_config(&s);
assert!(config.record_steps);
assert!(config.record_state_diff);
// not required for StateDiff
assert!(!config.record_state_diff);
}
}