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

Use frame umbrella crate in pallet-babe & pallet-staking-reward-curve #6412

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
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.

18 changes: 18 additions & 0 deletions prdoc/pr_6412.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# 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: Use frame umbrella crate in pallet-babe and pallet-staking-reward-curve

doc:
- audience: Runtime Dev
description: |
Extends the FRAME umbrella crate and uses it in pallet-babe and pallet-staking-reward-curve.
Migrates benchmarking from v1 to v2 for pallet-babe.

crates:
- name: polkadot-sdk-frame
bump: minor
- name: pallet-babe
bump: patch
- name: pallet-staking-reward-curve
bump: patch
77 changes: 30 additions & 47 deletions substrate/frame/babe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,12 @@ targets = ["x86_64-unknown-linux-gnu"]
codec = { features = ["derive"], workspace = true }
log = { workspace = true }
scale-info = { features = ["derive", "serde"], workspace = true }
frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
frame = { workspace = true, features = ["experimental", "runtime"] }
pallet-authorship = { workspace = true }
pallet-session = { workspace = true }
pallet-timestamp = { workspace = true }
sp-application-crypto = { features = ["serde"], workspace = true }
sp-consensus-babe = { features = ["serde"], workspace = true }
sp-core = { features = ["serde"], workspace = true }
sp-io = { workspace = true }
sp-runtime = { features = ["serde"], workspace = true }
sp-session = { workspace = true }
sp-staking = { features = ["serde"], workspace = true }

Expand All @@ -39,53 +34,41 @@ pallet-balances = { workspace = true, default-features = true }
pallet-offences = { workspace = true, default-features = true }
pallet-staking = { workspace = true, default-features = true }
pallet-staking-reward-curve = { workspace = true, default-features = true }
sp-core = { workspace = true, default-features = true }

[features]
default = ["std"]
std = [
"codec/std",
"frame-benchmarking?/std",
"frame-election-provider-support/std",
"frame-support/std",
"frame-system/std",
"log/std",
"pallet-authorship/std",
"pallet-balances/std",
"pallet-offences/std",
"pallet-session/std",
"pallet-staking/std",
"pallet-timestamp/std",
"scale-info/std",
"sp-application-crypto/std",
"sp-consensus-babe/std",
"sp-core/std",
"sp-io/std",
"sp-runtime/std",
"sp-session/std",
"sp-staking/std",
"codec/std",
"frame/std",
"log/std",
"pallet-authorship/std",
"pallet-balances/std",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not be needed if these pallets are only needed for tests.

"pallet-offences/std",
"pallet-session/std",
"pallet-staking/std",
"pallet-timestamp/std",
"scale-info/std",
"sp-application-crypto/std",
"sp-consensus-babe/std",
"sp-session/std",
"sp-staking/std",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-election-provider-support/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"pallet-balances/runtime-benchmarks",
"pallet-offences/runtime-benchmarks",
"pallet-staking/runtime-benchmarks",
"pallet-timestamp/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
"sp-staking/runtime-benchmarks",
"frame-election-provider-support/runtime-benchmarks",
"frame/runtime-benchmarks",
"pallet-balances/runtime-benchmarks",
"pallet-offences/runtime-benchmarks",
"pallet-staking/runtime-benchmarks",
"pallet-timestamp/runtime-benchmarks",
"sp-staking/runtime-benchmarks",
]
try-runtime = [
"frame-election-provider-support/try-runtime",
"frame-support/try-runtime",
"frame-system/try-runtime",
"pallet-authorship/try-runtime",
"pallet-balances/try-runtime",
"pallet-offences/try-runtime",
"pallet-session/try-runtime",
"pallet-staking/try-runtime",
"pallet-timestamp/try-runtime",
"sp-runtime/try-runtime",
"frame-election-provider-support/try-runtime",
"frame/try-runtime",
"pallet-authorship/try-runtime",
"pallet-balances/try-runtime",
"pallet-offences/try-runtime",
"pallet-session/try-runtime",
"pallet-staking/try-runtime",
"pallet-timestamp/try-runtime",
]
29 changes: 15 additions & 14 deletions substrate/frame/babe/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@
#![cfg(feature = "runtime-benchmarks")]

use super::*;
use frame_benchmarking::v1::benchmarks;
use frame::benchmarking::prelude::*;

type Header = sp_runtime::generic::Header<u64, sp_runtime::traits::BlakeTwo256>;

