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

Proposal: Flatten AllPallets and similar types #11813

Merged
merged 18 commits into from
Aug 14, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 4 additions & 0 deletions frame/support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,7 @@ no-metadata-docs = ["frame-support-procedural/no-metadata-docs"]
# By default some types have documentation, `full-metadata-docs` allows to add documentation to
# more types in the metadata.
full-metadata-docs = ["scale-info/docs"]
# Generate impl-trait for tuples with the given number of tuples. Will be needed as the number of
# pallets in a runtime grows. Does increase the compile time!
tuples-96 = []
tuples-128 = []
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
54 changes: 15 additions & 39 deletions frame/support/procedural/src/construct_runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,47 +308,26 @@ fn decl_all_pallets<'a>(
names.push(&pallet_declaration.name);
}

// Make nested tuple structure like:
// `((FirstPallet, (SecondPallet, ( ... , LastPallet) ... ))))`
// But ignore the system pallet.
let all_pallets_without_system = names
.iter()
.filter(|n| **n != SYSTEM_PALLET_NAME)
.rev()
.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));

// Make nested tuple structure like:
// `((FirstPallet, (SecondPallet, ( ... , LastPallet) ... ))))`
let all_pallets_with_system = names
.iter()
.rev()
.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));

// Make nested tuple structure like:
// `((LastPallet, (SecondLastPallet, ( ... , FirstPallet) ... ))))`
// But ignore the system pallet.
let all_pallets_without_system_reversed = names
.iter()
.filter(|n| **n != SYSTEM_PALLET_NAME)
.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));

