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

Generate storage info for aura pallet #9371

Merged
10 commits merged into from
Sep 2, 2021
Merged

Generate storage info for aura pallet #9371

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
60595de
Generate storage info for aura pallet
KiChjang Jul 18, 2021
57d8d82
Add MaxAuthorities to node-template aura pallet config
KiChjang Jul 19, 2021
e57f30e
Fix compilation errors on node-template
KiChjang Jul 19, 2021
a8fb933
Use WeakBoundedVec instead of BoundedVec
KiChjang Jul 19, 2021
3774b7d
Improve comment on BoundedSlice's EncodeLike impl
KiChjang Aug 18, 2021
8b7888e
Bump MaxAuthorities count to 32 for node template
KiChjang Aug 18, 2021
ca21401
Merge remote-tracking branch 'origin/master' into kckyeung/bounded-aura
KiChjang Aug 31, 2021
b68d9d6
Merge branch 'kckyeung/bounded-aura' of github.com:paritytech/substra…
KiChjang Sep 1, 2021
756e10f
cargo fmt
KiChjang Sep 1, 2021
5a75a6e
cargo fmt
KiChjang Sep 1, 2021
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
7 changes: 6 additions & 1 deletion bin/node-template/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,13 @@ impl frame_system::Config for Runtime {

impl pallet_randomness_collective_flip::Config for Runtime {}

parameter_types! {
pub const MaxAuthorities: u32 = 10;
KiChjang marked this conversation as resolved.
Show resolved Hide resolved
}

impl pallet_aura::Config for Runtime {
type AuthorityId = AuraId;
type MaxAuthorities = MaxAuthorities;
}

impl pallet_grandpa::Config for Runtime {
Expand Down Expand Up @@ -383,7 +388,7 @@ impl_runtime_apis! {
}

fn authorities() -> Vec<AuraId> {
Aura::authorities()
Aura::authorities().into_inner()
}
}

Expand Down
2 changes: 1 addition & 1 deletion frame/aura/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
sp-application-crypto = { version = "4.0.0-dev", default-features = false, path = "../../primitives/application-crypto" }
codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
codec = { package = "parity-scale-codec", version = "2.2.0", default-features = false, features = ["derive", "max-encoded-len"] }
sp-std = { version = "4.0.0-dev", default-features = false, path = "../../primitives/std" }
pallet-session = { version = "4.0.0-dev", default-features = false, path = "../session" }
sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../primitives/runtime" }
Expand Down
30 changes: 21 additions & 9 deletions frame/aura/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,11 @@

#![cfg_attr(not(feature = "std"), no_std)]

use sp_std::prelude::*;
use codec::{Encode, Decode};
use sp_std::{convert::{TryFrom, TryInto}, prelude::*};
use codec::{Encode, Decode, MaxEncodedLen};
use frame_support::{
Parameter, traits::{Get, FindAuthor, OneSessionHandler, OnTimestampSet}, ConsensusEngineId,
BoundedSlice, BoundedVec,
};
use sp_runtime::{
RuntimeAppPublic,
Expand All @@ -63,10 +64,14 @@ pub mod pallet {
#[pallet::config]
pub trait Config: pallet_timestamp::Config + frame_system::Config {
/// The identifier type for an authority.
type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + MaybeSerializeDeserialize;
type AuthorityId: Member + Parameter + RuntimeAppPublic + Default
+ MaybeSerializeDeserialize + MaxEncodedLen;
/// The maximum number of authorities that the pallet can hold.
type MaxAuthorities: Get<u32>;
}

#[pallet::pallet]
#[pallet::generate_storage_info]
pub struct Pallet<T>(sp_std::marker::PhantomData<T>);

#[pallet::hooks]
Expand All @@ -90,7 +95,8 @@ pub mod pallet {
/// The current authority set.
#[pallet::storage]
#[pallet::getter(fn authorities)]
pub(super) type Authorities<T: Config> = StorageValue<_, Vec<T::AuthorityId>, ValueQuery>;
pub(super) type Authorities<T: Config> =
StorageValue<_, BoundedVec<T::AuthorityId, T::MaxAuthorities>, ValueQuery>;

/// The current slot of this block.
///
Expand Down Expand Up @@ -120,20 +126,22 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
fn change_authorities(new: Vec<T::AuthorityId>) {
fn change_authorities(new: BoundedVec<T::AuthorityId, T::MaxAuthorities>) {
<Authorities<T>>::put(&new);

let log: DigestItem<T::Hash> = DigestItem::Consensus(
AURA_ENGINE_ID,
ConsensusLog::AuthoritiesChange(new).encode()
ConsensusLog::AuthoritiesChange(new.into_inner()).encode()
);
<frame_system::Pallet<T>>::deposit_log(log.into());
}

fn initialize_authorities(authorities: &[T::AuthorityId]) {
if !authorities.is_empty() {
assert!(<Authorities<T>>::get().is_empty(), "Authorities are already initialized!");
<Authorities<T>>::put(authorities);
let bounded = <BoundedSlice<'_, _, T::MaxAuthorities>>::try_from(authorities)
.expect("Initial authority set must be less than T::MaxAuthorities");
<Authorities<T>>::put(bounded);
}
}

Expand Down Expand Up @@ -179,8 +187,12 @@ impl<T: Config> OneSessionHandler<T::AccountId> for Pallet<T> {
if changed {
let next_authorities = validators.map(|(_, k)| k).collect::<Vec<_>>();
let last_authorities = Self::authorities();
if next_authorities != last_authorities {
Self::change_authorities(next_authorities);
if last_authorities != next_authorities {
let bounded = match next_authorities.try_into() {
Ok(v) => v,
Err(_) => return,
Copy link
Contributor

Choose a reason for hiding this comment

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

we should at least log an error, on_new_session shouldn't really be allowed to fail to set the authorities.
I don't know what is consequences of this if it happens.

Maybe better to use a WeakBoundedVec no ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In #8640, what we did instead is to truncate the authorities so that we're only left with the first n number of authorities, where n is the value of T::MaxAuthorities. I guess a WeakBoundedVec would work here as well -- it'll just mean that no additional authorities can be inserted, a removal must happen in order to reduce the number back down to n or below it.

};
Self::change_authorities(bounded);
KiChjang marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions frame/aura/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,13 @@ impl pallet_timestamp::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub const MaxAuthorities: u32 = 10;
}

impl pallet_aura::Config for Test {
type AuthorityId = AuthorityId;
type MaxAuthorities = MaxAuthorities;
}

pub fn new_test_ext(authorities: Vec<u64>) -> sp_io::TestExternalities {
Expand Down
1 change: 1 addition & 0 deletions primitives/application-crypto/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ macro_rules! app_crypto_public_not_full_crypto {
$crate::codec::Encode,
$crate::codec::Decode,
$crate::RuntimeDebug,
$crate::codec::MaxEncodedLen,
)]
pub struct Public($public);
}
Expand Down
2 changes: 1 addition & 1 deletion primitives/consensus/slots/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ readme = "README.md"
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
codec = { package = "parity-scale-codec", version = "2.2.0", default-features = false, features = ["derive", "max-encoded-len"] }
sp-runtime = { version = "4.0.0-dev", default-features = false, path = "../../runtime" }
sp-arithmetic = { version = "4.0.0-dev", default-features = false, path = "../../arithmetic" }

Expand Down
4 changes: 2 additions & 2 deletions primitives/consensus/slots/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

#![cfg_attr(not(feature = "std"), no_std)]

use codec::{Decode, Encode};
use codec::{Decode, Encode, MaxEncodedLen};

/// Unit type wrapper that represents a slot.
#[derive(Debug, Encode, Decode, Eq, Clone, Copy, Default, Ord)]
#[derive(Debug, Encode, Decode, Eq, Clone, Copy, Default, Ord, MaxEncodedLen)]
pub struct Slot(u64);

impl core::ops::Deref for Slot {
Expand Down