TrieDBMutBase added: no commit on drop (#226)

* explicit commit on drop

* doc updated

* trie-db: version bumped

* review: docs

* version bump

* bool flag: commit_on_drop

* tests added

* fmt

* fixes

* versions bumped

Author: michalkucharczyk

test-support/reference-trie/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "reference-trie"
-version = "0.29.1"
+version = "0.29.2"
 authors = ["Parity Technologies <admin@parity.io>"]
 description = "Simple reference trie format"
 repository = "https://github.com/paritytech/trie/"
@@ -10,7 +10,7 @@ edition = "2018"
 [dependencies]
 hash-db = { path = "../../hash-db" , version = "0.16.0"}
 keccak-hasher = { path = "../keccak-hasher", version = "0.16.0" }
-trie-db = { path = "../../trie-db", default-features = false, version = "0.30.0" }
+trie-db = { path = "../../trie-db", default-features = false, version = "0.31.0" }
 trie-root = { path = "../../trie-root", default-features = false, version = "0.18.0" }
 parity-scale-codec = { version = "3.0.0", features = ["derive"] }
 hashbrown = { version = "0.14.1", default-features = false, features = ["ahash"] }

test-support/trie-bench/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "trie-bench"
 description = "Standard benchmarking suite for tries"
-version = "0.42.0"
+version = "0.42.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 repository = "https://github.com/paritytech/trie/"
 license = "Apache-2.0"
@@ -13,6 +13,6 @@ trie-standardmap = { path = "../trie-standardmap", version = "0.16.0" }
 hash-db = { path = "../../hash-db" , version = "0.16.0"}
 memory-db = { path = "../../memory-db", version = "0.34.0" }
 trie-root = { path = "../../trie-root", version = "0.18.0" }
-trie-db = { path = "../../trie-db", version = "0.30.0" }
+trie-db = { path = "../../trie-db", version = "0.31.0" }
 criterion = "0.5.1"
 parity-scale-codec = "3.0.0"

trie-db/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "trie-db"
-version = "0.30.0"
+version = "0.31.0"
 authors = ["Parity Technologies <admin@parity.io>"]
 description = "Merkle-Patricia Trie generic over key hasher and node encoding"
 repository = "https://github.com/paritytech/trie"

trie-db/src/triedbmut.rs

@@ -641,6 +641,7 @@ pub struct TrieDBMutBuilder<'db, L: TrieLayout> {
 	root: &'db mut TrieHash<L>,
 	cache: Option<&'db mut dyn TrieCache<L::Codec>>,
 	recorder: Option<&'db mut dyn TrieRecorder<TrieHash<L>>>,
+	commit_on_drop: bool,
 }
 
 impl<'db, L: TrieLayout> TrieDBMutBuilder<'db, L> {
@@ -649,7 +650,7 @@ impl<'db, L: TrieLayout> TrieDBMutBuilder<'db, L> {
 	pub fn new(db: &'db mut dyn HashDB<L::Hash, DBValue>, root: &'db mut TrieHash<L>) -> Self {
 		*root = L::Codec::hashed_null_node();
 
-		Self { root, db, cache: None, recorder: None }
+		Self { root, db, cache: None, recorder: None, commit_on_drop: true }
 	}
 
 	/// Create a builder for constructing a new trie with the backing database `db` and `root`.
@@ -660,7 +661,7 @@ impl<'db, L: TrieLayout> TrieDBMutBuilder<'db, L> {
 		db: &'db mut dyn HashDB<L::Hash, DBValue>,
 		root: &'db mut TrieHash<L>,
 	) -> Self {
-		Self { db, root, cache: None, recorder: None }
+		Self { db, root, cache: None, recorder: None, commit_on_drop: true }
 	}
 
 	/// Use the given `cache` for the db.
@@ -695,7 +696,24 @@ impl<'db, L: TrieLayout> TrieDBMutBuilder<'db, L> {
 		self
 	}
 
+	/// Disable automatic commit on drop.
+	///
+	/// By default, [`TrieDBMut`] automatically commits changes when dropped. Calling this method
+	/// disables that behavior, requiring explicit calls to [`TrieDBMut::commit`] to persist
+	/// changes to the database.
+	///
+	/// This is useful when you want fine-grained control over when changes are committed, or when
+	/// you want to avoid the performance cost of committing if you're just doing temporary
+	/// operations.
+	pub fn disable_commit_on_drop(mut self) -> Self {
+		self.commit_on_drop = false;
+		self
+	}
+
 	/// Build the [`TrieDBMut`].
+	///
+	/// By default, the returned trie will automatically commit changes when dropped. Use
+	/// [`disable_commit_on_drop`](Self::disable_commit_on_drop) to disable this behavior.
 	pub fn build(self) -> TrieDBMut<'db, L> {
 		let root_handle = NodeHandle::Hash(*self.root);
 
@@ -707,17 +725,19 @@ impl<'db, L: TrieLayout> TrieDBMutBuilder<'db, L> {
 			storage: NodeStorage::empty(),
 			death_row: Default::default(),
 			root_handle,
+			commit_on_drop: self.commit_on_drop,
 		}
 	}
 }
 
 /// A `Trie` implementation using a generic `HashDB` backing database.
 ///
