Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Deprecate V1 Weights #13699

Merged
merged 12 commits into from
Apr 4, 2023
33 changes: 2 additions & 31 deletions frame/alliance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ use frame_support::{
ChangeMembers, Currency, Get, InitializeMembers, IsSubType, OnUnbalanced,
ReservableCurrency,
},
weights::{OldWeight, Weight},
weights::Weight,
};
use pallet_identity::IdentityField;

Expand Down Expand Up @@ -539,36 +539,7 @@ pub mod pallet {
Ok(())
}

/// Close a vote that is either approved, disapproved, or whose voting period has ended.
///
/// Must be called by a Fellow.
#[pallet::call_index(2)]
#[pallet::weight({
let b = *length_bound;
let m = T::MaxFellows::get();
let p1 = *proposal_weight_bound;
let p2 = T::MaxProposals::get();
T::WeightInfo::close_early_approved(b, m, p2)
.max(T::WeightInfo::close_early_disapproved(m, p2))
.max(T::WeightInfo::close_approved(b, m, p2))
.max(T::WeightInfo::close_disapproved(m, p2))
.saturating_add(p1.into())
})]
#[allow(deprecated)]
#[deprecated(note = "1D weight is used in this extrinsic, please migrate to use `close`")]
pub fn close_old_weight(
origin: OriginFor<T>,
proposal_hash: T::Hash,
#[pallet::compact] index: ProposalIndex,
#[pallet::compact] proposal_weight_bound: OldWeight,
#[pallet::compact] length_bound: u32,
) -> DispatchResultWithPostInfo {
let proposal_weight_bound: Weight = proposal_weight_bound.into();
let who = ensure_signed(origin)?;
ensure!(Self::has_voting_rights(&who), Error::<T, I>::NoVotingRights);

Self::do_close(proposal_hash, index, proposal_weight_bound, length_bound)
}
// Index 2 was `close_old_weight`; it was removed due to weights v1 deprecation.

/// Initialize the Alliance, onboard fellows and allies.
///
Expand Down
56 changes: 2 additions & 54 deletions frame/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use frame_support::{
traits::{
Backing, ChangeMembers, EnsureOrigin, Get, GetBacking, InitializeMembers, StorageVersion,
},
weights::{OldWeight, Weight},
weights::Weight,
};

#[cfg(test)]
Expand Down Expand Up @@ -557,59 +557,7 @@ pub mod pallet {
}
}

/// Close a vote that is either approved, disapproved or whose voting period has ended.
///
/// May be called by any signed account in order to finish voting and close the proposal.
///
/// If called before the end of the voting period it will only close the vote if it is
/// has enough votes to be approved or disapproved.
///
/// If called after the end of the voting period abstentions are counted as rejections
/// unless there is a prime member set and the prime member cast an approval.
///
/// If the close operation completes successfully with disapproval, the transaction fee will
/// be waived. Otherwise execution of the approved operation will be charged to the caller.
///
/// + `proposal_weight_bound`: The maximum amount of weight consumed by executing the closed
/// proposal.
/// + `length_bound`: The upper bound for the length of the proposal in storage. Checked via
/// `storage::read` so it is `size_of::<u32>() == 4` larger than the pure length.
///
/// ## Complexity
/// - `O(B + M + P1 + P2)` where:
/// - `B` is `proposal` size in bytes (length-fee-bounded)
/// - `M` is members-count (code- and governance-bounded)
/// - `P1` is the complexity of `proposal` preimage.
/// - `P2` is proposal-count (code-bounded)
#[pallet::call_index(4)]
#[pallet::weight((
{
let b = *length_bound;
let m = T::MaxMembers::get();
let p1 = *proposal_weight_bound;
let p2 = T::MaxProposals::get();
T::WeightInfo::close_early_approved(b, m, p2)
.max(T::WeightInfo::close_early_disapproved(m, p2))
.max(T::WeightInfo::close_approved(b, m, p2))
.max(T::WeightInfo::close_disapproved(m, p2))
.saturating_add(p1.into())
},
DispatchClass::Operational
))]
#[allow(deprecated)]
#[deprecated(note = "1D weight is used in this extrinsic, please migrate to `close`")]
pub fn close_old_weight(
origin: OriginFor<T>,
proposal_hash: T::Hash,
#[pallet::compact] index: ProposalIndex,
#[pallet::compact] proposal_weight_bound: OldWeight,
#[pallet::compact] length_bound: u32,
) -> DispatchResultWithPostInfo {
let proposal_weight_bound: Weight = proposal_weight_bound.into();
let _ = ensure_signed(origin)?;

Self::do_close(proposal_hash, index, proposal_weight_bound, length_bound)
}
// Index 4 was `close_old_weight`; it was removed due to weights v1 deprecation.

