From 3ebd9f959c4fab2f390f9442a1c17d965c9525a4 Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Thu, 18 Apr 2024 19:03:09 +0300 Subject: [PATCH] feat: extend trace decoder err info (#148) * feat: extend trace decoder err info * fix: fix clippy issue * feat: swap out the internal U512 inside nibbles (#132) * feat: swap out the internal U512 inside nibbles * fix: comment fix * fix: fix clippy pr issues * fix: fix clippy issue * fix: fix pr comments * docs: update changelog * fix: update impl_to_nibbles --------- Co-authored-by: Vladimir Trifonov * Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods * fix: add pr comments fixes * fix: add pr comments fix * fix: add pr comment fix * docs: update changelog --------- Co-authored-by: Vladimir Trifonov Co-authored-by: Ben --- CHANGELOG.md | 2 + trace_decoder/src/decoding.rs | 336 +++++++++++++++++++++++++--------- trace_decoder/src/utils.rs | 8 + 3 files changed, 257 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d7adc06e..9b98e5fc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - fix(keccak-sponge): properly constrain padding bytes ([#158](https://github.com/0xPolygonZero/zk_evm/pull/158)) - Reduce verbosity in logs ([#160](https://github.com/0xPolygonZero/zk_evm/pull/160)) - Bump with latest starky ([#161](https://github.com/0xPolygonZero/zk_evm/pull/161)) +- Extend trace decoder err info ([#148](https://github.com/0xPolygonZero/zk_evm/pull/148)) +- Add debug function for better public values logging in development ([#134](https://github.com/0xPolygonZero/zk_evm/pull/134)) - Simplify withdrawals logic ([#168](https://github.com/0xPolygonZero/zk_evm/pull/168)) ## [0.3.0] - 2024-04-03 diff --git a/trace_decoder/src/decoding.rs b/trace_decoder/src/decoding.rs index 9e9de93a6..c0128bb54 100644 --- a/trace_decoder/src/decoding.rs +++ b/trace_decoder/src/decoding.rs @@ -4,7 +4,7 @@ use std::{ iter::{self, empty, once}, }; -use ethereum_types::{Address, H256, U256}; +use ethereum_types::{Address, H256, U256, U512}; use evm_arithmetization::{ generation::{mpt::AccountRlp, GenerationInputs, TrieInputs}, proof::{ExtraBlockData, TrieRoots}, @@ -21,22 +21,126 @@ use mpt_trie::{ use thiserror::Error; use crate::{ - processed_block_trace::{NodesUsedByTxn, ProcessedBlockTrace, StateTrieWrites, TxnMetaState}, + processed_block_trace::{ + NodesUsedByTxn, ProcessedBlockTrace, ProcessedTxnInfo, StateTrieWrites, TxnMetaState, + }, types::{ HashedAccountAddr, HashedNodeAddr, HashedStorageAddr, HashedStorageAddrNibbles, OtherBlockData, TriePathIter, TrieRootHash, TxnIdx, EMPTY_ACCOUNT_BYTES_RLPED, ZERO_STORAGE_SLOT_VAL_RLPED, }, - utils::{hash, update_val_if_some}, + utils::{hash, optional_field, optional_field_hex, update_val_if_some}, }; /// Stores the result of parsing tries. Returns a [TraceParsingError] upon /// failure. -pub type TraceParsingResult = Result; +pub type TraceParsingResult = Result>; + +/// Represents errors that can occur during the processing of a block trace. +/// +/// This struct is intended to encapsulate various kinds of errors that might +/// arise when parsing, validating, or otherwise processing the trace data of +/// blockchain blocks. It could include issues like malformed trace data, +/// inconsistencies found during processing, or any other condition that +/// prevents successful completion of the trace processing task. +#[derive(Debug)] +pub struct TraceParsingError { + block_num: Option, + block_chain_id: Option, + txn_idx: Option, + addr: Option
, + h_addr: Option, + slot: Option, + slot_value: Option, + reason: TraceParsingErrorReason, // The original error type +} + +impl std::fmt::Display for TraceParsingError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + let h_slot = self.slot.map(|slot| { + let mut buf = [0u8; 64]; + slot.to_big_endian(&mut buf); + hash(&buf) + }); + write!( + f, + "Error processing trace: {}\n{}{}{}{}{}{}{}{}", + self.reason, + optional_field("Block num", self.block_num), + optional_field("Block chain id", self.block_chain_id), + optional_field("Txn idx", self.txn_idx), + optional_field("Address", self.addr.as_ref()), + optional_field("Hashed address", self.h_addr.as_ref()), + optional_field_hex("Slot", self.slot), + optional_field("Hashed Slot", h_slot), + optional_field_hex("Slot value", self.slot_value), + ) + } +} + +impl std::error::Error for TraceParsingError {} + +impl TraceParsingError { + /// Function to create a new TraceParsingError with mandatory fields + pub(crate) fn new(reason: TraceParsingErrorReason) -> Self { + Self { + block_num: None, + block_chain_id: None, + txn_idx: None, + addr: None, + h_addr: None, + slot: None, + slot_value: None, + reason, + } + } + + /// Builder method to set block_num + pub(crate) fn block_num(&mut self, block_num: U256) -> &mut Self { + self.block_num = Some(block_num); + self + } -/// An error type for trie parsing. + /// Builder method to set block_chain_id + pub(crate) fn block_chain_id(&mut self, block_chain_id: U256) -> &mut Self { + self.block_chain_id = Some(block_chain_id); + self + } + + /// Builder method to set txn_idx + pub fn txn_idx(&mut self, txn_idx: usize) -> &mut Self { + self.txn_idx = Some(txn_idx); + self + } + + /// Builder method to set addr + pub fn addr(&mut self, addr: Address) -> &mut Self { + self.addr = Some(addr); + self + } + + /// Builder method to set h_addr + pub fn h_addr(&mut self, h_addr: H256) -> &mut Self { + self.h_addr = Some(h_addr); + self + } + + /// Builder method to set slot + pub fn slot(&mut self, slot: U512) -> &mut Self { + self.slot = Some(slot); + self + } + + /// Builder method to set slot_value + pub fn slot_value(&mut self, slot_value: U512) -> &mut Self { + self.slot_value = Some(slot_value); + self + } +} + +/// An error reason for trie parsing. #[derive(Debug, Error)] -pub enum TraceParsingError { +pub enum TraceParsingErrorReason { /// Failure to decode an Ethereum [Account]. #[error("Failed to decode RLP bytes ({0}) as an Ethereum account due to the error: {1}")] AccountDecode(String, String), @@ -66,7 +170,7 @@ pub enum TraceParsingError { impl From for TraceParsingError { fn from(err: TrieOpError) -> Self { // Convert TrieOpError into TraceParsingError - TraceParsingError::TrieOpError(err) + TraceParsingError::new(TraceParsingErrorReason::TrieOpError(err)) } } @@ -147,70 +251,24 @@ impl ProcessedBlockTrace { .into_iter() .enumerate() .map(|(txn_idx, txn_info)| { - trace!("Generating proof IR for txn {}...", txn_idx); - - Self::init_any_needed_empty_storage_tries( - &mut curr_block_tries.storage, - txn_info - .nodes_used_by_txn - .storage_accesses - .iter() - .map(|(k, _)| k), - &txn_info - .nodes_used_by_txn - .state_accounts_with_no_accesses_but_storage_tries, - ); - // For each non-dummy txn, we increment `txn_number_after` by 1, and - // update `gas_used_after` accordingly. - extra_data.txn_number_after += U256::one(); - extra_data.gas_used_after += txn_info.meta.gas_used.into(); - - // Because we need to run delta application before creating the minimal - // sub-tries (we need to detect if deletes collapsed any branches), we need to - // do this clone every iteration. - let tries_at_start_of_txn = curr_block_tries.clone(); - - Self::update_txn_and_receipt_tries(&mut curr_block_tries, &txn_info.meta, txn_idx); - - let delta_out = Self::apply_deltas_to_trie_state( - &mut curr_block_tries, - &txn_info.nodes_used_by_txn, - &txn_info.meta, - )?; - - let tries = Self::create_minimal_partial_tries_needed_by_txn( - &tries_at_start_of_txn, - &txn_info.nodes_used_by_txn, + Self::process_txn_info( txn_idx, - delta_out, - &other_data.b_data.b_meta.block_beneficiary, - )?; - - let trie_roots_after = calculate_trie_input_hashes(&curr_block_tries); - let gen_inputs = GenerationInputs { - txn_number_before: extra_data.txn_number_before, - gas_used_before: extra_data.gas_used_before, - gas_used_after: extra_data.gas_used_after, - signed_txn: txn_info.meta.txn_bytes, - withdrawals: Vec::default(), /* Only ever set in a dummy txn at the end of - * the block (see `[add_withdrawals_to_txns]` - * for more info). */ - tries, - trie_roots_after, - checkpoint_state_trie_root: extra_data.checkpoint_state_trie_root, - contract_code: txn_info.contract_code_accessed, - block_metadata: other_data.b_data.b_meta.clone(), - block_hashes: other_data.b_data.b_hashes.clone(), - }; - - // After processing a transaction, we update the remaining accumulators - // for the next transaction. - extra_data.txn_number_before += U256::one(); - extra_data.gas_used_before = extra_data.gas_used_after; - - Ok(gen_inputs) + txn_info, + &mut curr_block_tries, + &mut extra_data, + &other_data, + ) + .map_err(|mut e| { + e.txn_idx(txn_idx); + e + }) }) - .collect::>>()?; + .collect::>>() + .map_err(|mut e| { + e.block_num(other_data.b_data.b_meta.block_number); + e.block_chain_id(other_data.b_data.b_meta.block_chain_id); + e + })?; let dummies_added = Self::pad_gen_inputs_with_dummy_inputs_if_needed( &mut txn_gen_inputs, @@ -315,9 +373,15 @@ impl ProcessedBlockTrace { let mut out = TrieDeltaApplicationOutput::default(); for (hashed_acc_addr, storage_writes) in deltas.storage_writes.iter() { - let mut storage_trie = trie_state.storage.get_mut(hashed_acc_addr).ok_or( - TraceParsingError::MissingAccountStorageTrie(*hashed_acc_addr), - )?; + let mut storage_trie = + trie_state.storage.get_mut(hashed_acc_addr).ok_or_else(|| { + let hashed_acc_addr = *hashed_acc_addr; + let mut e = TraceParsingError::new( + TraceParsingErrorReason::MissingAccountStorageTrie(hashed_acc_addr), + ); + e.h_addr(hashed_acc_addr); + e + })?; for (slot, val) in storage_writes .iter() @@ -325,7 +389,13 @@ impl ProcessedBlockTrace { { // If we are writing a zero, then we actually need to perform a delete. match val == &ZERO_STORAGE_SLOT_VAL_RLPED { - false => storage_trie.insert(slot, val.clone())?, + false => storage_trie.insert(slot, val.clone()).map_err(|err| { + let mut e = + TraceParsingError::new(TraceParsingErrorReason::TrieOpError(err)); + e.slot(U512::from_big_endian(slot.bytes_be().as_slice())); + e.slot_value(U512::from_big_endian(val.as_slice())); + e + })?, true => { if let Some(remaining_slot_key) = Self::delete_node_and_report_remaining_key_if_branch_collapsed( @@ -370,10 +440,14 @@ impl ProcessedBlockTrace { for hashed_addr in deltas.self_destructed_accounts.iter() { let k = Nibbles::from_h256_be(*hashed_addr); - trie_state - .storage - .remove(hashed_addr) - .ok_or(TraceParsingError::MissingAccountStorageTrie(*hashed_addr))?; + trie_state.storage.remove(hashed_addr).ok_or_else(|| { + let hashed_addr = *hashed_addr; + let mut e = TraceParsingError::new( + TraceParsingErrorReason::MissingAccountStorageTrie(hashed_addr), + ); + e.h_addr(hashed_addr); + e + })?; // TODO: Once the mechanism for resolving code hashes settles, we probably want // to also delete the code hash mapping here as well... @@ -535,12 +609,14 @@ impl ProcessedBlockTrace { for (addr, h_addr, amt) in withdrawals { let h_addr_nibs = Nibbles::from_h256_be(h_addr); - let acc_bytes = - state - .get(h_addr_nibs) - .ok_or(TraceParsingError::MissingWithdrawalAccount( - addr, h_addr, amt, - ))?; + let acc_bytes = state.get(h_addr_nibs).ok_or_else(|| { + let mut e = TraceParsingError::new( + TraceParsingErrorReason::MissingWithdrawalAccount(addr, h_addr, amt), + ); + e.addr(addr); + e.h_addr(h_addr); + e + })?; let mut acc_data = account_from_rlped_bytes(acc_bytes)?; acc_data.balance += amt; @@ -550,6 +626,78 @@ impl ProcessedBlockTrace { Ok(()) } + + /// Processes a single transaction in the trace. + fn process_txn_info( + txn_idx: usize, + txn_info: ProcessedTxnInfo, + curr_block_tries: &mut PartialTrieState, + extra_data: &mut ExtraBlockData, + other_data: &OtherBlockData, + ) -> TraceParsingResult { + trace!("Generating proof IR for txn {}...", txn_idx); + + Self::init_any_needed_empty_storage_tries( + &mut curr_block_tries.storage, + txn_info + .nodes_used_by_txn + .storage_accesses + .iter() + .map(|(k, _)| k), + &txn_info + .nodes_used_by_txn + .state_accounts_with_no_accesses_but_storage_tries, + ); + // For each non-dummy txn, we increment `txn_number_after` by 1, and + // update `gas_used_after` accordingly. + extra_data.txn_number_after += U256::one(); + extra_data.gas_used_after += txn_info.meta.gas_used.into(); + + // Because we need to run delta application before creating the minimal + // sub-tries (we need to detect if deletes collapsed any branches), we need to + // do this clone every iteration. + let tries_at_start_of_txn = curr_block_tries.clone(); + + Self::update_txn_and_receipt_tries(curr_block_tries, &txn_info.meta, txn_idx); + + let delta_out = Self::apply_deltas_to_trie_state( + curr_block_tries, + &txn_info.nodes_used_by_txn, + &txn_info.meta, + )?; + + let tries = Self::create_minimal_partial_tries_needed_by_txn( + &tries_at_start_of_txn, + &txn_info.nodes_used_by_txn, + txn_idx, + delta_out, + &other_data.b_data.b_meta.block_beneficiary, + )?; + + let trie_roots_after = calculate_trie_input_hashes(curr_block_tries); + let gen_inputs = GenerationInputs { + txn_number_before: extra_data.txn_number_before, + gas_used_before: extra_data.gas_used_before, + gas_used_after: extra_data.gas_used_after, + signed_txn: txn_info.meta.txn_bytes, + withdrawals: Vec::default(), /* Only ever set in a dummy txn at the end of + * the block (see `[add_withdrawals_to_txns]` + * for more info). */ + tries, + trie_roots_after, + checkpoint_state_trie_root: extra_data.checkpoint_state_trie_root, + contract_code: txn_info.contract_code_accessed, + block_metadata: other_data.b_data.b_meta.clone(), + block_hashes: other_data.b_data.b_hashes.clone(), + }; + + // After processing a transaction, we update the remaining accumulators + // for the next transaction. + extra_data.txn_number_before += U256::one(); + extra_data.gas_used_before = extra_data.gas_used_after; + + Ok(gen_inputs) + } } impl StateTrieWrites { @@ -562,9 +710,14 @@ impl StateTrieWrites { let storage_root_hash_change = match self.storage_trie_change { false => None, true => { - let storage_trie = acc_storage_tries - .get(h_addr) - .ok_or(TraceParsingError::MissingAccountStorageTrie(*h_addr))?; + let storage_trie = acc_storage_tries.get(h_addr).ok_or_else(|| { + let h_addr = *h_addr; + let mut e = TraceParsingError::new( + TraceParsingErrorReason::MissingAccountStorageTrie(h_addr), + ); + e.h_addr(h_addr); + e + })?; Some(storage_trie.hash()) } @@ -756,13 +909,18 @@ fn create_trie_subset_wrapped( SubsetTrieError::UnexpectedKey(key, _) => key, }; - TraceParsingError::MissingKeysCreatingSubPartialTrie(key, trie_type) + Box::new(TraceParsingError::new( + TraceParsingErrorReason::MissingKeysCreatingSubPartialTrie(key, trie_type), + )) }) } fn account_from_rlped_bytes(bytes: &[u8]) -> TraceParsingResult { - rlp::decode(bytes) - .map_err(|err| TraceParsingError::AccountDecode(hex::encode(bytes), err.to_string())) + rlp::decode(bytes).map_err(|err| { + Box::new(TraceParsingError::new( + TraceParsingErrorReason::AccountDecode(hex::encode(bytes), err.to_string()), + )) + }) } impl TxnMetaState { diff --git a/trace_decoder/src/utils.rs b/trace_decoder/src/utils.rs index b0da716ac..ee48e770f 100644 --- a/trace_decoder/src/utils.rs +++ b/trace_decoder/src/utils.rs @@ -58,3 +58,11 @@ pub(crate) fn h_addr_nibs_to_h256(h_addr_nibs: &Nibbles) -> H256 { H256::from_slice(&nib_bytes) } + +pub(crate) fn optional_field(label: &str, value: Option) -> String { + value.map_or(String::new(), |v| format!("{}: {:?}\n", label, v)) +} + +pub(crate) fn optional_field_hex(label: &str, value: Option) -> String { + value.map_or(String::new(), |v| format!("{}: 0x{:064X}\n", label, v)) +}