From ce60127b0458418df40bb516cfcbb238f1388618 Mon Sep 17 00:00:00 2001 From: Shaopeng Wang Date: Tue, 9 Feb 2021 22:37:11 +1300 Subject: [PATCH 1/5] Migrate pallet-timestamp to pallet attribute macro. --- frame/timestamp/src/lib.rs | 135 +++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 64 deletions(-) diff --git a/frame/timestamp/src/lib.rs b/frame/timestamp/src/lib.rs index ae7ba4814694b..570d842dd723a 100644 --- a/frame/timestamp/src/lib.rs +++ b/frame/timestamp/src/lib.rs @@ -15,23 +15,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # Timestamp Module +//! # Timestamp Pallet //! -//! The Timestamp module provides functionality to get and set the on-chain time. +//! The Timestamp pallet provides functionality to get and set the on-chain time. //! //! - [`timestamp::Config`](./trait.Config.html) //! - [`Call`](./enum.Call.html) -//! - [`Module`](./struct.Module.html) +//! - [`Pallet`](./struct.Pallet.html) //! //! ## Overview //! -//! The Timestamp module allows the validators to set and validate a timestamp with each block. +//! The Timestamp pallet allows the validators to set and validate a timestamp with each block. //! //! It uses inherents for timestamp data, which is provided by the block author and validated/verified //! by other validators. The timestamp can be set only once per block and must be set each block. //! There could be a constraint on how much time must pass before setting the new timestamp. //! -//! **NOTE:** The Timestamp module is the recommended way to query the on-chain time instead of using +//! **NOTE:** The Timestamp pallet is the recommended way to query the on-chain time instead of using //! an approach based on block numbers. The block number based time measurement can cause issues //! because of cumulative calculation errors and hence should be avoided. //! @@ -52,11 +52,11 @@ //! //! ## Usage //! -//! The following example shows how to use the Timestamp module in your custom module to query the current timestamp. +//! The following example shows how to use the Timestamp pallet in your custom pallet to query the current timestamp. //! //! ### Prerequisites //! -//! Import the Timestamp module into your custom module and derive the module configuration +//! Import the Timestamp pallet into your custom pallet and derive the pallet configuration //! trait from the timestamp trait. //! //! ### Get current timestamp @@ -83,10 +83,10 @@ //! //! ### Example from the FRAME //! -//! The [Session module](https://github.com/paritytech/substrate/blob/master/frame/session/src/lib.rs) uses -//! the Timestamp module for session management. +//! The [Session pallet](https://github.com/paritytech/substrate/blob/master/frame/session/src/lib.rs) uses +//! the Timestamp pallet for session management. //! -//! ## Related Modules +//! ## Related Pallets //! //! * [Session](../pallet_session/index.html) @@ -99,51 +99,80 @@ use sp_std::{result, cmp}; use sp_inherents::{ProvideInherent, InherentData, InherentIdentifier}; #[cfg(feature = "std")] use frame_support::debug; -use frame_support::{ - Parameter, decl_storage, decl_module, - traits::{Time, UnixTime, Get}, - weights::{DispatchClass, Weight}, -}; +use frame_support::traits::{Time, UnixTime, Get}; use sp_runtime::{ RuntimeString, traits::{ AtLeast32Bit, Zero, SaturatedConversion, Scale, } }; -use frame_system::ensure_none; use sp_timestamp::{ InherentError, INHERENT_IDENTIFIER, InherentType, OnTimestampSet, }; pub use weights::WeightInfo; -/// The module configuration trait -pub trait Config: frame_system::Config { - /// Type used for expressing timestamp. - type Moment: Parameter + Default + AtLeast32Bit - + Scale + Copy; +pub use pallet::*; - /// Something which can be notified when the timestamp is set. Set this to `()` if not needed. - type OnTimestampSet: OnTimestampSet; +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::*; - /// The minimum period between blocks. Beware that this is different to the *expected* period - /// that the block production apparatus provides. Your chosen consensus system will generally - /// work with this to determine a sensible block time. e.g. For Aura, it will be double this - /// period on default settings. - type MinimumPeriod: Get; + /// The pallet configuration trait + #[pallet::config] + pub trait Config: frame_system::Config { + /// Type used for expressing timestamp. + type Moment: Parameter + Default + AtLeast32Bit + + Scale + Copy; - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} + /// Something which can be notified when the timestamp is set. Set this to `()` if not needed. + type OnTimestampSet: OnTimestampSet; -decl_module! { - pub struct Module for enum Call where origin: T::Origin { /// The minimum period between blocks. Beware that this is different to the *expected* period /// that the block production apparatus provides. Your chosen consensus system will generally /// work with this to determine a sensible block time. e.g. For Aura, it will be double this /// period on default settings. - const MinimumPeriod: T::Moment = T::MinimumPeriod::get(); + #[pallet::constant] + type MinimumPeriod: Get; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + } + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(PhantomData); + + /// Current time for the current block. + #[pallet::storage] + #[pallet::getter(fn now)] + pub type Now = StorageValue<_, T::Moment, ValueQuery>; + + /// Did the timestamp get updated in this block? + #[pallet::storage] + pub(crate) type DidUpdate = StorageValue<_, bool, ValueQuery>; + + #[pallet::hooks] + impl Hooks> for Pallet { + /// dummy `on_initialize` to return the weight used in `on_finalize`. + fn on_initialize(_n: BlockNumberFor) -> Weight { + // weight of `on_finalize` + T::WeightInfo::on_finalize() + } + + /// # + /// - `O(1)` + /// - 1 storage deletion (codec `O(1)`). + /// # + fn on_finalize(_n: BlockNumberFor) { + assert!(::DidUpdate::take(), "Timestamp must be updated once in the block"); + } + } + #[pallet::call] + impl Pallet { /// Set the current time. /// /// This call should be invoked exactly once per block. It will panic at the finalization @@ -159,11 +188,11 @@ decl_module! { /// - 1 storage read and 1 storage mutation (codec `O(1)`). (because of `DidUpdate::take` in `on_finalize`) /// - 1 event handler `on_timestamp_set`. Must be `O(1)`. /// # - #[weight = ( + #[pallet::weight(( T::WeightInfo::set(), DispatchClass::Mandatory - )] - fn set(origin, #[compact] now: T::Moment) { + ))] + pub(crate) fn set(origin: OriginFor, #[pallet::compact] now: T::Moment) -> DispatchResultWithPostInfo { ensure_none(origin)?; assert!(!::DidUpdate::exists(), "Timestamp must be updated only once in the block"); let prev = Self::now(); @@ -175,35 +204,13 @@ decl_module! { ::DidUpdate::put(true); >::on_timestamp_set(now); - } - - /// dummy `on_initialize` to return the weight used in `on_finalize`. - fn on_initialize() -> Weight { - // weight of `on_finalize` - T::WeightInfo::on_finalize() - } - /// # - /// - `O(1)` - /// - 1 storage deletion (codec `O(1)`). - /// # - fn on_finalize() { - assert!(::DidUpdate::take(), "Timestamp must be updated once in the block"); + Ok(().into()) } } } -decl_storage! { - trait Store for Module as Timestamp { - /// Current time for the current block. - pub Now get(fn now): T::Moment; - - /// Did the timestamp get updated in this block? - DidUpdate: bool; - } -} - -impl Module { +impl Pallet { /// Get the current time for the current block. /// /// NOTE: if this function is called prior to setting the timestamp, @@ -225,7 +232,7 @@ fn extract_inherent_data(data: &InherentData) -> Result ProvideInherent for Module { +impl ProvideInherent for Pallet { type Call = Call; type Error = InherentError; const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; @@ -260,7 +267,7 @@ impl ProvideInherent for Module { } } -impl Time for Module { +impl Time for Pallet { type Moment = T::Moment; /// Before the first set of now with inherent the value returned is zero. @@ -272,7 +279,7 @@ impl Time for Module { /// Before the timestamp inherent is applied, it returns the time of previous block. /// /// On genesis the time returned is not valid. -impl UnixTime for Module { +impl UnixTime for Pallet { fn now() -> core::time::Duration { // now is duration since unix epoch in millisecond as documented in // `sp_timestamp::InherentDataProvider`. From 1936ba3f344ce53457155f6340ff40509cd7469a Mon Sep 17 00:00:00 2001 From: Shaopeng Wang Date: Tue, 9 Feb 2021 23:59:30 +1300 Subject: [PATCH 2/5] Migrate inherent. --- frame/timestamp/src/lib.rs | 75 +++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 37 deletions(-) diff --git a/frame/timestamp/src/lib.rs b/frame/timestamp/src/lib.rs index 570d842dd723a..809c2742001c9 100644 --- a/frame/timestamp/src/lib.rs +++ b/frame/timestamp/src/lib.rs @@ -96,10 +96,10 @@ mod benchmarking; pub mod weights; use sp_std::{result, cmp}; -use sp_inherents::{ProvideInherent, InherentData, InherentIdentifier}; +use sp_inherents::InherentData; #[cfg(feature = "std")] use frame_support::debug; -use frame_support::traits::{Time, UnixTime, Get}; +use frame_support::traits::{Time, UnixTime}; use sp_runtime::{ RuntimeString, traits::{ @@ -208,6 +208,42 @@ pub mod pallet { Ok(().into()) } } + + #[pallet::inherent] + impl ProvideInherent for Pallet { + type Call = Call; + type Error = InherentError; + const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; + + fn create_inherent(data: &InherentData) -> Option { + let data: T::Moment = extract_inherent_data(data) + .expect("Gets and decodes timestamp inherent data") + .saturated_into(); + + let next_time = cmp::max(data, Self::now() + T::MinimumPeriod::get()); + Some(Call::set(next_time.into())) + } + + fn check_inherent(call: &Self::Call, data: &InherentData) -> result::Result<(), Self::Error> { + const MAX_TIMESTAMP_DRIFT_MILLIS: u64 = 30 * 1000; + + let t: u64 = match call { + Call::set(ref t) => t.clone().saturated_into::(), + _ => return Ok(()), + }; + + let data = extract_inherent_data(data).map_err(|e| InherentError::Other(e))?; + + let minimum = (Self::now() + T::MinimumPeriod::get()).saturated_into::(); + if t > data + MAX_TIMESTAMP_DRIFT_MILLIS { + Err(InherentError::Other("Timestamp too far in future to accept".into())) + } else if t < minimum { + Err(InherentError::ValidAtTimestamp(minimum)) + } else { + Ok(()) + } + } + } } impl Pallet { @@ -232,41 +268,6 @@ fn extract_inherent_data(data: &InherentData) -> Result ProvideInherent for Pallet { - type Call = Call; - type Error = InherentError; - const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; - - fn create_inherent(data: &InherentData) -> Option { - let data: T::Moment = extract_inherent_data(data) - .expect("Gets and decodes timestamp inherent data") - .saturated_into(); - - let next_time = cmp::max(data, Self::now() + T::MinimumPeriod::get()); - Some(Call::set(next_time.into())) - } - - fn check_inherent(call: &Self::Call, data: &InherentData) -> result::Result<(), Self::Error> { - const MAX_TIMESTAMP_DRIFT_MILLIS: u64 = 30 * 1000; - - let t: u64 = match call { - Call::set(ref t) => t.clone().saturated_into::(), - _ => return Ok(()), - }; - - let data = extract_inherent_data(data).map_err(|e| InherentError::Other(e))?; - - let minimum = (Self::now() + T::MinimumPeriod::get()).saturated_into::(); - if t > data + MAX_TIMESTAMP_DRIFT_MILLIS { - Err(InherentError::Other("Timestamp too far in future to accept".into())) - } else if t < minimum { - Err(InherentError::ValidAtTimestamp(minimum)) - } else { - Ok(()) - } - } -} - impl Time for Pallet { type Moment = T::Moment; From fa2d670883d6ea37aa328b92d1c1753170f225a3 Mon Sep 17 00:00:00 2001 From: Shaopeng Wang Date: Wed, 10 Feb 2021 00:25:25 +1300 Subject: [PATCH 3/5] Unify private visbility. --- frame/timestamp/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/timestamp/src/lib.rs b/frame/timestamp/src/lib.rs index 809c2742001c9..8a265968354b0 100644 --- a/frame/timestamp/src/lib.rs +++ b/frame/timestamp/src/lib.rs @@ -152,7 +152,7 @@ pub mod pallet { /// Did the timestamp get updated in this block? #[pallet::storage] - pub(crate) type DidUpdate = StorageValue<_, bool, ValueQuery>; + pub(super) type DidUpdate = StorageValue<_, bool, ValueQuery>; #[pallet::hooks] impl Hooks> for Pallet { @@ -192,7 +192,7 @@ pub mod pallet { T::WeightInfo::set(), DispatchClass::Mandatory ))] - pub(crate) fn set(origin: OriginFor, #[pallet::compact] now: T::Moment) -> DispatchResultWithPostInfo { + pub(super) fn set(origin: OriginFor, #[pallet::compact] now: T::Moment) -> DispatchResultWithPostInfo { ensure_none(origin)?; assert!(!::DidUpdate::exists(), "Timestamp must be updated only once in the block"); let prev = Self::now(); From b100fc2a38abce0f928cb40ef1d4016fcb127d58 Mon Sep 17 00:00:00 2001 From: Shaopeng Wang Date: Wed, 10 Feb 2021 11:54:08 +1300 Subject: [PATCH 4/5] Update benchmarking. --- frame/timestamp/src/benchmarking.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/timestamp/src/benchmarking.rs b/frame/timestamp/src/benchmarking.rs index 024e6967826cd..ad249cbae69f2 100644 --- a/frame/timestamp/src/benchmarking.rs +++ b/frame/timestamp/src/benchmarking.rs @@ -33,7 +33,7 @@ benchmarks! { set { let t = MAX_TIME; // Ignore write to `DidUpdate` since it transient. - let did_update_key = crate::DidUpdate::hashed_key().to_vec(); + let did_update_key = crate::DidUpdate::::hashed_key().to_vec(); frame_benchmarking::benchmarking::add_to_whitelist(TrackedStorageKey { key: did_update_key, has_been_read: false, @@ -47,13 +47,13 @@ benchmarks! { on_finalize { let t = MAX_TIME; Timestamp::::set(RawOrigin::None.into(), t.into())?; - ensure!(DidUpdate::exists(), "Time was not set."); + ensure!(DidUpdate::::exists(), "Time was not set."); // Ignore read/write to `DidUpdate` since it is transient. - let did_update_key = crate::DidUpdate::hashed_key().to_vec(); + let did_update_key = crate::DidUpdate::::hashed_key().to_vec(); frame_benchmarking::benchmarking::add_to_whitelist(did_update_key.into()); }: { Timestamp::::on_finalize(t.into()); } verify { - ensure!(!DidUpdate::exists(), "Time was not removed."); + ensure!(!DidUpdate::::exists(), "Time was not removed."); } } From 8c027aa1145da4877bae654eececad5ae7a33d18 Mon Sep 17 00:00:00 2001 From: Shaopeng Wang Date: Wed, 10 Feb 2021 15:38:06 +1300 Subject: [PATCH 5/5] Update storage usages. --- frame/timestamp/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/timestamp/src/lib.rs b/frame/timestamp/src/lib.rs index 8a265968354b0..86ca0c11a70c8 100644 --- a/frame/timestamp/src/lib.rs +++ b/frame/timestamp/src/lib.rs @@ -167,7 +167,7 @@ pub mod pallet { /// - 1 storage deletion (codec `O(1)`). /// # fn on_finalize(_n: BlockNumberFor) { - assert!(::DidUpdate::take(), "Timestamp must be updated once in the block"); + assert!(DidUpdate::::take(), "Timestamp must be updated once in the block"); } } @@ -194,14 +194,14 @@ pub mod pallet { ))] pub(super) fn set(origin: OriginFor, #[pallet::compact] now: T::Moment) -> DispatchResultWithPostInfo { ensure_none(origin)?; - assert!(!::DidUpdate::exists(), "Timestamp must be updated only once in the block"); + assert!(!DidUpdate::::exists(), "Timestamp must be updated only once in the block"); let prev = Self::now(); assert!( prev.is_zero() || now >= prev + T::MinimumPeriod::get(), "Timestamp must increment by at least between sequential blocks" ); - ::Now::put(now); - ::DidUpdate::put(true); + Now::::put(now); + DidUpdate::::put(true); >::on_timestamp_set(now); @@ -258,7 +258,7 @@ impl Pallet { /// Set the timestamp to something in particular. Only used for tests. #[cfg(feature = "std")] pub fn set_timestamp(now: T::Moment) { - ::Now::put(now); + Now::::put(now); } }