/// Disapprove a proposal, close, and remove it from the system, regardless of its current
/// state.
Expand Down
10 changes: 8 additions & 2 deletions frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ use frame_support::{
tokens::fungible::Inspect, ConstU32, Contains, Currency, Get, Randomness,
ReservableCurrency, Time,
},
weights::{OldWeight, Weight},
weights::Weight,
BoundedVec, WeakBoundedVec,
};
use frame_system::Pallet as System;
Expand Down Expand Up @@ -150,6 +150,12 @@ type RelaxedCodeVec<T> = WeakBoundedVec<u8, <T as Config>::MaxCodeLen>;
type AccountIdLookupOf<T> = <<T as frame_system::Config>::Lookup as StaticLookup>::Source;
type DebugBufferVec<T> = BoundedVec<u8, <T as Config>::MaxDebugBufferLen>;

/// The old weight type.
///
/// This is a copy of the [`frame_support::weights::OldWeight`] type since the contracts pallet
/// needs to support it indefinitely.
type OldWeight = u64;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved

/// Used as a sentinel value when reading and writing contract memory.
///
/// It is usually used to signal `None` to a contract when only a primitive is allowed
Expand Down Expand Up @@ -1281,7 +1287,7 @@ impl<T: Config> Pallet<T> {
/// Used by backwards compatible extrinsics. We cannot just set the proof_size weight limit to
/// zero or an old `Call` will just fail with OutOfGas.
fn compat_weight_limit(gas_limit: OldWeight) -> Weight {
Weight::from_parts(gas_limit.0, u64::from(T::MaxCodeLen::get()) * 2)
Weight::from_parts(gas_limit, u64::from(T::MaxCodeLen::get()) * 2)
}
}

Expand Down
19 changes: 14 additions & 5 deletions frame/contracts/src/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,13 +370,11 @@ mod tests {
gas::GasMeter,
storage::WriteOutcome,
tests::{RuntimeCall, Test, ALICE, BOB},
BalanceOf, CodeHash, Error, Pallet as Contracts,
BalanceOf, CodeHash, Error, OldWeight, Pallet as Contracts,
};
use assert_matches::assert_matches;
use frame_support::{
assert_err, assert_ok,
dispatch::DispatchResultWithPostInfo,
weights::{OldWeight, Weight},
assert_err, assert_ok, dispatch::DispatchResultWithPostInfo, weights::Weight,
};
use pallet_contracts_primitives::{ExecReturnValue, ReturnFlags};
use pretty_assertions::assert_eq;
Expand Down Expand Up @@ -1632,13 +1630,24 @@ mod tests {

let output = execute(CODE_GAS_LEFT, vec![], &mut ext).unwrap();

let OldWeight(gas_left) = OldWeight::decode(&mut &*output.data).unwrap();
let gas_left = OldWeight::decode(&mut &*output.data).unwrap();
let actual_left = ext.gas_meter.gas_left();
// TODO: account for proof size weight
assert!(gas_left < gas_limit.ref_time(), "gas_left must be less than initial");
assert!(gas_left > actual_left.ref_time(), "gas_left must be greater than final");
}

/// Test that [`frame_support::weights::OldWeight`] en/decodes the same as our
/// [`crate::OldWeight`].
#[test]
fn old_weight_decode() {
#![allow(deprecated)]
let sp = frame_support::weights::OldWeight(42).encode();
let our = crate::OldWeight::decode(&mut &*sp).unwrap();

assert_eq!(our, 42);
}