benchmarks! {
check_equivocation_proof {
let x in 0 .. 1;
#[benchmarks]
mod benchmarks {
use super::*;

#[benchmark]
fn check_equivocation_proof(x: Linear<0, 1>) -> Result<(), BenchmarkError> {
// NOTE: generated with the test below `test_generate_equivocation_report_blob`.
// the output is not deterministic since keys are generated randomly (and therefore
// signature content changes). it should not affect the benchmark.
Expand All @@ -53,22 +55,21 @@ benchmarks! {
124, 11, 167, 227, 103, 88, 78, 23, 228, 33, 96, 41, 207, 183, 227, 189, 114, 70, 254,
30, 128, 243, 233, 83, 214, 45, 74, 182, 120, 119, 64, 243, 219, 119, 63, 240, 205,
123, 231, 82, 205, 174, 143, 70, 2, 86, 182, 20, 16, 141, 145, 91, 116, 195, 58, 223,
175, 145, 255, 7, 121, 133
175, 145, 255, 7, 121, 133,
];

let equivocation_proof1: sp_consensus_babe::EquivocationProof<Header> =
Decode::decode(&mut &EQUIVOCATION_PROOF_BLOB[..]).unwrap();

let equivocation_proof2 = equivocation_proof1.clone();
}: {
sp_consensus_babe::check_equivocation_proof::<Header>(equivocation_proof1);
} verify {

#[extrinsic_call]
_(equivocation_proof1);
Copy link
Contributor

Choose a reason for hiding this comment

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

check_equivocation_proof is not a Runtime call, you need #[block] here


assert!(sp_consensus_babe::check_equivocation_proof::<Header>(equivocation_proof2));
}

impl_benchmark_test_suite!(
Pallet,
crate::mock::new_test_ext(3),
crate::mock::Test,
)
impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(3), crate::mock::Test,);

Ok(())
}
}
17 changes: 9 additions & 8 deletions substrate/frame/babe/src/default_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,11 @@
//! Default weights for the Babe Pallet
//! This file was not auto-generated.

use frame_support::weights::{
constants::{RocksDbWeight as DbWeight, WEIGHT_REF_TIME_PER_MICROS, WEIGHT_REF_TIME_PER_NANOS},
Weight,
};
use frame::{prelude::*, weights_prelude::*};

impl crate::WeightInfo for () {
fn plan_config_change() -> Weight {
DbWeight::get().writes(1)
RocksDbWeight::get().writes(1)
}

fn report_equivocation(validator_count: u32, max_nominators_per_validator: u32) -> Weight {
Expand All @@ -39,7 +36,7 @@ impl crate::WeightInfo for () {
Weight::from_parts(175u64 * WEIGHT_REF_TIME_PER_NANOS, 0)
.saturating_mul(validator_count),
)
.saturating_add(DbWeight::get().reads(5))
.saturating_add(RocksDbWeight::get().reads(5))
// check equivocation proof
.saturating_add(Weight::from_parts(110u64 * WEIGHT_REF_TIME_PER_MICROS, 0))
// report offence
Expand All @@ -48,7 +45,11 @@ impl crate::WeightInfo for () {
25u64 * WEIGHT_REF_TIME_PER_MICROS * max_nominators_per_validator as u64,
0,
))
.saturating_add(DbWeight::get().reads(14 + 3 * max_nominators_per_validator as u64))
.saturating_add(DbWeight::get().writes(10 + 3 * max_nominators_per_validator as u64))
.saturating_add(
RocksDbWeight::get().reads(14 + 3 * max_nominators_per_validator as u64),
)
.saturating_add(
RocksDbWeight::get().writes(10 + 3 * max_nominators_per_validator as u64),
)
}
}
10 changes: 1 addition & 9 deletions substrate/frame/babe/src/equivocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,10 @@
//! definition.

use alloc::{boxed::Box, vec, vec::Vec};
use frame_support::traits::{Get, KeyOwnerProofSystem};
use frame_system::pallet_prelude::HeaderFor;
use frame::{prelude::*, runtime::apis::KeyTypeId, traits::KeyOwnerProofSystem};
use log::{error, info};