-/// Use it as a `TrieMut` trait object. You can use `db()` to get the backing database object.
+/// Use it as a [`TrieMut`] trait object. You can use `db()` to get the backing database object.
 /// Note that changes are not committed to the database until `commit` is called.
 ///
-/// Querying the root or dropping the trie will commit automatically.
+/// Querying the root of the trie will commit automatically.
 ///
+/// Dropping the instance may or may not commit depending on [Self::commit_on_drop] flag.
 ///
 /// # Example
 /// ```ignore
@@ -751,6 +771,8 @@ where
 	cache: Option<&'a mut dyn TrieCache<L::Codec>>,
 	/// Optional trie recorder for recording trie accesses.
 	recorder: Option<core::cell::RefCell<&'a mut dyn TrieRecorder<TrieHash<L>>>>,
+	/// Whether to commit on drop.
+	commit_on_drop: bool,
 }
 
 impl<'a, L> TrieDBMut<'a, L>
@@ -2103,7 +2125,9 @@ where
 	L: TrieLayout,
 {
 	fn drop(&mut self) {
-		self.commit();
+		if self.commit_on_drop {
+			self.commit();
+		}
 	}
 }
 

trie-db/test/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "trie-db-test"
-version = "0.31.0"
+version = "0.31.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 description = "Tests for trie-db crate"
 repository = "https://github.com/paritytech/trie"
@@ -12,7 +12,7 @@ name = "bench"
 harness = false
 
 [dependencies]
-trie-db = { path = "..", version = "0.30.0"}
+trie-db = { path = "..", version = "0.31.0"}
 hash-db = { path = "../../hash-db", version = "0.16.0"}
 memory-db = { path = "../../memory-db", version = "0.34.0" }
 rand = { version = "0.8", default-features = false, features = ["small_rng"] }

trie-db/test/src/triedbmut.rs

@@ -890,3 +890,93 @@ fn test_two_assets_memory_db_inner_2<T: TrieLayout>() {
 	assert_eq!(state.get(key2.as_ref()).unwrap().unwrap(), data2);
 	assert_eq!(state.get(key3.as_ref()).unwrap().unwrap(), data3);
 }
