diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 4af488ca98..f4ab495c24 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -8014,7 +8014,6 @@ dependencies = [ "sc-client-api", "sc-consensus", "sc-telemetry", - "sp-api", "sp-arithmetic", "sp-blockchain", "sp-consensus", diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index fbc91c5f7d..fabf8d0adf 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -196,7 +196,7 @@ pub fn new_partial( let justification_import = grandpa_block_import.clone(); let (block_import, babe_link) = sc_consensus_babe::block_import( - sc_consensus_babe::Config::get_or_compute(&*client)?, + sc_consensus_babe::Config::get(&*client)?, grandpa_block_import, client.clone(), )?; diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index ec577b0844..4579b2d73d 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -85,7 +85,7 @@ type AuthorityId

=

::Public; /// Slot duration type for Aura. pub type SlotDuration = sc_consensus_slots::SlotDuration; -/// Get type of `SlotDuration` for Aura. +/// Get the slot duration for Aura. pub fn slot_duration(client: &C) -> CResult where A: Codec, @@ -93,7 +93,10 @@ where C: AuxStore + ProvideRuntimeApi + UsageProvider, C::Api: AuraApi, { - SlotDuration::get_or_compute(client, |a, b| a.slot_duration(b).map_err(Into::into)) + let best_block_id = BlockId::Hash(client.usage_info().chain.best_hash); + let slot_duration = client.runtime_api().slot_duration(&best_block_id)?; + + Ok(SlotDuration::new(slot_duration)) } /// Get slot author for given block along with authorities. diff --git a/substrate/client/consensus/babe/rpc/src/lib.rs b/substrate/client/consensus/babe/rpc/src/lib.rs index eeec7b86b1..463b05ef9c 100644 --- a/substrate/client/consensus/babe/rpc/src/lib.rs +++ b/substrate/client/consensus/babe/rpc/src/lib.rs @@ -247,7 +247,7 @@ mod tests { let builder = TestClientBuilder::new(); let (client, longest_chain) = builder.build_with_longest_chain(); let client = Arc::new(client); - let config = Config::get_or_compute(&*client).expect("config available"); + let config = Config::get(&*client).expect("config available"); let (_, link) = block_import(config.clone(), client.clone(), client.clone()) .expect("can initialize block-import"); diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 21c3c883a3..7a05c7a926 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -329,7 +329,9 @@ pub struct BabeIntermediate { /// Intermediate key for Babe engine. pub static INTERMEDIATE_KEY: &[u8] = b"babe1"; -/// A slot duration. Create with `get_or_compute`. +/// A slot duration. +/// +/// Create with [`Self::get`]. // FIXME: Once Rust has higher-kinded types, the duplication between this // and `super::babe::Config` can be eliminated. // https://github.com/paritytech/substrate/issues/2434 @@ -337,39 +339,33 @@ pub static INTERMEDIATE_KEY: &[u8] = b"babe1"; pub struct Config(sc_consensus_slots::SlotDuration); impl Config { - /// Either fetch the slot duration from disk or compute it from the genesis - /// state. - pub fn get_or_compute(client: &C) -> ClientResult + /// Fetch the config from the runtime. + pub fn get(client: &C) -> ClientResult where C: AuxStore + ProvideRuntimeApi + UsageProvider, C::Api: BabeApi, { trace!(target: "babe", "Getting slot duration"); - match sc_consensus_slots::SlotDuration::get_or_compute(client, |a, b| { - let has_api_v1 = a.has_api_with::, _>(&b, |v| v == 1)?; - let has_api_v2 = a.has_api_with::, _>(&b, |v| v == 2)?; - if has_api_v1 { - #[allow(deprecated)] - { - Ok(a.configuration_before_version_2(b)?.into()) - } - } else if has_api_v2 { - a.configuration(b).map_err(Into::into) - } else { - Err(sp_blockchain::Error::VersionInvalid( - "Unsupported or invalid BabeApi version".to_string(), - )) + let best_block_id = BlockId::Hash(client.usage_info().chain.best_hash); + let runtime_api = client.runtime_api(); + + let version = runtime_api.api_version::>(&best_block_id)?; + + let slot_duration = if version == Some(1) { + #[allow(deprecated)] + { + runtime_api.configuration_before_version_2(&best_block_id)?.into() } - }) - .map(Self) - { - Ok(s) => Ok(s), - Err(s) => { - warn!(target: "babe", "Failed to get slot duration"); - Err(s) - }, - } + } else if version == Some(2) { + runtime_api.configuration(&best_block_id)? + } else { + return Err(sp_blockchain::Error::VersionInvalid( + "Unsupported or invalid BabeApi version".to_string(), + )) + }; + + Ok(Self(sc_consensus_slots::SlotDuration::new(slot_duration))) } /// Get the inner slot duration diff --git a/substrate/client/consensus/babe/src/tests.rs b/substrate/client/consensus/babe/src/tests.rs index 73cc453812..23c34d21ec 100644 --- a/substrate/client/consensus/babe/src/tests.rs +++ b/substrate/client/consensus/babe/src/tests.rs @@ -297,7 +297,7 @@ impl TestNetFactory for BabeTestNet { ) { let client = client.as_client(); - let config = Config::get_or_compute(&*client).expect("config available"); + let config = Config::get(&*client).expect("config available"); let (block_import, link) = crate::block_import(config, client.clone(), client.clone()) .expect("can initialize block-import"); diff --git a/substrate/client/consensus/manual-seal/src/consensus/babe.rs b/substrate/client/consensus/manual-seal/src/consensus/babe.rs index e06c544aae..499a82c63e 100644 --- a/substrate/client/consensus/manual-seal/src/consensus/babe.rs +++ b/substrate/client/consensus/manual-seal/src/consensus/babe.rs @@ -154,7 +154,7 @@ where return Err(Error::StringError("Cannot supply empty authority set!".into())) } - let config = Config::get_or_compute(&*client)?; + let config = Config::get(&*client)?; Ok(Self { config, client, keystore, epoch_changes, authorities }) } @@ -327,7 +327,7 @@ impl SlotTimestampProvider { C: AuxStore + HeaderBackend + ProvideRuntimeApi + UsageProvider, C::Api: BabeApi, { - let slot_duration = Config::get_or_compute(&*client)?.slot_duration; + let slot_duration = Config::get(&*client)?.slot_duration; let info = client.info(); // looks like this isn't the first block, rehydrate the fake time. diff --git a/substrate/client/consensus/slots/Cargo.toml b/substrate/client/consensus/slots/Cargo.toml index bf2bd59bb9..782e979ed6 100644 --- a/substrate/client/consensus/slots/Cargo.toml +++ b/substrate/client/consensus/slots/Cargo.toml @@ -23,7 +23,6 @@ sc-consensus = { version = "0.10.0-dev", path = "../../../client/consensus/commo sp-consensus-slots = { version = "0.10.0-dev", path = "../../../primitives/consensus/slots" } sp-runtime = { version = "4.0.0", path = "../../../primitives/runtime" } sp-state-machine = { version = "0.10.0", path = "../../../primitives/state-machine" } -sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } sc-telemetry = { version = "4.0.0-dev", path = "../../telemetry" } sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" } sp-inherents = { version = "4.0.0-dev", path = "../../../primitives/inherents" } diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index 3174eacaff..905165aa9e 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -38,7 +38,6 @@ use futures_timer::Delay; use log::{debug, error, info, warn}; use sc_consensus::{BlockImport, JustificationSyncLink}; use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO, CONSENSUS_WARN}; -use sp_api::{ApiRef, ProvideRuntimeApi}; use sp_arithmetic::traits::BaseArithmetic; use sp_consensus::{CanAuthorWith, Proposer, SelectChain, SlotData, SyncOracle}; use sp_consensus_slots::Slot; @@ -537,9 +536,7 @@ where SlotDurationInvalid(SlotDuration), } -/// A slot duration. Create with [`get_or_compute`](Self::get_or_compute). -// The internal member should stay private here to maintain invariants of -// `get_or_compute`. +/// A slot duration. Create with [`Self::new`]. #[derive(Clone, Copy, Debug, Encode, Decode, Hash, PartialOrd, Ord, PartialEq, Eq)] pub struct SlotDuration(T); @@ -554,54 +551,12 @@ impl SlotData for SlotDuration { fn slot_duration(&self) -> std::time::Duration { self.0.slot_duration() } - - const SLOT_KEY: &'static [u8] = T::SLOT_KEY; } impl SlotDuration { - /// Either fetch the slot duration from disk or compute it from the - /// genesis state. - /// - /// `slot_key` is marked as `'static`, as it should really be a - /// compile-time constant. - pub fn get_or_compute(client: &C, cb: CB) -> sp_blockchain::Result - where - C: sc_client_api::backend::AuxStore + sc_client_api::UsageProvider, - C: ProvideRuntimeApi, - CB: FnOnce(ApiRef, &BlockId) -> sp_blockchain::Result, - T: SlotData + Encode + Decode + Debug, - { - let slot_duration = match client.get_aux(T::SLOT_KEY)? { - Some(v) => ::decode(&mut &v[..]).map(SlotDuration).map_err(|_| { - sp_blockchain::Error::Backend({ - error!(target: "slots", "slot duration kept in invalid format"); - "slot duration kept in invalid format".to_string() - }) - }), - None => { - let best_hash = client.usage_info().chain.best_hash; - let slot_duration = cb(client.runtime_api(), &BlockId::hash(best_hash))?; - - info!( - "⏱ Loaded block-time = {:?} from block {:?}", - slot_duration.slot_duration(), - best_hash, - ); - - slot_duration - .using_encoded(|s| client.insert_aux(&[(T::SLOT_KEY, &s[..])], &[]))?; - - Ok(SlotDuration(slot_duration)) - }, - }?; - - if slot_duration.slot_duration() == Default::default() { - return Err(sp_blockchain::Error::Application(Box::new(Error::SlotDurationInvalid( - slot_duration, - )))) - } - - Ok(slot_duration) + /// Create a new instance of `Self`. + pub fn new(val: T) -> Self { + Self(val) } /// Returns slot data value. @@ -875,7 +830,7 @@ impl BackoffAuthoringBlocksStrategy for () { #[cfg(test)] mod test { use super::*; - use sp_api::NumberFor; + use sp_runtime::traits::NumberFor; use std::time::{Duration, Instant}; use substrate_test_runtime_client::runtime::{Block, Header}; diff --git a/substrate/primitives/consensus/aura/src/lib.rs b/substrate/primitives/consensus/aura/src/lib.rs index e6a319c1d1..b85443e091 100644 --- a/substrate/primitives/consensus/aura/src/lib.rs +++ b/substrate/primitives/consensus/aura/src/lib.rs @@ -117,6 +117,4 @@ impl sp_consensus::SlotData for SlotDuration { fn slot_duration(&self) -> std::time::Duration { std::time::Duration::from_millis(self.0) } - - const SLOT_KEY: &'static [u8] = b"aura_slot_duration"; } diff --git a/substrate/primitives/consensus/babe/src/lib.rs b/substrate/primitives/consensus/babe/src/lib.rs index 560866cfb2..1971563ff1 100644 --- a/substrate/primitives/consensus/babe/src/lib.rs +++ b/substrate/primitives/consensus/babe/src/lib.rs @@ -242,8 +242,6 @@ impl sp_consensus::SlotData for BabeGenesisConfiguration { fn slot_duration(&self) -> std::time::Duration { std::time::Duration::from_millis(self.slot_duration) } - - const SLOT_KEY: &'static [u8] = b"babe_configuration"; } /// Configuration data used by the BABE consensus engine. diff --git a/substrate/primitives/consensus/common/src/lib.rs b/substrate/primitives/consensus/common/src/lib.rs index ce834fd0a4..456ba965f4 100644 --- a/substrate/primitives/consensus/common/src/lib.rs +++ b/substrate/primitives/consensus/common/src/lib.rs @@ -332,7 +332,4 @@ impl CanAuthorWith for NeverCanAuthor { pub trait SlotData { /// Gets the slot duration. fn slot_duration(&self) -> sp_std::time::Duration; - - /// The static slot key - const SLOT_KEY: &'static [u8]; } diff --git a/substrate/test-utils/test-runner/src/client.rs b/substrate/test-utils/test-runner/src/client.rs index 21039d3bc4..b4a82b1c89 100644 --- a/substrate/test-utils/test-runner/src/client.rs +++ b/substrate/test-utils/test-runner/src/client.rs @@ -127,7 +127,7 @@ where None, )?; - let slot_duration = sc_consensus_babe::Config::get_or_compute(&*client)?; + let slot_duration = sc_consensus_babe::Config::get(&*client)?; let (block_import, babe_link) = sc_consensus_babe::block_import( slot_duration.clone(), grandpa_block_import, diff --git a/substrate/test-utils/test-runner/src/lib.rs b/substrate/test-utils/test-runner/src/lib.rs index 9f51442ed7..d5e2873fe4 100644 --- a/substrate/test-utils/test-runner/src/lib.rs +++ b/substrate/test-utils/test-runner/src/lib.rs @@ -154,7 +154,7 @@ //! sc_finality_grandpa::block_import(client.clone(), &(client.clone() as Arc<_>), select_chain.clone())?; //! //! let (block_import, babe_link) = sc_consensus_babe::block_import( -//! sc_consensus_babe::Config::get_or_compute(&*client)?, +//! sc_consensus_babe::Config::get(&*client)?, //! grandpa_block_import, //! client.clone(), //! )?;