migrations: take()should consume read and write operation weight (p…

…aritytech#4302)

#### Problem
`take()` consumes only 1 read worth of weight in
`single-block-migrations` example, while `take()`
[is](https://github.com/paritytech/polkadot-sdk/blob/master/substrate/frame/support/src/storage/unhashed.rs#L63)
`get() + kill()`, i.e should be 1 read + 1 write. I think this could
mislead developers who follow this example to write their migrations

---------

Co-authored-by: Bastian Köcher <[email protected]>

prdoc/pr_4302.prdoc

@@ -0,0 +1,15 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "migrations: take() should consume read and write operation weight"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      `take()` consumes only 1 read worth of weight in `single-block-migrations` example, while `take()` is `get() + kill()`, 
+      i.e should be 1 read + 1 write. Since this could mislead developers writing migrations following the example, 
+      this PR fixes the weight calculation.
+
+crates: 
+  - name: pallet-example-single-block-migrations
+    bump: minor

substrate/frame/balances/src/migration.rs

@@ -91,7 +91,7 @@ impl<T: Config<I>, I: 'static> OnRuntimeUpgrade for ResetInactive<T, I> {
 			StorageVersion::new(0).put::<Pallet<T, I>>();
 
 			log::info!(target: LOG_TARGET, "Storage to version 0");
-			T::DbWeight::get().reads_writes(1, 2)
+			T::DbWeight::get().reads_writes(1, 3)
 		} else {
 			log::info!(
 				target: LOG_TARGET,

substrate/frame/democracy/src/migrations/v1.rs

@@ -108,7 +108,7 @@ pub mod v1 {
 				.collect::<Vec<_>>();
 			let bounded = BoundedVec::<_, T::MaxProposals>::truncate_from(props.clone());
 			PublicProps::<T>::put(bounded);
-			weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));
+			weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 2));
 
 			if props.len() as u32 > T::MaxProposals::get() {
 				log::error!(
@@ -126,7 +126,7 @@ pub mod v1 {
 
 			StorageVersion::new(1).put::<Pallet<T>>();
 
-			weight.saturating_add(T::DbWeight::get().reads_writes(1, 2))
+			weight.saturating_add(T::DbWeight::get().reads_writes(1, 3))
 		}
 
 		#[cfg(feature = "try-runtime")]

substrate/frame/examples/single-block-migrations/src/migrations/v1.rs

@@ -67,10 +67,10 @@ impl<T: crate::Config> UncheckedOnRuntimeUpgrade for InnerMigrateV0ToV1<T> {
 			// Write the new value to storage
 			let new = crate::CurrentAndPreviousValue { current: old_value, previous: None };
 			crate::Value::<T>::put(new);
-			// One read for the old value, one write for the new value
-			T::DbWeight::get().reads_writes(1, 1)
+			// One read + write for taking the old value, and one write for setting the new value
+			T::DbWeight::get().reads_writes(1, 2)
 		} else {
-			// One read for trying to access the old value
+			// No writes since there was no old value, just one read for checking
 			T::DbWeight::get().reads(1)
 		}
 	}
@@ -184,7 +184,7 @@ mod test {
 			// value.
 			assert_eq!(
 				weight,
-				<MockRuntime as frame_system::Config>::DbWeight::get().reads_writes(1, 1)
+				<MockRuntime as frame_system::Config>::DbWeight::get().reads_writes(1, 2)
 			);
 
 			// After the migration, the new value should be set as the `current` value.

substrate/frame/staking/src/migrations.rs

@@ -370,7 +370,7 @@ pub mod v10 {
 				StorageVersion::<T>::put(ObsoleteReleases::V10_0_0);
 
 				log!(info, "MigrateToV10 executed successfully");
-				T::DbWeight::get().reads_writes(1, 1)
+				T::DbWeight::get().reads_writes(1, 2)
 			} else {
 				log!(warn, "MigrateToV10 should be removed.");
 				T::DbWeight::get().reads(1)