+
+/// Prepare a non-empty trie for testing commit_on_drop behavior.
+/// Returns a database and root with one committed key-value pair.
+fn prepare_test_trie<T: TrieLayout>() -> (PrefixedMemoryDB<T>, <T::Hash as Hasher>::Out) {
+	use hash_db::Hasher;
+
+	let mut memdb = PrefixedMemoryDB::<T>::default();
+	let mut root = <T::Hash as Hasher>::Out::default();
+
+	let initial_data = vec![(b"existing_key".to_vec(), b"existing_value".to_vec())];
+	populate_trie::<T>(&mut memdb, &mut root, &initial_data);
+
+	(memdb, root)
+}
+
+test_layouts!(test_commit_on_drop_disabled, test_commit_on_drop_disabled_internal);
+fn test_commit_on_drop_disabled_internal<T: TrieLayout>() {
+	let (mut memdb, mut root) = prepare_test_trie::<T>();
+	let root_before = root.clone();
+	let db_key_count_before = memdb.keys().len();
+
+	{
+		let mut trie = TrieDBMutBuilder::<T>::from_existing(&mut memdb, &mut root)
+			.disable_commit_on_drop()
+			.build();
+		trie.insert(b"test_key_1", b"test_value_1").unwrap();
+		trie.insert(b"test_key_2", b"test_value_2").unwrap();
+		trie.insert(b"test_key_3", b"test_value_3").unwrap();
+	}
+
+	assert_eq!(root, root_before, "Root should not change after drop without commit");
+
+	let db_key_count_after = memdb.keys().len();
+	assert_eq!(
+		db_key_count_before, db_key_count_after,
+		"Database should not gain new entries after drop without commit (before: {}, after: {})",
+		db_key_count_before, db_key_count_after
+	);
+}
+
+test_layouts!(test_commit_on_drop_enabled, test_commit_on_drop_enabled_internal);
+fn test_commit_on_drop_enabled_internal<T: TrieLayout>() {
+	let (mut memdb, mut root) = prepare_test_trie::<T>();
+	let root_before = root.clone();
+	let db_key_count_before = memdb.keys().len();
+
+	{
+		let mut trie = TrieDBMutBuilder::<T>::from_existing(&mut memdb, &mut root).build();
+		trie.insert(b"test_key_1", b"test_value_1").unwrap();
+		trie.insert(b"test_key_2", b"test_value_2").unwrap();
+		trie.insert(b"test_key_3", b"test_value_3").unwrap();
+	}
+
+	assert_ne!(root, root_before, "Root should change after drop with auto-commit");
+
+	let db_key_count_after = memdb.keys().len();
+	assert!(
+		db_key_count_after > db_key_count_before,
+		"Database should contain more nodes after drop with auto-commit (before: {}, after: {})",
+		db_key_count_before,
+		db_key_count_after
+	);
+}
+
+test_layouts!(test_commit_on_drop_explicit, test_commit_on_drop_explicit_internal);
+fn test_commit_on_drop_explicit_internal<T: TrieLayout>() {
+	let (mut memdb, mut root) = prepare_test_trie::<T>();
+	let root_before = root.clone();
+	let db_key_count_before = memdb.keys().len();
+
+	{
+		let mut trie = TrieDBMutBuilder::<T>::from_existing(&mut memdb, &mut root)
+			.disable_commit_on_drop()
+			.build();
+		trie.insert(b"test_key_1", b"test_value_1").unwrap();
+		trie.insert(b"test_key_2", b"test_value_2").unwrap();
+		trie.insert(b"test_key_3", b"test_value_3").unwrap();
+		trie.commit();
+	}
+
+	assert_ne!(root, root_before, "Root should change after explicit commit");
+
+	let db_key_count_after = memdb.keys().len();
+	assert!(
+		db_key_count_after > db_key_count_before,
+		"Database should contain more nodes after explicit commit (before: {}, after: {})",
+		db_key_count_before,
+		db_key_count_after
+	);
+}

trie-eip1186/Cargo.toml

@@ -1,14 +1,14 @@
 [package]
 name = "trie-eip1186"
-version = "0.5.0"
+version = "0.5.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 description = "EIP-1186 compliant proof generation and verification"
 repository = "https://github.com/paritytech/trie"
 license = "Apache-2.0"
 edition = "2018"
 
 [dependencies]
-trie-db = { path = "../trie-db", default-features = false, version = "0.30.0"}
+trie-db = { path = "../trie-db", default-features = false, version = "0.31.0"}
 hash-db = { path = "../hash-db", default-features = false, version = "0.16.0"}
 
 [features]

trie-eip1186/test/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "trie-eip1186-test"
-version = "0.7.0"
+version = "0.7.1"
 authors = ["Parity Technologies <admin@parity.io>"]
 description = "Tests for trie-eip1186 crate"
 repository = "https://github.com/paritytech/trie"
@@ -9,7 +9,7 @@ edition = "2018"
 
 [dependencies]
 trie-eip1186 = { path = "..", version = "0.5.0"}
-trie-db = { path = "../../trie-db", version = "0.30.0"}
+trie-db = { path = "../../trie-db", version = "0.31.0"}
 hash-db = { path = "../../hash-db", version = "0.16.0"}
 reference-trie = { path = "../../test-support/reference-trie", version = "0.29.0" }
 memory-db = { path = "../../memory-db", version = "0.34.0" }