contracts: Don't read the previous value when overwriting a storage item (#7879)

* Add `len` function that can return the length of a storage item efficiently

* Make use of the new len function in contracts

* Fix benchmarks

* cargo run --release --features=runtime-benchmarks --manifest-path=bin/node/cli/Cargo.toml -- benchmark --chain=dev --steps=50 --repeat=20 --pallet=pallet_contracts --extrinsic=* --execution=wasm --wasm-execution=compiled --heap-pages=4096 --output=./frame/contracts/src/weights.rs --template=./.maintain/frame-weight-template.hbs

* Remove unused imports

Co-authored-by: Parity Benchmarking Bot <admin@parity.io>

substrate/frame/contracts/src/benchmarking/mod.rs

@@ -109,21 +109,18 @@ where
 		endowment: Endow,
 	) -> Result<Contract<T>, &'static str>
 	{
-		use sp_runtime::traits::{CheckedDiv, SaturatedConversion};
 		let (storage_size, endowment) = match endowment {
 			Endow::CollectRent => {
 				// storage_size cannot be zero because otherwise a contract that is just above
 				// the subsistence threshold does not pay rent given a large enough subsistence
 				// threshold. But we need rent payments to occur in order to benchmark for worst cases.
-				let storage_size = ConfigCache::<T>::subsistence_threshold_uncached()
+				let storage_size = u32::max_value() / 10;
-					.checked_div(&T::DepositPerStorageByte::get())
-					.unwrap_or_else(Zero::zero);
 
 				// Endowment should be large but not as large to inhibit rent payments.
+				// Balance will only cover half the storage
 				let endowment = T::DepositPerStorageByte::get()
-					.saturating_mul(storage_size)
+					.saturating_mul(<BalanceOf<T>>::from(storage_size) / 2u32.into())
-					.saturating_add(T::DepositPerContract::get())
+					.saturating_add(T::DepositPerContract::get());
-					.saturating_sub(1u32.into());
 
 				(storage_size, endowment)
 			},
@@ -159,7 +156,7 @@ where
 		};
 
 		let mut contract = result.alive_info()?;
-		contract.storage_size = storage_size.saturated_into::<u32>();
+		contract.storage_size = storage_size;
 		ContractInfoOf::<T>::insert(&result.account_id, ContractInfo::Alive(contract));
 
 		Ok(result)

substrate/frame/contracts/src/storage.rs

@@ -89,19 +89,10 @@ where
 		let hashed_key = blake2_256(key);
 		let child_trie_info = &crate::child_trie_info(&trie_id);
 
-		// In order to correctly update the book keeping we need to fetch the previous
+		let opt_prev_len = child::len(&child_trie_info, &hashed_key);
-		// value of the key-value pair.
-		//
-		// It might be a bit more clean if we had an API that supported getting the size
-		// of the value without going through the loading of it. But at the moment of
-		// writing, there is no such API.
-		//
-		// That's not a show stopper in any case, since the performance cost is
-		// dominated by the trie traversal anyway.
-		let opt_prev_value = child::get_raw(&child_trie_info, &hashed_key);
 
 		// Update the total number of KV pairs and the number of empty pairs.
-		match (&opt_prev_value, &opt_new_value) {
+		match (&opt_prev_len, &opt_new_value) {
 			(Some(_), None) => {
 				new_info.pair_count -= 1;
 			},
@@ -113,10 +104,7 @@ where
 		}
 
 		// Update the total storage size.
-		let prev_value_len = opt_prev_value
+		let prev_value_len = opt_prev_len.unwrap_or(0);
-			.as_ref()
-			.map(|old_value| old_value.len() as u32)
-			.unwrap_or(0);
 		let new_value_len = opt_new_value
 			.as_ref()
 			.map(|new_value| new_value.len() as u32)

substrate/frame/support/src/storage/child.rs

@@ -243,3 +243,21 @@ pub fn root(
 		),
 	}
 }
+
+/// Return the length in bytes of the value without reading it. `None` if it does not exist.
+pub fn len(
+	child_info: &ChildInfo,
+	key: &[u8],
+) -> Option<u32> {
+	match child_info.child_type() {
+		ChildType::ParentKeyId => {
+			let mut buffer = [0; 0];
+			sp_io::default_child_storage::read(
+				child_info.storage_key(),
+				key,
+				&mut buffer,
+				0,
+			)
+		}
+	}
+}