From edd062fc32c17fee4985d322bbc1aa1a8060120b Mon Sep 17 00:00:00 2001 From: zeapoz Date: Mon, 8 Apr 2024 10:26:19 +0200 Subject: [PATCH 1/4] fix: correct wrong endianness --- src/processor/snapshot/database.rs | 2 +- src/processor/snapshot/mod.rs | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/processor/snapshot/database.rs b/src/processor/snapshot/database.rs index 80f1c47..99f66bd 100644 --- a/src/processor/snapshot/database.rs +++ b/src/processor/snapshot/database.rs @@ -53,7 +53,7 @@ impl SnapshotDB { PackingType::NoCompression(v) | PackingType::Transform(v) => v, PackingType::Add(_) | PackingType::Sub(_) => { let mut buffer = [0; 32]; - key.to_little_endian(&mut buffer); + key.to_big_endian(&mut buffer); if let Ok(Some(log)) = self.get_storage_log(&buffer) { let existing_value = U256::from(log.value.to_fixed_bytes()); // NOTE: We're explicitly allowing over-/underflow as per the spec. diff --git a/src/processor/snapshot/mod.rs b/src/processor/snapshot/mod.rs index 448593c..10b9930 100644 --- a/src/processor/snapshot/mod.rs +++ b/src/processor/snapshot/mod.rs @@ -69,7 +69,9 @@ impl Processor for SnapshotBuilder { self.database .insert_storage_log(&mut SnapshotStorageLog { key: U256::from_little_endian(key), - value: self.database.process_value(U256::from(key), *value), + value: self + .database + .process_value(U256::from_little_endian(key), *value), miniblock_number_of_initial_write: U64::from(0), l1_batch_number_of_initial_write: U64::from( block.l1_block_number.unwrap_or(0), @@ -86,7 +88,9 @@ impl Processor for SnapshotBuilder { .database .get_key_from_index(index as u64) .expect("missing key"); - let value = self.database.process_value(U256::from(&key[0..32]), *value); + let value = self + .database + .process_value(U256::from_big_endian(&key[0..32]), *value); if self .database From c99c9b1b3ab2cc44a2b9e1ad326bd75300956b84 Mon Sep 17 00:00:00 2001 From: zeapoz Date: Wed, 10 Apr 2024 11:56:42 +0200 Subject: [PATCH 2/4] fix(snapshot): initial inserts should always be initialized as 0 Previously we would `panic!` when trying to retrieve the value for a missing key, but as it turns out initial inserts can actually contain operation like `Add` and `Sub`. In those cases it's expected that the value should be added to 0. --- src/processor/snapshot/database.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/processor/snapshot/database.rs b/src/processor/snapshot/database.rs index 99f66bd..b277446 100644 --- a/src/processor/snapshot/database.rs +++ b/src/processor/snapshot/database.rs @@ -54,16 +54,17 @@ impl SnapshotDB { PackingType::Add(_) | PackingType::Sub(_) => { let mut buffer = [0; 32]; key.to_big_endian(&mut buffer); - if let Ok(Some(log)) = self.get_storage_log(&buffer) { - let existing_value = U256::from(log.value.to_fixed_bytes()); - // NOTE: We're explicitly allowing over-/underflow as per the spec. - match value { - PackingType::Add(v) => existing_value.overflowing_add(v).0, - PackingType::Sub(v) => existing_value.overflowing_sub(v).0, - _ => unreachable!(), - } + let existing_value = if let Some(log) = self.get_storage_log(&buffer).unwrap() { + U256::from(log.value.to_fixed_bytes()) } else { - panic!("no key found for version") + U256::from(0) + }; + + // NOTE: We're explicitly allowing over-/underflow as per the spec. + match value { + PackingType::Add(v) => existing_value.overflowing_add(v).0, + PackingType::Sub(v) => existing_value.overflowing_sub(v).0, + _ => unreachable!(), } } }; From 5c89a606cf9be93f7e3a5125034c7abd5764c0ad Mon Sep 17 00:00:00 2001 From: zeapoz Date: Wed, 10 Apr 2024 12:13:10 +0200 Subject: [PATCH 3/4] chore: better error handling --- src/processor/snapshot/database.rs | 6 +++--- src/processor/snapshot/mod.rs | 15 ++++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/processor/snapshot/database.rs b/src/processor/snapshot/database.rs index b277446..02b34ea 100644 --- a/src/processor/snapshot/database.rs +++ b/src/processor/snapshot/database.rs @@ -48,13 +48,13 @@ impl SnapshotDB { Ok(Self(db)) } - pub fn process_value(&self, key: U256, value: PackingType) -> H256 { + pub fn process_value(&self, key: U256, value: PackingType) -> Result { let processed_value = match value { PackingType::NoCompression(v) | PackingType::Transform(v) => v, PackingType::Add(_) | PackingType::Sub(_) => { let mut buffer = [0; 32]; key.to_big_endian(&mut buffer); - let existing_value = if let Some(log) = self.get_storage_log(&buffer).unwrap() { + let existing_value = if let Some(log) = self.get_storage_log(&buffer)? { U256::from(log.value.to_fixed_bytes()) } else { U256::from(0) @@ -71,7 +71,7 @@ impl SnapshotDB { let mut buffer = [0; 32]; processed_value.to_big_endian(&mut buffer); - H256::from(buffer) + Ok(H256::from(buffer)) } pub fn new_read_only(db_path: PathBuf) -> Result { diff --git a/src/processor/snapshot/mod.rs b/src/processor/snapshot/mod.rs index 10b9930..83e0cec 100644 --- a/src/processor/snapshot/mod.rs +++ b/src/processor/snapshot/mod.rs @@ -66,12 +66,16 @@ impl Processor for SnapshotBuilder { while let Some(block) = rx.recv().await { // Initial calldata. for (key, value) in &block.initial_storage_changes { + let key = U256::from_little_endian(key); + let value = self + .database + .process_value(key, *value) + .expect("failed to get key from database"); + self.database .insert_storage_log(&mut SnapshotStorageLog { - key: U256::from_little_endian(key), - value: self - .database - .process_value(U256::from_little_endian(key), *value), + key, + value, miniblock_number_of_initial_write: U64::from(0), l1_batch_number_of_initial_write: U64::from( block.l1_block_number.unwrap_or(0), @@ -90,7 +94,8 @@ impl Processor for SnapshotBuilder { .expect("missing key"); let value = self .database - .process_value(U256::from_big_endian(&key[0..32]), *value); + .process_value(U256::from_big_endian(&key[0..32]), *value) + .expect("failed to get key from database"); if self .database From fcd07e80b047a3eedf0ea2705e176557eaaf918f Mon Sep 17 00:00:00 2001 From: Vaclav Barta Date: Wed, 10 Apr 2024 15:38:29 +0200 Subject: [PATCH 4/4] added unit test --- src/processor/snapshot/mod.rs | 56 +++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/processor/snapshot/mod.rs b/src/processor/snapshot/mod.rs index 83e0cec..1001697 100644 --- a/src/processor/snapshot/mod.rs +++ b/src/processor/snapshot/mod.rs @@ -239,3 +239,59 @@ fn derive_final_address_for_params(address: &Address, key: &U256) -> [u8; 32] { result } + +#[cfg(test)] +mod tests { + use std::fs; + + use indexmap::IndexMap; + use state_reconstruct_fetcher::types::PackingType; + + use super::*; + + #[tokio::test] + async fn simple() { + let db_dir = "./test_db".to_string(); + let _ = fs::remove_dir_all(db_dir.clone()); + + { + let builder = SnapshotBuilder::new(Some(db_dir.clone())); + let (tx, rx) = mpsc::channel::(5); + + tokio::spawn(async move { + let key = U256::from_dec_str("1234").unwrap(); + let mut key1 = [0u8; 32]; + key.to_little_endian(&mut key1); + let val1 = U256::from_dec_str("5678").unwrap(); + let mut initial_storage_changes = IndexMap::new(); + initial_storage_changes.insert(key1, PackingType::NoCompression(val1)); + let repeated_storage_changes = IndexMap::new(); + let cb = CommitBlock { + l1_block_number: Some(1), + l2_block_number: 2, + index_repeated_storage_changes: 0, + new_state_root: Vec::new(), + initial_storage_changes, + repeated_storage_changes, + factory_deps: Vec::new(), + }; + tx.send(cb).await.unwrap(); + }); + + builder.run(rx).await; + } + + let db = SnapshotDB::new(PathBuf::from(db_dir.clone())).unwrap(); + + let key = U256::from_dec_str("1234").unwrap(); + let mut key1 = [0u8; 32]; + key.to_big_endian(&mut key1); + let Some(sl1) = db.get_storage_log(&key1).unwrap() else { + panic!("key1 not found") + }; + + assert_eq!(sl1.key, key1.into()); + + fs::remove_dir_all(db_dir).unwrap(); + } +}