// Make nested tuple structure like:
// `((LastPallet, (SecondLastPallet, ( ... , FirstPallet) ... ))))`
let all_pallets_with_system_reversed = names
.iter()
.fold(TokenStream2::default(), |combined, name| quote!((#name, #combined)));

let system_pallet = match names.iter().find(|n| **n == SYSTEM_PALLET_NAME) {
Some(name) => name,
None =>
return syn::Error::new(
proc_macro2::Span::call_site(),
"`System` pallet declaration is missing. \
Please add this line: `System: frame_system::{Pallet, Call, Storage, Config, Event<T>},`",
Please add this line: `System: frame_system::{Pallet, Call, Storage, Config, Event<T>},`",
)
.into_compile_error(),
};

let names_without_system =
names.iter().filter(|n| **n != SYSTEM_PALLET_NAME).collect::<Vec<_>>();
let names_reversed = names.clone().into_iter().rev().collect::<Vec<_>>();
let names_without_system_reverse =
names_without_system.clone().into_iter().rev().collect::<Vec<_>>();
let names_reversed_with_system_first = std::iter::once(system_pallet)
.chain(names_without_system_reverse.clone().into_iter())
.collect::<Vec<_>>();
bkchr marked this conversation as resolved.
Show resolved Hide resolved

quote!(
#types

Expand All @@ -364,25 +343,22 @@ fn decl_all_pallets<'a>(
pub type AllPallets = AllPalletsWithSystem;

/// All pallets included in the runtime as a nested tuple of types.
pub type AllPalletsWithSystem = ( #all_pallets_with_system );
pub type AllPalletsWithSystem = ( #(#names),* );

/// All pallets included in the runtime as a nested tuple of types.
/// Excludes the System pallet.
pub type AllPalletsWithoutSystem = ( #all_pallets_without_system );
pub type AllPalletsWithoutSystem = ( #(#names_without_system),* );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub type AllPalletsWithoutSystem = ( #(#names_without_system),* );
pub type AllPalletsWithoutSystem = ( #(#all_pallets_without_system),* );


/// All pallets included in the runtime as a nested tuple of types in reversed order.
/// Excludes the System pallet.
pub type AllPalletsWithoutSystemReversed = ( #all_pallets_without_system_reversed );
pub type AllPalletsWithoutSystemReversed =( #(#names_without_system_reverse),* );

/// All pallets included in the runtime as a nested tuple of types in reversed order.
pub type AllPalletsWithSystemReversed = ( #all_pallets_with_system_reversed );
pub type AllPalletsWithSystemReversed = ( #(#names_reversed),* );

/// All pallets included in the runtime as a nested tuple of types in reversed order.
/// With the system pallet first.
pub type AllPalletsReversedWithSystemFirst = (
#system_pallet,
AllPalletsWithoutSystemReversed
);
pub type AllPalletsReversedWithSystemFirst = ( #(#names_reversed_with_system_first),* );
Copy link
Member

Choose a reason for hiding this comment

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

I think we can probably now get rid of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would start by marking them as deprecated? or are you sure they are not needed anywhere?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah good idea.

)
}

Expand Down
6 changes: 2 additions & 4 deletions frame/support/procedural/src/pallet/expand/pallet_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,17 +240,15 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream {
#config_where_clause
{
fn count() -> usize { 1 }
fn accumulate(
acc: &mut #frame_support::sp_std::vec::Vec<#frame_support::traits::PalletInfoData>
) {
fn infos() -> #frame_support::sp_std::vec::Vec<#frame_support::traits::PalletInfoData> {
use #frame_support::traits::PalletInfoAccess;
let item = #frame_support::traits::PalletInfoData {
index: Self::index(),
name: Self::name(),
module_name: Self::module_name(),
crate_version: Self::crate_version(),
};
acc.push(item);
#frame_support::sp_std::vec![item]
}
}

Expand Down
4 changes: 2 additions & 2 deletions frame/support/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2204,15 +2204,15 @@ macro_rules! decl_module {
for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )*
{
fn count() -> usize { 1 }
fn accumulate(acc: &mut $crate::sp_std::vec::Vec<$crate::traits::PalletInfoData>) {
fn infos() -> $crate::sp_std::vec::Vec<$crate::traits::PalletInfoData> {
use $crate::traits::PalletInfoAccess;
let item = $crate::traits::PalletInfoData {
index: Self::index(),
name: Self::name(),
module_name: Self::module_name(),
crate_version: Self::crate_version(),
};
acc.push(item);
vec![item]
}
}

Expand Down
2 changes: 1 addition & 1 deletion frame/support/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<T: GetStorageVersion + PalletInfoAccess> PalletVersionToStorageVersionHelpe
}
}

#[impl_trait_for_tuples::impl_for_tuples(30)]
#[impl_trait_for_tuples::impl_for_tuples(100)]
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
impl PalletVersionToStorageVersionHelper for T {
fn migrate(db_weight: &RuntimeDbWeight) -> Weight {
let mut weight: Weight = 0;
Expand Down
5 changes: 3 additions & 2 deletions frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ mod error;
pub use error::PalletError;

mod filter;
pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter, IntegrityTest};
pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter};

mod misc;
pub use misc::{
Expand Down Expand Up @@ -81,7 +81,8 @@ mod hooks;
#[cfg(feature = "std")]
pub use hooks::GenesisBuild;
pub use hooks::{
Hooks, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade, OnTimestampSet,
Hooks, IntegrityTest, OnFinalize, OnGenesis, OnIdle, OnInitialize, OnRuntimeUpgrade,
OnTimestampSet,
};
#[cfg(feature = "try-runtime")]
pub use hooks::{OnRuntimeUpgradeHelpersExt, ON_RUNTIME_UPGRADE_PREFIX};
Expand Down
11 changes: 0 additions & 11 deletions frame/support/src/traits/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,6 @@ macro_rules! impl_filter_stack {
}
}

/// Type that provide some integrity tests.
///
/// This implemented for modules by `decl_module`.
#[impl_trait_for_tuples::impl_for_tuples(30)]
pub trait IntegrityTest {
/// Run integrity test.
///
/// The test is not executed in a externalities provided environment.
fn integrity_test() {}
}

#[cfg(test)]
pub mod test_impl_filter_stack {
use super::*;
Expand Down
37 changes: 31 additions & 6 deletions frame/support/src/traits/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ pub trait OnInitialize<BlockNumber> {
}
}

#[impl_for_tuples(30)]
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
impl<BlockNumber: Clone> OnInitialize<BlockNumber> for Tuple {
fn on_initialize(n: BlockNumber) -> crate::weights::Weight {
let mut weight = 0;
Expand All @@ -50,7 +52,9 @@ impl<BlockNumber: Clone> OnInitialize<BlockNumber> for Tuple {
/// The block finalization trait.
///
/// Implementing this lets you express what should happen for your pallet when the block is ending.
#[impl_for_tuples(30)]
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
pub trait OnFinalize<BlockNumber> {
/// The block is being finalized. Implement to have something happen.
///
Expand Down Expand Up @@ -79,7 +83,9 @@ pub trait OnIdle<BlockNumber> {
}
}

#[impl_for_tuples(30)]
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
impl<BlockNumber: Copy + AtLeast32BitUnsigned> OnIdle<BlockNumber> for Tuple {
fn on_idle(n: BlockNumber, remaining_weight: crate::weights::Weight) -> crate::weights::Weight {
let on_idle_functions: &[fn(
Expand All @@ -105,7 +111,9 @@ impl<BlockNumber: Copy + AtLeast32BitUnsigned> OnIdle<BlockNumber> for Tuple {
/// Implementing this trait for a pallet let's you express operations that should
/// happen at genesis. It will be called in an externalities provided environment and
/// will see the genesis state after all pallets have written their genesis state.
#[impl_for_tuples(30)]
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
pub trait OnGenesis {
/// Something that should happen at genesis.
fn on_genesis() {}
Expand Down Expand Up @@ -187,7 +195,9 @@ pub trait OnRuntimeUpgrade {
}
}

#[impl_for_tuples(30)]
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
impl OnRuntimeUpgrade for Tuple {
fn on_runtime_upgrade() -> crate::weights::Weight {
let mut weight = 0;
Expand All @@ -210,6 +220,19 @@ impl OnRuntimeUpgrade for Tuple {
}
}

/// Type that provide some integrity tests.
///
/// This implemented for modules by `decl_module`.
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
pub trait IntegrityTest {
/// Run integrity test.
///
/// The test is not executed in a externalities provided environment.
fn integrity_test() {}
}

/// The pallet hooks trait. Implementing this lets you express some logic to execute.
pub trait Hooks<BlockNumber> {
/// The block is being finalized. Implement to have something happen.
Expand Down Expand Up @@ -321,7 +344,9 @@ pub trait GenesisBuild<T, I = ()>: Default + sp_runtime::traits::MaybeSerializeD
}

/// A trait which is called when the timestamp is set in the runtime.
#[impl_for_tuples(30)]
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
pub trait OnTimestampSet<Moment> {
/// Called when the timestamp is set.
fn on_timestamp_set(moment: Moment);
Expand Down
9 changes: 7 additions & 2 deletions frame/support/src/traits/members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@
//! Traits for dealing with the idea of membership.

use sp_std::{marker::PhantomData, prelude::*};
use impl_trait_for_tuples::impl_for_tuples;

/// A trait for querying whether a type can be said to "contain" a value.
pub trait Contains<T> {
/// Return `true` if this "contains" the given value `t`.
fn contains(t: &T) -> bool;
}

#[impl_trait_for_tuples::impl_for_tuples(1, 30)]
#[impl_for_tuples(1, 64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(1, 96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(1, 128))]
impl<T> Contains<T> for Tuple {
fn contains(t: &T) -> bool {
for_tuples!( #(
Expand All @@ -41,7 +44,9 @@ pub trait ContainsPair<A, B> {
fn contains(a: &A, b: &B) -> bool;
}

#[impl_trait_for_tuples::impl_for_tuples(0, 30)]
#[impl_for_tuples(0, 64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(0, 96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(0, 128))]
impl<A, B> ContainsPair<A, B> for Tuple {
fn contains(a: &A, b: &B) -> bool {
for_tuples!( #(
Expand Down
45 changes: 16 additions & 29 deletions frame/support/src/traits/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use codec::{Decode, Encode};
use sp_runtime::RuntimeDebug;
use impl_trait_for_tuples::impl_for_tuples;
use sp_std::prelude::*;

/// Provides information about the pallet itself and its setup in the runtime.
Expand Down Expand Up @@ -70,40 +71,26 @@ pub trait PalletsInfoAccess {
///
/// You probably don't want this function but `infos()` instead.
fn count() -> usize {
0
// for backwards compatibility with XCM-3, Mark is deprecated.
Self::infos().len()
}

/// Extend the given vector by all of the pallets' information that this type represents.
///
/// You probably don't want this function but `infos()` instead.
fn accumulate(_accumulator: &mut Vec<PalletInfoData>) {}

/// All of the pallets' information that this type represents.
fn infos() -> Vec<PalletInfoData> {
let mut result = Vec::with_capacity(Self::count());
Self::accumulate(&mut result);
result
}
fn infos() -> Vec<PalletInfoData>;
}

impl PalletsInfoAccess for () {}
impl<T: PalletsInfoAccess> PalletsInfoAccess for (T,) {
fn count() -> usize {
T::count()
}
fn accumulate(acc: &mut Vec<PalletInfoData>) {
T::accumulate(acc)
}
}

impl<T1: PalletsInfoAccess, T2: PalletsInfoAccess> PalletsInfoAccess for (T1, T2) {
fn count() -> usize {
T1::count() + T2::count()
}
fn accumulate(acc: &mut Vec<PalletInfoData>) {
// The AllPallets type tuplises the pallets in reverse order, so we unreverse them here.
T2::accumulate(acc);
T1::accumulate(acc);
// TODO: this and needing to implement `PalletsInfoAccess` for any individual pallet is kinda wrong.
// all we need is:
// Impl<T: PalletInfoAccess> PalletsInfoAccess for (T) {}
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
// Impl<T: PalletInfoAccess, T1> PalletsInfoAccess for (T, T1) {}
#[impl_for_tuples(64)]
#[cfg_attr(feature = "tuples-96", impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
impl PalletsInfoAccess for Tuple {
fn infos() -> Vec<PalletInfoData> {
let mut res = vec![];
for_tuples!( #( res.extend_from_slice(&Tuple::infos()); )* );
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
res
}
}

Expand Down
Loading