Merge pull request #34 from cchudant/fix_inserts_remove_leaks

Smallvec, Perf improvements, MerkleTree refactor & a bunch more

.github/workflows/test.yml

@@ -37,7 +37,7 @@ jobs:
           key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('Cargo.lock') }}
       - name: Generate test result and coverage report
         run: |
-          cargo test $CARGO_OPTIONS --no-default-features
+          cargo test $CARGO_OPTIONS
       # - name: Upload test results
       #   uses: EnricoMi/publish-unit-test-result-action@v1
       #   with:

Cargo.toml

@@ -17,8 +17,8 @@ derive_more = { version = "0.99.17", default-features = false, features = [
 ] }
 hashbrown = "0.14.3"
 log = "0.4.20"
-smallvec = "1.11.2"
 rayon = { version = "1.9.0", optional = true }
+smallvec = { version = "1.11.2", features = ["serde"] }
 
 parity-scale-codec = { version = "3.0.0", default-features = false, features = [
     "derive",
@@ -27,9 +27,10 @@ serde = { version = "1.0.195", default-features = false, features = [
     "derive",
     "alloc",
 ] }
-starknet-types-core = { version = "0.1", default-features = false, features = [
+starknet-types-core = { version = "0.1.5", default-features = false, features = [
     "hash",
     "parity-scale-codec",
+    "alloc",
 ] }
 
 # Optionals
@@ -45,12 +46,14 @@ pathfinder-common = { git = "https://github.com/massalabs/pathfinder.git", packa
 pathfinder-crypto = { git = "https://github.com/massalabs/pathfinder.git", package = "pathfinder-crypto", rev = "b7b6d76a76ab0e10f92e5f84ce099b5f727cb4db" }
 pathfinder-merkle-tree = { git = "https://github.com/massalabs/pathfinder.git", package = "pathfinder-merkle-tree", rev = "b7b6d76a76ab0e10f92e5f84ce099b5f727cb4db" }
 pathfinder-storage = { git = "https://github.com/massalabs/pathfinder.git", package = "pathfinder-storage", rev = "b7b6d76a76ab0e10f92e5f84ce099b5f727cb4db" }
-rand = "0.8.5"
+rand = { version = "0.8.5", features = ["small_rng"] }
 tempfile = "3.8.0"
 rstest = "0.18.2"
 test-log = "0.2.15"
 indexmap = "2.2.6"
 criterion = "0.5.1"
+proptest = "1.4.0"
+proptest-derive = "0.4.0"
 serde_json = "1.0.68"
 
 [[bench]]

benches/storage.rs

@@ -15,6 +15,42 @@ use starknet_types_core::{
 
 mod flamegraph;
 
+fn drop_storage(c: &mut Criterion) {
+    c.bench_function("drop storage", move |b| {
+        b.iter_batched(
+            || {
+                let mut bonsai_storage: BonsaiStorage<BasicId, _, Pedersen> = BonsaiStorage::new(
+                    HashMapDb::<BasicId>::default(),
+                    BonsaiStorageConfig::default(),
+                )
+                .unwrap();
+
+                let mut rng = SmallRng::seed_from_u64(42);
+                let felt = Felt::from_hex("0x66342762FDD54D033c195fec3ce2568b62052e").unwrap();
+                for _ in 0..4000 {
+                    let bitvec = BitVec::from_vec(vec![
+                        rng.gen(),
+                        rng.gen(),
+                        rng.gen(),
+                        rng.gen(),
+                        rng.gen(),
+                        rng.gen(),
+                    ]);
+                    bonsai_storage.insert(&[], &bitvec, &felt).unwrap();
+                }
+
+                let mut id_builder = BasicIdBuilder::new();
+                let id1 = id_builder.new_id();
+                bonsai_storage.commit(id1).unwrap();
+
+                bonsai_storage
+            },
+            std::mem::drop,
+            BatchSize::LargeInput,
+        );
+    });
+}
+
 fn storage_with_insert(c: &mut Criterion) {
     c.bench_function("storage commit with insert", move |b| {
         let mut rng = thread_rng();
@@ -40,7 +76,6 @@ fn storage_with_insert(c: &mut Criterion) {
                     ]);
                     bonsai_storage.insert(&[], &bitvec, &felt).unwrap();
                 }
-
                 // let mut id_builder = BasicIdBuilder::new();
                 // bonsai_storage.commit(id_builder.new_id()).unwrap();
             },
@@ -56,7 +91,7 @@ fn storage(c: &mut Criterion) {
             BonsaiStorageConfig::default(),
         )
         .unwrap();
-        let mut rng = thread_rng();
+        let mut rng = SmallRng::seed_from_u64(42);
 
         let felt = Felt::from_hex("0x66342762FDD54D033c195fec3ce2568b62052e").unwrap();
         for _ in 0..1000 {
@@ -89,7 +124,7 @@ fn one_update(c: &mut Criterion) {
             BonsaiStorageConfig::default(),
         )
         .unwrap();
-        let mut rng = thread_rng();
+        let mut rng = SmallRng::seed_from_u64(42);
 
         let felt = Felt::from_hex("0x66342762FDD54D033c195fec3ce2568b62052e").unwrap();
         for _ in 0..1000 {
@@ -126,7 +161,7 @@ fn five_updates(c: &mut Criterion) {
             BonsaiStorageConfig::default(),
         )
         .unwrap();
-        let mut rng = thread_rng();
+        let mut rng = SmallRng::seed_from_u64(42);
 
         let felt = Felt::from_hex("0x66342762FDD54D033c195fec3ce2568b62052e").unwrap();
         for _ in 0..1000 {
@@ -226,6 +261,6 @@ fn hash(c: &mut Criterion) {
 criterion_group! {
     name = benches;
     config = Criterion::default(); // .with_profiler(flamegraph::FlamegraphProfiler::new(100));
-    targets = storage, one_update, five_updates, hash, storage_with_insert, multiple_contracts
+    targets = storage, one_update, five_updates, hash, drop_storage, storage_with_insert, multiple_contracts
 }
 criterion_main!(benches);

proptest-regressions/trie/merge.txt

@@ -0,0 +1,10 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 70ec50070d2ab1e02fbfc9e697b4db0c9c3584a4db39218dccca5b22e480378d # shrinks to pb = MerkleTreeMergeProblem { inserts_a: [], inserts_b: [(BitVec<u8, bitvec::order::Msb0> { addr: 0x778e9c046c30, head: 000, bits: 251, capacity: 256 } [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], Felt(FieldElement { value: UnsignedInteger { limbs: [576460752303388688, 18446744073709551615, 18446744073709551615, 18446744073709549569] } })), (BitVec<u8, bitvec::order::Msb0> { addr: 0x778e9c000b70, head: 000, bits: 251, capacity: 256 } [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0], Felt(FieldElement { value: UnsignedInteger { limbs: [576460752303388688, 18446744073709551615, 18446744073709551615, 18446744073709549569] } }))] }
+cc c3a0408a5f08bf1f5d8e31050cc4d95e1f377d8a56ba6fc50d5ebf3682dcc02b # shrinks to pb = MerkleTreeInsertProblem([Insert([01111], 0x20), Insert([01110], 0x20), Commit, Insert([01110], 0x40)])
+cc 185dc53af11731e46e174743ebbf050242f75bf405d301b2fd8b506f382cc4f4 # shrinks to pb = MerkleTreeInsertProblem([Insert([10011], 0x20), Remove([10011]), Insert([00000], 0x0), Insert([00000], 0x20), Commit, Insert([00000], 0x0)])
+cc c6239e16e78d8c8ec7441ef221279eef9c6541bea5b43f2ae58d0848e0960271 # shrinks to pb = MerkleTreeInsertProblem([Insert([10000], 0x20), Insert([11000], 0x40), Insert([11010], 0x40), Remove([10000]), Remove([10000]), Commit])

proptest-regressions/trie/merkle_tree.txt

@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 8ed2164817fe54de5eb23b134024a200ceb9c6fe2fc0835740731e9cd4326e45 # shrinks to pb = MerkleTreeInsertProblem([Insert(InsertStep(BitVec<u8, bitvec::order::Msb0> { addr: 0x7254a0004ca0, head: 000, bits: 3, capacity: 64 } [1, 0, 1], Felt(FieldElement { value: UnsignedInteger { limbs: [576460752303388688, 18446744073709551615, 18446744073709551615, 18446744073709549569] } }))), Insert(InsertStep(BitVec<u8, bitvec::order::Msb0> { addr: 0x7254a00037b0, head: 000, bits: 3, capacity: 64 } [1, 0, 0], Felt(FieldElement { value: UnsignedInteger { limbs: [576460752303406096, 18446744073709551615, 18446744073709551615, 18446744073709550593] } }))), Insert(InsertStep(BitVec<u8, bitvec::order::Msb0> { addr: 0x7254a0001b10, head: 000, bits: 3, capacity: 64 } [1, 0, 0], Felt(FieldElement { value: UnsignedInteger { limbs: [576460752303284240, 18446744073709551615, 18446744073709551615, 18446744073709543425] } })))])

src/bonsai_database.rs

@@ -1,4 +1,4 @@
-use crate::{id::Id, Vec};
+use crate::{id::Id, ByteVec, Vec};
 #[cfg(feature = "std")]
 use std::error::Error;
 
@@ -38,14 +38,14 @@ pub trait BonsaiDatabase {
     fn create_batch(&self) -> Self::Batch;
 
     /// Returns the value of the key if it exists
-    fn get(&self, key: &DatabaseKey) -> Result<Option<Vec<u8>>, Self::DatabaseError>;
+    fn get(&self, key: &DatabaseKey) -> Result<Option<ByteVec>, Self::DatabaseError>;
 
     #[allow(clippy::type_complexity)]
     /// Returns all values with keys that start with the given prefix
     fn get_by_prefix(
         &self,
         prefix: &DatabaseKey,
-    ) -> Result<Vec<(Vec<u8>, Vec<u8>)>, Self::DatabaseError>;
+    ) -> Result<Vec<(ByteVec, ByteVec)>, Self::DatabaseError>;
 
     /// Returns true if the key exists
     fn contains(&self, key: &DatabaseKey) -> Result<bool, Self::DatabaseError>;
@@ -57,15 +57,15 @@ pub trait BonsaiDatabase {
         key: &DatabaseKey,
         value: &[u8],
         batch: Option<&mut Self::Batch>,
-    ) -> Result<Option<Vec<u8>>, Self::DatabaseError>;
+    ) -> Result<Option<ByteVec>, Self::DatabaseError>;
 
     /// Remove a key-value pair, returns the old value if it existed.
     /// If a batch is provided, the change will be written in the batch instead of the database.
     fn remove(
         &mut self,
         key: &DatabaseKey,
         batch: Option<&mut Self::Batch>,
-    ) -> Result<Option<Vec<u8>>, Self::DatabaseError>;
+    ) -> Result<Option<ByteVec>, Self::DatabaseError>;
 
     /// Remove all keys that start with the given prefix
     fn remove_by_prefix(&mut self, prefix: &DatabaseKey) -> Result<(), Self::DatabaseError>;

src/changes.rs

@@ -1,10 +1,11 @@
-use crate::{hash_map::Entry, id::Id, trie::TrieKey, HashMap, Vec, VecDeque};
+use crate::{hash_map::Entry, id::Id, trie::TrieKey, ByteVec, HashMap, Vec, VecDeque};
+use core::iter;
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct Change {
-    pub old_value: Option<Vec<u8>>,
-    pub new_value: Option<Vec<u8>>,
+    pub old_value: Option<ByteVec>,
+    pub new_value: Option<ByteVec>,
 }
 
 #[derive(Debug, Default)]
@@ -31,7 +32,7 @@ impl ChangeBatch {
         }
     }
 
-    pub fn serialize<ID: Id>(&self, id: &ID) -> Vec<(Vec<u8>, &[u8])> {
+    pub fn serialize<ID: Id>(&self, id: &ID) -> Vec<(ByteVec, &[u8])> {
         self.0
             .iter()
             .flat_map(|(change_key, change)| {
@@ -56,7 +57,7 @@ impl ChangeBatch {
             .collect()
     }
 
-    pub fn deserialize<ID: Id>(id: &ID, changes: Vec<(Vec<u8>, Vec<u8>)>) -> Self {
+    pub fn deserialize<ID: Id>(id: &ID, changes: Vec<(ByteVec, ByteVec)>) -> Self {
         let id = id.to_bytes();
         let mut change_batch = ChangeBatch(HashMap::new());
         let mut current_change = Change::default();
@@ -69,8 +70,7 @@ impl ChangeBatch {
             let mut key = key.to_vec();
             let change_type = key.pop().unwrap();
             let key_type = key.pop().unwrap();
-            let change_key =
-                TrieKey::from_variant_and_bytes(key_type, key[id.len() + 1..].to_vec());
+            let change_key = TrieKey::from_variant_and_bytes(key_type, key[id.len() + 1..].into());
             if let Some(last_key) = last_key {
                 if last_key != change_key {
                     change_batch.insert_in_place(last_key, current_change);
@@ -93,29 +93,28 @@ impl ChangeBatch {
     }
 }
 
-pub fn key_old_value<ID: Id>(id: &ID, key: &TrieKey) -> Vec<u8> {
-    [
-        id.to_bytes().as_slice(),
-        &[KEY_SEPARATOR],
-        key.as_slice(),
-        &[key.into()],
-        &[OLD_VALUE],
-    ]
-    .concat()
+pub fn key_old_value<ID: Id>(id: &ID, key: &TrieKey) -> ByteVec {
+    id.to_bytes()
+        .into_iter()
+        .chain(iter::once(KEY_SEPARATOR))
+        .chain(key.as_slice().iter().copied())
+        .chain(iter::once(key.into()))
+        .chain(iter::once(OLD_VALUE))
+        .collect()
 }
 
-pub fn key_new_value<ID: Id>(id: &ID, key: &TrieKey) -> Vec<u8> {
-    [
-        id.to_bytes().as_slice(),
-        &[KEY_SEPARATOR],
-        key.as_slice(),
-        &[key.into()],
-        &[NEW_VALUE],
-    ]
-    .concat()
+pub fn key_new_value<ID: Id>(id: &ID, key: &TrieKey) -> ByteVec {
+    id.to_bytes()
+        .into_iter()
+        .chain(iter::once(KEY_SEPARATOR))
+        .chain(key.as_slice().iter().copied())
+        .chain(iter::once(key.into()))
+        .chain(iter::once(NEW_VALUE))
+        .collect()
 }
 
 #[cfg_attr(feature = "bench", derive(Clone))]
+#[derive(Debug)]
 pub struct ChangeStore<ID>
 where
     ID: Id,