From bf1987745804178009cfd57894a2daf539d4290c Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Thu, 3 Oct 2024 19:49:13 -0400 Subject: [PATCH 1/4] Fix missing code read in state write --- trace_decoder/src/core.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/trace_decoder/src/core.rs b/trace_decoder/src/core.rs index eb598b6ae..82afaa56f 100644 --- a/trace_decoder/src/core.rs +++ b/trace_decoder/src/core.rs @@ -433,7 +433,10 @@ fn middle( acct.code_hash = code_usage .map(|it| match it { ContractCodeUsage::Read(hash) => { - batch_contract_code.insert(code.get(hash)?); + let _ = code + .get(hash) + .map(|bytecode| batch_contract_code.insert(bytecode)); + anyhow::Ok(hash) } ContractCodeUsage::Write(bytes) => { From 3a750e9b327edcf8da0e75d56036fd3bb5399dab Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Thu, 3 Oct 2024 20:24:03 -0400 Subject: [PATCH 2/4] Tweak --- trace_decoder/src/core.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/trace_decoder/src/core.rs b/trace_decoder/src/core.rs index 82afaa56f..31312d250 100644 --- a/trace_decoder/src/core.rs +++ b/trace_decoder/src/core.rs @@ -176,7 +176,7 @@ fn start( .map(|v| (k, v)) }) .collect::>()?; - (state, storage, Hash2Code::new()) + (state, storage, Hash2Code::new(false)) } BlockTraceTriePreImages::Combined(CombinedPreImages { compact }) => { let instructions = crate::wire::parse(&compact) @@ -433,9 +433,9 @@ fn middle( acct.code_hash = code_usage .map(|it| match it { ContractCodeUsage::Read(hash) => { - let _ = code - .get(hash) - .map(|bytecode| batch_contract_code.insert(bytecode)); + if let Some(bytecode) = code.get(hash)? { + batch_contract_code.insert(bytecode); + }; anyhow::Ok(hash) } @@ -760,20 +760,29 @@ fn map_receipt_bytes(bytes: Vec) -> anyhow::Result> { struct Hash2Code { /// Key must always be [`hash`] of value. inner: HashMap>, + /// Flag to allow missing values in the inner map. + allow_missing_code: bool, } impl Hash2Code { - pub fn new() -> Self { + pub fn new(allow_missing_code: bool) -> Self { let mut this = Self { inner: HashMap::new(), + allow_missing_code, }; this.insert(vec![]); this } - pub fn get(&mut self, hash: H256) -> anyhow::Result> { + pub fn get(&mut self, hash: H256) -> anyhow::Result>> { match self.inner.get(&hash) { - Some(code) => Ok(code.clone()), - None => bail!("no code for hash {:x}", hash), + Some(code) => Ok(Some(code.clone())), + None => { + if self.allow_missing_code { + Ok(None) + } else { + bail!("no code for hash {:x}", hash) + } + } } } pub fn insert(&mut self, code: Vec) { @@ -791,7 +800,7 @@ impl Extend> for Hash2Code { impl FromIterator> for Hash2Code { fn from_iter>>(iter: II) -> Self { - let mut this = Self::new(); + let mut this = Self::new(true); this.extend(iter); this } From 5d31cf6c825d43d82e0e0408e68b33743b34249d Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Thu, 3 Oct 2024 21:22:26 -0400 Subject: [PATCH 3/4] Revert "Tweak" This reverts commit 3a750e9b327edcf8da0e75d56036fd3bb5399dab. --- trace_decoder/src/core.rs | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/trace_decoder/src/core.rs b/trace_decoder/src/core.rs index 31312d250..82afaa56f 100644 --- a/trace_decoder/src/core.rs +++ b/trace_decoder/src/core.rs @@ -176,7 +176,7 @@ fn start( .map(|v| (k, v)) }) .collect::>()?; - (state, storage, Hash2Code::new(false)) + (state, storage, Hash2Code::new()) } BlockTraceTriePreImages::Combined(CombinedPreImages { compact }) => { let instructions = crate::wire::parse(&compact) @@ -433,9 +433,9 @@ fn middle( acct.code_hash = code_usage .map(|it| match it { ContractCodeUsage::Read(hash) => { - if let Some(bytecode) = code.get(hash)? { - batch_contract_code.insert(bytecode); - }; + let _ = code + .get(hash) + .map(|bytecode| batch_contract_code.insert(bytecode)); anyhow::Ok(hash) } @@ -760,29 +760,20 @@ fn map_receipt_bytes(bytes: Vec) -> anyhow::Result> { struct Hash2Code { /// Key must always be [`hash`] of value. inner: HashMap>, - /// Flag to allow missing values in the inner map. - allow_missing_code: bool, } impl Hash2Code { - pub fn new(allow_missing_code: bool) -> Self { + pub fn new() -> Self { let mut this = Self { inner: HashMap::new(), - allow_missing_code, }; this.insert(vec![]); this } - pub fn get(&mut self, hash: H256) -> anyhow::Result>> { + pub fn get(&mut self, hash: H256) -> anyhow::Result> { match self.inner.get(&hash) { - Some(code) => Ok(Some(code.clone())), - None => { - if self.allow_missing_code { - Ok(None) - } else { - bail!("no code for hash {:x}", hash) - } - } + Some(code) => Ok(code.clone()), + None => bail!("no code for hash {:x}", hash), } } pub fn insert(&mut self, code: Vec) { @@ -800,7 +791,7 @@ impl Extend> for Hash2Code { impl FromIterator> for Hash2Code { fn from_iter>>(iter: II) -> Self { - let mut this = Self::new(true); + let mut this = Self::new(); this.extend(iter); this } From 317aedf31fa544362705bbc4e58045aa14851382 Mon Sep 17 00:00:00 2001 From: Robin Salen Date: Thu, 3 Oct 2024 21:32:55 -0400 Subject: [PATCH 4/4] Apply suggestion Co-authored-by: 0xaatif --- trace_decoder/src/core.rs | 40 +++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/trace_decoder/src/core.rs b/trace_decoder/src/core.rs index 82afaa56f..c5aa890da 100644 --- a/trace_decoder/src/core.rs +++ b/trace_decoder/src/core.rs @@ -40,6 +40,12 @@ pub fn entrypoint( code_db, txn_info, } = trace; + + let fatal_missing_code = match trie_pre_images { + BlockTraceTriePreImages::Separate(_) => FatalMissingCode(true), + BlockTraceTriePreImages::Combined(_) => FatalMissingCode(false), + }; + let (state, storage, mut code) = start(trie_pre_images)?; code.extend(code_db); @@ -68,6 +74,7 @@ pub fn entrypoint( &b_meta, ger_data, withdrawals, + fatal_missing_code, observer, )?; @@ -270,6 +277,14 @@ pub struct IntraBlockTries { pub receipt: ReceiptTrie, } +/// Hacky handling of possibly missing contract bytecode in `Hash2Code` inner +/// map. +/// Allows incomplete payloads fetched with the zero tracer to skip these +/// silently. +// TODO(Nashtare): https://github.com/0xPolygonZero/zk_evm/issues/700 +#[derive(Copy, Clone)] +pub struct FatalMissingCode(pub bool); + /// Does the main work mentioned in the [module documentation](super). #[allow(clippy::too_many_arguments)] fn middle( @@ -285,6 +300,7 @@ fn middle( ger_data: Option<(H256, H256)>, // added to final batch mut withdrawals: Vec<(Address, U256)>, + fatal_missing_code: FatalMissingCode, // called with the untrimmed tries after each batch observer: &mut impl Observer, ) -> anyhow::Result>> { @@ -433,9 +449,20 @@ fn middle( acct.code_hash = code_usage .map(|it| match it { ContractCodeUsage::Read(hash) => { - let _ = code - .get(hash) - .map(|bytecode| batch_contract_code.insert(bytecode)); + // TODO(Nashtare): https://github.com/0xPolygonZero/zk_evm/issues/700 + // This is a bug in the zero tracer, which shouldn't be giving us + // this read at all. Workaround for now. + match (fatal_missing_code, code.get(hash)) { + (FatalMissingCode(true), None) => { + bail!("no code for hash {hash:x}") + } + (_, Some(byte_code)) => { + batch_contract_code.insert(byte_code); + } + (_, None) => { + log::warn!("no code for {hash:x}") + } + } anyhow::Ok(hash) } @@ -770,11 +797,8 @@ impl Hash2Code { this.insert(vec![]); this } - pub fn get(&mut self, hash: H256) -> anyhow::Result> { - match self.inner.get(&hash) { - Some(code) => Ok(code.clone()), - None => bail!("no code for hash {:x}", hash), - } + pub fn get(&mut self, hash: H256) -> Option> { + self.inner.get(&hash).cloned() } pub fn insert(&mut self, code: Vec) { self.inner.insert(keccak_hash::keccak(&code), code);