use sp_consensus_babe::{AuthorityId, EquivocationProof, Slot, KEY_TYPE};
use sp_runtime::{
transaction_validity::{
InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity,
TransactionValidityError, ValidTransaction,
},
DispatchError, KeyTypeId, Perbill,
};
use sp_session::{GetSessionNumber, GetValidatorCount};
use sp_staking::{
offence::{Kind, Offence, OffenceReportSystem, ReportOffence},
Expand Down
40 changes: 14 additions & 26 deletions substrate/frame/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,20 @@ extern crate alloc;

use alloc::{boxed::Box, vec, vec::Vec};
use codec::{Decode, Encode};
use frame_support::{
dispatch::{DispatchResultWithPostInfo, Pays},
ensure,
traits::{ConstU32, DisabledValidators, FindAuthor, Get, OnTimestampSet, OneSessionHandler},
weights::Weight,
BoundedVec, WeakBoundedVec,
use frame::{
deps::sp_runtime::generic::DigestItem,
prelude::*,
traits::{
DisabledValidators, EstimateNextSessionRotation, FindAuthor, IsMember,
Copy link
Contributor

Choose a reason for hiding this comment

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

A few of these seems general enough to go to prelude:

IsMember, FindAuthor, OnTimestampSet.

Rule of thumb: If you can foresee it being useful in any pallet, then prelude. Else, domain specifc mod (e.g. mod traits).

Lateness as LatenessT, OnTimestampSet, OneSessionHandler, StorageInstance,
},
};
use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor};
use sp_consensus_babe::{
digests::{NextConfigDescriptor, NextEpochDescriptor, PreDigest},
AllowedSlots, BabeAuthorityWeight, BabeEpochConfiguration, ConsensusLog, Epoch,
EquivocationProof, Randomness as BabeRandomness, Slot, BABE_ENGINE_ID, RANDOMNESS_LENGTH,
RANDOMNESS_VRF_CONTEXT,
};
use sp_core::crypto::Wraps;
use sp_runtime::{
generic::DigestItem,
traits::{IsMember, One, SaturatedConversion, Saturating, Zero},
ConsensusEngineId, Permill,
};
use sp_session::{GetSessionNumber, GetValidatorCount};
use sp_staking::{offence::OffenceReportSystem, SessionIndex};

Expand Down Expand Up @@ -109,11 +103,9 @@ impl EpochChangeTrigger for SameAuthoritiesForever {

const UNDER_CONSTRUCTION_SEGMENT_LENGTH: u32 = 256;

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

/// The BABE Pallet
#[pallet::pallet]
Expand Down Expand Up @@ -311,7 +303,7 @@ pub mod pallet {
pub type SkippedEpochs<T> =
StorageValue<_, BoundedVec<(u64, SessionIndex), ConstU32<100>>, ValueQuery>;

#[derive(frame_support::DefaultNoBound)]
#[derive(frame::prelude::DefaultNoBound)]
#[pallet::genesis_config]
pub struct GenesisConfig<T: Config> {
pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>,
Expand Down Expand Up @@ -369,7 +361,6 @@ pub mod pallet {
// execution. We don't run the verification again here to avoid slowing
// down the runtime.
debug_assert!({
use sp_core::crypto::VrfPublic;
public.vrf_verify(&transcript.clone().into_sign_data(), &signature)
});

Expand Down Expand Up @@ -943,9 +934,7 @@ impl<T: Config> OnTimestampSet<T::Moment> for Pallet<T> {
}
}

impl<T: Config> frame_support::traits::EstimateNextSessionRotation<BlockNumberFor<T>>
for Pallet<T>
{
impl<T: Config> EstimateNextSessionRotation<BlockNumberFor<T>> for Pallet<T> {
fn average_session_length() -> BlockNumberFor<T> {
T::EpochDuration::get().saturated_into()
}
Expand All @@ -971,13 +960,13 @@ impl<T: Config> frame_support::traits::EstimateNextSessionRotation<BlockNumberFo
}
}

impl<T: Config> frame_support::traits::Lateness<BlockNumberFor<T>> for Pallet<T> {
impl<T: Config> LatenessT<BlockNumberFor<T>> for Pallet<T> {
fn lateness(&self) -> BlockNumberFor<T> {
Lateness::<T>::get()
}
}

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

Expand Down Expand Up @@ -1045,20 +1034,19 @@ fn compute_randomness(
s.extend_from_slice(&vrf_output[..]);
}

sp_io::hashing::blake2_256(&s)
blake2_256(&s)
}

pub mod migrations {
use super::*;
use frame_support::pallet_prelude::{StorageValue, ValueQuery};

/// Something that can return the storage prefix of the `Babe` pallet.
pub trait BabePalletPrefix: Config {
fn pallet_prefix() -> &'static str;
}

struct __OldNextEpochConfig<T>(core::marker::PhantomData<T>);
impl<T: BabePalletPrefix> frame_support::traits::StorageInstance for __OldNextEpochConfig<T> {
impl<T: BabePalletPrefix> StorageInstance for __OldNextEpochConfig<T> {
fn pallet_prefix() -> &'static str {
T::pallet_prefix()
}
Expand Down
Loading
Loading