const CODE_VALUE_TRANSFERRED: &str = r#"
(module
(import "seal0" "seal_value_transferred" (func $seal_value_transferred (param i32 i32)))
Expand Down
75 changes: 40 additions & 35 deletions frame/support/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,27 +550,6 @@ impl<T> ClassifyDispatch<T> for (Weight, DispatchClass, Pays) {

// TODO: Eventually remove these

ggwpez marked this conversation as resolved.
Show resolved Hide resolved
impl From<Option<u64>> for PostDispatchInfo {
fn from(maybe_actual_computation: Option<u64>) -> Self {
let actual_weight = match maybe_actual_computation {
Some(actual_computation) => Some(Weight::from_parts(actual_computation, 0)),
None => None,
};
Self { actual_weight, pays_fee: Default::default() }
}
}

impl From<(Option<u64>, Pays)> for PostDispatchInfo {
fn from(post_weight_info: (Option<u64>, Pays)) -> Self {
let (maybe_actual_time, pays_fee) = post_weight_info;
let actual_weight = match maybe_actual_time {
Some(actual_time) => Some(Weight::from_parts(actual_time, 0)),
None => None,
};
Self { actual_weight, pays_fee }
}
}

impl<T> ClassifyDispatch<T> for u64 {
fn classify_dispatch(&self, _: T) -> DispatchClass {
DispatchClass::Normal
Expand Down Expand Up @@ -730,7 +709,7 @@ impl<T> PaysFee<T> for (u64, Pays) {
/// ```
/// # #[macro_use]
/// # extern crate frame_support;
/// # use frame_support::{weights::Weight, dispatch::{DispatchResultWithPostInfo, WithPostDispatchInfo}};
/// # use frame_support::{weights::Weight, dispatch::{DispatchResultWithPostInfo, WithPostDispatchInfo, PostDispatchInfo}};
/// # use frame_system::{Config, ensure_signed};
/// decl_module! {
/// pub struct Module<T: Config> for enum Call where origin: T::RuntimeOrigin {
Expand All @@ -744,7 +723,7 @@ impl<T> PaysFee<T> for (u64, Pays) {
/// return Ok(None::<Weight>.into());
/// }
/// // expensive calculation not executed: use only a portion of the weight
/// Ok(Some(100_000).into())
/// Ok(PostDispatchInfo { actual_weight: Some(Weight::from_parts(100_000, 0)), ..Default::default() })
/// }
/// }
/// }
Expand Down Expand Up @@ -3214,7 +3193,7 @@ mod tests {
OnInitialize, OnRuntimeUpgrade, PalletInfo,
},
};
use sp_weights::RuntimeDbWeight;
use sp_weights::{RuntimeDbWeight, Weight};

pub trait Config: system::Config + Sized
where
Expand Down Expand Up @@ -3535,13 +3514,24 @@ mod tests {
fn test_new_call_variant() {
Call::<TraitImpl>::new_call_variant_aux_0();
}

pub fn from_actual_ref_time(ref_time: Option<u64>) -> PostDispatchInfo {
PostDispatchInfo {
actual_weight: ref_time.map(|t| Weight::from_all(t)),
pays_fee: Default::default(),
}
}

pub fn from_post_weight_info(ref_time: Option<u64>, pays_fee: Pays) -> PostDispatchInfo {
PostDispatchInfo { actual_weight: ref_time.map(|t| Weight::from_all(t)), pays_fee }
}
}

#[cfg(test)]
// Do not complain about unused `dispatch` and `dispatch_aux`.
#[allow(dead_code)]
mod weight_tests {
use super::*;
use super::{tests::*, *};
use sp_core::{parameter_types, Get};
use sp_weights::RuntimeDbWeight;

Expand Down Expand Up @@ -3655,9 +3645,12 @@ mod weight_tests {
#[test]
fn extract_actual_weight_works() {
let pre = DispatchInfo { weight: Weight::from_parts(1000, 0), ..Default::default() };
assert_eq!(extract_actual_weight(&Ok(Some(7).into()), &pre), Weight::from_parts(7, 0));
assert_eq!(
extract_actual_weight(&Ok(Some(1000).into()), &pre),
extract_actual_weight(&Ok(from_actual_ref_time(Some(7))), &pre),
Weight::from_parts(7, 0)
);
assert_eq!(
extract_actual_weight(&Ok(from_actual_ref_time(Some(1000))), &pre),
Weight::from_parts(1000, 0)
);
assert_eq!(
Expand All @@ -3673,7 +3666,7 @@ mod weight_tests {
fn extract_actual_weight_caps_at_pre_weight() {
let pre = DispatchInfo { weight: Weight::from_parts(1000, 0), ..Default::default() };
assert_eq!(
extract_actual_weight(&Ok(Some(1250).into()), &pre),
extract_actual_weight(&Ok(from_actual_ref_time(Some(1250))), &pre),
Weight::from_parts(1000, 0)
);
assert_eq!(
Expand All @@ -3688,10 +3681,19 @@ mod weight_tests {
#[test]
fn extract_actual_pays_fee_works() {
let pre = DispatchInfo { weight: Weight::from_parts(1000, 0), ..Default::default() };
assert_eq!(extract_actual_pays_fee(&Ok(Some(7).into()), &pre), Pays::Yes);
assert_eq!(extract_actual_pays_fee(&Ok(Some(1000).into()), &pre), Pays::Yes);
assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::Yes).into()), &pre), Pays::Yes);
assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::No).into()), &pre), Pays::No);
assert_eq!(extract_actual_pays_fee(&Ok(from_actual_ref_time(Some(7))), &pre), Pays::Yes);
assert_eq!(
extract_actual_pays_fee(&Ok(from_actual_ref_time(Some(1000)).into()), &pre),
Pays::Yes
);
assert_eq!(
extract_actual_pays_fee(&Ok(from_post_weight_info(Some(1000), Pays::Yes)), &pre),
Pays::Yes
);
assert_eq!(
extract_actual_pays_fee(&Ok(from_post_weight_info(Some(1000), Pays::No)), &pre),
Pays::No
);
assert_eq!(
extract_actual_pays_fee(
&Err(DispatchError::BadOrigin.with_weight(Weight::from_parts(9, 0))),
Expand All @@ -3715,9 +3717,12 @@ mod weight_tests {
pays_fee: Pays::No,
..Default::default()
};
assert_eq!(extract_actual_pays_fee(&Ok(Some(7).into()), &pre), Pays::No);
assert_eq!(extract_actual_pays_fee(&Ok(Some(1000).into()), &pre), Pays::No);
assert_eq!(extract_actual_pays_fee(&Ok((Some(1000), Pays::Yes).into()), &pre), Pays::No);
assert_eq!(extract_actual_pays_fee(&Ok(from_actual_ref_time(Some(7))), &pre), Pays::No);
assert_eq!(extract_actual_pays_fee(&Ok(from_actual_ref_time(Some(1000))), &pre), Pays::No);
assert_eq!(
extract_actual_pays_fee(&Ok(from_post_weight_info(Some(1000), Pays::Yes)), &pre),
Pays::No
);
}
}

Expand Down
31 changes: 24 additions & 7 deletions frame/system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,17 +236,23 @@ fn deposit_event_uses_actual_weight_and_pays_fee() {
.get(DispatchClass::Normal)
.base_extrinsic;
let pre_info = DispatchInfo { weight: Weight::from_parts(1000, 0), ..Default::default() };
System::note_applied_extrinsic(&Ok(Some(300).into()), pre_info);
System::note_applied_extrinsic(&Ok(Some(1000).into()), pre_info);
System::note_applied_extrinsic(&Ok(from_actual_ref_time(Some(300))), pre_info);
System::note_applied_extrinsic(&Ok(from_actual_ref_time(Some(1000))), pre_info);
System::note_applied_extrinsic(
// values over the pre info should be capped at pre dispatch value
&Ok(Some(1200).into()),
&Ok(from_actual_ref_time(Some(1200))),
pre_info,
);
System::note_applied_extrinsic(
&Ok(from_post_weight_info(Some(2_500_000), Pays::Yes)),
pre_info,
);
System::note_applied_extrinsic(&Ok((Some(2_500_000), Pays::Yes).into()), pre_info);
System::note_applied_extrinsic(&Ok(Pays::No.into()), pre_info);
System::note_applied_extrinsic(&Ok((Some(2_500_000), Pays::No).into()), pre_info);
System::note_applied_extrinsic(&Ok((Some(500), Pays::No).into()), pre_info);
System::note_applied_extrinsic(
&Ok(from_post_weight_info(Some(2_500_000), Pays::No)),
pre_info,
);
System::note_applied_extrinsic(&Ok(from_post_weight_info(Some(500), Pays::No)), pre_info);
System::note_applied_extrinsic(
&Err(DispatchError::BadOrigin.with_weight(Weight::from_parts(999, 0))),
pre_info,
Expand Down Expand Up @@ -289,7 +295,7 @@ fn deposit_event_uses_actual_weight_and_pays_fee() {
class: DispatchClass::Operational,
..Default::default()
};
System::note_applied_extrinsic(&Ok(Some(300).into()), pre_info);
System::note_applied_extrinsic(&Ok(from_actual_ref_time(Some(300))), pre_info);

let got = System::events();
let want = vec![
Expand Down Expand Up @@ -691,3 +697,14 @@ fn ensure_signed_stuff_works() {
assert_ok!(EnsureSignedBy::<Members, _>::try_origin(successful_origin));
}
}

pub fn from_actual_ref_time(ref_time: Option<u64>) -> PostDispatchInfo {
PostDispatchInfo {
actual_weight: ref_time.map(|t| Weight::from_all(t)),
pays_fee: Default::default(),
}
}

pub fn from_post_weight_info(ref_time: Option<u64>, pays_fee: Pays) -> PostDispatchInfo {
PostDispatchInfo { actual_weight: ref_time.map(|t| Weight::from_all(t)), pays_fee }
}
Loading