Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate pallet-aura to umbrella crate #6622

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions prdoc/pr_6622.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# 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: migrate pallet-aura to umbrella crate

doc:
- audience: Runtime Dev
description: |
This PR
- Imports frame umbrella crate systems into pallet-aura.
- Includes BoundedSlice as a runtime::prelude export from the polkadot-sdk-frame crate.
- Includes `sp-application-crypto` in polkadot-sdk-frame crate, re-exports
`RuntimeAppPublic` and `BoundToRuntimeAppPublic` as part of the cryptography module.
- Adds a consensus module for commonly used consensus types.

crates:
- name: pallet-aura
bump: minor
- name: polkadot-sdk-frame
bump: minor
3 changes: 3 additions & 0 deletions substrate/frame/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ sp-genesis-builder = { optional = true, workspace = true }
sp-inherents = { optional = true, workspace = true }
sp-storage = { optional = true, workspace = true }
sp-keyring = { optional = true, workspace = true }
sp-application-crypto = { optional = true, workspace = true }

frame-executive = { optional = true, workspace = true }
frame-system-rpc-runtime-api = { optional = true, workspace = true }
Expand All @@ -72,6 +73,7 @@ runtime = [
"frame-executive",
"frame-system-rpc-runtime-api",
"sp-api",
"sp-application-crypto",
"sp-block-builder",
"sp-consensus-aura",
"sp-consensus-grandpa",
Expand All @@ -96,6 +98,7 @@ std = [
"log/std",
"scale-info/std",
"sp-api?/std",
"sp-application-crypto/std",
"sp-arithmetic/std",
"sp-block-builder?/std",
"sp-consensus-aura?/std",
Expand Down
22 changes: 3 additions & 19 deletions substrate/frame/aura/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,35 +19,19 @@ targets = ["x86_64-unknown-linux-gnu"]
codec = { features = ["derive", "max-encoded-len"], workspace = true }
log = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
frame = { workspace = true, features = ["experimental", "runtime"] }
pallet-timestamp = { workspace = true }
sp-application-crypto = { workspace = true }
sp-consensus-aura = { workspace = true }
sp-runtime = { workspace = true }

[dev-dependencies]
sp-core = { workspace = true }
sp-io = { workspace = true, default-features = true }

[features]
default = ["std"]
std = [
"codec/std",
"frame-support/std",
"frame-system/std",
"frame/std",
"log/std",
"pallet-timestamp/std",
"scale-info/std",
"sp-application-crypto/std",
"sp-consensus-aura/std",
"sp-core/std",
"sp-io/std",
"sp-runtime/std",
]
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"frame/try-runtime",
"pallet-timestamp/try-runtime",
"sp-runtime/try-runtime",
]
33 changes: 14 additions & 19 deletions substrate/frame/aura/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,14 @@ extern crate alloc;

use alloc::vec::Vec;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
traits::{DisabledValidators, FindAuthor, Get, OnTimestampSet, OneSessionHandler},
BoundedSlice, BoundedVec, ConsensusEngineId, Parameter,
use frame::{
runtime::prelude::*,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the main prelude frame::prelude::* instead of the runtime one

traits::{DisabledValidators, FindAuthor, IsMember, OnTimestampSet, OneSessionHandler},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add IsMember to the main prelude of the umbrella crate

Copy link
Contributor Author

@runcomet runcomet Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also moved OnTimestampSet to top level prelude, used in Babe and Aura pallets.

FindAuthor & DisableValidators to account mod

};
use log;
use sp_consensus_aura::{AuthorityIndex, ConsensusLog, Slot, AURA_ENGINE_ID};
use sp_runtime::{
generic::DigestItem,
traits::{IsMember, Member, SaturatedConversion, Saturating, Zero},
RuntimeAppPublic,
};

#[cfg(feature = "try-runtime")]
use frame::try_runtime::TryRuntimeError;

pub mod migrations;
mod mock;
Expand All @@ -76,11 +73,9 @@ impl<T: pallet_timestamp::Config> Get<T::Moment> for MinimumPeriodTimesTwo<T> {
}
}

