From 9b693466c927e525a71cdd1afc0dffa5ee9b5b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Str=C3=B6m?= Date: Thu, 5 Jun 2025 09:58:03 +0000 Subject: [PATCH 1/3] Initial model testing using quickcheck --- Cargo.lock | 42 +++++++++++++-- Cargo.toml | 2 + crates/eth-sparse-mpt/Cargo.toml | 4 +- crates/eth-sparse-mpt/src/lib.rs | 2 + crates/eth-sparse-mpt/src/model_test.rs | 69 ++++++++++++++++++++++++ crates/eth-sparse-mpt/src/v2/trie/mod.rs | 60 +++++++++++++++++++++ 6 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 crates/eth-sparse-mpt/src/model_test.rs diff --git a/Cargo.lock b/Cargo.lock index 9fcc5004..790bb628 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3743,6 +3743,30 @@ dependencies = [ ] [[package]] +<<<<<<< HEAD +======= +name = "enumn" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f9ed6b3789237c8a0c1c505af1c7eb2c560df6186f01b098c3a1064ea532f38" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.100", +] + +[[package]] +name = "env_logger" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3" +dependencies = [ + "log", + "regex", +] + +[[package]] +>>>>>>> 88df572 (Initial model testing using quickcheck) name = "equivalent" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -3794,6 +3818,7 @@ dependencies = [ "hash-db", "parking_lot", "proptest", + "quickcheck", "rand 0.8.5", "rayon", "reth-errors", @@ -8251,6 +8276,17 @@ dependencies = [ "parking_lot", ] +[[package]] +name = "quickcheck" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6" +dependencies = [ + "env_logger", + "log", + "rand 0.8.5", +] + [[package]] name = "quinn" version = "0.11.7" @@ -11817,9 +11853,9 @@ dependencies = [ [[package]] name = "rustix" -version = "1.0.5" +version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d97817398dd4bb2e6da002002db259209759911da105da92bec29ccb12cf58bf" +checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" dependencies = [ "bitflags 2.9.0", "errno", @@ -13287,7 +13323,7 @@ dependencies = [ "fastrand 2.3.0", "getrandom 0.3.2", "once_cell", - "rustix 1.0.5", + "rustix 1.0.7", "windows-sys 0.59.0", ] diff --git a/Cargo.toml b/Cargo.toml index a47366b0..2791be89 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -160,6 +160,8 @@ warp = "0.3.7" flate2 = "1.0.35" prometheus = "0.13.4" ctor = "0.2" +quickcheck = "1.0.3" +rand = "0.8" libc = { version = "0.2.161" } lazy_static = "1.4.0" diff --git a/crates/eth-sparse-mpt/Cargo.toml b/crates/eth-sparse-mpt/Cargo.toml index ca99f2d1..7d6705ac 100644 --- a/crates/eth-sparse-mpt/Cargo.toml +++ b/crates/eth-sparse-mpt/Cargo.toml @@ -45,6 +45,9 @@ hash-db = { version = "0.15.2", optional = true } triehash = { version = "0.8.4", optional = true } flate2 = { workspace = true, optional = true } +quickcheck = { workspace = true } +rand = { workspace = true } + [features] benchmark-utils = ["dep:hash-db", "dep:triehash", "dep:flate2"] @@ -69,4 +72,3 @@ harness = false [[bench]] name = "trie_do_bench" harness = false - diff --git a/crates/eth-sparse-mpt/src/lib.rs b/crates/eth-sparse-mpt/src/lib.rs index dba05856..22d961f4 100644 --- a/crates/eth-sparse-mpt/src/lib.rs +++ b/crates/eth-sparse-mpt/src/lib.rs @@ -15,6 +15,8 @@ use reth_provider::{ StateCommitmentProvider, }; +pub mod model_test; + #[cfg(any(test, feature = "benchmark-utils"))] pub mod test_utils; pub mod utils; diff --git a/crates/eth-sparse-mpt/src/model_test.rs b/crates/eth-sparse-mpt/src/model_test.rs new file mode 100644 index 00000000..f27bf3c5 --- /dev/null +++ b/crates/eth-sparse-mpt/src/model_test.rs @@ -0,0 +1,69 @@ +use crate::v2::trie::Trie; +use quickcheck::{quickcheck, Arbitrary, Gen}; +use std::collections::HashMap; + +// The maximum key size. keeping it relatively +// small increases the chance of multiple +// operations being executed against the same +// key, which will tease out more bugs. +const KEY_SPACE: u8 = 16; + +#[derive(Clone, Debug)] +enum Op { + Insert(Vec, Vec), + Get(Vec), +} + +trait ChooseNonempty { + fn one_of<'a, T>(&'a mut self, entries: &'a [T]) -> &'a T; +} + +impl ChooseNonempty for Gen { + fn one_of<'a, T>(&'a mut self, entries: &'a [T]) -> &'a T { + self.choose(entries).expect("empty list in choose nonempty") + } +} + +// Arbitrary lets you create randomized instances +// of types that you're interested in testing +// properties with. QuickCheck will look for +// this trait for things that are the arguments +// to properties that it is testing. +impl Arbitrary for Op { + fn arbitrary(g: &mut Gen) -> Self { + // pick a random key to perform an operation on + let key = g + .one_of(&["key00", "key01", "odd", "key010"]) + .as_bytes() + .to_owned(); + + if *g.one_of(&[true, false]) { + Op::Insert(key, "value".into()) + } else { + Op::Get(key) + } + } +} + +quickcheck! { + fn model_test_v2(ops: Vec) -> bool { + let mut model = HashMap::new(); + let mut implementation = Trie::new_empty(); + + for op in ops { + match op { + Op::Insert(k, v) => { + implementation.insert(k.as_slice(), v.as_slice()); + model.insert(k, v); + } + Op::Get(k) => { + // if implementation.get(&k) != model.get(&k).map(AsRef::as_ref) { + // return false; + // } + } + } + } + + true + } +} diff --git a/crates/eth-sparse-mpt/src/v2/trie/mod.rs b/crates/eth-sparse-mpt/src/v2/trie/mod.rs index 80a7b91f..e43a3fb6 100644 --- a/crates/eth-sparse-mpt/src/v2/trie/mod.rs +++ b/crates/eth-sparse-mpt/src/v2/trie/mod.rs @@ -361,6 +361,66 @@ impl Trie { self.delete_nibbles_key(&n) } + pub fn get(&mut self, key: &[u8]) -> Option<&[u8]> { + let n = Nibbles::unpack(key); + self.get_nibbles_key(&n) + } + + pub fn get_nibbles_key(&mut self, nibbles_key: &Nibbles) -> Option<&[u8]> { + let get_key = nibbles_key.as_slice(); + self.walk_path.clear(); + + let mut current_node = 0; + let mut path_walked = 0; + + loop { + let node = self.nodes.get(current_node)?; + + match node { + DiffTrieNode::Branch { children } => { + // deleting from branch, key not found + if get_key.len() == path_walked { + return None; + } + + let children = *children; + + let n = get_key[path_walked]; + self.walk_path.push((current_node, n)); + path_walked += 1; + + if let Some(child_ptr) = self.branch_node_children[children][n as usize] { + current_node = child_ptr.local_ptr()?; + continue; + } else { + return None; + } + } + DiffTrieNode::Extension { key, next_node } => { + let key = key.clone(); + let next_node = *next_node; + + if get_key[path_walked..].starts_with(&self.keys[key.clone()]) { + self.walk_path.push((current_node, 0)); + path_walked += key.len(); + current_node = next_node.local_ptr()?; + continue; + } + + return None; + } + DiffTrieNode::Leaf { key, value } => { + if self.keys[key.clone()] == get_key[path_walked..] { + self.walk_path.push((current_node, 0)); + return Some(b"test"); + } + return None; + } + DiffTrieNode::Null => return None, + } + } + } + pub fn delete_nibbles_key( &mut self, nibbles_key: &Nibbles, From 033005bbc72378891ac4278bce95bee05ba78e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Str=C3=B6m?= Date: Thu, 5 Jun 2025 17:58:04 +0000 Subject: [PATCH 2/3] Model testing of v1 and v2 tries against a HasMap model --- crates/eth-sparse-mpt/src/model_test.rs | 133 ++++++++++++++++++----- crates/eth-sparse-mpt/src/v2/trie/mod.rs | 60 ---------- 2 files changed, 107 insertions(+), 86 deletions(-) diff --git a/crates/eth-sparse-mpt/src/model_test.rs b/crates/eth-sparse-mpt/src/model_test.rs index f27bf3c5..307b7975 100644 --- a/crates/eth-sparse-mpt/src/model_test.rs +++ b/crates/eth-sparse-mpt/src/model_test.rs @@ -1,19 +1,17 @@ -use crate::v2::trie::Trie; +use crate::v1::sparse_mpt::{DeletionError as DeletionErrorV1, DiffTrie}; +use crate::v2::trie::{DeletionError as DeletionErrorV2, Trie}; + +use alloy_primitives::{hex, keccak256, Bytes, FixedBytes}; use quickcheck::{quickcheck, Arbitrary, Gen}; use std::collections::HashMap; -// The maximum key size. keeping it relatively -// small increases the chance of multiple -// operations being executed against the same -// key, which will tease out more bugs. -const KEY_SPACE: u8 = 16; - #[derive(Clone, Debug)] enum Op { - Insert(Vec, Vec), - Get(Vec), + Insert(FixedKey, Vec), + Delete(FixedKey), } +// helper trait to extend `choose` with exception handling trait ChooseNonempty { fn one_of<'a, T>(&'a mut self, entries: &'a [T]) -> &'a T; } @@ -24,46 +22,129 @@ impl ChooseNonempty for Gen { } } -// Arbitrary lets you create randomized instances -// of types that you're interested in testing -// properties with. QuickCheck will look for -// this trait for things that are the arguments -// to properties that it is testing. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +struct FixedKey(FixedBytes<32>); + +impl FixedKey { + fn as_slice(&self) -> &[u8; 32] { + &self.0 + } + + fn from_string(s: &str) -> Self { + Self(keccak256("a")) + } + + fn from_bytes(bytes: [u8; 32]) -> Self { + Self(FixedBytes::new(bytes)) + } + + fn into_bytes(self) -> Bytes { + Bytes::from(self.0) + } +} + +impl From for FixedKey { + fn from(bytes: Bytes) -> FixedKey { + let fbytes = FixedBytes::from_slice(bytes.as_ref()); + FixedKey(fbytes) + } +} + +// We chose a small number of keys, to make sure our error cases handle key collisions, +// as well as shared prefixes, properties that would be very unlikely for random keys +impl Arbitrary for FixedKey { + fn arbitrary(g: &mut Gen) -> Self { + let keys = [ + FixedKey::from_bytes(hex!( + "0000000000000000000000000000000000000000000000000000000000000000" + )), + FixedKey::from_bytes(hex!( + "0000000000000000000000000000000000000000000000000000000000000001" + )), + FixedKey::from_bytes(hex!( + "0000000000000000000000000000001000000000000000000000000000000001" + )), + FixedKey::from_string("0"), + FixedKey::from_string("1"), + FixedKey::from_string("2"), + FixedKey::from_string("3"), + FixedKey::from_string("4"), + FixedKey::from_string("5"), + FixedKey::from_string("6"), + FixedKey::from_string("7"), + ]; + *g.one_of(&keys) + } +} + impl Arbitrary for Op { fn arbitrary(g: &mut Gen) -> Self { // pick a random key to perform an operation on - let key = g - .one_of(&["key00", "key01", "odd", "key010"]) - .as_bytes() - .to_owned(); + let key = FixedKey::arbitrary(g); if *g.one_of(&[true, false]) { Op::Insert(key, "value".into()) } else { - Op::Get(key) + Op::Delete(key) } } } +/// This test fails, since the Trie is designed for fixed key sizes +#[ignore] +#[test] +fn crash_example_v2() { + let mut trie = Trie::new_empty(); + trie.insert(b"00aeee", b"ok").unwrap(); + trie.insert(b"00ae", b"ok").unwrap(); +} + quickcheck! { - fn model_test_v2(ops: Vec) -> bool { + fn model_test_v1_map(ops: Vec) -> bool { + let mut model = HashMap::new(); + let mut implementation = DiffTrie::new_empty(); + + for op in ops { + match op { + Op::Insert(key, value) => { + implementation.insert(key.into_bytes(), Bytes::from(value.clone())).unwrap(); + model.insert(key, value); + } + Op::Delete(key) => { + match (implementation.delete(key.into_bytes()), model.remove(&key)) { + (Err(DeletionErrorV1::KeyNotFound), None) => (), + (Err(err), _) => panic!("Implementation error {err:?}"), + (Ok(_), Some(_)) => (), + (Ok(returned), None) => panic!("Implementation returned {returned:?} on delete"), + } + } + } + } + true + } +} + +quickcheck! { + fn model_test_v2_map(ops: Vec) -> bool { let mut model = HashMap::new(); let mut implementation = Trie::new_empty(); for op in ops { match op { Op::Insert(k, v) => { - implementation.insert(k.as_slice(), v.as_slice()); + implementation.insert(k.as_slice(), v.as_slice()).unwrap(); model.insert(k, v); } - Op::Get(k) => { - // if implementation.get(&k) != model.get(&k).map(AsRef::as_ref) { - // return false; - // } + Op::Delete(k) => { + match (implementation.delete(k.as_slice()), model.remove(&k)) { + (Err(DeletionErrorV2::KeyNotFound), None) => (), + (Err(e), _) => panic!("Implementation error {e:?}"), + (Ok(_), Some(_)) => (), + (Ok(a), None) => panic!("Implementation returned {a:?} on delete"), + } } } } - true } } diff --git a/crates/eth-sparse-mpt/src/v2/trie/mod.rs b/crates/eth-sparse-mpt/src/v2/trie/mod.rs index e43a3fb6..80a7b91f 100644 --- a/crates/eth-sparse-mpt/src/v2/trie/mod.rs +++ b/crates/eth-sparse-mpt/src/v2/trie/mod.rs @@ -361,66 +361,6 @@ impl Trie { self.delete_nibbles_key(&n) } - pub fn get(&mut self, key: &[u8]) -> Option<&[u8]> { - let n = Nibbles::unpack(key); - self.get_nibbles_key(&n) - } - - pub fn get_nibbles_key(&mut self, nibbles_key: &Nibbles) -> Option<&[u8]> { - let get_key = nibbles_key.as_slice(); - self.walk_path.clear(); - - let mut current_node = 0; - let mut path_walked = 0; - - loop { - let node = self.nodes.get(current_node)?; - - match node { - DiffTrieNode::Branch { children } => { - // deleting from branch, key not found - if get_key.len() == path_walked { - return None; - } - - let children = *children; - - let n = get_key[path_walked]; - self.walk_path.push((current_node, n)); - path_walked += 1; - - if let Some(child_ptr) = self.branch_node_children[children][n as usize] { - current_node = child_ptr.local_ptr()?; - continue; - } else { - return None; - } - } - DiffTrieNode::Extension { key, next_node } => { - let key = key.clone(); - let next_node = *next_node; - - if get_key[path_walked..].starts_with(&self.keys[key.clone()]) { - self.walk_path.push((current_node, 0)); - path_walked += key.len(); - current_node = next_node.local_ptr()?; - continue; - } - - return None; - } - DiffTrieNode::Leaf { key, value } => { - if self.keys[key.clone()] == get_key[path_walked..] { - self.walk_path.push((current_node, 0)); - return Some(b"test"); - } - return None; - } - DiffTrieNode::Null => return None, - } - } - } - pub fn delete_nibbles_key( &mut self, nibbles_key: &Nibbles, From e195e06394230d2ceadc8592a0831911f70a1287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristoffer=20Str=C3=B6m?= Date: Thu, 5 Jun 2025 18:31:13 +0000 Subject: [PATCH 3/3] Fix mistake in key generation --- crates/eth-sparse-mpt/src/model_test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/eth-sparse-mpt/src/model_test.rs b/crates/eth-sparse-mpt/src/model_test.rs index 307b7975..33361ec7 100644 --- a/crates/eth-sparse-mpt/src/model_test.rs +++ b/crates/eth-sparse-mpt/src/model_test.rs @@ -31,7 +31,7 @@ impl FixedKey { } fn from_string(s: &str) -> Self { - Self(keccak256("a")) + Self(keccak256(s)) } fn from_bytes(bytes: [u8; 32]) -> Self {