diff --git a/prdoc/pr_4302.prdoc b/prdoc/pr_4302.prdoc new file mode 100644 index 0000000000000..bb4331f280239 --- /dev/null +++ b/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 diff --git a/substrate/frame/balances/src/migration.rs b/substrate/frame/balances/src/migration.rs index 38d9c07ff7e00..568c3fbb7cf09 100644 --- a/substrate/frame/balances/src/migration.rs +++ b/substrate/frame/balances/src/migration.rs @@ -91,7 +91,7 @@ impl, I: 'static> OnRuntimeUpgrade for ResetInactive { StorageVersion::new(0).put::>(); 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, diff --git a/substrate/frame/democracy/src/migrations/v1.rs b/substrate/frame/democracy/src/migrations/v1.rs index 5e423b9ab6eff..47f8df017f1e1 100644 --- a/substrate/frame/democracy/src/migrations/v1.rs +++ b/substrate/frame/democracy/src/migrations/v1.rs @@ -108,7 +108,7 @@ pub mod v1 { .collect::>(); let bounded = BoundedVec::<_, T::MaxProposals>::truncate_from(props.clone()); PublicProps::::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::>(); - weight.saturating_add(T::DbWeight::get().reads_writes(1, 2)) + weight.saturating_add(T::DbWeight::get().reads_writes(1, 3)) } #[cfg(feature = "try-runtime")] diff --git a/substrate/frame/examples/single-block-migrations/src/migrations/v1.rs b/substrate/frame/examples/single-block-migrations/src/migrations/v1.rs index 18ef4e72cc4f5..7b543d72c9840 100644 --- a/substrate/frame/examples/single-block-migrations/src/migrations/v1.rs +++ b/substrate/frame/examples/single-block-migrations/src/migrations/v1.rs @@ -67,10 +67,10 @@ impl UncheckedOnRuntimeUpgrade for InnerMigrateV0ToV1 { // Write the new value to storage let new = crate::CurrentAndPreviousValue { current: old_value, previous: None }; crate::Value::::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, - ::DbWeight::get().reads_writes(1, 1) + ::DbWeight::get().reads_writes(1, 2) ); // After the migration, the new value should be set as the `current` value. diff --git a/substrate/frame/staking/src/migrations.rs b/substrate/frame/staking/src/migrations.rs index 510252be26c93..b2ddf77004f95 100644 --- a/substrate/frame/staking/src/migrations.rs +++ b/substrate/frame/staking/src/migrations.rs @@ -370,7 +370,7 @@ pub mod v10 { StorageVersion::::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)