diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 62969a0efa..93ddce4971 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -3738,6 +3738,7 @@ dependencies = [ "sr-std 2.0.0", "srml-support 2.0.0", "srml-system 2.0.0", + "substrate-inherents 2.0.0", "substrate-primitives 2.0.0", ] @@ -4457,6 +4458,7 @@ dependencies = [ "substrate-consensus-babe-primitives 2.0.0", "substrate-consensus-common 2.0.0", "substrate-consensus-slots 2.0.0", + "substrate-consensus-uncles 2.0.0", "substrate-executor 2.0.0", "substrate-inherents 2.0.0", "substrate-keyring 2.0.0", @@ -4553,6 +4555,19 @@ dependencies = [ "substrate-test-runtime-client 2.0.0", ] +[[package]] +name = "substrate-consensus-uncles" +version = "2.0.0" +dependencies = [ + "log 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", + "sr-primitives 2.0.0", + "srml-authorship 0.1.0", + "substrate-client 2.0.0", + "substrate-consensus-common 2.0.0", + "substrate-inherents 2.0.0", + "substrate-primitives 2.0.0", +] + [[package]] name = "substrate-executor" version = "2.0.0" diff --git a/substrate/Cargo.toml b/substrate/Cargo.toml index 298882e9a6..be6c89c2b7 100644 --- a/substrate/Cargo.toml +++ b/substrate/Cargo.toml @@ -28,6 +28,7 @@ members = [ "core/consensus/common", "core/consensus/rhd", "core/consensus/slots", + "core/consensus/uncles", "core/executor", "core/executor/runtime-test", "core/finality-grandpa", diff --git a/substrate/core/client/src/client.rs b/substrate/core/client/src/client.rs index 7213db0c9f..fa60c80e3a 100644 --- a/substrate/core/client/src/client.rs +++ b/substrate/core/client/src/client.rs @@ -170,6 +170,13 @@ pub trait BlockBody { ) -> error::Result::Extrinsic>>>; } +/// Provide a list of potential uncle headers for a given block. +pub trait ProvideUncles { + /// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors. + fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) + -> error::Result>; +} + /// Client info #[derive(Debug)] pub struct ClientInfo { @@ -1329,7 +1336,7 @@ impl Client where ancestor_hash = *current.parent_hash(); ancestor = load_header(ancestor_hash)?; } - + trace!("Collected {} uncles", uncles.len()); Ok(uncles) } @@ -1353,6 +1360,20 @@ impl Client where } } +impl ProvideUncles for Client where + B: backend::Backend, + E: CallExecutor, + Block: BlockT, +{ + fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) -> error::Result> { + Ok(Client::uncles(self, target_hash, max_generation)? + .into_iter() + .filter_map(|hash| Client::header(self, &BlockId::Hash(hash)).unwrap_or(None)) + .collect() + ) + } +} + impl ChainHeaderBackend for Client where B: backend::Backend, E: CallExecutor + Send + Sync, diff --git a/substrate/core/client/src/lib.rs b/substrate/core/client/src/lib.rs index ba5f316d0c..99cbecbe89 100644 --- a/substrate/core/client/src/lib.rs +++ b/substrate/core/client/src/lib.rs @@ -115,7 +115,7 @@ pub use crate::client::{ new_in_mem, BlockBody, BlockStatus, ImportNotifications, FinalityNotifications, BlockchainEvents, BlockImportNotification, Client, ClientInfo, ExecutionStrategies, FinalityNotification, - LongestChain, BlockOf, + LongestChain, BlockOf, ProvideUncles, utils, }; #[cfg(feature = "std")] diff --git a/substrate/core/consensus/babe/Cargo.toml b/substrate/core/consensus/babe/Cargo.toml index 9e1fb6bd4b..8184033735 100644 --- a/substrate/core/consensus/babe/Cargo.toml +++ b/substrate/core/consensus/babe/Cargo.toml @@ -22,6 +22,7 @@ keystore = { package = "substrate-keystore", path = "../../keystore" } srml-babe = { path = "../../../srml/babe" } client = { package = "substrate-client", path = "../../client" } consensus-common = { package = "substrate-consensus-common", path = "../common" } +uncles = { package = "substrate-consensus-uncles", path = "../uncles" } slots = { package = "substrate-consensus-slots", path = "../slots" } sr-primitives = { path = "../../sr-primitives" } fork-tree = { path = "../../utils/fork-tree" } diff --git a/substrate/core/consensus/babe/src/lib.rs b/substrate/core/consensus/babe/src/lib.rs index 873913e59b..eb4b0c5195 100644 --- a/substrate/core/consensus/babe/src/lib.rs +++ b/substrate/core/consensus/babe/src/lib.rs @@ -67,6 +67,7 @@ use client::{ block_builder::api::BlockBuilder as BlockBuilderApi, blockchain::{self, HeaderBackend, ProvideCache}, BlockchainEvents, CallExecutor, Client, runtime_api::ApiExt, error::Result as ClientResult, backend::{AuxStore, Backend}, + ProvideUncles, utils::is_descendent_of, }; use fork_tree::ForkTree; @@ -182,9 +183,9 @@ pub fn start_babe(BabeParams { consensus_common::Error, > where B: BlockT, - C: ProvideRuntimeApi + ProvideCache, + C: ProvideRuntimeApi + ProvideCache + ProvideUncles + Send + Sync + 'static, C::Api: BabeApi, - SC: SelectChain, + SC: SelectChain + 'static, E::Proposer: Proposer, >::Create: Unpin + Send + 'static, H: Header, @@ -203,6 +204,11 @@ pub fn start_babe(BabeParams { keystore, }; register_babe_inherent_data_provider(&inherent_data_providers, config.0.slot_duration())?; + uncles::register_uncles_inherent_data_provider( + client.clone(), + select_chain.clone(), + &inherent_data_providers, + )?; Ok(slots::start_slot_worker( config.0, select_chain, diff --git a/substrate/core/consensus/uncles/Cargo.toml b/substrate/core/consensus/uncles/Cargo.toml new file mode 100644 index 0000000000..98b8adee8b --- /dev/null +++ b/substrate/core/consensus/uncles/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "substrate-consensus-uncles" +version = "2.0.0" +authors = ["Parity Technologies "] +description = "Generic uncle inclusion utilities for consensus" +edition = "2018" + +[dependencies] +client = { package = "substrate-client", path = "../../client" } +primitives = { package = "substrate-primitives", path = "../../primitives" } +sr-primitives = { path = "../../sr-primitives" } +srml-authorship = { path = "../../../srml/authorship" } +consensus_common = { package = "substrate-consensus-common", path = "../common" } +inherents = { package = "substrate-inherents", path = "../../inherents" } +log = "0.4" diff --git a/substrate/core/consensus/uncles/src/lib.rs b/substrate/core/consensus/uncles/src/lib.rs new file mode 100644 index 0000000000..5638a23175 --- /dev/null +++ b/substrate/core/consensus/uncles/src/lib.rs @@ -0,0 +1,66 @@ +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate 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. + +// Substrate 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 Substrate. If not, see . + +//! Uncles functionality for Substrate. +//! +#![deny(warnings)] +#![forbid(unsafe_code, missing_docs)] + +use consensus_common::SelectChain; +use inherents::{InherentDataProviders}; +use log::warn; +use client::ProvideUncles; +use sr_primitives::traits::{Block as BlockT, Header}; +use std::sync::Arc; + +/// Maximum uncles generations we may provide to the runtime. +const MAX_UNCLE_GENERATIONS: u32 = 8; + +/// Register uncles inherent data provider, if not registered already. +pub fn register_uncles_inherent_data_provider( + client: Arc, + select_chain: SC, + inherent_data_providers: &InherentDataProviders, +) -> Result<(), consensus_common::Error> where + B: BlockT, + C: ProvideUncles + Send + Sync + 'static, + SC: SelectChain + 'static, +{ + if !inherent_data_providers.has_provider(&srml_authorship::INHERENT_IDENTIFIER) { + inherent_data_providers + .register_provider(srml_authorship::InherentDataProvider::new(move || { + { + let chain_head = match select_chain.best_chain() { + Ok(x) => x, + Err(e) => { + warn!(target: "uncles", "Unable to get chain head: {:?}", e); + return Vec::new(); + } + }; + match client.uncles(chain_head.hash(), MAX_UNCLE_GENERATIONS.into()) { + Ok(uncles) => uncles, + Err(e) => { + warn!(target: "uncles", "Unable to get uncles: {:?}", e); + Vec::new() + } + } + } + })) + .map_err(|err| consensus_common::Error::InherentData(err.into()))?; + } + Ok(()) +} + diff --git a/substrate/node/runtime/src/lib.rs b/substrate/node/runtime/src/lib.rs index 2d6a8eb2ca..9f247c6320 100644 --- a/substrate/node/runtime/src/lib.rs +++ b/substrate/node/runtime/src/lib.rs @@ -80,8 +80,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 131, - impl_version: 133, + spec_version: 134, + impl_version: 134, apis: RUNTIME_API_VERSIONS, }; @@ -179,7 +179,7 @@ impl timestamp::Trait for Runtime { } parameter_types! { - pub const UncleGenerations: u64 = 0; + pub const UncleGenerations: u64 = 5; } impl authorship::Trait for Runtime { @@ -421,7 +421,7 @@ construct_runtime!( System: system::{Module, Call, Storage, Config, Event}, Babe: babe::{Module, Call, Storage, Config, Inherent(Timestamp)}, Timestamp: timestamp::{Module, Call, Storage, Inherent}, - Authorship: authorship::{Module, Call, Storage}, + Authorship: authorship::{Module, Call, Storage, Inherent}, Indices: indices, Balances: balances, Staking: staking::{default, OfflineWorker}, diff --git a/substrate/srml/authorship/Cargo.toml b/substrate/srml/authorship/Cargo.toml index f470675b4b..e7f7b0941b 100644 --- a/substrate/srml/authorship/Cargo.toml +++ b/substrate/srml/authorship/Cargo.toml @@ -8,6 +8,7 @@ edition = "2018" [dependencies] primitives = { package = "substrate-primitives", path = "../../core/primitives", default-features = false } codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] } +inherents = { package = "substrate-inherents", path = "../../core/inherents", default-features = false } rstd = { package = "sr-std", path = "../../core/sr-std", default-features = false } sr-primitives = { path = "../../core/sr-primitives", default-features = false } srml-support = { path = "../support", default-features = false } @@ -19,6 +20,7 @@ default = ["std"] std = [ "codec/std", "primitives/std", + "inherents/std", "sr-primitives/std", "rstd/std", "srml-support/std", diff --git a/substrate/srml/authorship/src/lib.rs b/substrate/srml/authorship/src/lib.rs index fb0f451930..d6ea0326fa 100644 --- a/substrate/srml/authorship/src/lib.rs +++ b/substrate/srml/authorship/src/lib.rs @@ -20,7 +20,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use rstd::prelude::*; +use rstd::{result, prelude::*}; use rstd::collections::btree_set::BTreeSet; use srml_support::{decl_module, decl_storage, for_each_tuple, StorageValue}; use srml_support::traits::{FindAuthor, VerifySeal, Get}; @@ -29,6 +29,61 @@ use codec::{Encode, Decode}; use system::ensure_none; use sr_primitives::traits::{SimpleArithmetic, Header as HeaderT, One, Zero}; use sr_primitives::weights::SimpleDispatchInfo; +use inherents::{ + RuntimeString, InherentIdentifier, ProvideInherent, + InherentData, MakeFatalError, +}; + +/// The identifier for the `uncles` inherent. +pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"uncles00"; + +/// Auxiliary trait to extract uncles inherent data. +pub trait UnclesInherentData { + /// Get uncles. + fn uncles(&self) -> Result, RuntimeString>; +} + +impl UnclesInherentData for InherentData { + fn uncles(&self) -> Result, RuntimeString> { + Ok(self.get_data(&INHERENT_IDENTIFIER)?.unwrap_or_default()) + } +} + +/// Provider for inherent data. +#[cfg(feature = "std")] +pub struct InherentDataProvider { + inner: F, + _marker: std::marker::PhantomData, +} + +#[cfg(feature = "std")] +impl InherentDataProvider { + pub fn new(uncles_oracle: F) -> Self { + InherentDataProvider { inner: uncles_oracle, _marker: Default::default() } + } +} + +#[cfg(feature = "std")] +impl inherents::ProvideInherentData for InherentDataProvider +where F: Fn() -> Vec +{ + fn inherent_identifier(&self) -> &'static InherentIdentifier { + &INHERENT_IDENTIFIER + } + + fn provide_inherent_data(&self, inherent_data: &mut InherentData) -> Result<(), RuntimeString> { + let uncles = (self.inner)(); + if !uncles.is_empty() { + inherent_data.put_data(INHERENT_IDENTIFIER, &uncles) + } else { + Ok(()) + } + } + + fn error_to_string(&self, _error: &[u8]) -> Option { + Some(format!("no further information")) + } +} pub trait Trait: system::Trait { /// Find the author of a block. @@ -101,16 +156,16 @@ pub trait FilterUncle { /// Do additional filtering on a seal-checked uncle block, with the accumulated /// filter. - fn filter_uncle(header: &Header, acc: Self::Accumulator) - -> Result<(Option, Self::Accumulator), &'static str>; + fn filter_uncle(header: &Header, acc: &mut Self::Accumulator) + -> Result, &'static str>; } impl FilterUncle for () { type Accumulator = (); - fn filter_uncle(_: &H, acc: Self::Accumulator) - -> Result<(Option, Self::Accumulator), &'static str> + fn filter_uncle(_: &H, _acc: &mut Self::Accumulator) + -> Result, &'static str> { - Ok((None, acc)) + Ok(None) } } @@ -124,10 +179,10 @@ impl> FilterUncle { type Accumulator = (); - fn filter_uncle(header: &Header, _acc: ()) - -> Result<(Option, ()), &'static str> + fn filter_uncle(header: &Header, _acc: &mut ()) + -> Result, &'static str> { - T::verify_seal(header).map(|author| (author, ())) + T::verify_seal(header) } } @@ -147,8 +202,8 @@ where { type Accumulator = BTreeSet<(Header::Number, Author)>; - fn filter_uncle(header: &Header, mut acc: Self::Accumulator) - -> Result<(Option, Self::Accumulator), &'static str> + fn filter_uncle(header: &Header, acc: &mut Self::Accumulator) + -> Result, &'static str> { let author = T::verify_seal(header)?; let number = header.number(); @@ -159,7 +214,7 @@ where } } - Ok((author, acc)) + Ok(author) } } @@ -256,6 +311,39 @@ impl Module { fn verify_and_import_uncles(new_uncles: Vec) -> DispatchResult { let now = >::block_number(); + let mut uncles = ::Uncles::get(); + uncles.push(UncleEntryItem::InclusionHeight(now)); + + let mut acc: >::Accumulator = Default::default(); + + for uncle in new_uncles { + let prev_uncles = uncles.iter().filter_map(|entry| + match entry { + UncleEntryItem::InclusionHeight(_) => None, + UncleEntryItem::Uncle(h, _) => Some(h), + }); + let author = Self::verify_uncle(&uncle, prev_uncles, &mut acc)?; + let hash = uncle.hash(); + + T::EventHandler::note_uncle( + author.clone().unwrap_or_default(), + now - uncle.number().clone(), + ); + uncles.push(UncleEntryItem::Uncle(hash, author)); + } + + ::Uncles::put(&uncles); + Ok(()) + } + + fn verify_uncle<'a, I: IntoIterator>( + uncle: &T::Header, + existing_uncles: I, + accumulator: &mut >::Accumulator, + ) -> Result, &'static str> + { + let now = >::block_number(); + let (minimum_height, maximum_height) = { let uncle_generations = T::UncleGenerations::get(); let min = if now >= uncle_generations { @@ -267,55 +355,82 @@ impl Module { (min, now) }; - let mut uncles = ::Uncles::get(); - uncles.push(UncleEntryItem::InclusionHeight(now)); + let hash = uncle.hash(); - let mut acc: >::Accumulator = Default::default(); - - for uncle in new_uncles { - let hash = uncle.hash(); - - if uncle.number() < &One::one() { - return Err("uncle is genesis"); - } - - if uncle.number() > &maximum_height { - return Err("uncles too high in chain"); - } - - { - let parent_number = uncle.number().clone() - One::one(); - let parent_hash = >::block_hash(&parent_number); - if &parent_hash != uncle.parent_hash() { - return Err("uncle parent not in chain"); - } - } - - if uncle.number() < &minimum_height { - return Err("uncle not recent enough to be included"); - } - - let duplicate = uncles.iter().find(|entry| match entry { - UncleEntryItem::InclusionHeight(_) => false, - UncleEntryItem::Uncle(h, _) => h == &hash, - }).is_some(); - - let in_chain = >::block_hash(uncle.number()) == hash; - - if duplicate || in_chain { return Err("uncle already included") } - - // check uncle validity. - let (author, temp_acc) = T::FilterUncle::filter_uncle(&uncle, acc)?; - acc = temp_acc; - - T::EventHandler::note_uncle( - author.clone().unwrap_or_default(), - now - uncle.number().clone(), - ); - uncles.push(UncleEntryItem::Uncle(hash, author)); + if uncle.number() < &One::one() { + return Err("uncle is genesis"); } - ::Uncles::put(&uncles); + if uncle.number() > &maximum_height { + return Err("uncle is too high in chain"); + } + + { + let parent_number = uncle.number().clone() - One::one(); + let parent_hash = >::block_hash(&parent_number); + if &parent_hash != uncle.parent_hash() { + return Err("uncle parent not in chain"); + } + } + + if uncle.number() < &minimum_height { + return Err("uncle not recent enough to be included"); + } + + let duplicate = existing_uncles.into_iter().find(|h| **h == hash).is_some(); + let in_chain = >::block_hash(uncle.number()) == hash; + + if duplicate || in_chain { + return Err("uncle already included") + } + + // check uncle validity. + T::FilterUncle::filter_uncle(&uncle, accumulator) + } +} + +impl ProvideInherent for Module { + type Call = Call; + type Error = MakeFatalError<()>; + const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; + + fn create_inherent(data: &InherentData) -> Option { + let uncles = data.uncles().unwrap_or_default(); + let mut set_uncles = Vec::new(); + + if !uncles.is_empty() { + let prev_uncles = ::Uncles::get(); + let mut existing_hashes: Vec<_> = prev_uncles.into_iter().filter_map(|entry| + match entry { + UncleEntryItem::InclusionHeight(_) => None, + UncleEntryItem::Uncle(h, _) => Some(h), + } + ).collect(); + + let mut acc: >::Accumulator = Default::default(); + + for uncle in uncles { + match Self::verify_uncle(&uncle, &existing_hashes, &mut acc) { + Ok(_) => { + let hash = uncle.hash(); + set_uncles.push(uncle); + existing_hashes.push(hash); + } + Err(_) => { + // skip this uncle + } + } + } + } + + if set_uncles.is_empty() { + None + } else { + Some(Call::set_uncles(set_uncles)) + } + } + + fn check_inherent(_call: &Self::Call, _data: &InherentData) -> result::Result<(), Self::Error> { Ok(()) } } @@ -601,7 +716,7 @@ mod tests { let author_a = 42; let author_b = 43; - let mut acc: Option<>::Accumulator> = Some(Default::default()); + let mut acc: >::Accumulator = Default::default(); let header_a1 = seal_header( create_header(1, Default::default(), [1; 32].into()), author_a, @@ -621,13 +736,7 @@ mod tests { ); let mut check_filter = move |uncle| { - match Filter::filter_uncle(uncle, acc.take().unwrap()) { - Ok((author, a)) => { - acc = Some(a); - Ok(author) - } - Err(e) => Err(e), - } + Filter::filter_uncle(uncle, &mut acc) }; // same height, different author is OK. diff --git a/substrate/srml/finality-tracker/src/lib.rs b/substrate/srml/finality-tracker/src/lib.rs index f074b27aa0..a4026f21b3 100644 --- a/substrate/srml/finality-tracker/src/lib.rs +++ b/substrate/srml/finality-tracker/src/lib.rs @@ -244,15 +244,16 @@ impl ProvideInherent for Module { const INHERENT_IDENTIFIER: InherentIdentifier = INHERENT_IDENTIFIER; fn create_inherent(data: &InherentData) -> Option { - let final_num = - data.finalized_number().expect("Gets and decodes final number inherent data"); - - // make hint only when not same as last to avoid bloat. - Self::recent_hints().last().and_then(|last| if last == &final_num { - None + if let Ok(final_num) = data.finalized_number() { + // make hint only when not same as last to avoid bloat. + Self::recent_hints().last().and_then(|last| if last == &final_num { + None + } else { + Some(Call::final_hint(final_num)) + }) } else { - Some(Call::final_hint(final_num)) - }) + None + } } fn check_inherent(_call: &Self::Call, _data: &InherentData) -> result::Result<(), Self::Error> {