From bfb7733f8b35da49264c62ba5df98618f53537c8 Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Fri, 5 Apr 2024 16:23:57 +0300 Subject: [PATCH 1/8] feat: extend trace decoder err info --- trace_decoder/src/decoding.rs | 340 +++++++++++++++++++++++++--------- 1 file changed, 253 insertions(+), 87 deletions(-) diff --git a/trace_decoder/src/decoding.rs b/trace_decoder/src/decoding.rs index 06b4e7996..902a027d0 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,7 +21,9 @@ 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, TxnProofGenIR, @@ -34,9 +36,118 @@ use crate::{ /// failure. pub type TraceParsingResult = Result; -/// An error type for trie parsing. +/// 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 { + write!(f, "Error processing trace: {}", self.reason)?; + + if let Some(block_num) = self.block_num { + write!(f, "\nBlock num: {}", block_num)?; + } + if let Some(block_chain_id) = self.block_chain_id { + write!(f, "\nBlock chain id: {}", block_chain_id)?; + } + if let Some(txn_idx) = self.txn_idx { + write!(f, "\nTxn idx: {}", txn_idx)?; + } + if let Some(addr) = self.addr { + write!(f, "\nAddress: {}", addr)?; + } + if let Some(h_addr) = self.h_addr { + write!(f, "\nHashed address: {}", h_addr)?; + } + if let Some(slot) = self.slot { + write!(f, "\nSlot: {:x}", slot)?; + } + if let Some(slot_value) = self.slot_value { + write!(f, "\nSlot value: {:x}", slot_value)?; + } + + Ok(()) + } +} + +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 + } + + /// 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 +177,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 +258,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, @@ -318,9 +383,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() @@ -328,7 +399,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( @@ -373,10 +450,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... @@ -567,12 +648,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; @@ -582,6 +665,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 { @@ -594,9 +749,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()) } @@ -788,13 +948,19 @@ fn create_trie_subset_wrapped( SubsetTrieError::UnexpectedKey(key, _) => key, }; - TraceParsingError::MissingKeysCreatingSubPartialTrie(key, trie_type) + 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| { + TraceParsingError::new(TraceParsingErrorReason::AccountDecode( + hex::encode(bytes), + err.to_string(), + )) + }) } impl TxnMetaState { From bb4620680fca0df8ee19c06fbf62e0f1531d08de Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Fri, 5 Apr 2024 16:34:51 +0300 Subject: [PATCH 2/8] fix: fix clippy issue --- trace_decoder/src/decoding.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/trace_decoder/src/decoding.rs b/trace_decoder/src/decoding.rs index 902a027d0..fb77602e6 100644 --- a/trace_decoder/src/decoding.rs +++ b/trace_decoder/src/decoding.rs @@ -34,7 +34,7 @@ use crate::{ /// 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. /// @@ -713,7 +713,7 @@ impl ProcessedBlockTrace { &other_data.b_data.b_meta.block_beneficiary, )?; - let trie_roots_after = calculate_trie_input_hashes(&curr_block_tries); + 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, @@ -948,17 +948,16 @@ fn create_trie_subset_wrapped( SubsetTrieError::UnexpectedKey(key, _) => key, }; - TraceParsingError::new(TraceParsingErrorReason::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::new(TraceParsingErrorReason::AccountDecode( - hex::encode(bytes), - err.to_string(), + Box::new(TraceParsingError::new( + TraceParsingErrorReason::AccountDecode(hex::encode(bytes), err.to_string()), )) }) } From 944eb2ce1bbc63bb736909598d8a9e9658bfbe90 Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Mon, 8 Apr 2024 07:45:34 +0300 Subject: [PATCH 3/8] 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 --- CHANGELOG.md | 1 + .../src/cpu/kernel/tests/account_code.rs | 2 +- .../src/cpu/kernel/tests/balance.rs | 2 +- .../src/cpu/kernel/tests/mpt/delete.rs | 8 +- .../src/cpu/kernel/tests/mpt/insert.rs | 2 +- evm_arithmetization/src/generation/mpt.rs | 12 +- .../src/generation/trie_extractor.rs | 6 +- mpt_trie/Cargo.toml | 4 + mpt_trie/src/nibbles.rs | 377 +++++++++++++++--- mpt_trie/src/testing_utils.rs | 6 +- mpt_trie/src/utils.rs | 8 +- 11 files changed, 345 insertions(+), 83 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dabf15584..cbfe7e52b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Update plonky2 dependencies ([#119](https://github.com/0xPolygonZero/zk_evm/pull/119)) +- Swap out the internal U512 inside nibbles to [u64;5] ([#132](https://github.com/0xPolygonZero/zk_evm/pull/132)) - Charge gas before SLOAD and refactor `insert_accessed_storage_keys` ([#117](https://github.com/0xPolygonZero/zk_evm/pull/117)) - Increased the public interface for `trie_tools` ([#123](https://github.com/0xPolygonZero/zk_evm/pull/123)) - Mpt trie panic refactor ([#118](https://github.com/0xPolygonZero/zk_evm/pull/118)) diff --git a/evm_arithmetization/src/cpu/kernel/tests/account_code.rs b/evm_arithmetization/src/cpu/kernel/tests/account_code.rs index a8102f36c..5ad2d8485 100644 --- a/evm_arithmetization/src/cpu/kernel/tests/account_code.rs +++ b/evm_arithmetization/src/cpu/kernel/tests/account_code.rs @@ -115,7 +115,7 @@ fn prepare_interpreter( .push(value_ptr.into()) .expect("The stack should not overflow"); // value_ptr interpreter - .push(k.try_into_u256().unwrap()) + .push(k.try_into().unwrap()) .expect("The stack should not overflow"); // key interpreter.run()?; diff --git a/evm_arithmetization/src/cpu/kernel/tests/balance.rs b/evm_arithmetization/src/cpu/kernel/tests/balance.rs index 984772027..034df53a8 100644 --- a/evm_arithmetization/src/cpu/kernel/tests/balance.rs +++ b/evm_arithmetization/src/cpu/kernel/tests/balance.rs @@ -67,7 +67,7 @@ fn prepare_interpreter( .push(value_ptr.into()) .expect("The stack should not overflow"); // value_ptr interpreter - .push(k.try_into_u256().unwrap()) + .push(k.try_into().unwrap()) .expect("The stack should not overflow"); // key interpreter.run()?; diff --git a/evm_arithmetization/src/cpu/kernel/tests/mpt/delete.rs b/evm_arithmetization/src/cpu/kernel/tests/mpt/delete.rs index 3f9153cda..ef8830ae7 100644 --- a/evm_arithmetization/src/cpu/kernel/tests/mpt/delete.rs +++ b/evm_arithmetization/src/cpu/kernel/tests/mpt/delete.rs @@ -1,6 +1,6 @@ use anyhow::Result; use ethereum_types::{BigEndianHash, H256, U512}; -use mpt_trie::nibbles::Nibbles; +use mpt_trie::nibbles::{Nibbles, NibblesIntern}; use mpt_trie::partial_trie::{HashedPartialTrie, PartialTrie}; use plonky2::field::goldilocks_field::GoldilocksField as F; use rand::random; @@ -70,7 +70,7 @@ fn test_after_mpt_delete_extension_branch() -> Result<()> { } .into(); let key = nibbles.merge_nibbles(&Nibbles { - packed: U512::zero(), + packed: NibblesIntern::zero(), count: 64 - nibbles.count, }); test_state_trie(state_trie, key, test_account_2()) @@ -130,7 +130,7 @@ fn test_state_trie( .push(value_ptr.into()) .expect("The stack should not overflow"); // value_ptr interpreter - .push(k.try_into_u256().unwrap()) + .push(k.try_into().unwrap()) .expect("The stack should not overflow"); // key interpreter.run()?; assert_eq!( @@ -147,7 +147,7 @@ fn test_state_trie( .push(0xDEADBEEFu32.into()) .expect("The stack should not overflow"); interpreter - .push(k.try_into_u256().unwrap()) + .push(k.try_into().unwrap()) .expect("The stack should not overflow"); interpreter .push(64.into()) diff --git a/evm_arithmetization/src/cpu/kernel/tests/mpt/insert.rs b/evm_arithmetization/src/cpu/kernel/tests/mpt/insert.rs index fcb2b5323..771921173 100644 --- a/evm_arithmetization/src/cpu/kernel/tests/mpt/insert.rs +++ b/evm_arithmetization/src/cpu/kernel/tests/mpt/insert.rs @@ -205,7 +205,7 @@ fn test_state_trie( .push(value_ptr.into()) .expect("The stack should not overflow"); // value_ptr interpreter - .push(k.try_into_u256().unwrap()) + .push(k.try_into().unwrap()) .expect("The stack should not overflow"); // key interpreter.run()?; diff --git a/evm_arithmetization/src/generation/mpt.rs b/evm_arithmetization/src/generation/mpt.rs index 79e923068..febf2a2c1 100644 --- a/evm_arithmetization/src/generation/mpt.rs +++ b/evm_arithmetization/src/generation/mpt.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use bytes::Bytes; use ethereum_types::{Address, BigEndianHash, H256, U256, U512}; use keccak_hash::keccak; -use mpt_trie::nibbles::Nibbles; +use mpt_trie::nibbles::{Nibbles, NibblesIntern}; use mpt_trie::partial_trie::{HashedPartialTrie, PartialTrie}; use rlp::{Decodable, DecoderError, Encodable, PayloadInfo, Rlp, RlpStream}; use rlp_derive::{RlpDecodable, RlpEncodable}; @@ -120,7 +120,7 @@ fn parse_storage_value(value_rlp: &[u8]) -> Result, ProgramError> { const fn empty_nibbles() -> Nibbles { Nibbles { count: 0, - packed: U512::zero(), + packed: NibblesIntern::zero(), } } @@ -171,7 +171,7 @@ where trie_data.push(nibbles.count.into()); trie_data.push( nibbles - .try_into_u256() + .try_into() .map_err(|_| ProgramError::IntegerTooLarge)?, ); trie_data.push((trie_data.len() + 1).into()); @@ -187,7 +187,7 @@ where trie_data.push(nibbles.count.into()); trie_data.push( nibbles - .try_into_u256() + .try_into() .map_err(|_| ProgramError::IntegerTooLarge)?, ); @@ -250,7 +250,7 @@ fn load_state_trie( trie_data.push(nibbles.count.into()); trie_data.push( nibbles - .try_into_u256() + .try_into() .map_err(|_| ProgramError::IntegerTooLarge)?, ); // Set `value_ptr_ptr`. @@ -286,7 +286,7 @@ fn load_state_trie( trie_data.push(nibbles.count.into()); trie_data.push( nibbles - .try_into_u256() + .try_into() .map_err(|_| ProgramError::IntegerTooLarge)?, ); // Set `value_ptr_ptr`. diff --git a/evm_arithmetization/src/generation/trie_extractor.rs b/evm_arithmetization/src/generation/trie_extractor.rs index dfea16234..0d825643f 100644 --- a/evm_arithmetization/src/generation/trie_extractor.rs +++ b/evm_arithmetization/src/generation/trie_extractor.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use ethereum_types::{BigEndianHash, H256, U256, U512}; -use mpt_trie::nibbles::Nibbles; +use mpt_trie::nibbles::{Nibbles, NibblesIntern}; use mpt_trie::partial_trie::{HashedPartialTrie, Node, PartialTrie, WrappedNode}; use super::mpt::{AccountRlp, LegacyReceiptRlp, LogRlp}; @@ -47,7 +47,7 @@ pub(crate) fn read_trie( let mut res = HashMap::new(); let empty_nibbles = Nibbles { count: 0, - packed: U512::zero(), + packed: NibblesIntern::zero(), }; read_trie_helper::(memory, ptr, read_value, empty_nibbles, &mut res)?; Ok(res) @@ -259,7 +259,7 @@ pub(crate) fn get_trie( ) -> Result { let empty_nibbles = Nibbles { count: 0, - packed: U512::zero(), + packed: NibblesIntern::zero(), }; Ok(N::new(get_trie_helper( memory, diff --git a/mpt_trie/Cargo.toml b/mpt_trie/Cargo.toml index 8eed7534c..b4ef7ded2 100644 --- a/mpt_trie/Cargo.toml +++ b/mpt_trie/Cargo.toml @@ -27,6 +27,10 @@ num-traits = "0.2.15" uint = "0.9.5" rlp = { workspace = true } serde = { workspace = true, features = ["derive", "rc"] } +impl-rlp = "0.3.0" +impl-codec = "0.6.0" +impl-serde = "0.4.0" +impl-num-traits = "0.1.2" [dev-dependencies] eth_trie = "0.4.0" diff --git a/mpt_trie/src/nibbles.rs b/mpt_trie/src/nibbles.rs index d2426dad2..ecdc2dfba 100644 --- a/mpt_trie/src/nibbles.rs +++ b/mpt_trie/src/nibbles.rs @@ -1,6 +1,7 @@ +#![allow(clippy::assign_op_pattern)] + //! Define [`Nibbles`] and how to convert bytes, hex prefix encodings and //! strings into nibbles. - use std::mem::size_of; use std::{ fmt::{self, Debug}, @@ -13,9 +14,14 @@ use std::{ }; use bytes::{Bytes, BytesMut}; -use ethereum_types::{H256, U128, U256, U512}; +use ethereum_types::{H256, U128, U256}; +use impl_codec::impl_uint_codec; +use impl_num_traits::impl_uint_num_traits; +use impl_rlp::impl_uint_rlp; +use impl_serde::impl_uint_serde; use serde::{Deserialize, Serialize}; use thiserror::Error; +use uint::construct_uint; use uint::FromHexError; use crate::utils::{create_mask_of_1s, is_even}; @@ -23,8 +29,19 @@ use crate::utils::{create_mask_of_1s, is_even}; // Use a whole byte for a Nibble just for convenience /// A Nibble has 4 bits and is stored as `u8`. pub type Nibble = u8; -/// Used for the internal representation of a sequence of nibbles. -pub type NibblesIntern = U512; + +construct_uint! { + /// Used for the internal representation of a sequence of nibbles. + /// The choice of [u64; 5] accommodates the 260-bit key requirement efficiently by + /// leveraging 64-bit instructions for performance, while providing an additional u64 + /// to handle the overflow case beyond the 256-bit capacity of [u64; 4]. + pub struct NibblesIntern(5); +} + +impl_uint_num_traits!(NibblesIntern, 5); +impl_uint_serde!(NibblesIntern, 5); +impl_uint_codec!(NibblesIntern, 5); +impl_uint_rlp!(NibblesIntern, 5); const MULTIPLE_NIBBLES_APPEND_ASSERT_ERR_MSG: &str = "Attempted to create a nibbles sequence longer than 64!"; @@ -74,11 +91,56 @@ pub enum FromHexPrefixError { /// The hex prefix encoding flag is invalid. InvalidFlags(Nibble), - #[error("Tried to convert a hex prefix byte string into `Nibbles` that was longer than 64 bytes: (length: {0}, bytes: {1})")] + #[error("Tried to convert a hex prefix byte string into `Nibbles` that was longer than 40 bytes: (length: {0}, bytes: {1})")] /// The hex prefix encoding is too large. TooLong(String, usize), } +/// Error type for conversion. +#[derive(Debug, Error)] +pub enum NibblesToTypeError { + #[error("Overflow encountered when converting to U256: {0}")] + /// Overflow encountered. + Overflow(NibblesIntern), +} + +trait AsU64s { + fn as_u64s(&self) -> impl Iterator + '_; +} + +macro_rules! impl_as_u64s_for_primitive { + ($type:ty) => { + impl AsU64s for $type { + fn as_u64s(&self) -> impl Iterator + '_ { + std::iter::once(*self as u64) + } + } + }; +} + +impl_as_u64s_for_primitive!(u8); +impl_as_u64s_for_primitive!(u16); +impl_as_u64s_for_primitive!(u32); +impl_as_u64s_for_primitive!(u64); + +impl AsU64s for U128 { + fn as_u64s(&self) -> impl Iterator + '_ { + self.0.iter().copied() + } +} + +impl AsU64s for U256 { + fn as_u64s(&self) -> impl Iterator + '_ { + self.0.iter().copied() + } +} + +impl AsU64s for NibblesIntern { + fn as_u64s(&self) -> impl Iterator + '_ { + self.0.iter().copied() + } +} + #[derive(Debug, Error)] #[error(transparent)] /// An error encountered when converting a string to a sequence of nibbles. @@ -103,11 +165,14 @@ macro_rules! impl_to_nibbles { #[allow(clippy::manual_bits)] let size_bits = size_of::() * 8; let count = (size_bits - self.leading_zeros() as usize + 3) / 4; + let mut packed = NibblesIntern::zero(); - Nibbles { - count, - packed: self.into(), + let parts = self.as_u64s(); + for (i, part) in parts.enumerate().take(packed.0.len()) { + packed.0[i] = part; } + + Nibbles { count, packed } } } }; @@ -119,7 +184,60 @@ impl_to_nibbles!(u32); impl_to_nibbles!(u64); impl_to_nibbles!(U128); impl_to_nibbles!(U256); -impl_to_nibbles!(U512); +impl_to_nibbles!(NibblesIntern); + +impl<'a> TryFrom<&'a Nibbles> for U256 { + type Error = NibblesToTypeError; + + fn try_from(value: &'a Nibbles) -> Result { + let NibblesIntern(ref arr) = value.packed; + if arr[4] != 0 { + return Err(NibblesToTypeError::Overflow(value.packed)); + } + + let ret = [arr[0], arr[1], arr[2], arr[3]]; + Ok(U256(ret)) + } +} + +impl<'a> TryFrom<&'a NibblesIntern> for U256 { + type Error = NibblesToTypeError; + + fn try_from(value: &'a NibblesIntern) -> Result { + if value.0[4] != 0 { + return Err(NibblesToTypeError::Overflow(*value)); + } + + let ret = [value.0[0], value.0[1], value.0[2], value.0[3]]; + Ok(U256(ret)) + } +} + +impl TryInto for Nibbles { + type Error = NibblesToTypeError; + + fn try_into(self) -> Result { + let arr = self.packed; + if arr.0[4] != 0 { + return Err(NibblesToTypeError::Overflow(arr)); + } + + let ret = [arr.0[0], arr.0[1], arr.0[2], arr.0[3]]; + Ok(U256(ret)) + } +} + +impl From for NibblesIntern { + fn from(val: U256) -> Self { + let arr = val.as_u64s(); + + let mut ret = NibblesIntern::zero(); + for (i, part) in arr.enumerate() { + ret.0[i] = part; + } + ret + } +} #[derive(Copy, Clone, Deserialize, Default, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] /// A sequence of nibbles which is used as the key type into @@ -190,7 +308,6 @@ impl Debug for Nibbles { } } -// TODO: This impl is going to need to change once we move away from `U256`s... impl FromStr for Nibbles { type Err = StrToNibblesError; @@ -201,7 +318,7 @@ impl FromStr for Nibbles { .chars() .position(|c| c != '0') .unwrap_or(stripped_str.len()); - let packed = U512::from_str(s)?; + let packed = NibblesIntern::from_str(s)?; Ok(Self { count: leading_zeros + Self::get_num_nibbles_in_key(&packed), @@ -228,7 +345,7 @@ impl Nibbles { /// Returns an error if the byte slice is empty or is longer than `32` /// bytes. pub fn from_bytes_be(bytes: &[u8]) -> Result { - Self::from_bytes(bytes, U512::from_big_endian) + Self::from_bytes(bytes, NibblesIntern::from_big_endian) } /// Creates `Nibbles` from little endian bytes. @@ -236,7 +353,7 @@ impl Nibbles { /// Returns an error if the byte slice is empty or is longer than `32` /// bytes. pub fn from_bytes_le(bytes: &[u8]) -> Result { - Self::from_bytes(bytes, U512::from_little_endian) + Self::from_bytes(bytes, NibblesIntern::from_little_endian) } fn from_bytes(bytes: &[u8], conv_f: F) -> Result @@ -350,7 +467,7 @@ impl Nibbles { let shift_amt = 4 * self.count; self.count += 1; - self.packed |= U512::from(n) << shift_amt; + self.packed |= NibblesIntern::from(n) << shift_amt; } /// Pushes a nibble to the back. @@ -564,8 +681,8 @@ impl Nibbles { /// Reverses the `Nibbles` such that the last `Nibble` is now the first /// `Nibble`. pub fn reverse(&self) -> Nibbles { - let mut mask = U512::from(0xf); - let mut reversed_packed = U512::zero(); + let mut mask = NibblesIntern::from(0xf); + let mut reversed_packed = NibblesIntern::zero(); for i in 0..self.count { reversed_packed <<= 4; @@ -615,7 +732,7 @@ impl Nibbles { return n1.count; } - let mut curr_mask: U512 = (U512::from(0xf)) << ((n1.count - 1) * 4); + let mut curr_mask: NibblesIntern = (NibblesIntern::from(0xf)) << ((n1.count - 1) * 4); for i in 0..n1.count { if n1.packed & curr_mask != n2.packed & curr_mask { return i; @@ -632,11 +749,11 @@ impl Nibbles { where F: Fn(&[u8]) -> String, { - let mut byte_buf = [0; 64]; + let mut byte_buf = [0; 40]; self.packed.to_big_endian(&mut byte_buf); let count_bytes = self.min_bytes(); - let hex_string_raw = hex_encode_f(&byte_buf[(64 - count_bytes)..64]); + let hex_string_raw = hex_encode_f(&byte_buf[(40 - count_bytes)..40]); let hex_char_iter_raw = hex_string_raw.chars(); // We need this skip to make both match arms have the same type. @@ -656,10 +773,10 @@ impl Nibbles { pub fn to_hex_prefix_encoding(&self, is_leaf: bool) -> Bytes { let num_nibbles = self.count + 1; let num_bytes = (num_nibbles + 1) / 2; - let flag_byte_idx = 65 - num_bytes; + let flag_byte_idx = 41 - num_bytes; // Needed because `to_big_endian` always writes `32` bytes. - let mut bytes = BytesMut::zeroed(65); + let mut bytes = BytesMut::zeroed(41); let is_even = is_even(self.count); let odd_bit = match is_even { @@ -673,10 +790,10 @@ impl Nibbles { }; let flags: u8 = (odd_bit | (term_bit << 1)) << 4; - self.packed.to_big_endian(&mut bytes[1..65]); + self.packed.to_big_endian(&mut bytes[1..41]); bytes[flag_byte_idx] |= flags; - Bytes::copy_from_slice(&bytes[flag_byte_idx..65]) + Bytes::copy_from_slice(&bytes[flag_byte_idx..41]) } /// Converts a hex prefix byte string ("AKA "compact") into `Nibbles`. @@ -710,7 +827,7 @@ impl Nibbles { let hex_prefix_byes_iter = hex_prefix_bytes.iter().skip(1).take(32).cloned(); let mut i = 0; - let mut nibbles_raw = [0; 64]; + let mut nibbles_raw = [0; 40]; if odd_nib_count { Self::write_byte_iter_to_byte_buf( @@ -730,7 +847,7 @@ impl Nibbles { let x = Self { count, - packed: U512::from_big_endian(&nibbles_raw[..tot_bytes]), + packed: NibblesIntern::from_big_endian(&nibbles_raw[..tot_bytes]), }; Ok(x) @@ -749,31 +866,29 @@ impl Nibbles { } /// Returns the minimum number of nibbles needed to represent a `U256` key. - pub fn get_num_nibbles_in_key(k: &U512) -> usize { + pub fn get_num_nibbles_in_key(k: &NibblesIntern) -> usize { (k.bits() + 3) / 4 } - // TODO: Make not terrible at some point... Consider moving away from `U256` - // internally? /// Returns the nibbles bytes in big-endian format. pub fn bytes_be(&self) -> Vec { - let mut byte_buf = [0; 64]; + let mut byte_buf = [0; 40]; self.packed.to_big_endian(&mut byte_buf); - byte_buf[64 - self.min_bytes()..64].to_vec() + byte_buf[40 - self.min_bytes()..40].to_vec() } /// Creates `Nibbles` from a big endian `H256`. pub fn from_h256_be(v: H256) -> Self { - Self::from_h256_common(|v| U512::from_big_endian(v.as_bytes()), v) + Self::from_h256_common(|v| NibblesIntern::from_big_endian(v.as_bytes()), v) } /// Creates `Nibbles` from a little endian `H256`. pub fn from_h256_le(v: H256) -> Self { - Self::from_h256_common(|v| U512::from_little_endian(v.as_bytes()), v) + Self::from_h256_common(|v| NibblesIntern::from_little_endian(v.as_bytes()), v) } - fn from_h256_common U512>(conv_f: F, v: H256) -> Self { + fn from_h256_common NibblesIntern>(conv_f: F, v: H256) -> Self { Self { count: 64, packed: conv_f(v), @@ -796,18 +911,6 @@ impl Nibbles { MULTIPLE_NIBBLES_APPEND_ASSERT_ERR_MSG ); } - - // TODO: REMOVE BEFORE NEXT CRATE VERSION! THIS IS A TEMP HACK! - /// Converts to u256 returning an error if not possible. - pub fn try_into_u256(&self) -> Result { - match self.count <= 64 { - false => Err(format!( - "Tried getting a U256 from Nibbles that were too big! ({:?})", - self - )), - true => Ok(self.packed.try_into().unwrap()), - } - } } #[cfg(test)] @@ -816,7 +919,7 @@ mod tests { use ethereum_types::{H256, U256}; - use super::{Nibble, Nibbles, ToNibbles}; + use super::{Nibble, Nibbles, StrToNibblesError, ToNibbles}; use crate::nibbles::FromHexPrefixError; const ZERO_NIBS_63: &str = "0x000000000000000000000000000000000000000000000000000000000000000"; @@ -827,23 +930,42 @@ mod tests { "0x0000000000000000000000000000000000000000000000000000000000000001"; #[test] - fn get_nibble_works() { + fn get_nibble_works() -> Result<(), StrToNibblesError> { let n = Nibbles::from(0x1234); - assert_eq!(n.get_nibble(0), 0x1); assert_eq!(n.get_nibble(3), 0x4); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!(n.get_nibble(30), 0x1); + assert_eq!(n.get_nibble(33), 0xc); + + Ok(()) } #[test] fn pop_nibble_front_works() { pop_and_assert_nibbles(0x1, 0x0, 1, Nibbles::pop_next_nibble_front); pop_and_assert_nibbles(0x1234, 0x234, 1, Nibbles::pop_next_nibble_front); + pop_and_assert_nibbles( + 0x1234567890123, + 0x234567890123, + 1, + Nibbles::pop_next_nibble_front, + ); } #[test] fn pop_nibble_back_works() { pop_and_assert_nibbles(0x1, 0x0, 1, Nibbles::pop_next_nibble_back); pop_and_assert_nibbles(0x1234, 0x123, 4, Nibbles::pop_next_nibble_back); + pop_and_assert_nibbles( + 0x1234567890123, + 0x123456789012, + 3, + Nibbles::pop_next_nibble_back, + ); } fn pop_and_assert_nibbles Nibble>( @@ -868,7 +990,7 @@ mod tests { } #[test] - fn pop_nibbles_front_works() { + fn pop_nibbles_front_works() -> Result<(), StrToNibblesError> { let nib = 0x1234.into(); assert_pop_nibbles( @@ -899,10 +1021,30 @@ mod tests { 0x1234.into(), Nibbles::pop_nibbles_front, ); + assert_pop_nibbles( + &nib, + 4, + 0x0.into(), + 0x1234.into(), + Nibbles::pop_nibbles_front, + ); + + let nib = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_pop_nibbles( + &nib, + 24, + Nibbles::from_str("0x0ffd1e165c754b28a41a95922f9f70682c581353")?, + Nibbles::from_str("0x3ab76c381c0f8ea617ea9678")?, + Nibbles::pop_nibbles_front, + ); + + Ok(()) } #[test] - fn pop_nibbles_back_works() { + fn pop_nibbles_back_works() -> Result<(), StrToNibblesError> { let nib = 0x1234.into(); assert_pop_nibbles( @@ -921,6 +1063,19 @@ mod tests { 0x4321.into(), Nibbles::pop_nibbles_back, ); + + let nib = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_pop_nibbles( + &nib, + 24, + Nibbles::from_str("0x3ab76c381c0f8ea617ea96780ffd1e165c754b28")?, + Nibbles::from_str("0x353185c28607f9f22959a14a")?, + Nibbles::pop_nibbles_back, + ); + + Ok(()) } fn assert_pop_nibbles Nibbles>( @@ -997,7 +1152,7 @@ mod tests { } #[test] - fn get_next_nibbles_works() { + fn get_next_nibbles_works() -> Result<(), StrToNibblesError> { let n: Nibbles = 0x1234.into(); assert_eq!(n.get_next_nibbles(0), Nibbles::default()); @@ -1007,20 +1162,37 @@ mod tests { assert_eq!(n.get_next_nibbles(4), Nibbles::from(0x1234)); assert_eq!(Nibbles::from(0x0).get_next_nibbles(0), Nibbles::default()); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!( + n.get_next_nibbles(24), + Nibbles::from_str("0x3ab76c381c0f8ea617ea9678")? + ); + + Ok(()) } #[test] - fn get_nibble_range_works() { + fn get_nibble_range_works() -> Result<(), StrToNibblesError> { let n: Nibbles = 0x1234.into(); assert_eq!(n.get_nibble_range(0..0), 0x0.into()); assert_eq!(n.get_nibble_range(0..1), 0x1.into()); assert_eq!(n.get_nibble_range(0..2), 0x12.into()); assert_eq!(n.get_nibble_range(0..4), 0x1234.into()); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!(n.get_nibble_range(16..24), Nibbles::from_str("0x17ea9678")?); + + Ok(()) } #[test] - fn truncate_nibbles_works() { + fn truncate_nibbles_works() -> Result<(), StrToNibblesError> { let n: Nibbles = 0x1234.into(); assert_eq!(n.truncate_n_nibbles_front(0), n); @@ -1036,16 +1208,43 @@ mod tests { assert_eq!(n.truncate_n_nibbles_back(3), 0x1.into()); assert_eq!(n.truncate_n_nibbles_back(4), 0x0.into()); assert_eq!(n.truncate_n_nibbles_back(8), 0x0.into()); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!( + n.truncate_n_nibbles_front(16), + Nibbles::from_str("0x17ea96780ffd1e165c754b28a41a95922f9f70682c581353")? + ); + assert_eq!( + n.truncate_n_nibbles_back(16), + Nibbles::from_str("0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a9592")? + ); + + Ok(()) } #[test] - fn split_at_idx_works() { + fn split_at_idx_works() -> Result<(), StrToNibblesError> { let n: Nibbles = 0x1234.into(); assert_eq!(n.split_at_idx(0), (0x0.into(), 0x1234.into())); assert_eq!(n.split_at_idx(1), (0x1.into(), 0x234.into())); assert_eq!(n.split_at_idx(2), (0x12.into(), 0x34.into())); assert_eq!(n.split_at_idx(3), (0x123.into(), 0x4.into())); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!( + n.split_at_idx(24), + ( + Nibbles::from_str("0x3ab76c381c0f8ea617ea9678")?, + Nibbles::from_str("0x0ffd1e165c754b28a41a95922f9f70682c581353")? + ) + ); + + Ok(()) } #[test] @@ -1055,31 +1254,60 @@ mod tests { } #[test] - fn split_at_idx_prefix_works() { + fn split_at_idx_prefix_works() -> Result<(), StrToNibblesError> { let n: Nibbles = 0x1234.into(); assert_eq!(n.split_at_idx_prefix(0), 0x0.into()); assert_eq!(n.split_at_idx_prefix(1), 0x1.into()); assert_eq!(n.split_at_idx_prefix(3), 0x123.into()); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!( + n.split_at_idx_prefix(24), + Nibbles::from_str("0x3ab76c381c0f8ea617ea9678")? + ); + + Ok(()) } #[test] - fn split_at_idx_postfix_works() { + fn split_at_idx_postfix_works() -> Result<(), StrToNibblesError> { let n: Nibbles = 0x1234.into(); assert_eq!(n.split_at_idx_postfix(0), 0x1234.into()); assert_eq!(n.split_at_idx_postfix(1), 0x234.into()); assert_eq!(n.split_at_idx_postfix(3), 0x4.into()); + + let n = Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353", + )?; + assert_eq!( + n.split_at_idx_postfix(24), + Nibbles::from_str("0x0ffd1e165c754b28a41a95922f9f70682c581353")? + ); + + Ok(()) } #[test] - fn merge_nibble_works() { + fn merge_nibble_works() -> Result<(), StrToNibblesError> { assert_eq!(Nibbles::from(0x0).merge_nibble(1), 0x1.into()); assert_eq!(Nibbles::from(0x1234).merge_nibble(5), 0x12345.into()); + assert_eq!( + Nibbles::from_str("0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c58135")? + .merge_nibble(3), + Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353" + )? + ); + + Ok(()) } #[test] - fn merge_nibbles_works() { + fn merge_nibbles_works() -> Result<(), StrToNibblesError> { assert_eq!( Nibbles::from(0x12).merge_nibbles(&(0x34.into())), 0x1234.into() @@ -1093,18 +1321,34 @@ mod tests { 0x34.into() ); assert_eq!(Nibbles::from(0x0).merge_nibbles(&(0x0).into()), 0x0.into()); + assert_eq!( + Nibbles::from_str("0x3ab76c381c0f8ea617ea96780ffd1e1")? + .merge_nibbles(&Nibbles::from_str("0x65c754b28a41a95922f9f70682c581353")?), + Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c581353" + )? + ); + + Ok(()) } #[test] - fn reverse_works() { + fn reverse_works() -> Result<(), StrToNibblesError> { assert_eq!(Nibbles::from(0x0).reverse(), Nibbles::from(0x0_u64)); assert_eq!(Nibbles::from(0x1).reverse(), Nibbles::from(0x1_u64)); assert_eq!(Nibbles::from(0x12).reverse(), Nibbles::from(0x21_u64)); assert_eq!(Nibbles::from(0x1234).reverse(), Nibbles::from(0x4321_u64)); + assert_eq!( + Nibbles::from_str("0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c58135")? + .reverse(), + Nibbles::from_str("0x53185c28607f9f22959a14a82b457c561e1dff08769ae716ae8f0c183c67ba3")? + ); + + Ok(()) } #[test] - fn find_nibble_idx_that_differs_between_nibbles_works() { + fn find_nibble_idx_that_differs_between_nibbles_works() -> Result<(), StrToNibblesError> { assert_eq!( Nibbles::find_nibble_idx_that_differs_between_nibbles_equal_lengths( &(0x1234.into()), @@ -1140,6 +1384,19 @@ mod tests { ), 4 ); + assert_eq!( + Nibbles::find_nibble_idx_that_differs_between_nibbles_different_lengths( + &(Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41a95922f9f70682c58135" + )?), + &(Nibbles::from_str( + "0x3ab76c381c0f8ea617ea96780ffd1e165c754b28a41ae716ae8f0c183c67ba3" + )?), + ), + 44 + ); + + Ok(()) } #[test] diff --git a/mpt_trie/src/testing_utils.rs b/mpt_trie/src/testing_utils.rs index 58b344d79..cc8a0daef 100644 --- a/mpt_trie/src/testing_utils.rs +++ b/mpt_trie/src/testing_utils.rs @@ -3,12 +3,12 @@ use std::{ iter::{once, repeat}, }; -use ethereum_types::{H256, U256, U512}; +use ethereum_types::{H256, U256}; use log::info; use rand::{rngs::StdRng, seq::IteratorRandom, Rng, RngCore, SeedableRng}; use crate::{ - nibbles::Nibbles, + nibbles::{Nibbles, NibblesIntern}, partial_trie::{HashedPartialTrie, Node, PartialTrie}, trie_ops::{TrieOpResult, ValOrHash}, utils::is_even, @@ -29,7 +29,7 @@ type TestInsertEntry = (Nibbles, T); // Don't want this exposed publicly, but it is useful for testing. impl From for Nibbles { fn from(k: i32) -> Self { - let packed = U512::from(k); + let packed = NibblesIntern::from(k); Self { count: Self::get_num_nibbles_in_key(&packed), diff --git a/mpt_trie/src/utils.rs b/mpt_trie/src/utils.rs index 211c08861..5c1c1ca50 100644 --- a/mpt_trie/src/utils.rs +++ b/mpt_trie/src/utils.rs @@ -7,11 +7,11 @@ use std::{ sync::Arc, }; -use ethereum_types::{H256, U512}; +use ethereum_types::H256; use num_traits::PrimInt; use crate::{ - nibbles::{Nibble, Nibbles}, + nibbles::{Nibble, Nibbles, NibblesIntern}, partial_trie::{Node, PartialTrie}, trie_ops::TrieOpResult, }; @@ -71,8 +71,8 @@ pub(crate) fn is_even>(num: T) -> bool { (num & T::one()) == T::zero() } -pub(crate) fn create_mask_of_1s(amt: usize) -> U512 { - (U512::one() << amt) - 1 +pub(crate) fn create_mask_of_1s(amt: usize) -> NibblesIntern { + (NibblesIntern::one() << amt) - 1 } pub(crate) fn bytes_to_h256(b: &[u8; 32]) -> H256 { From f06be462ff0e8a568e9dc50b6c16e5830045b0bd Mon Sep 17 00:00:00 2001 From: Ben Date: Mon, 8 Apr 2024 13:22:56 +0500 Subject: [PATCH 4/8] Some clippy fixes (#149) * MAX fixes for clippy * fix transmut without annotations * please the fmt gods --- evm_arithmetization/src/arithmetic/modular.rs | 8 ++++---- evm_arithmetization/src/fixed_recursive_verifier.rs | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/evm_arithmetization/src/arithmetic/modular.rs b/evm_arithmetization/src/arithmetic/modular.rs index 70761d4be..f91bebe99 100644 --- a/evm_arithmetization/src/arithmetic/modular.rs +++ b/evm_arithmetization/src/arithmetic/modular.rs @@ -307,7 +307,7 @@ pub(crate) fn generate_modular_op( let (lo, hi) = quot_limbs.split_at_mut(N_LIMBS); // Verify that the elements are in the expected range. - debug_assert!(lo.iter().all(|&c| c <= u16::max_value() as i64)); + debug_assert!(lo.iter().all(|&c| c <= u16::MAX as i64)); // Top half of quot_limbs should be zero. debug_assert!(hi.iter().all(|&d| d.is_zero())); @@ -318,7 +318,7 @@ pub(crate) fn generate_modular_op( // it's in the range [0, 2^16 - 1] which will correctly // range-check. for c in lo { - *c += u16::max_value() as i64; + *c += u16::MAX as i64; } // Store the sign of the quotient after the quotient. hi[0] = 1; @@ -522,7 +522,7 @@ pub(crate) fn submod_constr_poly( let sign = hi[0]; // sign must be 1 (negative) or 0 (positive) yield_constr.constraint(filter * sign * (sign - P::ONES)); - let offset = P::Scalar::from_canonical_u16(u16::max_value()); + let offset = P::Scalar::from_canonical_u16(u16::MAX); for c in lo { *c -= offset * sign; } @@ -723,7 +723,7 @@ pub(crate) fn submod_constr_poly_ext_circuit, const let t = builder.mul_extension(filter, t); // sign must be 1 (negative) or 0 (positive) yield_constr.constraint(builder, t); - let offset = F::from_canonical_u16(u16::max_value()); + let offset = F::from_canonical_u16(u16::MAX); for c in lo { let t = builder.mul_const_extension(offset, sign); *c = builder.sub_extension(*c, t); diff --git a/evm_arithmetization/src/fixed_recursive_verifier.rs b/evm_arithmetization/src/fixed_recursive_verifier.rs index a60b9e7e3..be65eb413 100644 --- a/evm_arithmetization/src/fixed_recursive_verifier.rs +++ b/evm_arithmetization/src/fixed_recursive_verifier.rs @@ -399,7 +399,10 @@ where *table = MaybeUninit::new(value); } unsafe { - mem::transmute::<_, [RecursiveCircuitsForTable; NUM_TABLES]>(by_table) + mem::transmute::< + [std::mem::MaybeUninit>; NUM_TABLES], + [RecursiveCircuitsForTable; NUM_TABLES], + >(by_table) } } }; From ee0fabb910a364ba898de88d991d8ee044069b17 Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Tue, 9 Apr 2024 10:39:16 +0300 Subject: [PATCH 5/8] fix: add pr comments fixes --- trace_decoder/src/decoding.rs | 29 +++++++++-------------------- trace_decoder/src/utils.rs | 11 +++++++++++ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/trace_decoder/src/decoding.rs b/trace_decoder/src/decoding.rs index fb77602e6..e09bdcfc6 100644 --- a/trace_decoder/src/decoding.rs +++ b/trace_decoder/src/decoding.rs @@ -29,7 +29,7 @@ use crate::{ OtherBlockData, TriePathIter, TrieRootHash, TxnIdx, TxnProofGenIR, EMPTY_ACCOUNT_BYTES_RLPED, ZERO_STORAGE_SLOT_VAL_RLPED, }, - utils::{hash, update_val_if_some}, + utils::{hash, update_val_if_some, write_optional}, }; /// Stores the result of parsing tries. Returns a [TraceParsingError] upon @@ -57,28 +57,17 @@ pub struct TraceParsingError { impl std::fmt::Display for TraceParsingError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "Error processing trace: {}", self.reason)?; - - if let Some(block_num) = self.block_num { - write!(f, "\nBlock num: {}", block_num)?; - } - if let Some(block_chain_id) = self.block_chain_id { - write!(f, "\nBlock chain id: {}", block_chain_id)?; - } - if let Some(txn_idx) = self.txn_idx { - write!(f, "\nTxn idx: {}", txn_idx)?; - } - if let Some(addr) = self.addr { - write!(f, "\nAddress: {}", addr)?; - } - if let Some(h_addr) = self.h_addr { - write!(f, "\nHashed address: {}", h_addr)?; - } + writeln!(f, "Error processing trace: {}", self.reason)?; + write_optional(f, "Block num", self.block_num)?; + write_optional(f, "Block chain id", self.block_chain_id)?; + write_optional(f, "Txn idx", self.txn_idx)?; + write_optional(f, "Address", self.addr.as_ref())?; + write_optional(f, "Hashed address", self.h_addr.as_ref())?; if let Some(slot) = self.slot { - write!(f, "\nSlot: {:x}", slot)?; + writeln!(f, "Slot: {:x}", slot)?; } if let Some(slot_value) = self.slot_value { - write!(f, "\nSlot value: {:x}", slot_value)?; + writeln!(f, "Slot value: {:x}", slot_value)?; } Ok(()) diff --git a/trace_decoder/src/utils.rs b/trace_decoder/src/utils.rs index 678fcc064..8471597f5 100644 --- a/trace_decoder/src/utils.rs +++ b/trace_decoder/src/utils.rs @@ -57,3 +57,14 @@ pub(crate) fn h_addr_nibs_to_h256(h_addr_nibs: &Nibbles) -> H256 { H256::from_slice(&nib_bytes) } + +pub(crate) fn write_optional( + f: &mut std::fmt::Formatter, + label: &str, + value: Option, +) -> std::fmt::Result { + if let Some(v) = value { + writeln!(f, "{}: {}", label, v)?; + } + Ok(()) +} From cada63a810d829c82771f7a54c6d97669c0383e7 Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Mon, 15 Apr 2024 13:41:49 +0300 Subject: [PATCH 6/8] fix: add pr comments fix --- trace_decoder/src/decoding.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/trace_decoder/src/decoding.rs b/trace_decoder/src/decoding.rs index e09bdcfc6..0e171608f 100644 --- a/trace_decoder/src/decoding.rs +++ b/trace_decoder/src/decoding.rs @@ -65,6 +65,9 @@ impl std::fmt::Display for TraceParsingError { write_optional(f, "Hashed address", self.h_addr.as_ref())?; if let Some(slot) = self.slot { writeln!(f, "Slot: {:x}", slot)?; + let mut buf = [0u8; 64]; + slot.to_big_endian(&mut buf); + writeln!(f, "Hashed Slot: {}", hash(&buf))?; } if let Some(slot_value) = self.slot_value { writeln!(f, "Slot value: {:x}", slot_value)?; From 24b6da37f283130b78524979901f4ce05f44a77c Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Tue, 16 Apr 2024 11:42:51 +0300 Subject: [PATCH 7/8] fix: add pr comment fix --- trace_decoder/src/decoding.rs | 33 +++++++++++++++++---------------- trace_decoder/src/utils.rs | 15 ++++++--------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/trace_decoder/src/decoding.rs b/trace_decoder/src/decoding.rs index 0e171608f..723d884c2 100644 --- a/trace_decoder/src/decoding.rs +++ b/trace_decoder/src/decoding.rs @@ -29,7 +29,7 @@ use crate::{ OtherBlockData, TriePathIter, TrieRootHash, TxnIdx, TxnProofGenIR, EMPTY_ACCOUNT_BYTES_RLPED, ZERO_STORAGE_SLOT_VAL_RLPED, }, - utils::{hash, update_val_if_some, write_optional}, + utils::{hash, optional_field, optional_field_hex, update_val_if_some}, }; /// Stores the result of parsing tries. Returns a [TraceParsingError] upon @@ -57,23 +57,24 @@ pub struct TraceParsingError { impl std::fmt::Display for TraceParsingError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - writeln!(f, "Error processing trace: {}", self.reason)?; - write_optional(f, "Block num", self.block_num)?; - write_optional(f, "Block chain id", self.block_chain_id)?; - write_optional(f, "Txn idx", self.txn_idx)?; - write_optional(f, "Address", self.addr.as_ref())?; - write_optional(f, "Hashed address", self.h_addr.as_ref())?; - if let Some(slot) = self.slot { - writeln!(f, "Slot: {:x}", slot)?; + let h_slot = self.slot.map(|slot| { let mut buf = [0u8; 64]; slot.to_big_endian(&mut buf); - writeln!(f, "Hashed Slot: {}", hash(&buf))?; - } - if let Some(slot_value) = self.slot_value { - writeln!(f, "Slot value: {:x}", slot_value)?; - } - - Ok(()) + 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), + ) } } diff --git a/trace_decoder/src/utils.rs b/trace_decoder/src/utils.rs index 8471597f5..d70b227ca 100644 --- a/trace_decoder/src/utils.rs +++ b/trace_decoder/src/utils.rs @@ -58,13 +58,10 @@ pub(crate) fn h_addr_nibs_to_h256(h_addr_nibs: &Nibbles) -> H256 { H256::from_slice(&nib_bytes) } -pub(crate) fn write_optional( - f: &mut std::fmt::Formatter, - label: &str, - value: Option, -) -> std::fmt::Result { - if let Some(v) = value { - writeln!(f, "{}: {}", label, v)?; - } - Ok(()) +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)) } From 54a94807a9b65d2603b0643a0b9be20c85dec295 Mon Sep 17 00:00:00 2001 From: Vladimir Trifonov Date: Thu, 18 Apr 2024 18:47:06 +0300 Subject: [PATCH 8/8] docs: update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c1732afd..647fc8288 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)) ## [0.3.0] - 2024-04-03