From 83ae90aaaa09ede6e4986704a5b438ba6438b459 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 12:51:35 -0500 Subject: [PATCH 01/16] sketch that compiles --- Cargo.lock | 13 +++++ pallets/author-filter/Cargo.toml | 25 ++++++++++ pallets/author-filter/src/lib.rs | 85 ++++++++++++++++++++++++++++++++ pallets/stake/src/lib.rs | 2 +- runtime/Cargo.toml | 2 + 5 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 pallets/author-filter/Cargo.toml create mode 100644 pallets/author-filter/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 0319138656..0865b91bc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4117,6 +4117,7 @@ dependencies = [ "log", "moonbeam-rpc-primitives-txpool", "pallet-aura", + "pallet-author-filter", "pallet-balances", "pallet-ethereum", "pallet-ethereum-chain-id", @@ -4518,6 +4519,18 @@ dependencies = [ "sp-timestamp", ] +[[package]] +name = "pallet-author-filter" +version = "0.1.0" +dependencies = [ + "author-inherent", + "frame-support", + "frame-system", + "parity-scale-codec", + "serde", + "stake", +] + [[package]] name = "pallet-authority-discovery" version = "2.0.1" diff --git a/pallets/author-filter/Cargo.toml b/pallets/author-filter/Cargo.toml new file mode 100644 index 0000000000..ca2dfffea7 --- /dev/null +++ b/pallets/author-filter/Cargo.toml @@ -0,0 +1,25 @@ +[package] +authors = ["PureStake"] +edition = "2018" +name = "pallet-author-filter" +version = "0.1.0" + +[dependencies] +parity-scale-codec = { version = "1.3.0", default-features = false, features = ["derive"] } +serde = { version = "1.0.101", optional = true, features = ["derive"] } + +frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } +frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } +author-inherent = { path = "../author-inherent", default-features = false } +stake = { path = "../stake", default-features = false } + +[features] +default = ["std"] +std = [ + "parity-scale-codec/std", + "serde", + "frame-support/std", + "frame-system/std", + "author-inherent/std", + "stake/std", +] diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs new file mode 100644 index 0000000000..9ce8f9e4ea --- /dev/null +++ b/pallets/author-filter/src/lib.rs @@ -0,0 +1,85 @@ +// Copyright 2019-2020 PureStake Inc. +// This file is part of Moonbeam. + +// Moonbeam is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Moonbeam is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Moonbeam. If not, see . + +//! Small pallet responsible determining which accounts are eligible to author at the current +//! block height. +//! +//! Currently this pallet is tightly coupled to our stake pallet, but this design +//! should be generalized in the future. +//! +//! Using a randomness beacon supplied by the `Randomness` trait, this pallet takes the set of +//! currently staked accounts from pallet stake, and filters them down to a pseudorandom subset. +//! The current technique gives no preference to any particular author. In the future, we could +//! disfavor authors who are authoring a disproportionate amount of the time in an attempt to +//! "even the playing field". + +#![cfg_attr(not(feature = "std"), no_std)] + +use frame_support::pallet; + +pub use pallet::*; + +#[pallet] +pub mod pallet { + + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use frame_support::traits::Randomness; + use frame_support::traits::Vec; + + /// The Author Filter pallet + #[pallet::pallet] + pub struct Pallet(PhantomData); + + /// Configuration trait of this pallet. + #[pallet::config] + pub trait Config: frame_system::Config + stake::Config { + /// Deterministic on-chain pseudo-randomness used to do the filtering + type RandomnessSource: Randomness; + } + + impl author_inherent::CanAuthor for Pallet { + fn can_author(account: &T::AccountId) -> bool { + CurrentEligible::::get().contains(account) + } + } + + #[pallet::hooks] + impl Hooks> for Pallet { + // At the beginning of each block, we grab the randomness and use it to reduce the author set + fn on_initialize(_: T::BlockNumber) -> Weight { + let staked : Vec = stake::Module::::validators(); + + let randomness = T::RandomnessSource::random(&*b"author_filter"); + + //TODO actually do some reducing here. + let eligible_subset = staked; + + CurrentEligible::::put(eligible_subset); + + 0 //TODO actual weight? + } + } + + // No dispatchible calls + #[pallet::call] + impl Pallet {} + + /// Storage item that holds the set of authors who are eligible to author at this height. + #[pallet::storage] + #[pallet::getter(fn chain_id)] + pub type CurrentEligible = StorageValue<_, Vec, ValueQuery>; +} diff --git a/pallets/stake/src/lib.rs b/pallets/stake/src/lib.rs index acdebd1a5c..47b4485762 100644 --- a/pallets/stake/src/lib.rs +++ b/pallets/stake/src/lib.rs @@ -519,7 +519,7 @@ decl_storage! { /// Current candidates with associated state Candidates: map hasher(blake2_128_concat) T::AccountId => Option>; /// Current validator set - Validators: Vec; + Validators get(fn validators): Vec; /// Total Locked Total: BalanceOf; /// Pool of candidates, ordered by account id diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 5204887ec0..578347dee5 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -17,6 +17,7 @@ precompiles = { path = "precompiles/", default-features = false } account = { path = "account/", default-features = false } pallet-ethereum-chain-id = { path = "../pallets/ethereum-chain-id", default-features = false } stake = { path = "../pallets/stake", default-features = false } +pallet-author-filter = { path = "../pallets/author-filter", default-features = false } # Substrate dependencies pallet-aura = { git = "https://github.com/paritytech/substrate.git", default-features = false, branch = "master", optional = true } @@ -108,6 +109,7 @@ std = [ "cumulus-primitives/std", "account/std", "stake/std", + "pallet-author-filter/std", ] # Will be enabled by the `wasm-builder` when building the runtime for WASM. From f76f64912a7638f460f0bdc7b4a1009aae138125 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 13:03:23 -0500 Subject: [PATCH 02/16] add debug event --- pallets/author-filter/src/lib.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 9ce8f9e4ea..9909def9c3 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -47,6 +47,8 @@ pub mod pallet { /// Configuration trait of this pallet. #[pallet::config] pub trait Config: frame_system::Config + stake::Config { + /// The overarching event type + type Event: From> + IsType<::Event>; /// Deterministic on-chain pseudo-randomness used to do the filtering type RandomnessSource: Randomness; } @@ -66,9 +68,14 @@ pub mod pallet { let randomness = T::RandomnessSource::random(&*b"author_filter"); //TODO actually do some reducing here. - let eligible_subset = staked; + let eligible_subset = staked.clone(); - CurrentEligible::::put(eligible_subset); + CurrentEligible::::put(&eligible_subset); + + //Emit an event for debugging purposes + >::deposit_event( + Event::Filtered(randomness, staked, eligible_subset) + ); 0 //TODO actual weight? } @@ -82,4 +89,13 @@ pub mod pallet { #[pallet::storage] #[pallet::getter(fn chain_id)] pub type CurrentEligible = StorageValue<_, Vec, ValueQuery>; + + #[pallet::event] + #[pallet::generate_deposit(fn deposit_event)] + pub enum Event { + /// The staked authors have been filtered in this block. Here's some debugging info + /// randomness, copmlete set, reduced set + Filtered(u32, Vec, Vec), + } + } From 57ec05e741568b19fb795dc5f97f90ace48fc7e4 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 13:34:53 -0500 Subject: [PATCH 03/16] actual filtering --- Cargo.lock | 1 + pallets/author-filter/Cargo.toml | 2 ++ pallets/author-filter/src/lib.rs | 36 +++++++++++++++++++++++++------- runtime/src/lib.rs | 8 ++++++- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0865b91bc1..25947a9351 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4528,6 +4528,7 @@ dependencies = [ "frame-system", "parity-scale-codec", "serde", + "sp-core", "stake", ] diff --git a/pallets/author-filter/Cargo.toml b/pallets/author-filter/Cargo.toml index ca2dfffea7..de9c201edb 100644 --- a/pallets/author-filter/Cargo.toml +++ b/pallets/author-filter/Cargo.toml @@ -10,6 +10,7 @@ serde = { version = "1.0.101", optional = true, features = ["derive"] } frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } +sp-core = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } author-inherent = { path = "../author-inherent", default-features = false } stake = { path = "../stake", default-features = false } @@ -22,4 +23,5 @@ std = [ "frame-system/std", "author-inherent/std", "stake/std", + "sp-core/std", ] diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 9909def9c3..59c53051d9 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -39,18 +39,23 @@ pub mod pallet { use frame_system::pallet_prelude::*; use frame_support::traits::Randomness; use frame_support::traits::Vec; + use sp_core::H256; /// The Author Filter pallet #[pallet::pallet] pub struct Pallet(PhantomData); + // The maximum number of eligible authors at each hight. + // TODO make this part of the config trait. Or maybe express it as a percent. + const MAX_ELIGIBLE: usize = 3; + /// Configuration trait of this pallet. #[pallet::config] pub trait Config: frame_system::Config + stake::Config { /// The overarching event type type Event: From> + IsType<::Event>; /// Deterministic on-chain pseudo-randomness used to do the filtering - type RandomnessSource: Randomness; + type RandomnessSource: Randomness; } impl author_inherent::CanAuthor for Pallet { @@ -61,14 +66,29 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { - // At the beginning of each block, we grab the randomness and use it to reduce the author set + // At the beginning of each block, we calculate the set of eligible authors for this block. + // TODO it might make more sense to calculate for the next block at the end of this block. fn on_initialize(_: T::BlockNumber) -> Weight { - let staked : Vec = stake::Module::::validators(); - + //TODO only need to grab randomness in else clause. For now its here to support the debugging event let randomness = T::RandomnessSource::random(&*b"author_filter"); - - //TODO actually do some reducing here. - let eligible_subset = staked.clone(); + let mut staked : Vec = stake::Module::::validators(); + + // Reduce it to a subset if there are more staked then the max eligible + let eligible_subset = if staked.len() <= MAX_ELIGIBLE as usize{ + staked.clone() + } else { + let mut eligible = Vec::new(); + for i in 0..MAX_ELIGIBLE { + // Calculate the index by grabbing the corresponding byte out of the randomness + // This will only work when MAX_ELIGIBLE < 32 because that's how many bytes + // there are. There's a lot hacky about this POC. + let index = randomness.as_fixed_bytes()[i] as usize; + + let selected = staked.remove(index % staked.len()); + eligible.push(selected); + } + eligible + }; CurrentEligible::::put(&eligible_subset); @@ -95,7 +115,7 @@ pub mod pallet { pub enum Event { /// The staked authors have been filtered in this block. Here's some debugging info /// randomness, copmlete set, reduced set - Filtered(u32, Vec, Vec), + Filtered(H256, Vec, Vec), } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index d3b93a0ada..224ea39c9f 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -337,7 +337,12 @@ impl stake::Config for Runtime { } impl author_inherent::Config for Runtime { type EventHandler = Stake; - type CanAuthor = Stake; + type CanAuthor = AuthorFilter; +} + +impl pallet_author_filter::Config for Runtime { + type Event = Event; + type RandomnessSource = RandomnessCollectiveFlip; } construct_runtime! { @@ -359,6 +364,7 @@ construct_runtime! { Ethereum: pallet_ethereum::{Module, Call, Storage, Event, Config, ValidateUnsigned}, Stake: stake::{Module, Call, Storage, Event, Config}, AuthorInherent: author_inherent::{Module, Call, Storage, Inherent}, + AuthorFilter: pallet_author_filter::{Module, Storage, Event,} } } From 868c3f4c6bf8759f06d3b2202ecdaf607c9d50e8 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 14:08:30 -0500 Subject: [PATCH 04/16] cargo fmt --- pallets/author-filter/src/lib.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 59c53051d9..c984ed8578 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -36,9 +36,9 @@ pub use pallet::*; pub mod pallet { use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::*; use frame_support::traits::Randomness; use frame_support::traits::Vec; + use frame_system::pallet_prelude::*; use sp_core::H256; /// The Author Filter pallet @@ -71,10 +71,10 @@ pub mod pallet { fn on_initialize(_: T::BlockNumber) -> Weight { //TODO only need to grab randomness in else clause. For now its here to support the debugging event let randomness = T::RandomnessSource::random(&*b"author_filter"); - let mut staked : Vec = stake::Module::::validators(); + let mut staked: Vec = stake::Module::::validators(); // Reduce it to a subset if there are more staked then the max eligible - let eligible_subset = if staked.len() <= MAX_ELIGIBLE as usize{ + let eligible_subset = if staked.len() <= MAX_ELIGIBLE as usize { staked.clone() } else { let mut eligible = Vec::new(); @@ -93,9 +93,7 @@ pub mod pallet { CurrentEligible::::put(&eligible_subset); //Emit an event for debugging purposes - >::deposit_event( - Event::Filtered(randomness, staked, eligible_subset) - ); + >::deposit_event(Event::Filtered(randomness, staked, eligible_subset)); 0 //TODO actual weight? } @@ -117,5 +115,4 @@ pub mod pallet { /// randomness, copmlete set, reduced set Filtered(H256, Vec, Vec), } - } From 2aa58a8e1aef83b653ccb79d0d60fd39e0147360 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 14:26:09 -0500 Subject: [PATCH 05/16] line length --- pallets/author-filter/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index c984ed8578..26fedc233f 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -69,7 +69,8 @@ pub mod pallet { // At the beginning of each block, we calculate the set of eligible authors for this block. // TODO it might make more sense to calculate for the next block at the end of this block. fn on_initialize(_: T::BlockNumber) -> Weight { - //TODO only need to grab randomness in else clause. For now its here to support the debugging event + //TODO only need to grab randomness in else clause. + // For now its here to support the debugging event let randomness = T::RandomnessSource::random(&*b"author_filter"); let mut staked: Vec = stake::Module::::validators(); From 9527ba15ff6e46bf59f8a14de3343af225c87c0b Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 16:44:40 -0500 Subject: [PATCH 06/16] braindump into comments --- pallets/author-filter/src/lib.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 26fedc233f..1a6e4727dd 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -67,7 +67,15 @@ pub mod pallet { #[pallet::hooks] impl Hooks> for Pallet { // At the beginning of each block, we calculate the set of eligible authors for this block. - // TODO it might make more sense to calculate for the next block at the end of this block. + // TODO Design Decision: + // If we move this logic to on_finalize to calculate for the next block, we get to know in + // advance who the next eligible authors are. That is nice because it is easy to know in + // from offchain who will author next. You just need to read storage. + // On the other hand, it leads to liveness attacks. If the eligible authors collude to not + // author, then the chain is bricked. We can 't even force them out with governance because + // governance stops when the chain is stalled. In that way, the `EligibleRatio` _is_ our + // security assumption. By leaving this in on_initialize, we can rely on Polkadot's + // randomness beacon having a different value when there is a different relay parent. fn on_initialize(_: T::BlockNumber) -> Weight { //TODO only need to grab randomness in else clause. // For now its here to support the debugging event From 77a7f057e3223c27cb68e1b1a3e5a826855803e1 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Thu, 4 Feb 2021 16:45:05 -0500 Subject: [PATCH 07/16] EligibleRation --- Cargo.lock | 1 + pallets/author-filter/Cargo.toml | 2 ++ pallets/author-filter/src/lib.rs | 59 ++++++++++++++++---------------- 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25947a9351..5a46c93492 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4529,6 +4529,7 @@ dependencies = [ "parity-scale-codec", "serde", "sp-core", + "sp-runtime", "stake", ] diff --git a/pallets/author-filter/Cargo.toml b/pallets/author-filter/Cargo.toml index de9c201edb..764029461a 100644 --- a/pallets/author-filter/Cargo.toml +++ b/pallets/author-filter/Cargo.toml @@ -11,6 +11,7 @@ serde = { version = "1.0.101", optional = true, features = ["derive"] } frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } sp-core = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } +sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } author-inherent = { path = "../author-inherent", default-features = false } stake = { path = "../stake", default-features = false } @@ -24,4 +25,5 @@ std = [ "author-inherent/std", "stake/std", "sp-core/std", + "sp-runtime/std", ] diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 1a6e4727dd..4d522535c6 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -40,15 +40,12 @@ pub mod pallet { use frame_support::traits::Vec; use frame_system::pallet_prelude::*; use sp_core::H256; + use sp_runtime::Percent; /// The Author Filter pallet #[pallet::pallet] pub struct Pallet(PhantomData); - // The maximum number of eligible authors at each hight. - // TODO make this part of the config trait. Or maybe express it as a percent. - const MAX_ELIGIBLE: usize = 3; - /// Configuration trait of this pallet. #[pallet::config] pub trait Config: frame_system::Config + stake::Config { @@ -77,32 +74,25 @@ pub mod pallet { // security assumption. By leaving this in on_initialize, we can rely on Polkadot's // randomness beacon having a different value when there is a different relay parent. fn on_initialize(_: T::BlockNumber) -> Weight { - //TODO only need to grab randomness in else clause. - // For now its here to support the debugging event - let randomness = T::RandomnessSource::random(&*b"author_filter"); let mut staked: Vec = stake::Module::::validators(); - // Reduce it to a subset if there are more staked then the max eligible - let eligible_subset = if staked.len() <= MAX_ELIGIBLE as usize { - staked.clone() - } else { - let mut eligible = Vec::new(); - for i in 0..MAX_ELIGIBLE { - // Calculate the index by grabbing the corresponding byte out of the randomness - // This will only work when MAX_ELIGIBLE < 32 because that's how many bytes - // there are. There's a lot hacky about this POC. - let index = randomness.as_fixed_bytes()[i] as usize; + let num_eligible = EligibleRatio::::get() * staked.len(); + let mut eligible = Vec::with_capacity(num_eligible); + + for i in 0..num_eligible { + // A context identifier for grabbing the randomness. + // Of the form *b"filter4" + let subject: [u8; 7] = [b'f', b'i', b'l', b't', b'e', b'r', i as u8]; + let index = T::RandomnessSource::random(&subject).to_low_u64_be() as usize; - let selected = staked.remove(index % staked.len()); - eligible.push(selected); - } - eligible - }; + // Move the selected author from the original vector into the eligible vector + eligible.push(staked.remove(index % staked.len())); + } - CurrentEligible::::put(&eligible_subset); + CurrentEligible::::put(&eligible); - //Emit an event for debugging purposes - >::deposit_event(Event::Filtered(randomness, staked, eligible_subset)); + // Emit an event for debugging purposes + >::deposit_event(Event::Filtered(eligible)); 0 //TODO actual weight? } @@ -112,16 +102,25 @@ pub mod pallet { #[pallet::call] impl Pallet {} - /// Storage item that holds the set of authors who are eligible to author at this height. + /// The set of authors who are eligible to author at this height. #[pallet::storage] - #[pallet::getter(fn chain_id)] + #[pallet::getter(fn current_eligible)] pub type CurrentEligible = StorageValue<_, Vec, ValueQuery>; + /// The percentage of active staked authors that will be eligible at each height. + #[pallet::storage] + pub type EligibleRatio = StorageValue<_, Percent, ValueQuery, Half>; + + // Default value for the `EligibleRatio` is one half. + #[pallet::type_value] + pub fn Half() -> Percent { + Percent::from_percent(50) + } + #[pallet::event] #[pallet::generate_deposit(fn deposit_event)] pub enum Event { - /// The staked authors have been filtered in this block. Here's some debugging info - /// randomness, copmlete set, reduced set - Filtered(H256, Vec, Vec), + /// The staked authors have been filtered to these eligible authors in this block + Filtered(Vec), } } From cf5e095827d79697428481886fedfc6393af7f51 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 10:08:07 -0500 Subject: [PATCH 08/16] Move author inherent validity check back on on_finalize So it is guaranteed to happen after the relay chain randomness has been injected via inherent. --- pallets/author-inherent/src/lib.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pallets/author-inherent/src/lib.rs b/pallets/author-inherent/src/lib.rs index 1391b3a61d..dfbd5847ae 100644 --- a/pallets/author-inherent/src/lib.rs +++ b/pallets/author-inherent/src/lib.rs @@ -41,7 +41,8 @@ pub trait EventHandler { pub trait CanAuthor { fn can_author(account: &AccountId) -> bool; } -/// Default permissions is none, see `stake` pallet for different impl used in runtime +/// Default implementation where anyone can author, see `stake` and `author-filter` pallets for +/// additional implementations. impl CanAuthor for () { fn can_author(_: &T) -> bool { true @@ -60,8 +61,6 @@ decl_error! { pub enum Error for Module { /// Author already set in block. AuthorAlreadySet, - /// The author in the inherent is not an eligible author. - CannotBeAuthor, } } @@ -89,7 +88,6 @@ decl_module! { fn set_author(origin, author: T::AccountId) { ensure_none(origin)?; ensure!(>::get().is_none(), Error::::AuthorAlreadySet); - ensure!(T::CanAuthor::can_author(&author), Error::::CannotBeAuthor); // Update storage Author::::put(&author); @@ -105,6 +103,13 @@ decl_module! { // Notify any other pallets that are listening (eg rewards) about the author T::EventHandler::note_author(author.clone()); } + + fn on_finalize() { + assert!( + T::CanAuthor::can_author(&Author::::get().expect("Author was set in block")), + "Author is not eligible in this block" + ); + } } } From 2473b1c0733732238e17fc1ee8a1dfbb25090c05 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 10:26:17 -0500 Subject: [PATCH 09/16] Calculate eligability on demand. Will happen in authorshi_inherent's on_finalize. --- pallets/author-filter/src/lib.rs | 70 ++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 4d522535c6..1ba9c8926a 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -55,57 +55,62 @@ pub mod pallet { type RandomnessSource: Randomness; } + // This code will be called by the author-inherent pallet in its on-finalize block to check + // whether the reported author of this block is eligible at this height. We calculate that + // result on demand for the currently ending block and do not record it instorage (although + // we do emit a debugging event for now.) impl author_inherent::CanAuthor for Pallet { fn can_author(account: &T::AccountId) -> bool { - CurrentEligible::::get().contains(account) - } - } - - #[pallet::hooks] - impl Hooks> for Pallet { - // At the beginning of each block, we calculate the set of eligible authors for this block. - // TODO Design Decision: - // If we move this logic to on_finalize to calculate for the next block, we get to know in - // advance who the next eligible authors are. That is nice because it is easy to know in - // from offchain who will author next. You just need to read storage. - // On the other hand, it leads to liveness attacks. If the eligible authors collude to not - // author, then the chain is bricked. We can 't even force them out with governance because - // governance stops when the chain is stalled. In that way, the `EligibleRatio` _is_ our - // security assumption. By leaving this in on_initialize, we can rely on Polkadot's - // randomness beacon having a different value when there is a different relay parent. - fn on_initialize(_: T::BlockNumber) -> Weight { let mut staked: Vec = stake::Module::::validators(); let num_eligible = EligibleRatio::::get() * staked.len(); let mut eligible = Vec::with_capacity(num_eligible); + //TODO actually grab the relay parent height and mod it into a u8 + let relay_height: u8 = 7; + for i in 0..num_eligible { - // A context identifier for grabbing the randomness. - // Of the form *b"filter4" - let subject: [u8; 7] = [b'f', b'i', b'l', b't', b'e', b'r', i as u8]; + // A context identifier for grabbing the randomness. Consists of three parts + // - The constant string *b"filter" - to identify this pallet + // - The index `i` when we're selecting the ith eligible author + // - The relay parent block number so that the eligible authors at the next height + // change. Avoids liveness attacks from colluding minorities of active authors. + // Third one will not be necessary once we dleverage the relay chain's randomness. + let subject: [u8; 8] = [b'f', b'i', b'l', b't', b'e', b'r', i as u8, relay_height]; let index = T::RandomnessSource::random(&subject).to_low_u64_be() as usize; // Move the selected author from the original vector into the eligible vector + // TODO we could short-circuit this check by returning early when the claimed + // author is selected. For now I'll leave it like this because: + // 1. it is easier to understand what our core filtering logic is + // 2. we currently show the entire filtered set in the debug event eligible.push(staked.remove(index % staked.len())); } - CurrentEligible::::put(&eligible); - // Emit an event for debugging purposes - >::deposit_event(Event::Filtered(eligible)); + >::deposit_event(Event::Filtered(eligible.clone())); - 0 //TODO actual weight? + eligible.contains(account) } } - // No dispatchible calls + // No hooks + #[pallet::hooks] + impl Hooks> for Pallet {} + #[pallet::call] - impl Pallet {} + impl Pallet { - /// The set of authors who are eligible to author at this height. - #[pallet::storage] - #[pallet::getter(fn current_eligible)] - pub type CurrentEligible = StorageValue<_, Vec, ValueQuery>; + /// Update the eligible ratio. Intended to be called by governance. + #[pallet::weight(0)] + pub fn set_eligible(origin: OriginFor, new: Percent) -> DispatchResultWithPostInfo { + ensure_root(origin)?; + EligibleRatio::::put(&new); + >::deposit_event(Event::EligibleUpdated(new)); + + Ok(Default::default()) + } + } /// The percentage of active staked authors that will be eligible at each height. #[pallet::storage] @@ -120,7 +125,10 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(fn deposit_event)] pub enum Event { - /// The staked authors have been filtered to these eligible authors in this block + /// The amount of eligible authors for the filter to select has been changed. + EligibleUpdated(Percent), + /// The staked authors have been filtered to these eligible authors in this block. + /// This is a debugging and development event and should be removed eventually. Filtered(Vec), } } From faa62cab851832eb1e67dc357767e910039d8a08 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 12:00:29 -0500 Subject: [PATCH 10/16] Proper multiplication with ceiling --- pallets/author-filter/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 1ba9c8926a..1cc85d1d7f 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -63,7 +63,7 @@ pub mod pallet { fn can_author(account: &T::AccountId) -> bool { let mut staked: Vec = stake::Module::::validators(); - let num_eligible = EligibleRatio::::get() * staked.len(); + let num_eligible = EligibleRatio::::get().mul_ceil(staked.len()); let mut eligible = Vec::with_capacity(num_eligible); //TODO actually grab the relay parent height and mod it into a u8 From b60345faac9d2d62e6b5c82ddcca7ffd7806e14b Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 13:03:05 -0500 Subject: [PATCH 11/16] use relay block height as a source of relay-based entropy to avoid liveness attacks --- Cargo.lock | 1 + pallets/author-filter/Cargo.toml | 2 ++ pallets/author-filter/src/lib.rs | 28 +++++++++++++++++++++------- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5a46c93492..712c0fdb9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4524,6 +4524,7 @@ name = "pallet-author-filter" version = "0.1.0" dependencies = [ "author-inherent", + "cumulus-parachain-upgrade", "frame-support", "frame-system", "parity-scale-codec", diff --git a/pallets/author-filter/Cargo.toml b/pallets/author-filter/Cargo.toml index 764029461a..f6094f3b84 100644 --- a/pallets/author-filter/Cargo.toml +++ b/pallets/author-filter/Cargo.toml @@ -14,6 +14,7 @@ sp-core = { git = "https://github.com/paritytech/substrate", default-features = sp-runtime = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } author-inherent = { path = "../author-inherent", default-features = false } stake = { path = "../stake", default-features = false } +cumulus-parachain-upgrade = { git = "https://github.com/paritytech/cumulus", default-features = false, branch = "master" } [features] default = ["std"] @@ -26,4 +27,5 @@ std = [ "stake/std", "sp-core/std", "sp-runtime/std", + "cumulus-parachain-upgrade/std", ] diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 1cc85d1d7f..168bde5070 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -48,7 +48,9 @@ pub mod pallet { /// Configuration trait of this pallet. #[pallet::config] - pub trait Config: frame_system::Config + stake::Config { + pub trait Config: + frame_system::Config + stake::Config + cumulus_parachain_upgrade::Config + { /// The overarching event type type Event: From> + IsType<::Event>; /// Deterministic on-chain pseudo-randomness used to do the filtering @@ -66,8 +68,10 @@ pub mod pallet { let num_eligible = EligibleRatio::::get().mul_ceil(staked.len()); let mut eligible = Vec::with_capacity(num_eligible); - //TODO actually grab the relay parent height and mod it into a u8 - let relay_height: u8 = 7; + // Grab the relay parent height as a temporary source of relay-based entropy + let validation_data = cumulus_parachain_upgrade::Module::::validation_data() + .expect("validation data was set in parachain system inherent"); + let relay_height = validation_data.persisted.block_number; for i in 0..num_eligible { // A context identifier for grabbing the randomness. Consists of three parts @@ -76,7 +80,16 @@ pub mod pallet { // - The relay parent block number so that the eligible authors at the next height // change. Avoids liveness attacks from colluding minorities of active authors. // Third one will not be necessary once we dleverage the relay chain's randomness. - let subject: [u8; 8] = [b'f', b'i', b'l', b't', b'e', b'r', i as u8, relay_height]; + let subject: [u8; 8] = [ + b'f', + b'i', + b'l', + b't', + b'e', + b'r', + i as u8, + relay_height as u8, + ]; let index = T::RandomnessSource::random(&subject).to_low_u64_be() as usize; // Move the selected author from the original vector into the eligible vector @@ -88,7 +101,8 @@ pub mod pallet { } // Emit an event for debugging purposes - >::deposit_event(Event::Filtered(eligible.clone())); + let our_height = frame_system::Module::::block_number(); + >::deposit_event(Event::Filtered(our_height, relay_height, eligible.clone())); eligible.contains(account) } @@ -100,7 +114,6 @@ pub mod pallet { #[pallet::call] impl Pallet { - /// Update the eligible ratio. Intended to be called by governance. #[pallet::weight(0)] pub fn set_eligible(origin: OriginFor, new: Percent) -> DispatchResultWithPostInfo { @@ -129,6 +142,7 @@ pub mod pallet { EligibleUpdated(Percent), /// The staked authors have been filtered to these eligible authors in this block. /// This is a debugging and development event and should be removed eventually. - Filtered(Vec), + /// Fields are: para block height, relay block height, eligible authors + Filtered(T::BlockNumber, u32, Vec), } } From 2a1a18e1aff613d8e703cc0655e35a3926f78281 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 15:20:37 -0500 Subject: [PATCH 12/16] Move author check back to inherent body (not on_finalize) to void ugly panic messages. --- pallets/author-filter/src/lib.rs | 11 +++++++---- pallets/author-inherent/src/lib.rs | 14 ++++++-------- runtime/src/lib.rs | 2 ++ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index 168bde5070..c804de31fd 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -57,10 +57,13 @@ pub mod pallet { type RandomnessSource: Randomness; } - // This code will be called by the author-inherent pallet in its on-finalize block to check - // whether the reported author of this block is eligible at this height. We calculate that - // result on demand for the currently ending block and do not record it instorage (although - // we do emit a debugging event for now.) + // This code will be called by the author-inherent pallet to check whether the reported author + // of this block is eligible at this height. We calculate that result on demand and do not + // record it instorage (although we do emit a debugging event for now). + // This implementation relies on the relay parent's block number from the validation data + // inherent. Therefore the validation data inherent **must** be included before this check is + // performed. Concretely the validataion data inherent must be included before the author + // inherent. impl author_inherent::CanAuthor for Pallet { fn can_author(account: &T::AccountId) -> bool { let mut staked: Vec = stake::Module::::validators(); diff --git a/pallets/author-inherent/src/lib.rs b/pallets/author-inherent/src/lib.rs index dfbd5847ae..897b0f1ee4 100644 --- a/pallets/author-inherent/src/lib.rs +++ b/pallets/author-inherent/src/lib.rs @@ -53,7 +53,9 @@ pub trait Config: System { /// Other pallets that want to be informed about block authorship type EventHandler: EventHandler; - /// Checks if account can be set as block author + /// Checks if account can be set as block author. + /// If the pallet that implements this trait depends on an inherent, that inherent **must** + /// be included before this one. type CanAuthor: CanAuthor; } @@ -61,6 +63,8 @@ decl_error! { pub enum Error for Module { /// Author already set in block. AuthorAlreadySet, + /// The author in the inherent is not an eligible author. + CannotBeAuthor, } } @@ -88,6 +92,7 @@ decl_module! { fn set_author(origin, author: T::AccountId) { ensure_none(origin)?; ensure!(>::get().is_none(), Error::::AuthorAlreadySet); + ensure!(T::CanAuthor::can_author(&author), Error::::CannotBeAuthor); // Update storage Author::::put(&author); @@ -103,13 +108,6 @@ decl_module! { // Notify any other pallets that are listening (eg rewards) about the author T::EventHandler::note_author(author.clone()); } - - fn on_finalize() { - assert!( - T::CanAuthor::can_author(&Author::::get().expect("Author was set in block")), - "Author is not eligible in this block" - ); - } } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 224ea39c9f..cb0db9cbf6 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -363,6 +363,8 @@ construct_runtime! { EVM: pallet_evm::{Module, Config, Call, Storage, Event}, Ethereum: pallet_ethereum::{Module, Call, Storage, Event, Config, ValidateUnsigned}, Stake: stake::{Module, Call, Storage, Event, Config}, + // The order matters here. Inherents will be included in the order specified here. + // Concretely wee need the author inherent to come after the parachain_upgrade inherent. AuthorInherent: author_inherent::{Module, Call, Storage, Inherent}, AuthorFilter: pallet_author_filter::{Module, Storage, Event,} } From aa6edfa1415e72517d9e9de46056c4f6ece4cbc2 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 15:44:40 -0500 Subject: [PATCH 13/16] disable event so ci passes --- pallets/author-filter/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index c804de31fd..e0751c6481 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -104,8 +104,8 @@ pub mod pallet { } // Emit an event for debugging purposes - let our_height = frame_system::Module::::block_number(); - >::deposit_event(Event::Filtered(our_height, relay_height, eligible.clone())); + // let our_height = frame_system::Module::::block_number(); + // >::deposit_event(Event::Filtered(our_height, relay_height, eligible.clone())); eligible.contains(account) } From 3d55d6163f9026d23d03926587d4102ccaf14e61 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Fri, 5 Feb 2021 15:48:43 -0500 Subject: [PATCH 14/16] bump runtime version --- runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index cb0db9cbf6..59b542a611 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -109,7 +109,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("moonbase-alphanet"), impl_name: create_runtime_str!("moonbase-alphanet"), authoring_version: 3, - spec_version: 16, + spec_version: 17, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 2, From 7ce9d96d8331e5179ffe42fbc123fc0d74625385 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 8 Feb 2021 09:04:57 -0500 Subject: [PATCH 15/16] Update pallets/author-filter/src/lib.rs Co-authored-by: Amar Singh --- pallets/author-filter/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/author-filter/src/lib.rs b/pallets/author-filter/src/lib.rs index e0751c6481..8ca0830dc5 100644 --- a/pallets/author-filter/src/lib.rs +++ b/pallets/author-filter/src/lib.rs @@ -62,7 +62,7 @@ pub mod pallet { // record it instorage (although we do emit a debugging event for now). // This implementation relies on the relay parent's block number from the validation data // inherent. Therefore the validation data inherent **must** be included before this check is - // performed. Concretely the validataion data inherent must be included before the author + // performed. Concretely the validation data inherent must be included before the author // inherent. impl author_inherent::CanAuthor for Pallet { fn can_author(account: &T::AccountId) -> bool { From 76f65ef87f6151c9982f46a1ba4b377b740d5fcc Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Mon, 8 Feb 2021 09:12:01 -0500 Subject: [PATCH 16/16] prune serde dependency --- Cargo.lock | 1 - pallets/author-filter/Cargo.toml | 2 -- 2 files changed, 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 712c0fdb9e..1a93ed945a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4528,7 +4528,6 @@ dependencies = [ "frame-support", "frame-system", "parity-scale-codec", - "serde", "sp-core", "sp-runtime", "stake", diff --git a/pallets/author-filter/Cargo.toml b/pallets/author-filter/Cargo.toml index f6094f3b84..3afe065657 100644 --- a/pallets/author-filter/Cargo.toml +++ b/pallets/author-filter/Cargo.toml @@ -6,7 +6,6 @@ version = "0.1.0" [dependencies] parity-scale-codec = { version = "1.3.0", default-features = false, features = ["derive"] } -serde = { version = "1.0.101", optional = true, features = ["derive"] } frame-support = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } frame-system = { git = "https://github.com/paritytech/substrate", default-features = false, branch = "master" } @@ -20,7 +19,6 @@ cumulus-parachain-upgrade = { git = "https://github.com/paritytech/cumulus", de default = ["std"] std = [ "parity-scale-codec/std", - "serde", "frame-support/std", "frame-system/std", "author-inherent/std",