diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 7340f35ca0..1b77246031 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -4399,7 +4399,6 @@ dependencies = [ "sp-application-crypto", "sp-consensus-aura", "sp-core", - "sp-inherents", "sp-io", "sp-runtime", "sp-std", diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs index 5efe5492b9..8d68dbdc96 100644 --- a/substrate/bin/node-template/runtime/src/lib.rs +++ b/substrate/bin/node-template/runtime/src/lib.rs @@ -273,7 +273,7 @@ construct_runtime!( System: frame_system::{Module, Call, Config, Storage, Event}, RandomnessCollectiveFlip: pallet_randomness_collective_flip::{Module, Call, Storage}, Timestamp: pallet_timestamp::{Module, Call, Storage, Inherent}, - Aura: pallet_aura::{Module, Config, Inherent}, + Aura: pallet_aura::{Module, Config}, Grandpa: pallet_grandpa::{Module, Call, Storage, Config, Event}, Balances: pallet_balances::{Module, Call, Storage, Config, Event}, TransactionPayment: pallet_transaction_payment::{Module, Storage}, diff --git a/substrate/frame/aura/Cargo.toml b/substrate/frame/aura/Cargo.toml index 2cd7e5c15f..9034e483f3 100644 --- a/substrate/frame/aura/Cargo.toml +++ b/substrate/frame/aura/Cargo.toml @@ -15,7 +15,6 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] sp-application-crypto = { version = "2.0.0", default-features = false, path = "../../primitives/application-crypto" } codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } -sp-inherents = { version = "2.0.0", default-features = false, path = "../../primitives/inherents" } sp-std = { version = "2.0.0", default-features = false, path = "../../primitives/std" } serde = { version = "1.0.101", optional = true } pallet-session = { version = "2.0.0", default-features = false, path = "../session" } @@ -37,7 +36,6 @@ default = ["std"] std = [ "sp-application-crypto/std", "codec/std", - "sp-inherents/std", "sp-std/std", "serde", "sp-runtime/std", diff --git a/substrate/frame/aura/src/lib.rs b/substrate/frame/aura/src/lib.rs index 61937da286..db639a4499 100644 --- a/substrate/frame/aura/src/lib.rs +++ b/substrate/frame/aura/src/lib.rs @@ -34,17 +34,10 @@ //! //! - [Timestamp](../pallet_timestamp/index.html): The Timestamp module is used in Aura to track //! consensus rounds (via `slots`). -//! -//! ## References -//! -//! If you're interested in hacking on this module, it is useful to understand the interaction with -//! `substrate/primitives/inherents/src/lib.rs` and, specifically, the required implementation of -//! [`ProvideInherent`](../sp_inherents/trait.ProvideInherent.html) and -//! [`ProvideInherentData`](../sp_inherents/trait.ProvideInherentData.html) to create and check inherents. #![cfg_attr(not(feature = "std"), no_std)] -use sp_std::{result, prelude::*}; +use sp_std::prelude::*; use codec::{Encode, Decode}; use frame_support::{Parameter, traits::{Get, FindAuthor, OneSessionHandler}, ConsensusEngineId}; use sp_runtime::{ @@ -52,14 +45,11 @@ use sp_runtime::{ traits::{SaturatedConversion, Saturating, Zero, Member, IsMember}, generic::DigestItem, }; use sp_timestamp::OnTimestampSet; -use sp_inherents::{InherentIdentifier, InherentData, ProvideInherent, MakeFatalError}; -use sp_consensus_aura::{ - AURA_ENGINE_ID, ConsensusLog, AuthorityIndex, Slot, - inherents::{INHERENT_IDENTIFIER, AuraInherentData}, -}; +use sp_consensus_aura::{AURA_ENGINE_ID, ConsensusLog, AuthorityIndex, Slot}; mod mock; mod tests; +pub mod migrations; pub use pallet::*; @@ -79,7 +69,22 @@ pub mod pallet { pub struct Pallet(sp_std::marker::PhantomData); #[pallet::hooks] - impl Hooks> for Pallet {} + impl Hooks> for Pallet { + fn on_initialize(_: T::BlockNumber) -> Weight { + if let Some(new_slot) = Self::current_slot_from_digests() { + let current_slot = CurrentSlot::::get(); + + assert!(current_slot < new_slot, "Slot must increase"); + CurrentSlot::::put(new_slot); + + // TODO [#3398] Generate offence report for all authorities that skipped their slots. + + T::DbWeight::get().reads_writes(2, 1) + } else { + T::DbWeight::get().reads(1) + } + } + } #[pallet::call] impl Pallet {} @@ -89,10 +94,12 @@ pub mod pallet { #[pallet::getter(fn authorities)] pub(super) type Authorities = StorageValue<_, Vec, ValueQuery>; - /// The last timestamp we have been notified of. + /// The current slot of this block. + /// + /// This will be set in `on_initialize`. #[pallet::storage] - #[pallet::getter(fn last_timestamp)] - pub(super) type LastTimestamp = StorageValue<_, T::Moment, ValueQuery>; + #[pallet::getter(fn current_slot)] + pub(super) type CurrentSlot = StorageValue<_, Slot, ValueQuery>; #[pallet::genesis_config] pub struct GenesisConfig { @@ -132,6 +139,19 @@ impl Pallet { } } + /// Get the current slot from the pre-runtime digests. + fn current_slot_from_digests() -> Option { + let digest = frame_system::Pallet::::digest(); + let pre_runtime_digests = digest.logs.iter().filter_map(|d| d.as_pre_runtime()); + for (id, mut data) in pre_runtime_digests { + if id == AURA_ENGINE_ID { + return Slot::decode(&mut data).ok(); + } + } + + None + } + /// Determine the Aura slot-duration based on the Timestamp module configuration. pub fn slot_duration() -> T::Moment { // we double the minimum block-period so each author can always propose within @@ -224,49 +244,12 @@ impl IsMember for Pallet { impl OnTimestampSet for Pallet { fn on_timestamp_set(moment: T::Moment) { - let last = Self::last_timestamp(); - LastTimestamp::::put(moment); - - if last.is_zero() { - return; - } - let slot_duration = Self::slot_duration(); assert!(!slot_duration.is_zero(), "Aura slot duration cannot be zero."); - let last_slot = last / slot_duration; - let cur_slot = moment / slot_duration; + let timestamp_slot = moment / slot_duration; + let timestamp_slot = Slot::from(timestamp_slot.saturated_into::()); - assert!(last_slot < cur_slot, "Only one block may be authored per slot."); - - // TODO [#3398] Generate offence report for all authorities that skipped their slots. - } -} - -impl ProvideInherent for Pallet { - type Call = pallet_timestamp::Call; - type Error = MakeFatalError; - const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; - - fn create_inherent(_: &InherentData) -> Option { - None - } - - /// Verify the validity of the inherent using the timestamp. - fn check_inherent(call: &Self::Call, data: &InherentData) -> result::Result<(), Self::Error> { - let timestamp = match call { - pallet_timestamp::Call::set(ref timestamp) => timestamp.clone(), - _ => return Ok(()), - }; - - let timestamp_based_slot = timestamp / Self::slot_duration(); - - let seal_slot = u64::from(data.aura_inherent_data()?).saturated_into(); - - if timestamp_based_slot == seal_slot { - Ok(()) - } else { - Err(sp_inherents::Error::from("timestamp set in block doesn't match slot in seal").into()) - } + assert!(CurrentSlot::::get() == timestamp_slot, "Timestamp slot must match `CurrentSlot`"); } } diff --git a/substrate/frame/aura/src/migrations.rs b/substrate/frame/aura/src/migrations.rs new file mode 100644 index 0000000000..038c5b3f3f --- /dev/null +++ b/substrate/frame/aura/src/migrations.rs @@ -0,0 +1,43 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Migrations for the AURA pallet. + +use frame_support::{traits::Get, weights::Weight, pallet_prelude::*}; + +struct __LastTimestamp(sp_std::marker::PhantomData); +impl frame_support::traits::StorageInstance for __LastTimestamp { + fn pallet_prefix() -> &'static str { T::PalletPrefix::get() } + const STORAGE_PREFIX: &'static str = "LastTimestamp"; +} + +type LastTimestamp = StorageValue<__LastTimestamp, (), ValueQuery>; + +pub trait RemoveLastTimestamp: super::Config { + type PalletPrefix: Get<&'static str>; +} + +/// Remove the `LastTimestamp` storage value. +/// +/// This storage value was removed and replaced by `CurrentSlot`. As we only remove this storage +/// value, it is safe to call this method multiple times. +/// +/// This migration requires a type `T` that implements [`RemoveLastTimestamp`]. +pub fn remove_last_timestamp() -> Weight { + LastTimestamp::::kill(); + T::DbWeight::get().writes(1) +} diff --git a/substrate/frame/aura/src/mock.rs b/substrate/frame/aura/src/mock.rs index 8eef18448d..a5ef12f593 100644 --- a/substrate/frame/aura/src/mock.rs +++ b/substrate/frame/aura/src/mock.rs @@ -21,12 +21,8 @@ use crate as pallet_aura; use sp_consensus_aura::ed25519::AuthorityId; -use sp_runtime::{ - traits::IdentityLookup, - testing::{Header, UintAuthorityId}, -}; +use sp_runtime::{traits::IdentityLookup, testing::{Header, UintAuthorityId}}; use frame_support::{parameter_types, traits::GenesisBuild}; -use sp_io; use sp_core::H256; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -40,7 +36,7 @@ frame_support::construct_runtime!( { System: frame_system::{Module, Call, Config, Storage, Event}, Timestamp: pallet_timestamp::{Module, Call, Storage, Inherent}, - Aura: pallet_aura::{Module, Call, Storage, Config, Inherent}, + Aura: pallet_aura::{Module, Call, Storage, Config}, } ); diff --git a/substrate/frame/aura/src/tests.rs b/substrate/frame/aura/src/tests.rs index 00b792c300..18e14e802b 100644 --- a/substrate/frame/aura/src/tests.rs +++ b/substrate/frame/aura/src/tests.rs @@ -24,7 +24,7 @@ use crate::mock::{Aura, new_test_ext}; #[test] fn initial_values() { new_test_ext(vec![0, 1, 2, 3]).execute_with(|| { - assert_eq!(Aura::last_timestamp(), 0u64); + assert_eq!(Aura::current_slot(), 0u64); assert_eq!(Aura::authorities().len(), 4); }); } diff --git a/substrate/primitives/consensus/aura/src/inherents.rs b/substrate/primitives/consensus/aura/src/inherents.rs index 2b73b22295..35f686d934 100644 --- a/substrate/primitives/consensus/aura/src/inherents.rs +++ b/substrate/primitives/consensus/aura/src/inherents.rs @@ -48,6 +48,7 @@ impl AuraInherentData for InherentData { } /// Provides the slot duration inherent data for `Aura`. +// TODO: Remove in the future. https://github.com/paritytech/substrate/issues/8029 #[cfg(feature = "std")] pub struct InherentDataProvider { slot_duration: u64,