#[frame_support::pallet]
#[frame::pallet]
pub mod pallet {
use super::*;
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

#[pallet::config]
pub trait Config: pallet_timestamp::Config + frame_system::Config {
Expand Down Expand Up @@ -157,7 +152,7 @@ pub mod pallet {
}

#[cfg(feature = "try-runtime")]
fn try_state(_: BlockNumberFor<T>) -> Result<(), sp_runtime::TryRuntimeError> {
fn try_state(_: BlockNumberFor<T>) -> Result<(), TryRuntimeError> {
Self::do_try_state()
}
}
Expand All @@ -174,7 +169,7 @@ pub mod pallet {
pub type CurrentSlot<T: Config> = StorageValue<_, Slot, ValueQuery>;

#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
#[derive(DefaultNoBound)]
pub struct GenesisConfig<T: Config> {
pub authorities: Vec<T::AuthorityId>,
}
Expand Down Expand Up @@ -265,7 +260,7 @@ impl<T: Config> Pallet<T> {
/// * The number of authorities must be less than or equal to `T::MaxAuthorities`. This however,
/// is guarded by the type system.
#[cfg(any(test, feature = "try-runtime"))]
pub fn do_try_state() -> Result<(), sp_runtime::TryRuntimeError> {
pub fn do_try_state() -> Result<(), TryRuntimeError> {
// We don't have any guarantee that we are already after `on_initialize` and thus we have to
// check the current slot from the digest or take the last known slot.
let current_slot =
Expand All @@ -274,7 +269,7 @@ impl<T: Config> Pallet<T> {
// Check that the current slot is less than the maximal slot number, unless we allow for
// multiple blocks per slot.
if !T::AllowMultipleBlocksPerSlot::get() {
frame_support::ensure!(
ensure!(
current_slot < u64::MAX,
"Current slot has reached maximum value and cannot be incremented further.",
);
Expand All @@ -284,11 +279,11 @@ impl<T: Config> Pallet<T> {
<Authorities<T>>::decode_len().ok_or("Failed to decode authorities length")?;

// Check that the authorities are non-empty.
frame_support::ensure!(!authorities_len.is_zero(), "Authorities must be non-empty.");
ensure!(!authorities_len.is_zero(), "Authorities must be non-empty.");

// Check that the current authority is not disabled.
let authority_index = *current_slot % authorities_len as u64;
frame_support::ensure!(
ensure!(
!T::DisabledValidators::is_disabled(authority_index as u32),
"Current validator is disabled and should not be attempting to author blocks.",
);
Expand All @@ -297,7 +292,7 @@ impl<T: Config> Pallet<T> {
}
}

impl<T: Config> sp_runtime::BoundToRuntimeAppPublic for Pallet<T> {
impl<T: Config> BoundToRuntimeAppPublic for Pallet<T> {
type Public = T::AuthorityId;
}

Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/aura/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@

//! Migrations for the AURA pallet.

use frame_support::{pallet_prelude::*, traits::Get, weights::Weight};
use frame::prelude::*;

struct __LastTimestamp<T>(core::marker::PhantomData<T>);
impl<T: RemoveLastTimestamp> frame_support::traits::StorageInstance for __LastTimestamp<T> {
impl<T: RemoveLastTimestamp> frame::traits::StorageInstance for __LastTimestamp<T> {
fn pallet_prefix() -> &'static str {
T::PalletPrefix::get()
}
Expand Down
13 changes: 4 additions & 9 deletions substrate/frame/aura/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,13 @@
#![cfg(test)]

use crate as pallet_aura;
use frame_support::{
derive_impl, parameter_types,
traits::{ConstU32, ConstU64, DisabledValidators},
};
use sp_consensus_aura::{ed25519::AuthorityId, AuthorityIndex};
use sp_runtime::{testing::UintAuthorityId, BuildStorage};
use frame::testing_prelude::*;

type Block = frame_system::mocking::MockBlock<Test>;

const SLOT_DURATION: u64 = 2;

frame_support::construct_runtime!(
construct_runtime!(
pub enum Test
{
System: frame_system,
Expand Down Expand Up @@ -69,7 +64,7 @@ impl MockDisabledValidators {
}
}

impl DisabledValidators for MockDisabledValidators {
impl crate::DisabledValidators for MockDisabledValidators {
fn is_disabled(index: AuthorityIndex) -> bool {
DisabledValidatorTestValue::get().binary_search(&index).is_ok()
}
Expand All @@ -87,7 +82,7 @@ impl pallet_aura::Config for Test {
type SlotDuration = ConstU64<SLOT_DURATION>;
}

fn build_ext(authorities: Vec<u64>) -> sp_io::TestExternalities {
fn build_ext(authorities: Vec<u64>) -> TestExternalities {
let mut storage = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap();
pallet_aura::GenesisConfig::<Test> {
authorities: authorities.into_iter().map(|a| UintAuthorityId(a).to_public_key()).collect(),
Expand Down
4 changes: 1 addition & 3 deletions substrate/frame/aura/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@
use super::pallet;
use crate::mock::{build_ext_and_execute_test, Aura, MockDisabledValidators, System, Test};
use codec::Encode;
use frame_support::traits::OnInitialize;
use sp_consensus_aura::{Slot, AURA_ENGINE_ID};
use sp_runtime::{Digest, DigestItem};
use frame::testing_prelude::*;

#[test]
fn initial_values() {
Expand Down
29 changes: 25 additions & 4 deletions substrate/frame/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,10 @@ pub mod prelude {
pub use super::derive::*;

/// All hashing related things
pub use super::hashing::*;
pub use super::cryptography::*;

/// Consensus types needed in runtime construction.
pub use super::consensus::*;

/// Runtime traits
#[doc(no_inline)]
Expand Down Expand Up @@ -308,7 +311,7 @@ pub mod testing_prelude {
/// Other helper macros from `frame_support` that help with asserting in tests.
pub use frame_support::{
assert_err, assert_err_ignore_postinfo, assert_error_encoded_size, assert_noop, assert_ok,
assert_storage_noop, storage_alias,
assert_storage_noop, storage_alias, ensure,
};

pub use frame_system::{self, mocking::*};
Expand Down Expand Up @@ -342,6 +345,9 @@ pub mod runtime {
/// Consider using the new version of this [`frame_construct_runtime`].
pub use frame_support::construct_runtime;

/// Related to runtime construction.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not relevant for me

pub use frame_support::{BoundedSlice, BoundedVec};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you added them just to use the runtime prelude in the aura pallet, I think it's better to change it to the main prelude (see comment) and remove this from here


/// Macro to amalgamate the runtime into `struct Runtime`.
///
/// This is the newer version of [`construct_runtime`].
Expand Down Expand Up @@ -494,6 +500,7 @@ pub mod runtime {
pub mod testing_prelude {
pub use sp_core::storage::Storage;
pub use sp_runtime::BuildStorage;
pub use sp_runtime::testing::{TestSignature, UintAuthorityId};
}
}

Expand All @@ -513,6 +520,13 @@ pub mod arithmetic {
pub use sp_arithmetic::{traits::*, *};
}

/// Consensus types
pub mod consensus {
pub use sp_consensus_aura::{ed25519::AuthorityId, AuthorityIndex, ConsensusLog, Slot, AURA_ENGINE_ID};
pub use sp_runtime::{DigestItem, Digest};
pub use sp_runtime::ConsensusEngineId;
}

/// All derive macros used in frame.
///
/// This is already part of the [`prelude`].
Expand All @@ -527,9 +541,14 @@ pub mod derive {
pub use sp_runtime::RuntimeDebug;
}

pub mod hashing {
pub use sp_core::{hashing::*, H160, H256, H512, U256, U512};
pub mod cryptography {
pub use sp_core::{
crypto::{VrfPublic, VrfSecret, Wraps},
hashing::*,
Pair, H160, H256, H512, U256, U512,
};
pub use sp_runtime::traits::{BlakeTwo256, Hash, Keccak256};
pub use sp_application_crypto::{RuntimeAppPublic, BoundToRuntimeAppPublic};
}

/// Access to all of the dependencies of this crate. In case the prelude re-exports are not enough,
Expand Down Expand Up @@ -575,6 +594,8 @@ pub mod deps {
pub use sp_storage;
#[cfg(feature = "runtime")]
pub use sp_version;
#[cfg(feature = "runtime")]
pub use sp_application_crypto;

#[cfg(feature = "runtime-benchmarks")]
pub use frame_benchmarking;
Expand Down
Loading