From 11804203f40f99ea93e4853d7367b88a4f9671d3 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 5 Apr 2019 17:38:42 -0300 Subject: [PATCH 01/24] move SelectChain trait out of client --- core/client/src/client.rs | 14 +++-------- core/client/src/lib.rs | 2 +- core/consensus/aura/slots/src/lib.rs | 7 +++--- core/consensus/aura/src/lib.rs | 7 +++--- core/consensus/common/src/lib.rs | 2 ++ core/consensus/common/src/select_chain.rs | 29 +++++++++++++++++++++++ core/consensus/rhd/src/service.rs | 4 ++-- 7 files changed, 43 insertions(+), 22 deletions(-) create mode 100644 core/consensus/common/src/select_chain.rs diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 041933cfffaff..0c35fd485feca 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -28,6 +28,7 @@ use runtime_primitives::{ use consensus::{ Error as ConsensusError, ErrorKind as ConsensusErrorKind, ImportBlock, ImportResult, BlockOrigin, ForkChoiceStrategy, well_known_cache_keys::Id as CacheKeyId, + SelectChain, self, }; use runtime_primitives::traits::{ Block as BlockT, Header as HeaderT, Zero, As, NumberFor, CurrentHeight, BlockNumberToHash, @@ -61,7 +62,6 @@ use crate::error::{self, ErrorKind}; use crate::in_mem; use crate::block_builder::{self, api::BlockBuilder as BlockBuilderAPI}; use crate::genesis; -use consensus; use substrate_telemetry::{telemetry, SUBSTRATE_INFO}; use log::{info, trace, warn}; @@ -140,15 +140,6 @@ pub trait BlockchainEvents { fn storage_changes_notification_stream(&self, filter_keys: Option<&[StorageKey]>) -> error::Result>; } -/// Chain head information. -pub trait ChainHead { - /// Get best block header. - fn best_block_header(&self) -> Result<::Header, error::Error>; - /// Get all leaves of the chain: block hashes that have no children currently. - /// Leaves that can never be finalized will not be returned. - fn leaves(&self) -> Result::Hash>, error::Error>; -} - /// Fetch block body by ID. pub trait BlockBody { /// Get block body by ID. Returns `None` if the body is not stored. @@ -1477,12 +1468,13 @@ where } } -impl ChainHead for Client +impl SelectChain for Client where B: backend::Backend, E: CallExecutor, Block: BlockT, { + type Error = Error; fn best_block_header(&self) -> error::Result<::Header> { Client::best_block_header(self) } diff --git a/core/client/src/lib.rs b/core/client/src/lib.rs index d2da243d14a4d..5937517878247 100644 --- a/core/client/src/lib.rs +++ b/core/client/src/lib.rs @@ -58,7 +58,7 @@ pub use crate::client::{ new_with_backend, new_in_mem, BlockBody, BlockStatus, ImportNotifications, FinalityNotifications, BlockchainEvents, - BlockImportNotification, Client, ClientInfo, ChainHead, ExecutionStrategies, + BlockImportNotification, Client, ClientInfo, ExecutionStrategies, }; #[cfg(feature = "std")] pub use crate::notifications::{StorageEventStream, StorageChangeSet}; diff --git a/core/consensus/aura/slots/src/lib.rs b/core/consensus/aura/slots/src/lib.rs index cfad5b69cdef1..f43905ab901e3 100644 --- a/core/consensus/aura/slots/src/lib.rs +++ b/core/consensus/aura/slots/src/lib.rs @@ -25,10 +25,9 @@ use futures::{Future, IntoFuture, future::{self, Either}}; use log::{warn, debug, info}; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{ProvideRuntimeApi, Block}; -use consensus_common::SyncOracle; +use consensus_common::{SyncOracle, SelectChain}; use inherents::{InherentData, InherentDataProviders}; use aura_primitives::AuraApi; -use client::ChainHead; use codec::Encode; /// A worker that should be invoked at every new slot. @@ -70,7 +69,7 @@ pub fn start_slot_worker_thread( inherent_data_providers: InherentDataProviders, ) -> Result<(), consensus_common::Error> where B: Block + 'static, - C: ChainHead + Send + Sync + 'static, + C: SelectChain + Send + Sync + 'static, W: SlotWorker + Send + Sync + 'static, SO: SyncOracle + Send + Clone + 'static, SC: SlotCompatible + 'static, @@ -127,7 +126,7 @@ pub fn start_slot_worker( inherent_data_providers: InherentDataProviders, ) -> Result, consensus_common::Error> where B: Block, - C: ChainHead, + C: SelectChain, W: SlotWorker, SO: SyncOracle + Send + Clone, SC: SlotCompatible, diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 9849594686b54..534fc0f5cfefc 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -31,10 +31,9 @@ use std::{sync::Arc, time::Duration, thread, marker::PhantomData, hash::Hash, fm use parity_codec::{Encode, Decode}; use consensus_common::{self, Authorities, BlockImport, Environment, Proposer, ForkChoiceStrategy, ImportBlock, BlockOrigin, Error as ConsensusError, + SelectChain, well_known_cache_keys }; -use consensus_common::well_known_cache_keys; use consensus_common::import_queue::{Verifier, BasicQueue, SharedBlockImport, SharedJustificationImport}; -use client::ChainHead; use client::block_builder::api::BlockBuilder as BlockBuilderApi; use client::blockchain::ProvideCache; use client::runtime_api::{ApiExt, Core as CoreApi}; @@ -178,7 +177,7 @@ pub fn start_aura_thread( force_authoring: bool, ) -> Result<(), consensus_common::Error> where B: Block + 'static, - C: ChainHead + ProvideRuntimeApi + ProvideCache + Send + Sync + 'static, + C: SelectChain + ProvideRuntimeApi + ProvideCache + Send + Sync + 'static, C::Api: AuthoritiesApi, E: Environment + Send + Sync + 'static, E::Proposer: Proposer + Send + 'static, @@ -226,7 +225,7 @@ pub fn start_aura( force_authoring: bool, ) -> Result, consensus_common::Error> where B: Block, - C: ChainHead + ProvideRuntimeApi + ProvideCache, + C: SelectChain + ProvideRuntimeApi + ProvideCache, C::Api: AuthoritiesApi, E: Environment, E::Proposer: Proposer, diff --git a/core/consensus/common/src/lib.rs b/core/consensus/common/src/lib.rs index 134a34454edf1..c5ed01f4ff636 100644 --- a/core/consensus/common/src/lib.rs +++ b/core/consensus/common/src/lib.rs @@ -40,6 +40,7 @@ pub use inherents::InherentData; pub mod offline_tracker; pub mod error; mod block_import; +mod select_chain; pub mod import_queue; pub mod evaluation; @@ -50,6 +51,7 @@ pub use self::error::{Error, ErrorKind}; pub use block_import::{ BlockImport, BlockOrigin, ForkChoiceStrategy, ImportedAux, ImportBlock, ImportResult, JustificationImport, }; +pub use select_chain::{SelectChain}; /// Trait for getting the authorities at a given block. pub trait Authorities { diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs new file mode 100644 index 0000000000000..885de8943551e --- /dev/null +++ b/core/consensus/common/src/select_chain.rs @@ -0,0 +1,29 @@ + +// Copyright 2019 Parity Technologies (UK) Ltd. +// This file is part of Substrate Consensus Common. + +// Substrate Demo 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 Consensus Common 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 Consensus Common. If not, see . + +use std::error::Error; +use runtime_primitives::traits::Block; + +// Chain head Selection +pub trait SelectChain { + type Error: Error; + /// Get best block header. + fn best_block_header(&self) -> Result<::Header, Self::Error>; + /// Get all leaves of the chain: block hashes that have no children currently. + /// Leaves that can never be finalized will not be returned. + fn leaves(&self) -> Result::Hash>, Self::Error>; +} diff --git a/core/consensus/rhd/src/service.rs b/core/consensus/rhd/src/service.rs index e2858f767a8c0..67f33c5e042f9 100644 --- a/core/consensus/rhd/src/service.rs +++ b/core/consensus/rhd/src/service.rs @@ -22,7 +22,7 @@ use std::thread; use std::time::{Duration, Instant}; use std::sync::Arc; -use client::{BlockchainEvents, ChainHead, BlockBody}; +use client::{BlockchainEvents, SelectChain, BlockBody}; use futures::prelude::*; use transaction_pool::txpool::{Pool as TransactionPool, ChainApi as PoolChainApi}; use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, BlockNumberToHash}; @@ -87,7 +87,7 @@ impl Service { A: AuthoringApi + BlockNumberToHash + 'static, P: PoolChainApi::Block> + 'static, C: BlockchainEvents<::Block> - + ChainHead<::Block> + + consensus::SelectChain<::Block> + BlockBody<::Block>, C: consensus::BlockImport<::Block> + consensus::Authorities<::Block> + Send + Sync + 'static, From b2048748a8ce4f3bb2dc4127684d5405d4f65725 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 5 Apr 2019 18:50:05 -0300 Subject: [PATCH 02/24] Extend SelectChain, move longest chain implementation into it --- core/client/src/client.rs | 254 +++++++++++----------- core/consensus/common/src/lib.rs | 2 +- core/consensus/common/src/select_chain.rs | 55 ++++- core/consensus/rhd/src/service.rs | 8 +- core/finality-grandpa/src/environment.rs | 2 + core/finality-grandpa/src/import.rs | 1 + core/service/src/lib.rs | 1 + 7 files changed, 187 insertions(+), 136 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 0c35fd485feca..129f1da71f59a 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1101,132 +1101,6 @@ impl Client where }) } - /// Get best block header. - pub fn best_block_header(&self) -> error::Result<::Header> { - let info = self.backend.blockchain().info().map_err(|e| error::Error::from_blockchain(Box::new(e)))?; - Ok(self.header(&BlockId::Hash(info.best_hash))?.expect("Best block header must always exist")) - } - - /// Get the most recent block hash of the best (longest) chains - /// that contain block with the given `target_hash`. - /// - /// The search space is always limited to blocks which are in the finalized - /// chain or descendents of it. - /// - /// If `maybe_max_block_number` is `Some(max_block_number)` - /// the search is limited to block `numbers <= max_block_number`. - /// in other words as if there were no blocks greater `max_block_number`. - /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) - /// Returns `Ok(None)` if `target_hash` is not found in search space. - /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) - pub fn best_containing(&self, target_hash: Block::Hash, maybe_max_number: Option>) - -> error::Result> - { - let target_header = { - match self.backend.blockchain().header(BlockId::Hash(target_hash))? { - Some(x) => x, - // target not in blockchain - None => { return Ok(None); }, - } - }; - - if let Some(max_number) = maybe_max_number { - // target outside search range - if target_header.number() > &max_number { - return Ok(None); - } - } - - let (leaves, best_already_checked) = { - // ensure no blocks are imported during this code block. - // an import could trigger a reorg which could change the canonical chain. - // we depend on the canonical chain staying the same during this code block. - let _import_lock = self.import_lock.lock(); - - let info = self.backend.blockchain().info()?; - - let canon_hash = self.backend.blockchain().hash(*target_header.number())? - .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; - - if canon_hash == target_hash { - // if no block at the given max depth exists fallback to the best block - if let Some(max_number) = maybe_max_number { - if let Some(header) = self.backend.blockchain().hash(max_number)? { - return Ok(Some(header)); - } - } - - return Ok(Some(info.best_hash)); - } else if info.finalized_number >= *target_header.number() { - // header is on a dead fork. - return Ok(None); - } - - (self.backend.blockchain().leaves()?, info.best_hash) - }; - - // for each chain. longest chain first. shortest last - for leaf_hash in leaves { - // ignore canonical chain which we already checked above - if leaf_hash == best_already_checked { - continue; - } - - // start at the leaf - let mut current_hash = leaf_hash; - - // if search is not restricted then the leaf is the best - let mut best_hash = leaf_hash; - - // go backwards entering the search space - // waiting until we are <= max_number - if let Some(max_number) = maybe_max_number { - loop { - let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? - .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - - if current_header.number() <= &max_number { - best_hash = current_header.hash(); - break; - } - - current_hash = *current_header.parent_hash(); - } - } - - // go backwards through the chain (via parent links) - loop { - // until we find target - if current_hash == target_hash { - return Ok(Some(best_hash)); - } - - let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? - .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - - // stop search in this chain once we go below the target's block number - if current_header.number() < target_header.number() { - break; - } - - current_hash = *current_header.parent_hash(); - } - } - - // header may be on a dead fork -- the only leaves that are considered are - // those which can still be finalized. - // - // FIXME #1558 only issue this warning when not on a dead fork - warn!( - "Block {:?} exists in chain but not found when following all \ - leaves backwards. Number limit = {:?}", - target_hash, - maybe_max_number, - ); - - Ok(None) - } - /// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors. pub fn uncles(&self, target_hash: Block::Hash, max_generation: NumberFor) -> error::Result> { let load_header = |id: Block::Hash| -> error::Result { @@ -1475,8 +1349,134 @@ where Block: BlockT, { type Error = Error; + fn best_block_header(&self) -> error::Result<::Header> { - Client::best_block_header(self) + let info = self.backend.blockchain().info() + .map_err(|e| error::Error::from_blockchain(Box::new(e)))?; + Ok(self.header(&BlockId::Hash(info.best_hash))? + .expect("Best block header must always exist")) + } + + /// Get the most recent block hash of the best (longest) chains + /// that contain block with the given `target_hash`. + /// + /// The search space is always limited to blocks which are in the finalized + /// chain or descendents of it. + /// + /// If `maybe_max_block_number` is `Some(max_block_number)` + /// the search is limited to block `numbers <= max_block_number`. + /// in other words as if there were no blocks greater `max_block_number`. + /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) + /// Returns `Ok(None)` if `target_hash` is not found in search space. + /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) + fn best_containing( + &self, + target_hash: Block::Hash, + maybe_max_number: Option> + ) -> error::Result> { + let target_header = { + match self.backend.blockchain().header(BlockId::Hash(target_hash))? { + Some(x) => x, + // target not in blockchain + None => { return Ok(None); }, + } + }; + + if let Some(max_number) = maybe_max_number { + // target outside search range + if target_header.number() > &max_number { + return Ok(None); + } + } + + let (leaves, best_already_checked) = { + // ensure no blocks are imported during this code block. + // an import could trigger a reorg which could change the canonical chain. + // we depend on the canonical chain staying the same during this code block. + let _import_lock = self.import_lock.lock(); + + let info = self.backend.blockchain().info()?; + + let canon_hash = self.backend.blockchain().hash(*target_header.number())? + .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; + + if canon_hash == target_hash { + // if no block at the given max depth exists fallback to the best block + if let Some(max_number) = maybe_max_number { + if let Some(header) = self.backend.blockchain().hash(max_number)? { + return Ok(Some(header)); + } + } + + return Ok(Some(info.best_hash)); + } else if info.finalized_number >= *target_header.number() { + // header is on a dead fork. + return Ok(None); + } + + (self.backend.blockchain().leaves()?, info.best_hash) + }; + + // for each chain. longest chain first. shortest last + for leaf_hash in leaves { + // ignore canonical chain which we already checked above + if leaf_hash == best_already_checked { + continue; + } + + // start at the leaf + let mut current_hash = leaf_hash; + + // if search is not restricted then the leaf is the best + let mut best_hash = leaf_hash; + + // go backwards entering the search space + // waiting until we are <= max_number + if let Some(max_number) = maybe_max_number { + loop { + let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? + .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; + + if current_header.number() <= &max_number { + best_hash = current_header.hash(); + break; + } + + current_hash = *current_header.parent_hash(); + } + } + + // go backwards through the chain (via parent links) + loop { + // until we find target + if current_hash == target_hash { + return Ok(Some(best_hash)); + } + + let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? + .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; + + // stop search in this chain once we go below the target's block number + if current_header.number() < target_header.number() { + break; + } + + current_hash = *current_header.parent_hash(); + } + } + + // header may be on a dead fork -- the only leaves that are considered are + // those which can still be finalized. + // + // FIXME #1558 only issue this warning when not on a dead fork + warn!( + "Block {:?} exists in chain but not found when following all \ + leaves backwards. Number limit = {:?}", + target_hash, + maybe_max_number, + ); + + Ok(None) } fn leaves(&self) -> Result::Hash>, error::Error> { diff --git a/core/consensus/common/src/lib.rs b/core/consensus/common/src/lib.rs index c5ed01f4ff636..3250e8c53962e 100644 --- a/core/consensus/common/src/lib.rs +++ b/core/consensus/common/src/lib.rs @@ -51,7 +51,7 @@ pub use self::error::{Error, ErrorKind}; pub use block_import::{ BlockImport, BlockOrigin, ForkChoiceStrategy, ImportedAux, ImportBlock, ImportResult, JustificationImport, }; -pub use select_chain::{SelectChain}; +pub use select_chain::SelectChain; /// Trait for getting the authorities at a given block. pub trait Authorities { diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs index 885de8943551e..e7309f558c138 100644 --- a/core/consensus/common/src/select_chain.rs +++ b/core/consensus/common/src/select_chain.rs @@ -16,14 +16,61 @@ // along with Substrate Consensus Common. If not, see . use std::error::Error; -use runtime_primitives::traits::Block; +use runtime_primitives::traits::{Block, NumberFor}; -// Chain head Selection +/// The SelectChain trait defines the strategy upon which the head is chosen +/// if multiple forks are present for an opaque definition of "best" in the +/// specific chain build. +//// +/// The Strategy can be customised for the two use cases of authoring new blocks +/// upon the best chain or which fork to finalise or just the main methods +/// `best_block_header` and `best_containing` can be implemented if the same strategy +/// is being used. pub trait SelectChain { + /// The Error type returned in case the call didn't succeed type Error: Error; - /// Get best block header. - fn best_block_header(&self) -> Result<::Header, Self::Error>; + /// Get all leaves of the chain: block hashes that have no children currently. /// Leaves that can never be finalized will not be returned. fn leaves(&self) -> Result::Hash>, Self::Error>; + + /// Get best block header. + fn best_block_header(&self) -> Result<::Header, Self::Error>; + + /// Get best block header for authoring + fn best_block_header_for_authoring(&self) -> Result<::Header, Self::Error> { + self.best_block_header() + } + + /// Get best block header for finalisation + fn best_block_header_for_finalisation(&self) -> Result<::Header, Self::Error> { + self.best_block_header() + } + + /// Get the most recent block hash of the best chain that contain block + /// with the given `target_hash`. + fn best_containing( + &self, + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Self::Error>; + + /// Get the most recent block hash of the best chain that contain block + /// with the given `target_hash` for authoring + fn best_containing_for_authoring( + &self, + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Self::Error> { + self.best_containing(target_hash, maybe_max_number) + } + /// Get the most recent block hash of the best chain that contain block + /// with the given `target_hash` for finalisation + fn best_containing_for_finalisation( + &self, + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Self::Error> { + self.best_containing(target_hash, maybe_max_number) + } } diff --git a/core/consensus/rhd/src/service.rs b/core/consensus/rhd/src/service.rs index 67f33c5e042f9..f59393c530356 100644 --- a/core/consensus/rhd/src/service.rs +++ b/core/consensus/rhd/src/service.rs @@ -22,7 +22,7 @@ use std::thread; use std::time::{Duration, Instant}; use std::sync::Arc; -use client::{BlockchainEvents, SelectChain, BlockBody}; +use client::{BlockchainEvents, BlockBody}; use futures::prelude::*; use transaction_pool::txpool::{Pool as TransactionPool, ChainApi as PoolChainApi}; use runtime_primitives::traits::{Block as BlockT, Header as HeaderT, BlockNumberToHash}; @@ -33,7 +33,7 @@ use tokio::runtime::current_thread::Runtime as LocalRuntime; use tokio::timer::Interval; use parking_lot::RwLock; -use consensus::offline_tracker::OfflineTracker; +use consensus::{self, offline_tracker::OfflineTracker}; use super::{Network, ProposerFactory, AuthoringApi}; use {consensus, primitives, ed25519, error, BftService, LocalProposer}; @@ -87,9 +87,9 @@ impl Service { A: AuthoringApi + BlockNumberToHash + 'static, P: PoolChainApi::Block> + 'static, C: BlockchainEvents<::Block> + + BlockBody<::Block> + consensus::SelectChain<::Block> - + BlockBody<::Block>, - C: consensus::BlockImport<::Block> + + consensus::BlockImport<::Block> + consensus::Authorities<::Block> + Send + Sync + 'static, primitives::H256: From<<::Block as BlockT>::Hash>, <::Block as BlockT>::Hash: PartialEq + PartialEq, diff --git a/core/finality-grandpa/src/environment.rs b/core/finality-grandpa/src/environment.rs index 556b4aead76e3..f6924b6539345 100644 --- a/core/finality-grandpa/src/environment.rs +++ b/core/finality-grandpa/src/environment.rs @@ -41,6 +41,8 @@ use crate::{ CommandOrError, NewAuthoritySet, VoterCommand, }; +use consensus_common::SelectChain; + use crate::authorities::SharedAuthoritySet; use crate::consensus_changes::SharedConsensusChanges; use crate::justification::GrandpaJustification; diff --git a/core/finality-grandpa/src/import.rs b/core/finality-grandpa/src/import.rs index 960ab3140d7c0..e41dccf96c897 100644 --- a/core/finality-grandpa/src/import.rs +++ b/core/finality-grandpa/src/import.rs @@ -28,6 +28,7 @@ use client::runtime_api::ApiExt; use consensus_common::{ BlockImport, Error as ConsensusError, ErrorKind as ConsensusErrorKind, ImportBlock, ImportResult, JustificationImport, well_known_cache_keys, + SelectChain, }; use fg_primitives::GrandpaApi; use runtime_primitives::Justification; diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 105bab8ca03cf..8e4c5d58b2440 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -40,6 +40,7 @@ use primitives::Pair; use runtime_primitives::generic::BlockId; use runtime_primitives::traits::{Header, As}; use substrate_executor::NativeExecutor; +use consensus_common::SelectChain; use tel::{telemetry, SUBSTRATE_INFO}; pub use self::error::{ErrorKind, Error}; From 32c0c725fa5f2f76e4ebbf8f5f7f90bfd204954a Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Tue, 9 Apr 2019 16:59:52 -0300 Subject: [PATCH 03/24] Bring SelectChain into service --- core/client/src/client.rs | 326 ++++++++++-------- core/consensus/aura/slots/src/lib.rs | 10 +- core/consensus/aura/src/lib.rs | 20 +- core/consensus/common/Cargo.toml | 1 + core/consensus/common/src/select_chain.rs | 46 ++- core/finality-grandpa/src/environment.rs | 17 +- core/finality-grandpa/src/import.rs | 39 ++- core/finality-grandpa/src/lib.rs | 30 +- .../src/service_integration.rs | 1 + core/service/src/components.rs | 47 ++- core/service/src/lib.rs | 5 +- 11 files changed, 335 insertions(+), 207 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 129f1da71f59a..53b5f24949df4 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -311,10 +311,16 @@ impl Client where } /// Expose backend reference. To be used in tests only + // FIXME: this is being used in non-test cases as well, we probably + // want to decide if that is correct or other ways are more appropriate. pub fn backend(&self) -> &Arc { &self.backend } + fn import_lock(&self) -> &Mutex<()> { + &self.import_lock + } + /// Return storage entry keys in state in a block of given hash with given prefix. pub fn storage_keys(&self, id: &BlockId, key_prefix: &StorageKey) -> error::Result> { let keys = self.state_at(id)?.keys(&key_prefix.0).into_iter().map(StorageKey).collect(); @@ -1342,147 +1348,185 @@ where } } -impl SelectChain for Client -where - B: backend::Backend, - E: CallExecutor, - Block: BlockT, -{ - type Error = Error; - - fn best_block_header(&self) -> error::Result<::Header> { - let info = self.backend.blockchain().info() - .map_err(|e| error::Error::from_blockchain(Box::new(e)))?; - Ok(self.header(&BlockId::Hash(info.best_hash))? - .expect("Best block header must always exist")) - } - - /// Get the most recent block hash of the best (longest) chains - /// that contain block with the given `target_hash`. - /// - /// The search space is always limited to blocks which are in the finalized - /// chain or descendents of it. - /// - /// If `maybe_max_block_number` is `Some(max_block_number)` - /// the search is limited to block `numbers <= max_block_number`. - /// in other words as if there were no blocks greater `max_block_number`. - /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) - /// Returns `Ok(None)` if `target_hash` is not found in search space. - /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) - fn best_containing( - &self, - target_hash: Block::Hash, - maybe_max_number: Option> - ) -> error::Result> { - let target_header = { - match self.backend.blockchain().header(BlockId::Hash(target_hash))? { - Some(x) => x, - // target not in blockchain - None => { return Ok(None); }, - } - }; - - if let Some(max_number) = maybe_max_number { - // target outside search range - if target_header.number() > &max_number { - return Ok(None); - } - } - - let (leaves, best_already_checked) = { - // ensure no blocks are imported during this code block. - // an import could trigger a reorg which could change the canonical chain. - // we depend on the canonical chain staying the same during this code block. - let _import_lock = self.import_lock.lock(); - - let info = self.backend.blockchain().info()?; - - let canon_hash = self.backend.blockchain().hash(*target_header.number())? - .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; - - if canon_hash == target_hash { - // if no block at the given max depth exists fallback to the best block - if let Some(max_number) = maybe_max_number { - if let Some(header) = self.backend.blockchain().hash(max_number)? { - return Ok(Some(header)); - } - } - - return Ok(Some(info.best_hash)); - } else if info.finalized_number >= *target_header.number() { - // header is on a dead fork. - return Ok(None); - } - - (self.backend.blockchain().leaves()?, info.best_hash) - }; - - // for each chain. longest chain first. shortest last - for leaf_hash in leaves { - // ignore canonical chain which we already checked above - if leaf_hash == best_already_checked { - continue; - } - - // start at the leaf - let mut current_hash = leaf_hash; - - // if search is not restricted then the leaf is the best - let mut best_hash = leaf_hash; - - // go backwards entering the search space - // waiting until we are <= max_number - if let Some(max_number) = maybe_max_number { - loop { - let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? - .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - - if current_header.number() <= &max_number { - best_hash = current_header.hash(); - break; - } - - current_hash = *current_header.parent_hash(); - } - } - - // go backwards through the chain (via parent links) - loop { - // until we find target - if current_hash == target_hash { - return Ok(Some(best_hash)); - } - - let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? - .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - - // stop search in this chain once we go below the target's block number - if current_header.number() < target_header.number() { - break; - } - - current_hash = *current_header.parent_hash(); - } - } - - // header may be on a dead fork -- the only leaves that are considered are - // those which can still be finalized. - // - // FIXME #1558 only issue this warning when not on a dead fork - warn!( - "Block {:?} exists in chain but not found when following all \ - leaves backwards. Number limit = {:?}", - target_hash, - maybe_max_number, - ); - - Ok(None) - } - - fn leaves(&self) -> Result::Hash>, error::Error> { - self.backend.blockchain().leaves() - } -} +// struct LongestChain +// where +// B: backend::Backend + Send + Sync, +// Block: BlockT + Send + Sync, +// E: CallExecutor + Send + Sync, +// RA: Send + Sync, +// { +// client: Arc> +// } + +// impl Clone for LongestChain +// where +// B: backend::Backend + Send + Sync, +// Block: BlockT + Send + Sync, +// E: CallExecutor + Send + Sync, +// RA: Send + Sync, +// { +// fn clone(&self) -> Self { +// let client = self.client.clone(); +// LongestChain { +// client +// } +// } +// } + + +// impl LongestChain +// where +// B: backend::Backend, +// Block: BlockT, +// { +// fn new(client: Arc>) -> Self { +// LongestChain { +// client +// } +// } +// } + +// impl SelectChain for LongestChain +// where +// B: backend::Backend + Send + Sync, +// E: CallExecutor + Send + Sync, +// Block: BlockT + Send + Sync, +// E: Send + Sync, +// RA: Send + Sync, +// { +// fn best_block_header(&self) -> error::Result<::Header> { +// let info = self.client.backend().blockchain().info() +// .map_err(|e| error::Error::from_blockchain(Box::new(e))).into()?; +// Ok(self.client.backend().blockchain().header(BlockId::Hash(info.best_hash))? +// .expect("Best block header must always exist")) +// } + +// /// Get the most recent block hash of the best (longest) chains +// /// that contain block with the given `target_hash`. +// /// +// /// The search space is always limited to blocks which are in the finalized +// /// chain or descendents of it. +// /// +// /// If `maybe_max_block_number` is `Some(max_block_number)` +// /// the search is limited to block `numbers <= max_block_number`. +// /// in other words as if there were no blocks greater `max_block_number`. +// /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) +// /// Returns `Ok(None)` if `target_hash` is not found in search space. +// /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) +// fn best_containing( +// &self, +// target_hash: Block::Hash, +// maybe_max_number: Option> +// ) -> error::Result> { +// let target_header = { +// match self.client.backend().blockchain().header(BlockId::Hash(target_hash))? { +// Some(x) => x, +// // target not in blockchain +// None => { return Ok(None); }, +// } +// }; + +// if let Some(max_number) = maybe_max_number { +// // target outside search range +// if target_header.number() > &max_number { +// return Ok(None); +// } +// } + +// let (leaves, best_already_checked) = { +// // ensure no blocks are imported during this code block. +// // an import could trigger a reorg which could change the canonical chain. +// // we depend on the canonical chain staying the same during this code block. +// let _import_lock = self.client.import_lock().lock(); + +// let info = self.client.backend().blockchain().info()?; + +// let canon_hash = self.client.backend().blockchain().hash(*target_header.number())? +// .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; + +// if canon_hash == target_hash { +// // if no block at the given max depth exists fallback to the best block +// if let Some(max_number) = maybe_max_number { +// if let Some(header) = self.client.backend().blockchain().hash(max_number)? { +// return Ok(Some(header)); +// } +// } + +// return Ok(Some(info.best_hash)); +// } else if info.finalized_number >= *target_header.number() { +// // header is on a dead fork. +// return Ok(None); +// } + +// (self.client.backend().blockchain().leaves()?, info.best_hash) +// }; + +// // for each chain. longest chain first. shortest last +// for leaf_hash in leaves { +// // ignore canonical chain which we already checked above +// if leaf_hash == best_already_checked { +// continue; +// } + +// // start at the leaf +// let mut current_hash = leaf_hash; + +// // if search is not restricted then the leaf is the best +// let mut best_hash = leaf_hash; + +// // go backwards entering the search space +// // waiting until we are <= max_number +// if let Some(max_number) = maybe_max_number { +// loop { +// let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? +// .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; + +// if current_header.number() <= &max_number { +// best_hash = current_header.hash(); +// break; +// } + +// current_hash = *current_header.parent_hash(); +// } +// } + +// // go backwards through the chain (via parent links) +// loop { +// // until we find target +// if current_hash == target_hash { +// return Ok(Some(best_hash)); +// } + +// let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? +// .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; + +// // stop search in this chain once we go below the target's block number +// if current_header.number() < target_header.number() { +// break; +// } + +// current_hash = *current_header.parent_hash(); +// } +// } + +// // header may be on a dead fork -- the only leaves that are considered are +// // those which can still be finalized. +// // +// // FIXME #1558 only issue this warning when not on a dead fork +// warn!( +// "Block {:?} exists in chain but not found when following all \ +// leaves backwards. Number limit = {:?}", +// target_hash, +// maybe_max_number, +// ); + +// Ok(None) +// } + +// fn leaves(&self) -> Result::Hash>, error::Error> { +// self.client.backend().blockchain().leaves() +// } +// } impl BlockBody for Client where diff --git a/core/consensus/aura/slots/src/lib.rs b/core/consensus/aura/slots/src/lib.rs index f43905ab901e3..b364c6aab8f7b 100644 --- a/core/consensus/aura/slots/src/lib.rs +++ b/core/consensus/aura/slots/src/lib.rs @@ -62,14 +62,14 @@ pub fn inherent_to_common_error(err: inherents::RuntimeString) -> consensus_comm /// Start a new slot worker in a separate thread. pub fn start_slot_worker_thread( slot_duration: SlotDuration, - client: Arc, + select_chain: C, worker: Arc, sync_oracle: SO, on_exit: OnExit, inherent_data_providers: InherentDataProviders, ) -> Result<(), consensus_common::Error> where B: Block + 'static, - C: SelectChain + Send + Sync + 'static, + C: SelectChain + Clone + 'static, W: SlotWorker + Send + Sync + 'static, SO: SyncOracle + Send + Clone + 'static, SC: SlotCompatible + 'static, @@ -90,7 +90,7 @@ pub fn start_slot_worker_thread( let slot_worker_future = match start_slot_worker::<_, _, _, _, SC, _>( slot_duration, - client, + select_chain, worker, sync_oracle, on_exit, @@ -119,14 +119,14 @@ pub fn start_slot_worker_thread( /// Start a new slot worker. pub fn start_slot_worker( slot_duration: SlotDuration, - client: Arc, + client: C, worker: Arc, sync_oracle: SO, on_exit: OnExit, inherent_data_providers: InherentDataProviders, ) -> Result, consensus_common::Error> where B: Block, - C: SelectChain, + C: SelectChain + Clone, W: SlotWorker, SO: SyncOracle + Send + Clone, SC: SlotCompatible, diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 534fc0f5cfefc..08649ff189186 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -31,7 +31,7 @@ use std::{sync::Arc, time::Duration, thread, marker::PhantomData, hash::Hash, fm use parity_codec::{Encode, Decode}; use consensus_common::{self, Authorities, BlockImport, Environment, Proposer, ForkChoiceStrategy, ImportBlock, BlockOrigin, Error as ConsensusError, - SelectChain, well_known_cache_keys + SelectChain, well_known_cache_keys }; use consensus_common::import_queue::{Verifier, BasicQueue, SharedBlockImport, SharedJustificationImport}; use client::block_builder::api::BlockBuilder as BlockBuilderApi; @@ -165,10 +165,11 @@ impl SlotCompatible for AuraSlotCompatible { } /// Start the aura worker in a separate thread. -pub fn start_aura_thread( +pub fn start_aura_thread( slot_duration: SlotDuration, local_key: Arc

, client: Arc, + select_chain: SC, block_import: Arc, env: Arc, sync_oracle: SO, @@ -177,8 +178,9 @@ pub fn start_aura_thread( force_authoring: bool, ) -> Result<(), consensus_common::Error> where B: Block + 'static, - C: SelectChain + ProvideRuntimeApi + ProvideCache + Send + Sync + 'static, + C: ProvideRuntimeApi + ProvideCache + Send + Sync + 'static, C::Api: AuthoritiesApi, + SC: SelectChain + Clone + 'static, E: Environment + Send + Sync + 'static, E::Proposer: Proposer + Send + 'static, <>::Create as IntoFuture>::Future: Send + 'static, @@ -204,7 +206,7 @@ pub fn start_aura_thread( aura_slots::start_slot_worker_thread::<_, _, _, _, AuraSlotCompatible, _>( slot_duration, - client, + select_chain, Arc::new(worker), sync_oracle, on_exit, @@ -213,10 +215,11 @@ pub fn start_aura_thread( } /// Start the aura worker. The returned future should be run in a tokio runtime. -pub fn start_aura( +pub fn start_aura( slot_duration: SlotDuration, local_key: Arc

, client: Arc, + select_chain: SC, block_import: Arc, env: Arc, sync_oracle: SO, @@ -225,8 +228,9 @@ pub fn start_aura( force_authoring: bool, ) -> Result, consensus_common::Error> where B: Block, - C: SelectChain + ProvideRuntimeApi + ProvideCache, + C: ProvideRuntimeApi + ProvideCache, C::Api: AuthoritiesApi, + SC: SelectChain + Clone, E: Environment, E::Proposer: Proposer, <>::Create as IntoFuture>::Future: Send + 'static, @@ -250,7 +254,7 @@ pub fn start_aura( }; aura_slots::start_slot_worker::<_, _, _, _, AuraSlotCompatible, _>( slot_duration, - client, + select_chain, Arc::new(worker), sync_oracle, on_exit, @@ -926,7 +930,7 @@ mod tests { &inherent_data_providers, slot_duration.get() ).expect("Registers aura inherent data provider"); - let aura = start_aura::<_, _, _, _, ed25519::Pair, _, _, _>( + let aura = start_aura::<_, _, _, _, _, ed25519::Pair, _, _, _>( slot_duration, Arc::new(key.clone().into()), client.clone(), diff --git a/core/consensus/common/Cargo.toml b/core/consensus/common/Cargo.toml index eb0061a133b36..33db83a20d36f 100644 --- a/core/consensus/common/Cargo.toml +++ b/core/consensus/common/Cargo.toml @@ -6,6 +6,7 @@ description = "Common utilities for substrate consensus" edition = "2018" [dependencies] +parking_lot = "0.7.1" crossbeam-channel = "0.3.4" libp2p = { version = "0.6.0", default-features = false } log = "0.4" diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs index e7309f558c138..6f9bc8f44f5b1 100644 --- a/core/consensus/common/src/select_chain.rs +++ b/core/consensus/common/src/select_chain.rs @@ -15,8 +15,9 @@ // You should have received a copy of the GNU General Public License // along with Substrate Consensus Common. If not, see . -use std::error::Error; -use runtime_primitives::traits::{Block, NumberFor}; +use crate::error::Error; +use runtime_primitives::traits::{Block as BlockT, NumberFor}; + /// The SelectChain trait defines the strategy upon which the head is chosen /// if multiple forks are present for an opaque definition of "best" in the @@ -26,24 +27,22 @@ use runtime_primitives::traits::{Block, NumberFor}; /// upon the best chain or which fork to finalise or just the main methods /// `best_block_header` and `best_containing` can be implemented if the same strategy /// is being used. -pub trait SelectChain { - /// The Error type returned in case the call didn't succeed - type Error: Error; +pub trait SelectChain: Sync + Send + Clone { /// Get all leaves of the chain: block hashes that have no children currently. /// Leaves that can never be finalized will not be returned. - fn leaves(&self) -> Result::Hash>, Self::Error>; + fn leaves(&self) -> Result::Hash>, Error>; /// Get best block header. - fn best_block_header(&self) -> Result<::Header, Self::Error>; + fn best_block_header(&self) -> Result<::Header, Error>; /// Get best block header for authoring - fn best_block_header_for_authoring(&self) -> Result<::Header, Self::Error> { + fn best_block_header_for_authoring(&self) -> Result<::Header, Error> { self.best_block_header() } /// Get best block header for finalisation - fn best_block_header_for_finalisation(&self) -> Result<::Header, Self::Error> { + fn best_block_header_for_finalisation(&self) -> Result<::Header, Error> { self.best_block_header() } @@ -51,26 +50,37 @@ pub trait SelectChain { /// with the given `target_hash`. fn best_containing( &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Self::Error>; + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Error>; /// Get the most recent block hash of the best chain that contain block /// with the given `target_hash` for authoring fn best_containing_for_authoring( &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Self::Error> { + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Error> { self.best_containing(target_hash, maybe_max_number) } /// Get the most recent block hash of the best chain that contain block /// with the given `target_hash` for finalisation fn best_containing_for_finalisation( &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Self::Error> { + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Error> { self.best_containing(target_hash, maybe_max_number) } } + + +// pub trait SelectChainClone { +// fn clone_box(&self) -> Box>; +// } + +// impl Clone for Box> { +// fn clone(&self) -> Box> { +// self.clone_box() +// } +// } diff --git a/core/finality-grandpa/src/environment.rs b/core/finality-grandpa/src/environment.rs index f6924b6539345..25c90ce193c91 100644 --- a/core/finality-grandpa/src/environment.rs +++ b/core/finality-grandpa/src/environment.rs @@ -78,8 +78,9 @@ impl LastCompletedRound { } /// The environment we run GRANDPA in. -pub(crate) struct Environment, RA> { +pub(crate) struct Environment, RA, SC> { pub(crate) inner: Arc>, + pub(crate) select_chain: SC, pub(crate) voters: Arc>, pub(crate) config: Config, pub(crate) authority_set: SharedAuthoritySet>, @@ -89,12 +90,16 @@ pub(crate) struct Environment, RA> { pub(crate) last_completed: LastCompletedRound>, } -impl, B, E, N, RA> grandpa::Chain> for Environment where +impl, B, E, N, RA, SC> + grandpa::Chain> +for Environment +where Block: 'static, B: Backend + 'static, E: CallExecutor + 'static, N: Network + 'static, N::In: 'static, + SC: SelectChain + 'static, NumberFor: BlockNumberOps, { fn ancestry(&self, base: Block::Hash, block: Block::Hash) -> Result, GrandpaError> { @@ -141,7 +146,7 @@ impl, B, E, N, RA> grandpa::Chain { let base_header = self.inner.header(&BlockId::Hash(block)).ok()? .expect("Header known to exist after `best_containing` call; qed"); @@ -200,13 +205,17 @@ impl, B, E, N, RA> grandpa::Chain, N, RA> voter::Environment> for Environment where +impl, N, RA, SC> + voter::Environment> +for Environment +where Block: 'static, B: Backend + 'static, E: CallExecutor + 'static + Send + Sync, N: Network + 'static + Send, N::In: 'static + Send, RA: 'static + Send + Sync, + SC: SelectChain + 'static, NumberFor: BlockNumberOps, { type Timer = Box + Send>; diff --git a/core/finality-grandpa/src/import.rs b/core/finality-grandpa/src/import.rs index e41dccf96c897..4f5032066f305 100644 --- a/core/finality-grandpa/src/import.rs +++ b/core/finality-grandpa/src/import.rs @@ -56,16 +56,17 @@ use ed25519::Public as AuthorityId; /// /// When using GRANDPA, the block import worker should be using this block import /// object. -pub struct GrandpaBlockImport, RA, PRA> { +pub struct GrandpaBlockImport, RA, PRA, SC> { inner: Arc>, + select_chain: SC, authority_set: SharedAuthoritySet>, send_voter_commands: mpsc::UnboundedSender>>, consensus_changes: SharedConsensusChanges>, api: Arc, } -impl, RA, PRA> JustificationImport - for GrandpaBlockImport where +impl, RA, PRA, SC> JustificationImport + for GrandpaBlockImport where NumberFor: grandpa::BlockNumberOps, B: Backend + 'static, E: CallExecutor + 'static + Clone + Send + Sync, @@ -74,6 +75,7 @@ impl, RA, PRA> JustificationImport RA: Send + Sync, PRA: ProvideRuntimeApi, PRA::Api: GrandpaApi, + SC: SelectChain, { type Error = ConsensusError; @@ -90,7 +92,7 @@ impl, RA, PRA> JustificationImport pending_change.effective_number() > chain_info.finalized_number && pending_change.effective_number() <= chain_info.best_number { - let effective_block_hash = self.inner.best_containing( + let effective_block_hash = self.select_chain.best_containing( pending_change.canon_hash, Some(pending_change.effective_number()), ); @@ -159,7 +161,9 @@ impl<'a, Block: 'a + BlockT> Drop for PendingSetChanges<'a, Block> { } } -impl, RA, PRA> GrandpaBlockImport where +impl, RA, PRA, SC> + GrandpaBlockImport +where NumberFor: grandpa::BlockNumberOps, B: Backend + 'static, E: CallExecutor + 'static + Clone + Send + Sync, @@ -376,8 +380,8 @@ impl, RA, PRA> GrandpaBlockImport, RA, PRA> BlockImport - for GrandpaBlockImport where +impl, RA, PRA, SC> BlockImport + for GrandpaBlockImport where NumberFor: grandpa::BlockNumberOps, B: Backend + 'static, E: CallExecutor + 'static + Clone + Send + Sync, @@ -510,16 +514,20 @@ impl, RA, PRA> BlockImport } } -impl, RA, PRA> GrandpaBlockImport { +impl, RA, PRA, SC> + GrandpaBlockImport +{ pub(crate) fn new( inner: Arc>, + select_chain: SC, authority_set: SharedAuthoritySet>, send_voter_commands: mpsc::UnboundedSender>>, consensus_changes: SharedConsensusChanges>, api: Arc, - ) -> GrandpaBlockImport { + ) -> GrandpaBlockImport { GrandpaBlockImport { inner, + select_chain, authority_set, send_voter_commands, consensus_changes, @@ -528,12 +536,13 @@ impl, RA, PRA> GrandpaBlockImport, RA, PRA> GrandpaBlockImport - where - NumberFor: grandpa::BlockNumberOps, - B: Backend + 'static, - E: CallExecutor + 'static + Clone + Send + Sync, - RA: Send + Sync, +impl, RA, PRA, SC> + GrandpaBlockImport +where + NumberFor: grandpa::BlockNumberOps, + B: Backend + 'static, + E: CallExecutor + 'static + Clone + Send + Sync, + RA: Send + Sync, { /// Import a block justification and finalize the block. diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index b07535963727e..9dda733c7e172 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -67,6 +67,7 @@ use runtime_primitives::traits::{ use fg_primitives::GrandpaApi; use inherents::InherentDataProviders; use runtime_primitives::generic::BlockId; +use consensus_common::SelectChain; use substrate_primitives::{ed25519, H256, Pair, Blake2Hasher}; use substrate_telemetry::{telemetry, CONSENSUS_INFO, CONSENSUS_DEBUG, CONSENSUS_WARN}; @@ -287,16 +288,21 @@ pub struct LinkHalf, RA> { /// Make block importer and link half necessary to tie the background voter /// to it. -pub fn block_import, RA, PRA>( +pub fn block_import, RA, PRA, SC>( client: Arc>, + select_chain: SC, api: Arc -) -> Result<(GrandpaBlockImport, LinkHalf), ClientError> - where - B: Backend + 'static, - E: CallExecutor + 'static + Clone + Send + Sync, - RA: Send + Sync, - PRA: ProvideRuntimeApi, - PRA::Api: GrandpaApi, +) -> Result<( + GrandpaBlockImport, + LinkHalf + ), ClientError> +where + B: Backend + 'static, + E: CallExecutor + 'static + Clone + Send + Sync, + RA: Send + Sync, + PRA: ProvideRuntimeApi, + PRA::Api: GrandpaApi, + SC: SelectChain, { use runtime_primitives::traits::Zero; @@ -322,6 +328,7 @@ pub fn block_import, RA, PRA>( Ok(( GrandpaBlockImport::new( client.clone(), + select_chain, persistent_data.authority_set.clone(), voter_commands_tx, persistent_data.consensus_changes.clone(), @@ -414,11 +421,12 @@ fn register_finality_tracker_inherent_data_provider, N, RA>( +pub fn run_grandpa, N, RA, SC>( config: Config, link: LinkHalf, network: N, inherent_data_providers: InherentDataProviders, + select_chain: SC, on_exit: impl Future + Send + 'static, ) -> ::client::error::Result + Send + 'static> where Block::Hash: Ord, @@ -426,6 +434,7 @@ pub fn run_grandpa, N, RA>( E: CallExecutor + Send + Sync + 'static, N: Network + Send + Sync + 'static, N::In: Send + 'static, + SC: SelectChain + 'static, NumberFor: BlockNumberOps, DigestFor: Encode, DigestItemFor: DigestItem, @@ -448,6 +457,7 @@ pub fn run_grandpa, N, RA>( let initial_environment = Arc::new(Environment { inner: client.clone(), config: config.clone(), + select_chain: select_chain.clone(), voters: Arc::new(voters), network: network.clone(), set_id: authority_set.set_id(), @@ -507,6 +517,7 @@ pub fn run_grandpa, N, RA>( let client = client.clone(); let config = config.clone(); let network = network.clone(); + let select_chain = select_chain.clone(); let authority_set = authority_set.clone(); let consensus_changes = consensus_changes.clone(); @@ -528,6 +539,7 @@ pub fn run_grandpa, N, RA>( let genesis_state = RoundState::genesis((new.canon_hash, new.canon_number)); let env = Arc::new(Environment { inner: client, + select_chain, config, voters: Arc::new(new.authorities.into_iter().collect()), set_id: new.set_id, diff --git a/core/finality-grandpa/src/service_integration.rs b/core/finality-grandpa/src/service_integration.rs index 3eee1dd9408d4..7e9264c5e5b3b 100644 --- a/core/finality-grandpa/src/service_integration.rs +++ b/core/finality-grandpa/src/service_integration.rs @@ -30,6 +30,7 @@ pub type BlockImportForService = crate::GrandpaBlockImport< ::Block, ::RuntimeApi >, + ::SelectChain >; pub type LinkHalfForService = crate::LinkHalf< diff --git a/core/service/src/components.rs b/core/service/src/components.rs index eb37a69a14ca2..23f09d97145fb 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -23,7 +23,7 @@ use crate::chain_spec::ChainSpec; use client_db; use client::{self, Client, runtime_api}; use crate::{error, Service, maybe_start_server}; -use consensus_common::import_queue::ImportQueue; +use consensus_common::{import_queue::ImportQueue, SelectChain}; use network::{self, OnDemand}; use substrate_executor::{NativeExecutor, NativeExecutionDispatch}; use transaction_pool::txpool::{self, Options as TransactionPoolOptions, Pool as TransactionPool}; @@ -302,9 +302,11 @@ pub trait ServiceFactory: 'static + Sized { /// Extended light service type. type LightService: ServiceTrait>; /// ImportQueue for full client - type FullImportQueue: consensus_common::import_queue::ImportQueue + 'static; + type FullImportQueue: ImportQueue + 'static; /// ImportQueue for light clients - type LightImportQueue: consensus_common::import_queue::ImportQueue + 'static; + type LightImportQueue: ImportQueue + 'static; + /// The Fork Choice Strategy for the chain + type SelectChain: SelectChain + 'static; //TODO: replace these with a constructor trait. that TransactionPool implements. (#1242) /// Extrinsic pool constructor for the full client. @@ -318,6 +320,12 @@ pub trait ServiceFactory: 'static + Sized { fn build_network_protocol(config: &FactoryFullConfiguration) -> Result; + /// Build the Fork Choice algorithm for full client + fn build_select_chain( + config: &mut FactoryFullConfiguration, + client: Arc> + ) -> Result; + /// Build full service. fn new_full(config: FactoryFullConfiguration, executor: TaskExecutor) -> Result; @@ -398,6 +406,13 @@ pub trait Components: Sized + 'static { config: &mut FactoryFullConfiguration, client: Arc> ) -> Result; + + /// Build fork choice selector + fn build_select_chain( + config: &mut FactoryFullConfiguration, + client: Arc> + ) -> Result<::SelectChain, error::Error>; + } /// A struct that implement `Components` for the full client. @@ -466,9 +481,10 @@ impl Components for FullComponents { )?), None)) } - fn build_transaction_pool(config: TransactionPoolOptions, client: Arc>) - -> Result, error::Error> - { + fn build_transaction_pool( + config: TransactionPoolOptions, + client: Arc> + ) -> Result, error::Error> { Factory::build_full_transaction_pool(config, client) } @@ -478,6 +494,14 @@ impl Components for FullComponents { ) -> Result { Factory::build_full_import_queue(config, client) } + + fn build_select_chain( + config: &mut FactoryFullConfiguration, + client: Arc> + ) -> Result<::SelectChain, error::Error> { + Self::Factory::build_select_chain(config, client) + } + } /// A struct that implement `Components` for the light client. @@ -554,6 +578,17 @@ impl Components for LightComponents { ) -> Result { Factory::build_light_import_queue(config, client) } + + /// Build fork choice selector + fn build_select_chain( + _config: &mut FactoryFullConfiguration, + _client: Arc> + ) -> Result<::SelectChain, error::Error> { + // FIXME: this (and other places) need a "NotAvailableInMode" for never-in-light-client-features + // that doesn't break creation of light clients + Err("Fork choice doesn't happen on light clients.".into()) + } + } #[cfg(test)] diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 8e4c5d58b2440..0de908f689bd7 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -72,6 +72,7 @@ const DEFAULT_PROTOCOL_ID: &str = "sup"; /// Substrate service. pub struct Service { client: Arc>, + select_chain: <::Factory as ServiceFactory>::SelectChain, network: Option>>, transaction_pool: Arc>, inherents_pool: Arc>>, @@ -129,7 +130,8 @@ impl Service { let (client, on_demand) = Components::build_client(&config, executor)?; let import_queue = Box::new(Components::build_import_queue(&mut config, client.clone())?); - let best_header = client.best_block_header()?; + let select_chain = Components::build_select_chain(&mut config, client.clone())?; + let best_header = select_chain.best_block_header()?; let version = config.full_version(); info!("Best block: #{}", best_header.number()); @@ -325,6 +327,7 @@ impl Service { Ok(Service { client, + select_chain, network: Some(network), transaction_pool, inherents_pool, From 1bfeb8338e7bccb68e04ec5290700d3fc911f9ac Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Tue, 9 Apr 2019 17:00:15 -0300 Subject: [PATCH 04/24] implement LongestChain SelectChain --- core/client/src/client.rs | 377 +++++++++++++++-------------- core/consensus/common/src/error.rs | 6 + 2 files changed, 203 insertions(+), 180 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 53b5f24949df4..f9ef7995016d3 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -28,7 +28,7 @@ use runtime_primitives::{ use consensus::{ Error as ConsensusError, ErrorKind as ConsensusErrorKind, ImportBlock, ImportResult, BlockOrigin, ForkChoiceStrategy, well_known_cache_keys::Id as CacheKeyId, - SelectChain, self, + SelectChain, self, }; use runtime_primitives::traits::{ Block as BlockT, Header as HeaderT, Zero, As, NumberFor, CurrentHeight, BlockNumberToHash, @@ -1348,185 +1348,202 @@ where } } -// struct LongestChain -// where -// B: backend::Backend + Send + Sync, -// Block: BlockT + Send + Sync, -// E: CallExecutor + Send + Sync, -// RA: Send + Sync, -// { -// client: Arc> -// } - -// impl Clone for LongestChain -// where -// B: backend::Backend + Send + Sync, -// Block: BlockT + Send + Sync, -// E: CallExecutor + Send + Sync, -// RA: Send + Sync, -// { -// fn clone(&self) -> Self { -// let client = self.client.clone(); -// LongestChain { -// client -// } -// } -// } - - -// impl LongestChain -// where -// B: backend::Backend, -// Block: BlockT, -// { -// fn new(client: Arc>) -> Self { -// LongestChain { -// client -// } -// } -// } - -// impl SelectChain for LongestChain -// where -// B: backend::Backend + Send + Sync, -// E: CallExecutor + Send + Sync, -// Block: BlockT + Send + Sync, -// E: Send + Sync, -// RA: Send + Sync, -// { -// fn best_block_header(&self) -> error::Result<::Header> { -// let info = self.client.backend().blockchain().info() -// .map_err(|e| error::Error::from_blockchain(Box::new(e))).into()?; -// Ok(self.client.backend().blockchain().header(BlockId::Hash(info.best_hash))? -// .expect("Best block header must always exist")) -// } - -// /// Get the most recent block hash of the best (longest) chains -// /// that contain block with the given `target_hash`. -// /// -// /// The search space is always limited to blocks which are in the finalized -// /// chain or descendents of it. -// /// -// /// If `maybe_max_block_number` is `Some(max_block_number)` -// /// the search is limited to block `numbers <= max_block_number`. -// /// in other words as if there were no blocks greater `max_block_number`. -// /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) -// /// Returns `Ok(None)` if `target_hash` is not found in search space. -// /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) -// fn best_containing( -// &self, -// target_hash: Block::Hash, -// maybe_max_number: Option> -// ) -> error::Result> { -// let target_header = { -// match self.client.backend().blockchain().header(BlockId::Hash(target_hash))? { -// Some(x) => x, -// // target not in blockchain -// None => { return Ok(None); }, -// } -// }; - -// if let Some(max_number) = maybe_max_number { -// // target outside search range -// if target_header.number() > &max_number { -// return Ok(None); -// } -// } - -// let (leaves, best_already_checked) = { -// // ensure no blocks are imported during this code block. -// // an import could trigger a reorg which could change the canonical chain. -// // we depend on the canonical chain staying the same during this code block. -// let _import_lock = self.client.import_lock().lock(); - -// let info = self.client.backend().blockchain().info()?; - -// let canon_hash = self.client.backend().blockchain().hash(*target_header.number())? -// .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; - -// if canon_hash == target_hash { -// // if no block at the given max depth exists fallback to the best block -// if let Some(max_number) = maybe_max_number { -// if let Some(header) = self.client.backend().blockchain().hash(max_number)? { -// return Ok(Some(header)); -// } -// } - -// return Ok(Some(info.best_hash)); -// } else if info.finalized_number >= *target_header.number() { -// // header is on a dead fork. -// return Ok(None); -// } - -// (self.client.backend().blockchain().leaves()?, info.best_hash) -// }; - -// // for each chain. longest chain first. shortest last -// for leaf_hash in leaves { -// // ignore canonical chain which we already checked above -// if leaf_hash == best_already_checked { -// continue; -// } - -// // start at the leaf -// let mut current_hash = leaf_hash; - -// // if search is not restricted then the leaf is the best -// let mut best_hash = leaf_hash; - -// // go backwards entering the search space -// // waiting until we are <= max_number -// if let Some(max_number) = maybe_max_number { -// loop { -// let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? -// .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - -// if current_header.number() <= &max_number { -// best_hash = current_header.hash(); -// break; -// } - -// current_hash = *current_header.parent_hash(); -// } -// } - -// // go backwards through the chain (via parent links) -// loop { -// // until we find target -// if current_hash == target_hash { -// return Ok(Some(best_hash)); -// } - -// let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? -// .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; - -// // stop search in this chain once we go below the target's block number -// if current_header.number() < target_header.number() { -// break; -// } - -// current_hash = *current_header.parent_hash(); -// } -// } - -// // header may be on a dead fork -- the only leaves that are considered are -// // those which can still be finalized. -// // -// // FIXME #1558 only issue this warning when not on a dead fork -// warn!( -// "Block {:?} exists in chain but not found when following all \ -// leaves backwards. Number limit = {:?}", -// target_hash, -// maybe_max_number, -// ); - -// Ok(None) -// } - -// fn leaves(&self) -> Result::Hash>, error::Error> { -// self.client.backend().blockchain().leaves() -// } -// } +struct LongestChain +where + B: backend::Backend + Send + Sync, + Block: BlockT + Send + Sync, + E: CallExecutor + Send + Sync, + RA: Send + Sync, +{ + client: Arc> +} + +impl Clone for LongestChain +where + B: backend::Backend + Send + Sync, + Block: BlockT + Send + Sync, + E: CallExecutor + Send + Sync, + RA: Send + Sync, +{ + fn clone(&self) -> Self { + let client = self.client.clone(); + LongestChain { + client + } + } +} + + +impl LongestChain +where + B: backend::Backend, + Block: BlockT, + E: CallExecutor + Send + Sync, + RA: Send + Sync, +{ + fn best_block_header(&self) -> error::Result<::Header> { + let info : ChainInfo = match self.client.backend().blockchain().info() { + Ok(i) => i, + Err(e) => return Err(error::Error::from_blockchain(Box::new(e))) + }; + Ok(self.client.backend().blockchain().header(BlockId::Hash(info.best_hash))? + .expect("Best block header must always exist")) + } + + /// Get the most recent block hash of the best (longest) chains + /// that contain block with the given `target_hash`. + /// + /// The search space is always limited to blocks which are in the finalized + /// chain or descendents of it. + /// + /// If `maybe_max_block_number` is `Some(max_block_number)` + /// the search is limited to block `numbers <= max_block_number`. + /// in other words as if there were no blocks greater `max_block_number`. + /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) + /// Returns `Ok(None)` if `target_hash` is not found in search space. + /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) + fn best_containing( + &self, + target_hash: Block::Hash, + maybe_max_number: Option> + ) -> error::Result> { + let target_header = { + match self.client.backend().blockchain().header(BlockId::Hash(target_hash))? { + Some(x) => x, + // target not in blockchain + None => { return Ok(None); }, + } + }; + + if let Some(max_number) = maybe_max_number { + // target outside search range + if target_header.number() > &max_number { + return Ok(None); + } + } + + let (leaves, best_already_checked) = { + // ensure no blocks are imported during this code block. + // an import could trigger a reorg which could change the canonical chain. + // we depend on the canonical chain staying the same during this code block. + let _import_lock = self.client.import_lock().lock(); + + let info = self.client.backend().blockchain().info()?; + + let canon_hash = self.client.backend().blockchain().hash(*target_header.number())? + .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; + + if canon_hash == target_hash { + // if no block at the given max depth exists fallback to the best block + if let Some(max_number) = maybe_max_number { + if let Some(header) = self.client.backend().blockchain().hash(max_number)? { + return Ok(Some(header)); + } + } + + return Ok(Some(info.best_hash)); + } else if info.finalized_number >= *target_header.number() { + // header is on a dead fork. + return Ok(None); + } + + (self.client.backend().blockchain().leaves()?, info.best_hash) + }; + + // for each chain. longest chain first. shortest last + for leaf_hash in leaves { + // ignore canonical chain which we already checked above + if leaf_hash == best_already_checked { + continue; + } + + // start at the leaf + let mut current_hash = leaf_hash; + + // if search is not restricted then the leaf is the best + let mut best_hash = leaf_hash; + + // go backwards entering the search space + // waiting until we are <= max_number + if let Some(max_number) = maybe_max_number { + loop { + let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? + .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; + + if current_header.number() <= &max_number { + best_hash = current_header.hash(); + break; + } + + current_hash = *current_header.parent_hash(); + } + } + + // go backwards through the chain (via parent links) + loop { + // until we find target + if current_hash == target_hash { + return Ok(Some(best_hash)); + } + + let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? + .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; + + // stop search in this chain once we go below the target's block number + if current_header.number() < target_header.number() { + break; + } + + current_hash = *current_header.parent_hash(); + } + } + + // header may be on a dead fork -- the only leaves that are considered are + // those which can still be finalized. + // + // FIXME #1558 only issue this warning when not on a dead fork + warn!( + "Block {:?} exists in chain but not found when following all \ + leaves backwards. Number limit = {:?}", + target_hash, + maybe_max_number, + ); + + Ok(None) + } + + fn leaves(&self) -> Result::Hash>, error::Error> { + self.client.backend().blockchain().leaves() + } +} + +impl SelectChain for LongestChain +where + B: backend::Backend + Send + Sync, + E: CallExecutor + Send + Sync, + Block: BlockT + Send + Sync, + E: Send + Sync, + RA: Send + Sync, +{ + fn best_block_header(&self) -> Result<::Header, ConsensusError> { + LongestChain::best_block_header(&self) + .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) + } + + fn best_containing( + &self, + target_hash: Block::Hash, + maybe_max_number: Option> + ) -> Result, ConsensusError> { + LongestChain::best_containing(self, target_hash, maybe_max_number) + .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) + } + + fn leaves(&self) -> Result::Hash>, ConsensusError> { + LongestChain::leaves(self) + .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) + } +} impl BlockBody for Client where diff --git a/core/consensus/common/src/error.rs b/core/consensus/common/src/error.rs index 0f1914087bf8c..01b6773d985fb 100644 --- a/core/consensus/common/src/error.rs +++ b/core/consensus/common/src/error.rs @@ -105,5 +105,11 @@ error_chain! { description("Import failed"), display("Import failed: {}", reason), } + + /// Error from the client while importing + ChainLookup(reason: String) { + description("Looking up chain failed"), + display("Chain lookup failed: {}", reason), + } } } From a46acc64ab3715f8c4e65d079f3d62ff71d8269d Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Tue, 9 Apr 2019 19:35:49 -0300 Subject: [PATCH 05/24] implement longest chain for node --- Cargo.lock | 1 + core/client/src/client.rs | 82 ++++++++++++++++---------------- core/client/src/lib.rs | 1 + core/finality-grandpa/src/lib.rs | 4 +- core/service/src/chain_ops.rs | 3 +- core/service/src/components.rs | 28 ++++++----- core/service/src/lib.rs | 30 ++++++++++-- node/cli/src/service.rs | 22 +++++++-- node/runtime/wasm/Cargo.lock | 1 + 9 files changed, 106 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 29886a032feab..1bda137fbbcc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3802,6 +3802,7 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", diff --git a/core/client/src/client.rs b/core/client/src/client.rs index f9ef7995016d3..b1364fca8bd82 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -110,7 +110,7 @@ pub struct Client where Block: BlockT { storage_notifications: Mutex>, import_notification_sinks: Mutex>>>, finality_notification_sinks: Mutex>>>, - import_lock: Mutex<()>, + import_lock: Arc>, // holds the block hash currently being imported. TODO: replace this with block queue importing_block: RwLock>, execution_strategies: ExecutionStrategies, @@ -242,7 +242,7 @@ pub fn new_in_mem( new_with_backend(Arc::new(in_mem::Backend::new()), executor, genesis_storage) } -/// Create a client with the explicitely provided backend. +/// Create a client with the explicitly provided backend. /// This is useful for testing backend implementations. pub fn new_with_backend( backend: Arc, @@ -317,8 +317,10 @@ impl Client where &self.backend } - fn import_lock(&self) -> &Mutex<()> { - &self.import_lock + /// Expose reference to import lock + // FIXME: the lock shouldn't be on client but on the import-queue + pub fn import_lock(&self) -> Arc> { + self.import_lock.clone() } /// Return storage entry keys in state in a block of given hash with given prefix. @@ -1348,45 +1350,44 @@ where } } -struct LongestChain -where - B: backend::Backend + Send + Sync, - Block: BlockT + Send + Sync, - E: CallExecutor + Send + Sync, - RA: Send + Sync, -{ - client: Arc> +/// Implement Longest Chain Select implementation +/// where 'longest' is defined as the highest number of blocks +pub struct LongestChain { + backend: Arc, + import_lock: Arc>, + _phantom: PhantomData } -impl Clone for LongestChain -where - B: backend::Backend + Send + Sync, - Block: BlockT + Send + Sync, - E: CallExecutor + Send + Sync, - RA: Send + Sync, -{ +impl Clone for LongestChain { fn clone(&self) -> Self { - let client = self.client.clone(); + let backend = self.backend.clone(); + let import_lock = self.import_lock.clone(); LongestChain { - client + backend, + import_lock, + _phantom: Default::default() } } } - -impl LongestChain +impl LongestChain where B: backend::Backend, Block: BlockT, - E: CallExecutor + Send + Sync, - RA: Send + Sync, { + pub fn new(backend: Arc, import_lock: Arc>) -> Self { + LongestChain { + backend, + import_lock, + _phantom: Default::default() + } + } fn best_block_header(&self) -> error::Result<::Header> { - let info : ChainInfo = match self.client.backend().blockchain().info() { + let info : ChainInfo = match self.backend.blockchain().info() { Ok(i) => i, Err(e) => return Err(error::Error::from_blockchain(Box::new(e))) }; - Ok(self.client.backend().blockchain().header(BlockId::Hash(info.best_hash))? + Ok(self.backend.blockchain().header(BlockId::Hash(info.best_hash))? .expect("Best block header must always exist")) } @@ -1408,7 +1409,7 @@ where maybe_max_number: Option> ) -> error::Result> { let target_header = { - match self.client.backend().blockchain().header(BlockId::Hash(target_hash))? { + match self.backend.blockchain().header(BlockId::Hash(target_hash))? { Some(x) => x, // target not in blockchain None => { return Ok(None); }, @@ -1426,17 +1427,17 @@ where // ensure no blocks are imported during this code block. // an import could trigger a reorg which could change the canonical chain. // we depend on the canonical chain staying the same during this code block. - let _import_lock = self.client.import_lock().lock(); + let _import_lock = self.import_lock.lock(); - let info = self.client.backend().blockchain().info()?; + let info = self.backend.blockchain().info()?; - let canon_hash = self.client.backend().blockchain().hash(*target_header.number())? + let canon_hash = self.backend.blockchain().hash(*target_header.number())? .ok_or_else(|| error::Error::from(format!("failed to get hash for block number {}", target_header.number())))?; if canon_hash == target_hash { // if no block at the given max depth exists fallback to the best block if let Some(max_number) = maybe_max_number { - if let Some(header) = self.client.backend().blockchain().hash(max_number)? { + if let Some(header) = self.backend.blockchain().hash(max_number)? { return Ok(Some(header)); } } @@ -1447,7 +1448,7 @@ where return Ok(None); } - (self.client.backend().blockchain().leaves()?, info.best_hash) + (self.backend.blockchain().leaves()?, info.best_hash) }; // for each chain. longest chain first. shortest last @@ -1467,7 +1468,7 @@ where // waiting until we are <= max_number if let Some(max_number) = maybe_max_number { loop { - let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? + let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; if current_header.number() <= &max_number { @@ -1486,7 +1487,7 @@ where return Ok(Some(best_hash)); } - let current_header = self.client.backend().blockchain().header(BlockId::Hash(current_hash.clone()))? + let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))? .ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?; // stop search in this chain once we go below the target's block number @@ -1513,17 +1514,14 @@ where } fn leaves(&self) -> Result::Hash>, error::Error> { - self.client.backend().blockchain().leaves() + self.backend.blockchain().leaves() } } -impl SelectChain for LongestChain +impl SelectChain for LongestChain where - B: backend::Backend + Send + Sync, - E: CallExecutor + Send + Sync, - Block: BlockT + Send + Sync, - E: Send + Sync, - RA: Send + Sync, + B: backend::Backend, + Block: BlockT, { fn best_block_header(&self) -> Result<::Header, ConsensusError> { LongestChain::best_block_header(&self) diff --git a/core/client/src/lib.rs b/core/client/src/lib.rs index 5937517878247..fe33c56262b3c 100644 --- a/core/client/src/lib.rs +++ b/core/client/src/lib.rs @@ -59,6 +59,7 @@ pub use crate::client::{ new_in_mem, BlockBody, BlockStatus, ImportNotifications, FinalityNotifications, BlockchainEvents, BlockImportNotification, Client, ClientInfo, ExecutionStrategies, + LongestChain }; #[cfg(feature = "std")] pub use crate::notifications::{StorageEventStream, StorageChangeSet}; diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 9dda733c7e172..ea39cff5c0fa4 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -290,8 +290,8 @@ pub struct LinkHalf, RA> { /// to it. pub fn block_import, RA, PRA, SC>( client: Arc>, - select_chain: SC, - api: Arc + api: Arc, + select_chain: SC ) -> Result<( GrandpaBlockImport, LinkHalf diff --git a/core/service/src/chain_ops.rs b/core/service/src/chain_ops.rs index 36cbee9039395..9f39e8088e7f8 100644 --- a/core/service/src/chain_ops.rs +++ b/core/service/src/chain_ops.rs @@ -127,7 +127,8 @@ pub fn import_blocks( { let client = new_client::(&config)?; // FIXME #1134 this shouldn't need a mutable config. - let queue = components::FullComponents::::build_import_queue(&mut config, client.clone())?; + let select_chain = components::FullComponents::::build_select_chain(&mut config, client.clone())?; + let queue = components::FullComponents::::build_import_queue(&mut config, client.clone(), select_chain)?; let (wait_send, wait_recv) = std::sync::mpsc::channel(); let wait_link = WaitLink::new(wait_send); diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 23f09d97145fb..4e1528b2d12f3 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -323,7 +323,7 @@ pub trait ServiceFactory: 'static + Sized { /// Build the Fork Choice algorithm for full client fn build_select_chain( config: &mut FactoryFullConfiguration, - client: Arc> + client: Arc>, ) -> Result; /// Build full service. @@ -336,7 +336,8 @@ pub trait ServiceFactory: 'static + Sized { /// ImportQueue for a full client fn build_full_import_queue( config: &mut FactoryFullConfiguration, - _client: Arc> + _client: Arc>, + _select_chain: Self::SelectChain ) -> Result { if let Some(name) = config.chain_spec.consensus_engine() { match name { @@ -384,6 +385,7 @@ pub trait Components: Sized + 'static { >; /// Our Import Queue type ImportQueue: ImportQueue> + 'static; + type SelectChain: SelectChain>; /// Create client. fn build_client( @@ -404,20 +406,21 @@ pub trait Components: Sized + 'static { /// instance of import queue for clients fn build_import_queue( config: &mut FactoryFullConfiguration, - client: Arc> + client: Arc>, + select_chain: Self::SelectChain, ) -> Result; /// Build fork choice selector fn build_select_chain( config: &mut FactoryFullConfiguration, client: Arc> - ) -> Result<::SelectChain, error::Error>; + ) -> Result; } /// A struct that implement `Components` for the full client. pub struct FullComponents { - _factory: PhantomData, + // _factory: PhantomData, service: Service>, } @@ -429,7 +432,6 @@ impl FullComponents { ) -> Result { Ok( Self { - _factory: Default::default(), service: Service::new(config, task_executor)?, } ) @@ -458,6 +460,7 @@ impl Components for FullComponents { type ImportQueue = Factory::FullImportQueue; type RuntimeApi = Factory::RuntimeApi; type RuntimeServices = Factory::FullService; + type SelectChain = Factory::SelectChain; fn build_client( config: &FactoryFullConfiguration, @@ -490,15 +493,16 @@ impl Components for FullComponents { fn build_import_queue( config: &mut FactoryFullConfiguration, - client: Arc> + client: Arc>, + select_chain: Self::SelectChain, ) -> Result { - Factory::build_full_import_queue(config, client) + Factory::build_full_import_queue(config, client, select_chain) } fn build_select_chain( config: &mut FactoryFullConfiguration, client: Arc> - ) -> Result<::SelectChain, error::Error> { + ) -> Result { Self::Factory::build_select_chain(config, client) } @@ -541,6 +545,7 @@ impl Components for LightComponents { type ImportQueue = ::LightImportQueue; type RuntimeApi = Factory::RuntimeApi; type RuntimeServices = Factory::LightService; + type SelectChain = Factory::SelectChain; fn build_client( config: &FactoryFullConfiguration, @@ -574,7 +579,8 @@ impl Components for LightComponents { fn build_import_queue( config: &mut FactoryFullConfiguration, - client: Arc> + client: Arc>, + _select_chain: Self::SelectChain, ) -> Result { Factory::build_light_import_queue(config, client) } @@ -583,7 +589,7 @@ impl Components for LightComponents { fn build_select_chain( _config: &mut FactoryFullConfiguration, _client: Arc> - ) -> Result<::SelectChain, error::Error> { + ) -> Result { // FIXME: this (and other places) need a "NotAvailableInMode" for never-in-light-client-features // that doesn't break creation of light clients Err("Fork choice doesn't happen on light clients.".into()) diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 0de908f689bd7..65c0c94c1ab4e 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -72,7 +72,7 @@ const DEFAULT_PROTOCOL_ID: &str = "sup"; /// Substrate service. pub struct Service { client: Arc>, - select_chain: <::Factory as ServiceFactory>::SelectChain, + select_chain: ::SelectChain, network: Option>>, transaction_pool: Arc>, inherents_pool: Arc>>, @@ -129,8 +129,12 @@ impl Service { }; let (client, on_demand) = Components::build_client(&config, executor)?; - let import_queue = Box::new(Components::build_import_queue(&mut config, client.clone())?); let select_chain = Components::build_select_chain(&mut config, client.clone())?; + let import_queue = Box::new(Components::build_import_queue( + &mut config, + client.clone(), + select_chain.clone() + )?); let best_header = select_chain.best_block_header()?; let version = config.full_version(); @@ -327,8 +331,8 @@ impl Service { Ok(Service { client, - select_chain, network: Some(network), + select_chain, transaction_pool, inherents_pool, signal: Some(signal), @@ -354,7 +358,7 @@ impl Service { } } - /// return a shared instance of Telemtry (if enabled) + /// return a shared instance of Telemetry (if enabled) pub fn telemetry(&self) -> Option> { self._telemetry.as_ref().map(|t| t.clone()) } @@ -366,6 +370,11 @@ impl Service where Components: components::Components { self.client.clone() } + /// Get clone of select chain. + pub fn select_chain(&self) -> ::SelectChain { + self.select_chain.clone() + } + /// Get shared network instance. pub fn network(&self) -> Arc> { self.network.as_ref().expect("self.network always Some").clone() @@ -549,6 +558,8 @@ macro_rules! construct_service_factory { { $( $full_import_queue_init:tt )* }, LightImportQueue = $light_import_queue:ty { $( $light_import_queue_init:tt )* }, + SelectChain = $select_chain:ty + { $( $select_chain_init:tt )* }, } ) => { $( #[$attr] )* @@ -568,6 +579,7 @@ macro_rules! construct_service_factory { type LightService = $light_service; type FullImportQueue = $full_import_queue; type LightImportQueue = $light_import_queue; + type SelectChain = $select_chain; fn build_full_transaction_pool( config: $crate::TransactionPoolOptions, @@ -591,11 +603,19 @@ macro_rules! construct_service_factory { ( $( $protocol_init )* ) (config) } + fn build_select_chain( + config: &mut $crate::FactoryFullConfiguration, + client: Arc<$crate::FullClient> + ) -> $crate::Result { + ( $( $select_chain_init )* ) (config, client) + } + fn build_full_import_queue( config: &mut $crate::FactoryFullConfiguration, client: $crate::Arc<$crate::FullClient>, + select_chain: Self::SelectChain ) -> $crate::Result { - ( $( $full_import_queue_init )* ) (config, client) + ( $( $full_import_queue_init )* ) (config, client, select_chain) } fn build_light_import_queue( diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index b9c7ddffeef64..b22cb6e8c1e99 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -21,8 +21,10 @@ use std::sync::Arc; use std::time::Duration; -use client; -use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration, NothingExtra}; +use client::{self, LongestChain}; +use consensus::{import_queue, start_aura, AuraImportQueue, + SlotDuration, NothingExtra +}; use grandpa; use node_executor; use primitives::{Pair as PairT, ed25519}; @@ -93,6 +95,7 @@ construct_service_factory! { SlotDuration::get_or_compute(&*client)?, key.clone(), client, + service.select_chain(), block_import.clone(), proposer, service.network(), @@ -121,6 +124,7 @@ construct_service_factory! { link_half, service.network(), service.config.custom.inherent_data_providers.clone(), + service.select_chain(), service.on_exit(), )?); @@ -130,11 +134,11 @@ construct_service_factory! { LightService = LightComponents { |config, executor| >::new(config, executor) }, FullImportQueue = AuraImportQueue - { |config: &mut FactoryFullConfiguration , client: Arc>| { + { |config: &mut FactoryFullConfiguration , client: Arc>, select_chain: Self::SelectChain| { let slot_duration = SlotDuration::get_or_compute(&*client)?; let (block_import, link_half) = - grandpa::block_import::<_, _, _, RuntimeApi, FullClient>( - client.clone(), client.clone() + grandpa::block_import::<_, _, _, RuntimeApi, FullClient, _>( + client.clone(), client.clone(), select_chain )?; let block_import = Arc::new(block_import); let justification_import = block_import.clone(); @@ -162,6 +166,14 @@ construct_service_factory! { ).map_err(Into::into) } }, + SelectChain = LongestChain, Self::Block> + { |config: &FactoryFullConfiguration, client: Arc>| { + Ok(LongestChain::new( + client.backend().clone(), + client.import_lock() + )) + } + }, } } diff --git a/node/runtime/wasm/Cargo.lock b/node/runtime/wasm/Cargo.lock index 83b2de5bd9048..792c68b39ea45 100644 --- a/node/runtime/wasm/Cargo.lock +++ b/node/runtime/wasm/Cargo.lock @@ -2156,6 +2156,7 @@ version = "1.0.0" dependencies = [ "hex-literal 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-std 1.0.0", "srml-session 1.0.0", From 30f5528a49966ff47ae70569c425bf2f3f506da5 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Tue, 9 Apr 2019 20:06:39 -0300 Subject: [PATCH 06/24] update Cargo.lock's --- core/test-runtime/wasm/Cargo.lock | 1 + node-template/runtime/wasm/Cargo.lock | 1 + node/runtime/wasm/Cargo.lock | 1 + 3 files changed, 3 insertions(+) diff --git a/core/test-runtime/wasm/Cargo.lock b/core/test-runtime/wasm/Cargo.lock index 7df0a2652463d..2b270c099f00d 100644 --- a/core/test-runtime/wasm/Cargo.lock +++ b/core/test-runtime/wasm/Cargo.lock @@ -2315,6 +2315,7 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", diff --git a/node-template/runtime/wasm/Cargo.lock b/node-template/runtime/wasm/Cargo.lock index 9d6c917b15fb8..a307d2992999a 100644 --- a/node-template/runtime/wasm/Cargo.lock +++ b/node-template/runtime/wasm/Cargo.lock @@ -2477,6 +2477,7 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", diff --git a/node/runtime/wasm/Cargo.lock b/node/runtime/wasm/Cargo.lock index 792c68b39ea45..5aa4f5c9cb709 100644 --- a/node/runtime/wasm/Cargo.lock +++ b/node/runtime/wasm/Cargo.lock @@ -2620,6 +2620,7 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", + "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", From 4cf2abf05d8faa28c948cc09bc86b1c95bab4596 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 15:58:52 -0300 Subject: [PATCH 07/24] in between erroring tests --- core/client/db/src/lib.rs | 2 +- core/client/src/client.rs | 202 +++++++++++++++++++++----------------- 2 files changed, 114 insertions(+), 90 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index ac22bcbb0169c..93f9325b26884 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -35,7 +35,7 @@ use std::path::PathBuf; use std::io; use std::collections::HashMap; -use client::backend::NewBlockState; +use client::backend::{NewBlockState, Backend as BackendT}; use client::blockchain::HeaderBackend; use client::ExecutionStrategies; use parity_codec::{Decode, Encode}; diff --git a/core/client/src/client.rs b/core/client/src/client.rs index b1364fca8bd82..15502c4c83a46 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1589,11 +1589,15 @@ pub(crate) mod tests { use runtime_primitives::traits::DigestItem as DigestItemT; use runtime_primitives::generic::DigestItem; use test_client::{self, TestClient, AccountKeyring}; - use consensus::BlockOrigin; + use consensus::{BlockOrigin, SelectChain}; + use crate::backend::Backend; use test_client::client::backend::Backend as TestBackend; use test_client::BlockBuilderExt; use test_client::runtime::{self, Block, Transfer, RuntimeApi, TestAPI}; + type InMemTestBackend = test_client::client::in_mem::Backend; + + /// Returns tuple, consisting of: /// 1) test client pre-filled with blocks changing balances; /// 2) roots of changes tries for these blocks @@ -1764,10 +1768,15 @@ pub(crate) mod tests { // G let client = test_client::new(); - + let genesis_hash = client.info().unwrap().chain.genesis_hash; + let longest_chain_select = LongestChain::new( + client.backend().clone(), + client.import_lock() + ); - assert_eq!(genesis_hash.clone(), client.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); + + // assert_eq!(genesis_hash.clone(), longest_chain_select.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); } #[test] @@ -1778,8 +1787,12 @@ pub(crate) mod tests { let client = test_client::new(); let uninserted_block = client.new_block().unwrap().bake().unwrap(); + let backend = client.backend().as_in_memory(); + let longest_chain_select = LongestChain::new( + Arc::new(backend), + client.import_lock()); - assert_eq!(None, client.best_containing(uninserted_block.hash().clone(), None).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(uninserted_block.hash().clone(), None).unwrap()); } #[test] @@ -1894,7 +1907,7 @@ pub(crate) mod tests { } #[test] - fn best_containing_with_single_chain_3_blocks() { + fn best_containing_on_longest_chain_with_single_chain_3_blocks() { // block tree: // G -> A1 -> A2 @@ -1910,13 +1923,17 @@ pub(crate) mod tests { let genesis_hash = client.info().unwrap().chain.genesis_hash; - assert_eq!(a2.hash(), client.best_containing(genesis_hash, None).unwrap().unwrap()); - assert_eq!(a2.hash(), client.best_containing(a1.hash(), None).unwrap().unwrap()); - assert_eq!(a2.hash(), client.best_containing(a2.hash(), None).unwrap().unwrap()); + let longest_chain_select = LongestChain::new( + Arc::new(client.backend().as_in_memory()), + client.import_lock()); + + // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); + // assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); + // assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); } #[test] - fn best_containing_with_multiple_forks() { + fn best_containing_on_longest_chain_with_multiple_forks() { // NOTE: we use the version of the trait from `test_client` // because that is actually different than the version linked to // in the test facade crate. @@ -1996,7 +2013,11 @@ pub(crate) mod tests { assert_eq!(client.info().unwrap().chain.best_hash, a5.hash()); let genesis_hash = client.info().unwrap().chain.genesis_hash; - let leaves = BlockchainBackendT::leaves(client.backend().blockchain()).unwrap(); + let longest_chain_select = LongestChain::new( + Arc::new(client.backend().as_in_memory()), + client.import_lock()); + + let leaves = longest_chain_select.leaves().unwrap(); assert!(leaves.contains(&a5.hash())); assert!(leaves.contains(&b4.hash())); @@ -2006,131 +2027,131 @@ pub(crate) mod tests { // search without restriction - assert_eq!(a5.hash(), client.best_containing(genesis_hash, None).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a1.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a2.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a3.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a4.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a5.hash(), None).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), None).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), None).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b2.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b3.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b4.hash(), None).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), None).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), None).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), None).unwrap().unwrap()); - assert_eq!(c3.hash(), client.best_containing(c3.hash(), None).unwrap().unwrap()); + // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), None).unwrap().unwrap()); - assert_eq!(d2.hash(), client.best_containing(d2.hash(), None).unwrap().unwrap()); + // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), None).unwrap().unwrap()); // search only blocks with number <= 5. equivalent to without restriction for this scenario - assert_eq!(a5.hash(), client.best_containing(genesis_hash, Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a1.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a2.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a4.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), client.best_containing(a5.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, Some(5)).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b2.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b4.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(c3.hash(), client.best_containing(c3.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(d2.hash(), client.best_containing(d2.hash(), Some(5)).unwrap().unwrap()); + // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(5)).unwrap().unwrap()); // search only blocks with number <= 4 - assert_eq!(a4.hash(), client.best_containing(genesis_hash, Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), client.best_containing(a1.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), client.best_containing(a2.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), client.best_containing(a3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), client.best_containing(a4.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(a5.hash(), Some(4)).unwrap()); + // assert_eq!(a4.hash(), longest_chain_select.best_containing(genesis_hash, Some(4)).unwrap().unwrap()); + // assert_eq!(a4.hash(), longest_chain_select.best_containing(a1.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(a4.hash(), longest_chain_select.best_containing(a2.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(a4.hash(), longest_chain_select.best_containing(a3.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(a4.hash(), longest_chain_select.best_containing(a4.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(4)).unwrap()); - assert_eq!(b4.hash(), client.best_containing(b2.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(b4.hash(), client.best_containing(b4.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(c3.hash(), client.best_containing(c3.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(d2.hash(), client.best_containing(d2.hash(), Some(4)).unwrap().unwrap()); + // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(4)).unwrap().unwrap()); // search only blocks with number <= 3 - assert_eq!(a3.hash(), client.best_containing(genesis_hash, Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), client.best_containing(a1.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), client.best_containing(a2.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), client.best_containing(a3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(a4.hash(), Some(3)).unwrap()); - assert_eq!(None, client.best_containing(a5.hash(), Some(3)).unwrap()); + // assert_eq!(a3.hash(), longest_chain_select.best_containing(genesis_hash, Some(3)).unwrap().unwrap()); + // assert_eq!(a3.hash(), longest_chain_select.best_containing(a1.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(a3.hash(), longest_chain_select.best_containing(a2.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(a3.hash(), longest_chain_select.best_containing(a3.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(3)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(3)).unwrap()); - assert_eq!(b3.hash(), client.best_containing(b2.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(b3.hash(), client.best_containing(b3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(b4.hash(), Some(3)).unwrap()); + // assert_eq!(b3.hash(), longest_chain_select.best_containing(b2.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(b3.hash(), longest_chain_select.best_containing(b3.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(3)).unwrap()); - assert_eq!(c3.hash(), client.best_containing(c3.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(d2.hash(), client.best_containing(d2.hash(), Some(3)).unwrap().unwrap()); + // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(3)).unwrap().unwrap()); // search only blocks with number <= 2 - assert_eq!(a2.hash(), client.best_containing(genesis_hash, Some(2)).unwrap().unwrap()); - assert_eq!(a2.hash(), client.best_containing(a1.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(a2.hash(), client.best_containing(a2.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(a3.hash(), Some(2)).unwrap()); - assert_eq!(None, client.best_containing(a4.hash(), Some(2)).unwrap()); - assert_eq!(None, client.best_containing(a5.hash(), Some(2)).unwrap()); + // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(2)).unwrap().unwrap()); + // assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), Some(2)).unwrap().unwrap()); + // assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), Some(2)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(2)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(2)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(2)).unwrap()); - assert_eq!(b2.hash(), client.best_containing(b2.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(b3.hash(), Some(2)).unwrap()); - assert_eq!(None, client.best_containing(b4.hash(), Some(2)).unwrap()); + // assert_eq!(b2.hash(), longest_chain_select.best_containing(b2.hash(), Some(2)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(2)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(2)).unwrap()); - assert_eq!(None, client.best_containing(c3.hash(), Some(2)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(2)).unwrap()); - assert_eq!(d2.hash(), client.best_containing(d2.hash(), Some(2)).unwrap().unwrap()); + // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(2)).unwrap().unwrap()); // search only blocks with number <= 1 - assert_eq!(a1.hash(), client.best_containing(genesis_hash, Some(1)).unwrap().unwrap()); - assert_eq!(a1.hash(), client.best_containing(a1.hash(), Some(1)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(a2.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(a3.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(a4.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(a5.hash(), Some(1)).unwrap()); + // assert_eq!(a1.hash(), longest_chain_select.best_containing(genesis_hash, Some(1)).unwrap().unwrap()); + // assert_eq!(a1.hash(), longest_chain_select.best_containing(a1.hash(), Some(1)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(b2.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(b3.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(b4.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(c3.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(1)).unwrap()); - assert_eq!(None, client.best_containing(d2.hash(), Some(1)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(d2.hash(), Some(1)).unwrap()); // search only blocks with number <= 0 - assert_eq!(genesis_hash, client.best_containing(genesis_hash, Some(0)).unwrap().unwrap()); - assert_eq!(None, client.best_containing(a1.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(a2.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(a3.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(a4.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(a5.hash(), Some(0)).unwrap()); + // assert_eq!(genesis_hash, longest_chain_select.best_containing(genesis_hash, Some(0)).unwrap().unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a1.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(b2.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(b3.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(b4.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(c3.hash().clone(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(c3.hash().clone(), Some(0)).unwrap()); - assert_eq!(None, client.best_containing(d2.hash().clone(), Some(0)).unwrap()); + // assert_eq!(None, longest_chain_select.best_containing(d2.hash().clone(), Some(0)).unwrap()); } #[test] - fn best_containing_with_max_depth_higher_than_best() { + fn best_containing_on_longest_chain_with_max_depth_higher_than_best() { // block tree: // G -> A1 -> A2 @@ -2145,8 +2166,11 @@ pub(crate) mod tests { client.import(BlockOrigin::Own, a2.clone()).unwrap(); let genesis_hash = client.info().unwrap().chain.genesis_hash; + let longest_chain_select = LongestChain::new( + Arc::new(client.backend().as_in_memory()), + client.import_lock()); - assert_eq!(a2.hash(), client.best_containing(genesis_hash, Some(10)).unwrap().unwrap()); + // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(10)).unwrap().unwrap()); } #[test] From 5ff112acabfbc2309157351851dc786e43a2ef68 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 16:10:55 -0300 Subject: [PATCH 08/24] deprecate ::backend and ::import_lock --- core/client/src/client.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 15502c4c83a46..381aadfad7315 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -311,14 +311,19 @@ impl Client where } /// Expose backend reference. To be used in tests only - // FIXME: this is being used in non-test cases as well, we probably - // want to decide if that is correct or other ways are more appropriate. + #[doc(hidden)] + #[deprecated(since="1.0.1", note="Rather than relying on `client` \ + to provide this, access to the backend should be handled at setup \ + only - see #1134. This function will be removed once that is in place.")] pub fn backend(&self) -> &Arc { &self.backend } /// Expose reference to import lock - // FIXME: the lock shouldn't be on client but on the import-queue + #[doc(hidden)] + #[deprecated(since="1.0.1", note="Rather than relying on `client` \ + to provide this, access to the backend should be handled at setup \ + only - see #1134. This function will be removed once that is in place.")] pub fn import_lock(&self) -> Arc> { self.import_lock.clone() } @@ -1382,6 +1387,7 @@ where _phantom: Default::default() } } + fn best_block_header(&self) -> error::Result<::Header> { let info : ChainInfo = match self.backend.blockchain().info() { Ok(i) => i, From a2b0dd1d52e4da55d80a3e61b1e59758ca0932e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 12 Apr 2019 16:12:56 -0300 Subject: [PATCH 09/24] Remove unneded space Co-Authored-By: gnunicorn --- core/consensus/aura/slots/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/consensus/aura/slots/src/lib.rs b/core/consensus/aura/slots/src/lib.rs index b364c6aab8f7b..96f6dc5ef4677 100644 --- a/core/consensus/aura/slots/src/lib.rs +++ b/core/consensus/aura/slots/src/lib.rs @@ -119,7 +119,7 @@ pub fn start_slot_worker_thread( /// Start a new slot worker. pub fn start_slot_worker( slot_duration: SlotDuration, - client: C, + client: C, worker: Arc, sync_oracle: SO, on_exit: OnExit, From 2db616305546fdba6db54253c6bca2ac18586870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 12 Apr 2019 16:16:03 -0300 Subject: [PATCH 10/24] Remove unneded space Co-Authored-By: gnunicorn --- core/consensus/aura/slots/src/lib.rs | 2 +- core/consensus/common/src/select_chain.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/core/consensus/aura/slots/src/lib.rs b/core/consensus/aura/slots/src/lib.rs index 96f6dc5ef4677..9adc8362070d5 100644 --- a/core/consensus/aura/slots/src/lib.rs +++ b/core/consensus/aura/slots/src/lib.rs @@ -126,7 +126,7 @@ pub fn start_slot_worker( inherent_data_providers: InherentDataProviders, ) -> Result, consensus_common::Error> where B: Block, - C: SelectChain + Clone, + C: SelectChain + Clone, W: SlotWorker, SO: SyncOracle + Send + Clone, SC: SlotCompatible, diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs index 6f9bc8f44f5b1..e0eef7047aa32 100644 --- a/core/consensus/common/src/select_chain.rs +++ b/core/consensus/common/src/select_chain.rs @@ -1,4 +1,3 @@ - // Copyright 2019 Parity Technologies (UK) Ltd. // This file is part of Substrate Consensus Common. From f1c4225cef50b2209f4c44684f5e6e233923a551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 12 Apr 2019 21:16:28 +0200 Subject: [PATCH 11/24] Fixes test compilation --- core/client/src/client.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 381aadfad7315..565897282592c 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -28,7 +28,7 @@ use runtime_primitives::{ use consensus::{ Error as ConsensusError, ErrorKind as ConsensusErrorKind, ImportBlock, ImportResult, BlockOrigin, ForkChoiceStrategy, well_known_cache_keys::Id as CacheKeyId, - SelectChain, self, + SelectChain, self, }; use runtime_primitives::traits::{ Block as BlockT, Header as HeaderT, Zero, As, NumberFor, CurrentHeight, BlockNumberToHash, @@ -1533,7 +1533,7 @@ where LongestChain::best_block_header(&self) .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) } - + fn best_containing( &self, target_hash: Block::Hash, @@ -1774,9 +1774,9 @@ pub(crate) mod tests { // G let client = test_client::new(); - + let genesis_hash = client.info().unwrap().chain.genesis_hash; - let longest_chain_select = LongestChain::new( + let longest_chain_select = test_client::client::LongestChain::new( client.backend().clone(), client.import_lock() ); @@ -1794,7 +1794,7 @@ pub(crate) mod tests { let uninserted_block = client.new_block().unwrap().bake().unwrap(); let backend = client.backend().as_in_memory(); - let longest_chain_select = LongestChain::new( + let longest_chain_select = test_client::client::LongestChain::new( Arc::new(backend), client.import_lock()); @@ -1929,7 +1929,7 @@ pub(crate) mod tests { let genesis_hash = client.info().unwrap().chain.genesis_hash; - let longest_chain_select = LongestChain::new( + let longest_chain_select = test_client::client::LongestChain::new( Arc::new(client.backend().as_in_memory()), client.import_lock()); @@ -2019,7 +2019,7 @@ pub(crate) mod tests { assert_eq!(client.info().unwrap().chain.best_hash, a5.hash()); let genesis_hash = client.info().unwrap().chain.genesis_hash; - let longest_chain_select = LongestChain::new( + let longest_chain_select = test_client::client::LongestChain::new( Arc::new(client.backend().as_in_memory()), client.import_lock()); @@ -2172,9 +2172,10 @@ pub(crate) mod tests { client.import(BlockOrigin::Own, a2.clone()).unwrap(); let genesis_hash = client.info().unwrap().chain.genesis_hash; - let longest_chain_select = LongestChain::new( - Arc::new(client.backend().as_in_memory()), - client.import_lock()); + let longest_chain_select = test_client::client::LongestChain::new( + Arc::new(client.backend().as_in_memory()), + client.import_lock() + ); // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(10)).unwrap().unwrap()); } From d2f21a4e15f97359dd45ff828a65bd575ac3bd3b Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 16:24:07 -0300 Subject: [PATCH 12/24] remove todo --- core/client/src/client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 565897282592c..2202a05ba94e9 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1406,7 +1406,6 @@ where /// If `maybe_max_block_number` is `Some(max_block_number)` /// the search is limited to block `numbers <= max_block_number`. /// in other words as if there were no blocks greater `max_block_number`. - /// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443) /// Returns `Ok(None)` if `target_hash` is not found in search space. /// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444) fn best_containing( From 097905a1d02ad89bd119a88daf62ae56474cd1a3 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 16:29:48 -0300 Subject: [PATCH 13/24] re-enable client test --- core/client/db/src/lib.rs | 2 +- core/client/src/client.rs | 175 ++++++++++++++++++-------------------- 2 files changed, 84 insertions(+), 93 deletions(-) diff --git a/core/client/db/src/lib.rs b/core/client/db/src/lib.rs index 93f9325b26884..ac22bcbb0169c 100644 --- a/core/client/db/src/lib.rs +++ b/core/client/db/src/lib.rs @@ -35,7 +35,7 @@ use std::path::PathBuf; use std::io; use std::collections::HashMap; -use client::backend::{NewBlockState, Backend as BackendT}; +use client::backend::NewBlockState; use client::blockchain::HeaderBackend; use client::ExecutionStrategies; use parity_codec::{Decode, Encode}; diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 2202a05ba94e9..dfe82b6beed07 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1595,14 +1595,10 @@ pub(crate) mod tests { use runtime_primitives::generic::DigestItem; use test_client::{self, TestClient, AccountKeyring}; use consensus::{BlockOrigin, SelectChain}; - use crate::backend::Backend; use test_client::client::backend::Backend as TestBackend; use test_client::BlockBuilderExt; use test_client::runtime::{self, Block, Transfer, RuntimeApi, TestAPI}; - type InMemTestBackend = test_client::client::in_mem::Backend; - - /// Returns tuple, consisting of: /// 1) test client pre-filled with blocks changing balances; /// 2) roots of changes tries for these blocks @@ -1781,7 +1777,7 @@ pub(crate) mod tests { ); - // assert_eq!(genesis_hash.clone(), longest_chain_select.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); + assert_eq!(genesis_hash.clone(), longest_chain_select.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); } #[test] @@ -1797,7 +1793,7 @@ pub(crate) mod tests { Arc::new(backend), client.import_lock()); - // assert_eq!(None, longest_chain_select.best_containing(uninserted_block.hash().clone(), None).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(uninserted_block.hash().clone(), None).unwrap()); } #[test] @@ -1932,18 +1928,13 @@ pub(crate) mod tests { Arc::new(client.backend().as_in_memory()), client.import_lock()); - // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); - // assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); - // assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); } #[test] fn best_containing_on_longest_chain_with_multiple_forks() { - // NOTE: we use the version of the trait from `test_client` - // because that is actually different than the version linked to - // in the test facade crate. - use test_client::blockchain::Backend as BlockchainBackendT; - // block tree: // G -> A1 -> A2 -> A3 -> A4 -> A5 // A1 -> B2 -> B3 -> B4 @@ -2032,127 +2023,127 @@ pub(crate) mod tests { // search without restriction - // assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), None).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), None).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), None).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), None).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), None).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), None).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), None).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), None).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), None).unwrap().unwrap()); - // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), None).unwrap().unwrap()); + assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), None).unwrap().unwrap()); - // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), None).unwrap().unwrap()); + assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), None).unwrap().unwrap()); // search only blocks with number <= 5. equivalent to without restriction for this scenario - // assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, Some(5)).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(5)).unwrap().unwrap()); - // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(5)).unwrap().unwrap()); // search only blocks with number <= 4 - // assert_eq!(a4.hash(), longest_chain_select.best_containing(genesis_hash, Some(4)).unwrap().unwrap()); - // assert_eq!(a4.hash(), longest_chain_select.best_containing(a1.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(a4.hash(), longest_chain_select.best_containing(a2.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(a4.hash(), longest_chain_select.best_containing(a3.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(a4.hash(), longest_chain_select.best_containing(a4.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(4)).unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing(genesis_hash, Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing(a1.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing(a2.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing(a3.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing(a4.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(4)).unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(4)).unwrap().unwrap()); - // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(4)).unwrap().unwrap()); // search only blocks with number <= 3 - // assert_eq!(a3.hash(), longest_chain_select.best_containing(genesis_hash, Some(3)).unwrap().unwrap()); - // assert_eq!(a3.hash(), longest_chain_select.best_containing(a1.hash(), Some(3)).unwrap().unwrap()); - // assert_eq!(a3.hash(), longest_chain_select.best_containing(a2.hash(), Some(3)).unwrap().unwrap()); - // assert_eq!(a3.hash(), longest_chain_select.best_containing(a3.hash(), Some(3)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(3)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(3)).unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing(genesis_hash, Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing(a1.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing(a2.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing(a3.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(3)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(3)).unwrap()); - // assert_eq!(b3.hash(), longest_chain_select.best_containing(b2.hash(), Some(3)).unwrap().unwrap()); - // assert_eq!(b3.hash(), longest_chain_select.best_containing(b3.hash(), Some(3)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(3)).unwrap()); + assert_eq!(b3.hash(), longest_chain_select.best_containing(b2.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(b3.hash(), longest_chain_select.best_containing(b3.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(3)).unwrap()); - // assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(3)).unwrap().unwrap()); - // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(3)).unwrap().unwrap()); // search only blocks with number <= 2 - // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(2)).unwrap().unwrap()); - // assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), Some(2)).unwrap().unwrap()); - // assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), Some(2)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(2)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(2)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(2)).unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(2)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(2)).unwrap()); - // assert_eq!(b2.hash(), longest_chain_select.best_containing(b2.hash(), Some(2)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(2)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(2)).unwrap()); + assert_eq!(b2.hash(), longest_chain_select.best_containing(b2.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(2)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(2)).unwrap()); - // assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(2)).unwrap().unwrap()); // search only blocks with number <= 1 - // assert_eq!(a1.hash(), longest_chain_select.best_containing(genesis_hash, Some(1)).unwrap().unwrap()); - // assert_eq!(a1.hash(), longest_chain_select.best_containing(a1.hash(), Some(1)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(1)).unwrap()); + assert_eq!(a1.hash(), longest_chain_select.best_containing(genesis_hash, Some(1)).unwrap().unwrap()); + assert_eq!(a1.hash(), longest_chain_select.best_containing(a1.hash(), Some(1)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(1)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(d2.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(d2.hash(), Some(1)).unwrap()); // search only blocks with number <= 0 - // assert_eq!(genesis_hash, longest_chain_select.best_containing(genesis_hash, Some(0)).unwrap().unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a1.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(0)).unwrap()); + assert_eq!(genesis_hash, longest_chain_select.best_containing(genesis_hash, Some(0)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a1.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(c3.hash().clone(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(c3.hash().clone(), Some(0)).unwrap()); - // assert_eq!(None, longest_chain_select.best_containing(d2.hash().clone(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing(d2.hash().clone(), Some(0)).unwrap()); } #[test] @@ -2176,7 +2167,7 @@ pub(crate) mod tests { client.import_lock() ); - // assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(10)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(10)).unwrap().unwrap()); } #[test] From 1c870e1de2a57ba415f5d8f01629c121c8c1d2da Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 16:30:27 -0300 Subject: [PATCH 14/24] add doc --- core/client/src/client.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index dfe82b6beed07..de0133d239895 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1380,6 +1380,7 @@ where B: backend::Backend, Block: BlockT, { + /// Instantiate a new LongestChain for Backend B pub fn new(backend: Arc, import_lock: Arc>) -> Self { LongestChain { backend, From 59fbf034d636d30cf58623f9ad49fe81a26add0b Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 18:09:52 -0300 Subject: [PATCH 15/24] fixing tests --- core/client/src/client.rs | 2 +- core/consensus/aura/src/lib.rs | 9 +++++-- core/finality-grandpa/src/environment.rs | 2 +- core/finality-grandpa/src/tests.rs | 31 +++++++++++++++++++++++- core/service/src/components.rs | 4 ++- node-template/src/service.rs | 13 ++++++++-- 6 files changed, 53 insertions(+), 8 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index de0133d239895..c81a6acd5cede 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1078,7 +1078,7 @@ impl Client where }; match hash_and_number { Some((hash, number)) => { - if self.backend().have_state_at(&hash, number) { + if self.backend.have_state_at(&hash, number) { Ok(BlockStatus::InChainWithState) } else { Ok(BlockStatus::InChainPruned) diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 08649ff189186..edaf792aa560a 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -25,7 +25,7 @@ //! //! Blocks from future steps will be either deferred or rejected depending on how //! far in the future they are. -#![deny(deprecated)] + use std::{sync::Arc, time::Duration, thread, marker::PhantomData, hash::Hash, fmt::Debug}; use parity_codec::{Encode, Decode}; @@ -803,7 +803,7 @@ mod tests { use tokio::runtime::current_thread; use keyring::ed25519::Keyring; use primitives::ed25519; - use client::BlockchainEvents; + use client::{LongestChain, BlockchainEvents}; use test_client; type Error = ::client::error::Error; @@ -915,6 +915,10 @@ mod tests { let mut runtime = current_thread::Runtime::new().unwrap(); for (peer_id, key) in peers { let client = net.lock().peer(*peer_id).client().clone(); + let select_chain = LongestChain::new( + client.backend().clone(), + client.import_lock().clone() + ); let environ = Arc::new(DummyFactory(client.clone())); import_notifications.push( client.import_notification_stream() @@ -934,6 +938,7 @@ mod tests { slot_duration, Arc::new(key.clone().into()), client.clone(), + select_chain, client, environ.clone(), DummyOracle, diff --git a/core/finality-grandpa/src/environment.rs b/core/finality-grandpa/src/environment.rs index f758f2dc1d67b..cae7fe4239658 100644 --- a/core/finality-grandpa/src/environment.rs +++ b/core/finality-grandpa/src/environment.rs @@ -276,7 +276,7 @@ pub(crate) struct Environment, RA, SC> { pub(crate) voter_set_state: SharedVoterSetState, } -impl, RA> Environment { +impl, RA, SC> Environment { /// Updates the voter set state using the given closure. The write lock is /// held during evaluation of the closure and the environment's voter set /// state is set to its result if successful. diff --git a/core/finality-grandpa/src/tests.rs b/core/finality-grandpa/src/tests.rs index b4a5d40bf9916..36f3eff732123 100644 --- a/core/finality-grandpa/src/tests.rs +++ b/core/finality-grandpa/src/tests.rs @@ -28,6 +28,7 @@ use client::{ BlockchainEvents, error::Result, blockchain::Backend as BlockchainBackend, runtime_api::{Core, RuntimeVersion, ApiExt}, + LongestChain, }; use test_client::{self, runtime::BlockNumber}; use consensus_common::{BlockOrigin, ForkChoiceStrategy, ImportedAux, ImportBlock, ImportResult}; @@ -106,9 +107,15 @@ impl TestNetFactory for GrandpaTestNet { fn make_block_import(&self, client: Arc) -> (SharedBlockImport, Option>, PeerData) { + + let select_chain = LongestChain::new( + client.backend().clone(), + client.import_lock().clone() + ); let (import, link) = block_import( client, - Arc::new(self.test_config.clone()) + Arc::new(self.test_config.clone()), + select_chain, ).expect("Could not create block import for fresh peer."); let shared_import = Arc::new(import); (shared_import.clone(), Some(shared_import), Mutex::new(Some(link))) @@ -379,6 +386,10 @@ fn run_to_completion_with( link, ) }; + let select_chain = LongestChain::new( + client.backend().clone(), + client.import_lock().clone() + ); finality_notifications.push( client.finality_notification_stream() .take_while(move |n| { @@ -402,6 +413,7 @@ fn run_to_completion_with( link, MessageRouting::new(net.clone(), peer_id), InherentDataProviders::new(), + select_chain, futures::empty(), ).expect("all in order with client and network"); @@ -488,6 +500,10 @@ fn finalize_3_voters_1_observer() { link, ) }; + let select_chain = LongestChain::new( + client.backend().clone(), + client.import_lock().clone() + ); finality_notifications.push( client.finality_notification_stream() .take_while(|n| Ok(n.header.number() < &20)) @@ -503,6 +519,7 @@ fn finalize_3_voters_1_observer() { link, MessageRouting::new(net.clone(), peer_id), InherentDataProviders::new(), + select_chain, futures::empty(), ).expect("all in order with client and network"); @@ -642,6 +659,12 @@ fn transition_3_voters_twice_1_observer() { link, ) }; + + let select_chain = LongestChain::new( + client.backend().clone(), + client.import_lock().clone() + ); + finality_notifications.push( client.finality_notification_stream() .take_while(|n| Ok(n.header.number() < &30)) @@ -665,6 +688,7 @@ fn transition_3_voters_twice_1_observer() { link, MessageRouting::new(net.clone(), peer_id), InherentDataProviders::new(), + select_chain, futures::empty(), ).expect("all in order with client and network"); @@ -1052,6 +1076,10 @@ fn voter_persists_its_votes() { let net = net.clone(); let voter = future::loop_fn(voter_rx, move |rx| { + let select_chain = LongestChain::new( + client.backend().clone(), + client.import_lock().clone() + ); let (_block_import, _, link) = net.lock().make_block_import(client.clone()); let link = link.lock().take().unwrap(); @@ -1065,6 +1093,7 @@ fn voter_persists_its_votes() { link, MessageRouting::new(net.clone(), 0), InherentDataProviders::new(), + select_chain, futures::empty(), ).expect("all in order with client and network"); diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 4e1528b2d12f3..0cbc53f010a78 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -602,6 +602,7 @@ mod tests { use super::*; use parity_codec::Encode; use consensus_common::BlockOrigin; + use client::LongestChain; use substrate_test_client::{self, TestClient, AccountKeyring, runtime::{Extrinsic, Transfer}}; #[test] @@ -618,8 +619,9 @@ mod tests { let signature = AccountKeyring::from_public(&transfer.from).unwrap().sign(&transfer.encode()).into(); Extrinsic::Transfer(transfer, signature) }; + let best = LongestChain::new(client.backend().clone(), client.import_lock()).best_block_header().unwrap(); // store the transaction in the pool - pool.submit_one(&BlockId::hash(client.best_block_header().unwrap().hash()), transaction.clone()).unwrap(); + pool.submit_one(&BlockId::hash(best.hash()), transaction.clone()).unwrap(); // import the block let mut builder = client.new_block().unwrap(); diff --git a/node-template/src/service.rs b/node-template/src/service.rs index 239f02f33da3f..2a4980688f8a7 100644 --- a/node-template/src/service.rs +++ b/node-template/src/service.rs @@ -13,7 +13,7 @@ use substrate_service::{ }; use basic_authorship::ProposerFactory; use consensus::{import_queue, start_aura, AuraImportQueue, SlotDuration, NothingExtra}; -use substrate_client as client; +use substrate_client::{self as client, LongestChain}; use primitives::{ed25519::Pair, Pair as PairT}; use inherents::InherentDataProviders; use network::construct_simple_protocol; @@ -69,6 +69,7 @@ construct_service_factory! { SlotDuration::get_or_compute(&*client)?, key.clone(), client.clone(), + service.select_chain(), client, proposer, service.network(), @@ -86,7 +87,7 @@ construct_service_factory! { FullImportQueue = AuraImportQueue< Self::Block, > - { |config: &mut FactoryFullConfiguration , client: Arc>| { + { |config: &mut FactoryFullConfiguration , client: Arc>, _select_chain: Self::SelectChain| { import_queue::<_, _, _, Pair>( SlotDuration::get_or_compute(&*client)?, client.clone(), @@ -111,5 +112,13 @@ construct_service_factory! { ).map_err(Into::into) } }, + SelectChain = LongestChain, Self::Block> + { |config: &FactoryFullConfiguration, client: Arc>| { + Ok(LongestChain::new( + client.backend().clone(), + client.import_lock() + )) + } + }, } } From 326a8c3ee71fbeddc392fa4a99b7d2de41200f81 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 18:57:03 -0300 Subject: [PATCH 16/24] Clarify SelectChain Interface, intended implementation and usage --- core/client/src/client.rs | 297 ++++++++++++++-------- core/consensus/aura/slots/src/lib.rs | 2 +- core/consensus/common/src/select_chain.rs | 77 +++--- core/finality-grandpa/src/import.rs | 2 +- core/service/src/components.rs | 3 +- core/service/src/lib.rs | 2 +- 6 files changed, 225 insertions(+), 158 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index c81a6acd5cede..5d549922ccb17 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1529,12 +1529,14 @@ where B: backend::Backend, Block: BlockT, { - fn best_block_header(&self) -> Result<::Header, ConsensusError> { + fn best_block_header_for_authoring(&self) + -> Result<::Header, ConsensusError> + { LongestChain::best_block_header(&self) .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) } - fn best_containing( + fn best_containing_for_authoring( &self, target_hash: Block::Hash, maybe_max_number: Option> @@ -1778,7 +1780,8 @@ pub(crate) mod tests { ); - assert_eq!(genesis_hash.clone(), longest_chain_select.best_containing(genesis_hash.clone(), None).unwrap().unwrap()); + assert_eq!(genesis_hash.clone(), longest_chain_select.best_containing_for_authoring( + genesis_hash.clone(), None).unwrap().unwrap()); } #[test] @@ -1794,7 +1797,8 @@ pub(crate) mod tests { Arc::new(backend), client.import_lock()); - assert_eq!(None, longest_chain_select.best_containing(uninserted_block.hash().clone(), None).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + uninserted_block.hash().clone(), None).unwrap()); } #[test] @@ -1929,9 +1933,12 @@ pub(crate) mod tests { Arc::new(client.backend().as_in_memory()), client.import_lock()); - assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, None).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), None).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + a2.hash(), None).unwrap().unwrap()); } #[test] @@ -2024,127 +2031,204 @@ pub(crate) mod tests { // search without restriction - assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), None).unwrap().unwrap()); - - assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), None).unwrap().unwrap()); - - assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), None).unwrap().unwrap()); - - assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a2.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a3.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a4.hash(), None).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a5.hash(), None).unwrap().unwrap()); + + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b2.hash(), None).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b3.hash(), None).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b4.hash(), None).unwrap().unwrap()); + + assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + c3.hash(), None).unwrap().unwrap()); + + assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + d2.hash(), None).unwrap().unwrap()); // search only blocks with number <= 5. equivalent to without restriction for this scenario - assert_eq!(a5.hash(), longest_chain_select.best_containing(genesis_hash, Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a1.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a2.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a4.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing(a5.hash(), Some(5)).unwrap().unwrap()); - - assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(5)).unwrap().unwrap()); - - assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(5)).unwrap().unwrap()); - - assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a2.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a3.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a4.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + a5.hash(), Some(5)).unwrap().unwrap()); + + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b2.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b3.hash(), Some(5)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b4.hash(), Some(5)).unwrap().unwrap()); + + assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + c3.hash(), Some(5)).unwrap().unwrap()); + + assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + d2.hash(), Some(5)).unwrap().unwrap()); // search only blocks with number <= 4 - assert_eq!(a4.hash(), longest_chain_select.best_containing(genesis_hash, Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing(a1.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing(a2.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing(a3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing(a4.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(4)).unwrap()); - - assert_eq!(b4.hash(), longest_chain_select.best_containing(b2.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing(b3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing(b4.hash(), Some(4)).unwrap().unwrap()); - - assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(4)).unwrap().unwrap()); - - assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + a2.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + a3.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + a4.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a5.hash(), Some(4)).unwrap()); + + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b2.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b3.hash(), Some(4)).unwrap().unwrap()); + assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + b4.hash(), Some(4)).unwrap().unwrap()); + + assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + c3.hash(), Some(4)).unwrap().unwrap()); + + assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + d2.hash(), Some(4)).unwrap().unwrap()); // search only blocks with number <= 3 - assert_eq!(a3.hash(), longest_chain_select.best_containing(genesis_hash, Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), longest_chain_select.best_containing(a1.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), longest_chain_select.best_containing(a2.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), longest_chain_select.best_containing(a3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(3)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(3)).unwrap()); - - assert_eq!(b3.hash(), longest_chain_select.best_containing(b2.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(b3.hash(), longest_chain_select.best_containing(b3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(3)).unwrap()); - - assert_eq!(c3.hash(), longest_chain_select.best_containing(c3.hash(), Some(3)).unwrap().unwrap()); - - assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + a2.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + a3.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a4.hash(), Some(3)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a5.hash(), Some(3)).unwrap()); + + assert_eq!(b3.hash(), longest_chain_select.best_containing_for_authoring( + b2.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(b3.hash(), longest_chain_select.best_containing_for_authoring( + b3.hash(), Some(3)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b4.hash(), Some(3)).unwrap()); + + assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + c3.hash(), Some(3)).unwrap().unwrap()); + + assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + d2.hash(), Some(3)).unwrap().unwrap()); // search only blocks with number <= 2 - assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(2)).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing(a1.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing(a2.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(2)).unwrap()); - - assert_eq!(b2.hash(), longest_chain_select.best_containing(b2.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(2)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(2)).unwrap()); - - assert_eq!(d2.hash(), longest_chain_select.best_containing(d2.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(2)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + a2.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a3.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a4.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a5.hash(), Some(2)).unwrap()); + + assert_eq!(b2.hash(), longest_chain_select.best_containing_for_authoring( + b2.hash(), Some(2)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b3.hash(), Some(2)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b4.hash(), Some(2)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + c3.hash(), Some(2)).unwrap()); + + assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + d2.hash(), Some(2)).unwrap().unwrap()); // search only blocks with number <= 1 - assert_eq!(a1.hash(), longest_chain_select.best_containing(genesis_hash, Some(1)).unwrap().unwrap()); - assert_eq!(a1.hash(), longest_chain_select.best_containing(a1.hash(), Some(1)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(1)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(1)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(c3.hash(), Some(1)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(d2.hash(), Some(1)).unwrap()); + assert_eq!(a1.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(1)).unwrap().unwrap()); + assert_eq!(a1.hash(), longest_chain_select.best_containing_for_authoring( + a1.hash(), Some(1)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a2.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a3.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a4.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a5.hash(), Some(1)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b2.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b3.hash(), Some(1)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b4.hash(), Some(1)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + c3.hash(), Some(1)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + d2.hash(), Some(1)).unwrap()); // search only blocks with number <= 0 - assert_eq!(genesis_hash, longest_chain_select.best_containing(genesis_hash, Some(0)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a1.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a2.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a3.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a4.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(a5.hash(), Some(0)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(b2.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b3.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing(b4.hash(), Some(0)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(c3.hash().clone(), Some(0)).unwrap()); - - assert_eq!(None, longest_chain_select.best_containing(d2.hash().clone(), Some(0)).unwrap()); + assert_eq!(genesis_hash, longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(0)).unwrap().unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a1.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a2.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a3.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a4.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + a5.hash(), Some(0)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b2.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b3.hash(), Some(0)).unwrap()); + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + b4.hash(), Some(0)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + c3.hash().clone(), Some(0)).unwrap()); + + assert_eq!(None, longest_chain_select.best_containing_for_authoring( + d2.hash().clone(), Some(0)).unwrap()); } #[test] @@ -2168,7 +2252,8 @@ pub(crate) mod tests { client.import_lock() ); - assert_eq!(a2.hash(), longest_chain_select.best_containing(genesis_hash, Some(10)).unwrap().unwrap()); + assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + genesis_hash, Some(10)).unwrap().unwrap()); } #[test] diff --git a/core/consensus/aura/slots/src/lib.rs b/core/consensus/aura/slots/src/lib.rs index 9adc8362070d5..2db4738d46efe 100644 --- a/core/consensus/aura/slots/src/lib.rs +++ b/core/consensus/aura/slots/src/lib.rs @@ -156,7 +156,7 @@ pub fn start_slot_worker( } let slot_num = slot_info.number; - let chain_head = match client.best_block_header() { + let chain_head = match client.best_block_header_for_authoring() { Ok(x) => x, Err(e) => { warn!(target: "aura", "Unable to author block in slot {}. \ diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs index e0eef7047aa32..84b2c0e3ec55b 100644 --- a/core/consensus/common/src/select_chain.rs +++ b/core/consensus/common/src/select_chain.rs @@ -23,63 +23,44 @@ use runtime_primitives::traits::{Block as BlockT, NumberFor}; /// specific chain build. //// /// The Strategy can be customised for the two use cases of authoring new blocks -/// upon the best chain or which fork to finalise or just the main methods -/// `best_block_header` and `best_containing` can be implemented if the same strategy -/// is being used. +/// upon the best chain or which fork to finalise. Unless implemented differently +/// by default finalisation methods fall back to use authoring, so as a minimum +/// `_authoring`-functions must be implemented. +/// +/// Any particular user must make explicit, however, whether they intend to finalise +/// or author through the using the right function call, as these might differ in +/// some implemntations. +/// +/// Non-deterministicly finalising chains may only use the `_authoring` functions. pub trait SelectChain: Sync + Send + Clone { /// Get all leaves of the chain: block hashes that have no children currently. /// Leaves that can never be finalized will not be returned. fn leaves(&self) -> Result::Hash>, Error>; - /// Get best block header. - fn best_block_header(&self) -> Result<::Header, Error>; - - /// Get best block header for authoring - fn best_block_header_for_authoring(&self) -> Result<::Header, Error> { - self.best_block_header() - } + /// Get best block header for authoring. + fn best_block_header_for_authoring(&self) -> Result<::Header, Error>; /// Get best block header for finalisation fn best_block_header_for_finalisation(&self) -> Result<::Header, Error> { - self.best_block_header() - } + self.best_block_header_for_authoring() + } /// Get the most recent block hash of the best chain that contain block - /// with the given `target_hash`. - fn best_containing( - &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Error>; - - /// Get the most recent block hash of the best chain that contain block - /// with the given `target_hash` for authoring - fn best_containing_for_authoring( - &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Error> { - self.best_containing(target_hash, maybe_max_number) - } - /// Get the most recent block hash of the best chain that contain block - /// with the given `target_hash` for finalisation - fn best_containing_for_finalisation( - &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Error> { - self.best_containing(target_hash, maybe_max_number) - } -} + /// with the given `target_hash` for authoring + fn best_containing_for_authoring( + &self, + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Error>; - -// pub trait SelectChainClone { -// fn clone_box(&self) -> Box>; -// } - -// impl Clone for Box> { -// fn clone(&self) -> Box> { -// self.clone_box() -// } -// } + /// Get the most recent block hash of the best chain that contain block + /// with the given `target_hash` for finalisation + fn best_containing_for_finalisation( + &self, + target_hash: ::Hash, + maybe_max_number: Option> + ) -> Result::Hash>, Error> { + self.best_containing_for_authoring(target_hash, maybe_max_number) + } +} \ No newline at end of file diff --git a/core/finality-grandpa/src/import.rs b/core/finality-grandpa/src/import.rs index 653a2f3a27e8d..5e98d4f1661e0 100644 --- a/core/finality-grandpa/src/import.rs +++ b/core/finality-grandpa/src/import.rs @@ -92,7 +92,7 @@ impl, RA, PRA, SC> JustificationImport pending_change.effective_number() > chain_info.finalized_number && pending_change.effective_number() <= chain_info.best_number { - let effective_block_hash = self.select_chain.best_containing( + let effective_block_hash = self.select_chain.best_containing_for_authoring( pending_change.canon_hash, Some(pending_change.effective_number()), ); diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 0cbc53f010a78..afe2d36062fdb 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -619,7 +619,8 @@ mod tests { let signature = AccountKeyring::from_public(&transfer.from).unwrap().sign(&transfer.encode()).into(); Extrinsic::Transfer(transfer, signature) }; - let best = LongestChain::new(client.backend().clone(), client.import_lock()).best_block_header().unwrap(); + let best = LongestChain::new(client.backend().clone(), client.import_lock()) + .best_block_header_for_authoring().unwrap(); // store the transaction in the pool pool.submit_one(&BlockId::hash(best.hash()), transaction.clone()).unwrap(); diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 422154a669e9d..d11aebee5adcb 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -135,7 +135,7 @@ impl Service { client.clone(), select_chain.clone() )?); - let best_header = select_chain.best_block_header()?; + let best_header = select_chain.best_block_header_for_authoring()?; let version = config.full_version(); info!("Best block: #{}", best_header.number()); From 5fb7a0969093fd7e0a152925bc177d7f9fceab82 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 19:01:28 -0300 Subject: [PATCH 17/24] minor components cleanups --- core/service/src/components.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/service/src/components.rs b/core/service/src/components.rs index afe2d36062fdb..974c74912015e 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -16,7 +16,7 @@ //! Substrate service components. -use std::{sync::Arc, net::SocketAddr, marker::PhantomData, ops::Deref, ops::DerefMut}; +use std::{sync::Arc, net::SocketAddr, ops::Deref, ops::DerefMut}; use serde::{Serialize, de::DeserializeOwned}; use tokio::runtime::TaskExecutor; use crate::chain_spec::ChainSpec; @@ -385,6 +385,7 @@ pub trait Components: Sized + 'static { >; /// Our Import Queue type ImportQueue: ImportQueue> + 'static; + /// The Fork Choice Strategy for the chain type SelectChain: SelectChain>; /// Create client. @@ -420,7 +421,6 @@ pub trait Components: Sized + 'static { /// A struct that implement `Components` for the full client. pub struct FullComponents { - // _factory: PhantomData, service: Service>, } @@ -510,7 +510,6 @@ impl Components for FullComponents { /// A struct that implement `Components` for the light client. pub struct LightComponents { - _factory: PhantomData, service: Service>, } @@ -522,7 +521,6 @@ impl LightComponents { ) -> Result { Ok( Self { - _factory: Default::default(), service: Service::new(config, task_executor)?, } ) From e6f8ed2ad65fbf15662cc8ccb2a1867ca3e17285 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 19:13:43 -0300 Subject: [PATCH 18/24] minor cleanups --- core/consensus/common/Cargo.toml | 1 - core/service/src/components.rs | 2 -- 2 files changed, 3 deletions(-) diff --git a/core/consensus/common/Cargo.toml b/core/consensus/common/Cargo.toml index 33db83a20d36f..eb0061a133b36 100644 --- a/core/consensus/common/Cargo.toml +++ b/core/consensus/common/Cargo.toml @@ -6,7 +6,6 @@ description = "Common utilities for substrate consensus" edition = "2018" [dependencies] -parking_lot = "0.7.1" crossbeam-channel = "0.3.4" libp2p = { version = "0.6.0", default-features = false } log = "0.4" diff --git a/core/service/src/components.rs b/core/service/src/components.rs index 974c74912015e..3bf474336b67c 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -588,8 +588,6 @@ impl Components for LightComponents { _config: &mut FactoryFullConfiguration, _client: Arc> ) -> Result { - // FIXME: this (and other places) need a "NotAvailableInMode" for never-in-light-client-features - // that doesn't break creation of light clients Err("Fork choice doesn't happen on light clients.".into()) } From d92d542ae01f619cb447adf383735946bdade727 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 12 Apr 2019 19:35:47 -0300 Subject: [PATCH 19/24] Update lock files --- Cargo.lock | 1 - core/test-runtime/wasm/Cargo.lock | 1 - node-template/runtime/wasm/Cargo.lock | 1 - node/runtime/wasm/Cargo.lock | 1 - 4 files changed, 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 04b7c7554b696..e7afec8f6a79a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3803,7 +3803,6 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", diff --git a/core/test-runtime/wasm/Cargo.lock b/core/test-runtime/wasm/Cargo.lock index 2b270c099f00d..7df0a2652463d 100644 --- a/core/test-runtime/wasm/Cargo.lock +++ b/core/test-runtime/wasm/Cargo.lock @@ -2315,7 +2315,6 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", diff --git a/node-template/runtime/wasm/Cargo.lock b/node-template/runtime/wasm/Cargo.lock index a307d2992999a..9d6c917b15fb8 100644 --- a/node-template/runtime/wasm/Cargo.lock +++ b/node-template/runtime/wasm/Cargo.lock @@ -2477,7 +2477,6 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", diff --git a/node/runtime/wasm/Cargo.lock b/node/runtime/wasm/Cargo.lock index 5aa4f5c9cb709..792c68b39ea45 100644 --- a/node/runtime/wasm/Cargo.lock +++ b/node/runtime/wasm/Cargo.lock @@ -2620,7 +2620,6 @@ dependencies = [ "libp2p 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.4.0 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "sr-primitives 1.0.0", "sr-version 1.0.0", "substrate-inherents 1.0.0", From f9c94a54dc8fdcf6d6d9ed8da16a974cde7fdaa7 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 6 May 2019 10:34:28 +0200 Subject: [PATCH 20/24] Implement cleaner interface for SelectChain --- core/client/src/client.rs | 15 +++++++------ core/consensus/common/src/select_chain.rs | 26 ++++++----------------- core/consensus/slots/src/lib.rs | 2 +- core/finality-grandpa/src/environment.rs | 2 +- core/finality-grandpa/src/import.rs | 2 +- core/finality-grandpa/src/lib.rs | 2 +- core/service/src/lib.rs | 2 +- 7 files changed, 20 insertions(+), 31 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index 5f70cb23aa7c9..b90fd5e6c66d0 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1567,14 +1567,20 @@ where B: backend::Backend, Block: BlockT, { - fn best_block_header_for_authoring(&self) + + fn leaves(&self) -> Result::Hash>, ConsensusError> { + LongestChain::leaves(self) + .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) + } + + fn best_chain(&self) -> Result<::Header, ConsensusError> { LongestChain::best_block_header(&self) .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) } - fn best_containing_for_authoring( + fn finality_target( &self, target_hash: Block::Hash, maybe_max_number: Option> @@ -1582,11 +1588,6 @@ where LongestChain::best_containing(self, target_hash, maybe_max_number) .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) } - - fn leaves(&self) -> Result::Hash>, ConsensusError> { - LongestChain::leaves(self) - .map_err(|e| ConsensusErrorKind::ChainLookup(e.to_string()).into()) - } } impl BlockBody for Client diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs index 84b2c0e3ec55b..c32538caa4360 100644 --- a/core/consensus/common/src/select_chain.rs +++ b/core/consensus/common/src/select_chain.rs @@ -29,7 +29,7 @@ use runtime_primitives::traits::{Block as BlockT, NumberFor}; /// /// Any particular user must make explicit, however, whether they intend to finalise /// or author through the using the right function call, as these might differ in -/// some implemntations. +/// some implementations. /// /// Non-deterministicly finalising chains may only use the `_authoring` functions. pub trait SelectChain: Sync + Send + Clone { @@ -38,29 +38,17 @@ pub trait SelectChain: Sync + Send + Clone { /// Leaves that can never be finalized will not be returned. fn leaves(&self) -> Result::Hash>, Error>; - /// Get best block header for authoring. - fn best_block_header_for_authoring(&self) -> Result<::Header, Error>; - - /// Get best block header for finalisation - fn best_block_header_for_finalisation(&self) -> Result<::Header, Error> { - self.best_block_header_for_authoring() - } - - /// Get the most recent block hash of the best chain that contain block - /// with the given `target_hash` for authoring - fn best_containing_for_authoring( - &self, - target_hash: ::Hash, - maybe_max_number: Option> - ) -> Result::Hash>, Error>; + /// Among those `leaves` deterministically pick one chain as the generally + /// best chain to author new blocks upon and probably finalize. + fn best_chain(&self) -> Result<::Header, Error>; /// Get the most recent block hash of the best chain that contain block /// with the given `target_hash` for finalisation - fn best_containing_for_finalisation( + fn finality_target( &self, target_hash: ::Hash, - maybe_max_number: Option> + _maybe_max_number: Option> ) -> Result::Hash>, Error> { - self.best_containing_for_authoring(target_hash, maybe_max_number) + Ok(Some(target_hash)) } } \ No newline at end of file diff --git a/core/consensus/slots/src/lib.rs b/core/consensus/slots/src/lib.rs index 5d2b57c4ef8e5..2f8eedb497d39 100644 --- a/core/consensus/slots/src/lib.rs +++ b/core/consensus/slots/src/lib.rs @@ -172,7 +172,7 @@ where } let slot_num = slot_info.number; - let chain_head = match client.best_block_header_for_authoring() { + let chain_head = match client.best_chain() { Ok(x) => x, Err(e) => { warn!(target: "slots", "Unable to author block in slot {}. \ diff --git a/core/finality-grandpa/src/environment.rs b/core/finality-grandpa/src/environment.rs index 3c471fe4f0809..cec64b254ccfe 100644 --- a/core/finality-grandpa/src/environment.rs +++ b/core/finality-grandpa/src/environment.rs @@ -324,7 +324,7 @@ where let limit = self.authority_set.current_limit(); debug!(target: "afg", "Finding best chain containing block {:?} with number limit {:?}", block, limit); - match self.select_chain.best_containing_for_finalisation(block, None) { + match self.select_chain.finality_target(block, None) { Ok(Some(mut best_hash)) => { let base_header = self.inner.header(&BlockId::Hash(block)).ok()? .expect("Header known to exist after `best_containing` call; qed"); diff --git a/core/finality-grandpa/src/import.rs b/core/finality-grandpa/src/import.rs index c1ec4151beb83..542617fbcf0b7 100644 --- a/core/finality-grandpa/src/import.rs +++ b/core/finality-grandpa/src/import.rs @@ -89,7 +89,7 @@ impl, RA, PRA, SC> JustificationImport pending_change.effective_number() > chain_info.finalized_number && pending_change.effective_number() <= chain_info.best_number { - let effective_block_hash = self.select_chain.best_containing_for_authoring( + let effective_block_hash = self.select_chain.finality_target( pending_change.canon_hash, Some(pending_change.effective_number()), ); diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 7c29c0836f530..823bd443361c4 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -52,7 +52,7 @@ //! or prune any signaled changes based on whether the signaling block is //! included in the newly-finalized chain. #![forbid(warnings)] -#![warn(deprecated)] +#![allow(deprecated)] use futures::prelude::*; use log::{debug, info, warn}; diff --git a/core/service/src/lib.rs b/core/service/src/lib.rs index 118e07510f4c1..aed14568cb682 100644 --- a/core/service/src/lib.rs +++ b/core/service/src/lib.rs @@ -158,7 +158,7 @@ impl Service { client.clone(), select_chain.clone() )?); - let best_header = select_chain.best_block_header_for_authoring()?; + let best_header = select_chain.best_chain()?; let version = config.full_version(); info!("Best block: #{}", best_header.number()); From 1de5c646c8a05bce0afca1f9917111f366100574 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 6 May 2019 11:04:04 +0200 Subject: [PATCH 21/24] addressing comments --- core/client/src/client.rs | 12 ++++++------ core/finality-grandpa/src/lib.rs | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index b90fd5e6c66d0..f3d08cbf0dc76 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -321,18 +321,18 @@ impl Client where /// Expose backend reference. To be used in tests only #[doc(hidden)] - #[deprecated(since="1.0.1", note="Rather than relying on `client` \ - to provide this, access to the backend should be handled at setup \ - only - see #1134. This function will be removed once that is in place.")] + #[deprecated(note="Rather than relying on `client` to provide this, access \ + to the backend should be handled at setup only - see #1134. This function \ + will be removed once that is in place.")] pub fn backend(&self) -> &Arc { &self.backend } /// Expose reference to import lock #[doc(hidden)] - #[deprecated(since="1.0.1", note="Rather than relying on `client` \ - to provide this, access to the backend should be handled at setup \ - only - see #1134. This function will be removed once that is in place.")] + #[deprecated(note="Rather than relying on `client` to provide this, access \ + to the backend should be handled at setup only - see #1134. This function \ + will be removed once that is in place.")] pub fn import_lock(&self) -> Arc> { self.import_lock.clone() } diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 823bd443361c4..503c256c77fb5 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -52,6 +52,7 @@ //! or prune any signaled changes based on whether the signaling block is //! included in the newly-finalized chain. #![forbid(warnings)] +/// FIXME #![allow(deprecated)] use futures::prelude::*; From 2b52445e29df9124611a0b7273e8f6276704233a Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 6 May 2019 11:59:41 +0200 Subject: [PATCH 22/24] Updating tests --- core/client/src/client.rs | 166 +++++++++++----------- core/consensus/babe/src/lib.rs | 22 +-- core/finality-grandpa/src/lib.rs | 3 +- core/finality-grandpa/src/tests.rs | 14 +- core/service/src/components.rs | 2 +- core/sr-api-macros/tests/runtime_calls.rs | 5 +- 6 files changed, 104 insertions(+), 108 deletions(-) diff --git a/core/client/src/client.rs b/core/client/src/client.rs index f3d08cbf0dc76..640484f9d5422 100644 --- a/core/client/src/client.rs +++ b/core/client/src/client.rs @@ -1819,7 +1819,7 @@ pub(crate) mod tests { ); - assert_eq!(genesis_hash.clone(), longest_chain_select.best_containing_for_authoring( + assert_eq!(genesis_hash.clone(), longest_chain_select.finality_target( genesis_hash.clone(), None).unwrap().unwrap()); } @@ -1836,7 +1836,7 @@ pub(crate) mod tests { Arc::new(backend), client.import_lock()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( uninserted_block.hash().clone(), None).unwrap()); } @@ -1972,11 +1972,11 @@ pub(crate) mod tests { Arc::new(client.backend().as_in_memory()), client.import_lock()); - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( genesis_hash, None).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( a1.hash(), None).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( a2.hash(), None).unwrap().unwrap()); } @@ -2070,203 +2070,203 @@ pub(crate) mod tests { // search without restriction - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( genesis_hash, None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a1.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a2.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a3.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a4.hash(), None).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a5.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b2.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b3.hash(), None).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b4.hash(), None).unwrap().unwrap()); - assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(c3.hash(), longest_chain_select.finality_target( c3.hash(), None).unwrap().unwrap()); - assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(d2.hash(), longest_chain_select.finality_target( d2.hash(), None).unwrap().unwrap()); // search only blocks with number <= 5. equivalent to without restriction for this scenario - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( genesis_hash, Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a1.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a2.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a4.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(a5.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a5.hash(), longest_chain_select.finality_target( a5.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b2.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b4.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(c3.hash(), longest_chain_select.finality_target( c3.hash(), Some(5)).unwrap().unwrap()); - assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(d2.hash(), longest_chain_select.finality_target( d2.hash(), Some(5)).unwrap().unwrap()); // search only blocks with number <= 4 - assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a4.hash(), longest_chain_select.finality_target( genesis_hash, Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a4.hash(), longest_chain_select.finality_target( a1.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a4.hash(), longest_chain_select.finality_target( a2.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a4.hash(), longest_chain_select.finality_target( a3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(a4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a4.hash(), longest_chain_select.finality_target( a4.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a5.hash(), Some(4)).unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b2.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(b4.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b4.hash(), longest_chain_select.finality_target( b4.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(c3.hash(), longest_chain_select.finality_target( c3.hash(), Some(4)).unwrap().unwrap()); - assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(d2.hash(), longest_chain_select.finality_target( d2.hash(), Some(4)).unwrap().unwrap()); // search only blocks with number <= 3 - assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a3.hash(), longest_chain_select.finality_target( genesis_hash, Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a3.hash(), longest_chain_select.finality_target( a1.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a3.hash(), longest_chain_select.finality_target( a2.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(a3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a3.hash(), longest_chain_select.finality_target( a3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a4.hash(), Some(3)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a5.hash(), Some(3)).unwrap()); - assert_eq!(b3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b3.hash(), longest_chain_select.finality_target( b2.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(b3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b3.hash(), longest_chain_select.finality_target( b3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b4.hash(), Some(3)).unwrap()); - assert_eq!(c3.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(c3.hash(), longest_chain_select.finality_target( c3.hash(), Some(3)).unwrap().unwrap()); - assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(d2.hash(), longest_chain_select.finality_target( d2.hash(), Some(3)).unwrap().unwrap()); // search only blocks with number <= 2 - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( genesis_hash, Some(2)).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( a1.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( a2.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a3.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a4.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a5.hash(), Some(2)).unwrap()); - assert_eq!(b2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(b2.hash(), longest_chain_select.finality_target( b2.hash(), Some(2)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b3.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b4.hash(), Some(2)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( c3.hash(), Some(2)).unwrap()); - assert_eq!(d2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(d2.hash(), longest_chain_select.finality_target( d2.hash(), Some(2)).unwrap().unwrap()); // search only blocks with number <= 1 - assert_eq!(a1.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a1.hash(), longest_chain_select.finality_target( genesis_hash, Some(1)).unwrap().unwrap()); - assert_eq!(a1.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a1.hash(), longest_chain_select.finality_target( a1.hash(), Some(1)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a2.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a3.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a4.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a5.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b2.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b3.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b4.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( c3.hash(), Some(1)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( d2.hash(), Some(1)).unwrap()); // search only blocks with number <= 0 - assert_eq!(genesis_hash, longest_chain_select.best_containing_for_authoring( + assert_eq!(genesis_hash, longest_chain_select.finality_target( genesis_hash, Some(0)).unwrap().unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a1.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a2.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a3.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a4.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( a5.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b2.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b3.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( b4.hash(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( c3.hash().clone(), Some(0)).unwrap()); - assert_eq!(None, longest_chain_select.best_containing_for_authoring( + assert_eq!(None, longest_chain_select.finality_target( d2.hash().clone(), Some(0)).unwrap()); } @@ -2291,7 +2291,7 @@ pub(crate) mod tests { client.import_lock() ); - assert_eq!(a2.hash(), longest_chain_select.best_containing_for_authoring( + assert_eq!(a2.hash(), longest_chain_select.finality_target( genesis_hash, Some(10)).unwrap().unwrap()); } diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index c86e14de31fe6..1ffa247e4c290 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -58,10 +58,9 @@ use srml_babe::{ BabeInherentData, timestamp::{TimestampInherentData, InherentType as TimestampInherent} }; -use consensus_common::well_known_cache_keys; +use consensus_common::{SelectChain, well_known_cache_keys}; use consensus_common::import_queue::{Verifier, BasicQueue}; use client::{ - ChainHead, block_builder::api::BlockBuilder as BlockBuilderApi, blockchain::ProvideCache, runtime_api::ApiExt, @@ -249,7 +248,7 @@ impl SlotCompatible for BabeSlotCompatible { } /// Parameters for BABE. -pub struct BabeParams { +pub struct BabeParams { /// The configuration for BABE. Includes the slot duration, threshold, and /// other parameters. @@ -261,6 +260,9 @@ pub struct BabeParams { /// The client to use pub client: Arc, + /// The SelectChain Strategy + pub select_chain: SC, + /// A block importer pub block_import: Arc, @@ -281,28 +283,30 @@ pub struct BabeParams { } /// Start the babe worker. The returned future should be run in a tokio runtime. -pub fn start_babe(BabeParams { +pub fn start_babe(BabeParams { config, local_key, client, + select_chain, block_import, env, sync_oracle, on_exit, inherent_data_providers, force_authoring, -}: BabeParams) -> Result< +}: BabeParams) -> Result< impl Future, consensus_common::Error, > where B: Block, - C: ChainHead + ProvideRuntimeApi + ProvideCache, + C: ProvideRuntimeApi + ProvideCache, C::Api: AuthoritiesApi, E: Environment, E::Proposer: Proposer, <>::Create as IntoFuture>::Future: Send + 'static, I: BlockImport + Send + Sync + 'static, SO: SyncOracle + Send + Sync + Clone, + SC: SelectChain, DigestItemFor: CompatibleDigestItem + DigestItem, Error: ::std::error::Error + Send + From<::consensus_common::Error> + From + 'static, OnExit: Future, @@ -319,7 +323,7 @@ pub fn start_babe(BabeParams { }; slots::start_slot_worker::<_, _, _, _, _, BabeSlotCompatible, _>( config.0, - client, + select_chain, Arc::new(worker), sync_oracle, on_exit, @@ -847,10 +851,11 @@ fn claim_slot( } #[cfg(test)] -#[allow(dead_code, unused_imports)] +#[allow(dead_code, unused_imports, deprecated)] //FIXME mod tests { use super::*; + use client::LongestChain; use consensus_common::NoNetwork as DummyOracle; use network::test::*; use network::test::{Block as TestBlock, PeersClient}; @@ -1014,6 +1019,7 @@ mod tests { config, local_key: Arc::new(key.clone().into()), block_import: client.clone(), + select_chain: LongestChain::new(client.backend().clone(), client.import_lock().clone()), client, env: environ.clone(), sync_oracle: DummyOracle, diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 503c256c77fb5..c2afeb27f2f7b 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -52,8 +52,7 @@ //! or prune any signaled changes based on whether the signaling block is //! included in the newly-finalized chain. #![forbid(warnings)] -/// FIXME -#![allow(deprecated)] +#![allow(deprecated)] // FIXME use futures::prelude::*; use log::{debug, info, warn}; diff --git a/core/finality-grandpa/src/tests.rs b/core/finality-grandpa/src/tests.rs index 1158a2dbb3d38..52746c5592420 100644 --- a/core/finality-grandpa/src/tests.rs +++ b/core/finality-grandpa/src/tests.rs @@ -51,6 +51,7 @@ type PeerData = test_client::Executor, Block, test_client::runtime::RuntimeApi, + LongestChain > > >; @@ -534,10 +535,6 @@ fn finalize_3_voters_1_full_observer() { link, ) }; - let select_chain = LongestChain::new( - client.backend().clone(), - client.import_lock().clone() - ); finality_notifications.push( client.finality_notification_stream() .take_while(|n| Ok(n.header.number() < &20)) @@ -696,11 +693,6 @@ fn transition_3_voters_twice_1_full_observer() { ) }; - let select_chain = LongestChain::new( - client.backend().clone(), - client.import_lock().clone() - ); - finality_notifications.push( client.finality_notification_stream() .take_while(|n| Ok(n.header.number() < &30)) @@ -1115,10 +1107,6 @@ fn voter_persists_its_votes() { let net = net.clone(); let voter = future::loop_fn(voter_rx, move |rx| { - let select_chain = LongestChain::new( - client.backend().clone(), - client.import_lock().clone() - ); let (_block_import, _, link) = net.lock().make_block_import(client.clone()); let link = link.lock().take().unwrap(); diff --git a/core/service/src/components.rs b/core/service/src/components.rs index fe56d5281f2a6..7e76a8f201a9b 100644 --- a/core/service/src/components.rs +++ b/core/service/src/components.rs @@ -615,7 +615,7 @@ mod tests { to: Default::default(), }.into_signed_tx(); let best = LongestChain::new(client.backend().clone(), client.import_lock()) - .best_block_header_for_authoring().unwrap(); + .best_chain().unwrap(); // store the transaction in the pool pool.submit_one(&BlockId::hash(best.hash()), transaction.clone()).unwrap(); diff --git a/core/sr-api-macros/tests/runtime_calls.rs b/core/sr-api-macros/tests/runtime_calls.rs index df1be11a74fb8..fb3cad3238e6f 100644 --- a/core/sr-api-macros/tests/runtime_calls.rs +++ b/core/sr-api-macros/tests/runtime_calls.rs @@ -27,6 +27,8 @@ use state_machine::{ execution_proof_check_on_trie_backend, }; +use client::LongestChain; +use consensus_common::SelectChain; use codec::Encode; fn calling_function_with_strat(strat: ExecutionStrategy) { @@ -156,7 +158,8 @@ fn record_proof_works() { let client = test_client::new_with_execution_strategy(ExecutionStrategy::Both); let block_id = BlockId::Number(client.info().unwrap().chain.best_number); - let storage_root = client.best_block_header().unwrap().state_root().clone(); + let storage_root = LongestChain::new(client.backend().clone(), client.import_lock()) + .best_chain().unwrap().state_root().clone(); let transaction = Transfer { amount: 1000, From 38f8f65df93fbfe30c57cf48e8ffe72dbbf24185 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Mon, 6 May 2019 13:26:29 +0200 Subject: [PATCH 23/24] bump node runtime impl version --- core/test-runtime/wasm/Cargo.lock | 4 +--- node-template/runtime/wasm/Cargo.lock | 4 +--- node/runtime/src/lib.rs | 2 +- node/runtime/wasm/Cargo.lock | 4 +--- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/core/test-runtime/wasm/Cargo.lock b/core/test-runtime/wasm/Cargo.lock index c6510ab383669..c2f107087540e 100644 --- a/core/test-runtime/wasm/Cargo.lock +++ b/core/test-runtime/wasm/Cargo.lock @@ -2391,7 +2391,6 @@ dependencies = [ "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)", "hash-db 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", - "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "hex-literal 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "kvdb 0.1.0 (git+https://github.com/paritytech/parity-common?rev=b0317f649ab2c665b7987b8475878fc4d2e1f81d)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2457,7 +2456,7 @@ dependencies = [ "sr-version 1.0.0", "substrate-inherents 1.0.0", "substrate-primitives 1.0.0", - "tokio 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", + "tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -2577,7 +2576,6 @@ name = "substrate-state-machine" version = "1.0.0" dependencies = [ "hash-db 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", - "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/node-template/runtime/wasm/Cargo.lock b/node-template/runtime/wasm/Cargo.lock index 221aa72729797..07d9fbf8fc87f 100644 --- a/node-template/runtime/wasm/Cargo.lock +++ b/node-template/runtime/wasm/Cargo.lock @@ -2543,7 +2543,6 @@ dependencies = [ "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)", "hash-db 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", - "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "hex-literal 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "kvdb 0.1.0 (git+https://github.com/paritytech/parity-common?rev=b0317f649ab2c665b7987b8475878fc4d2e1f81d)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2599,7 +2598,7 @@ dependencies = [ "sr-version 1.0.0", "substrate-inherents 1.0.0", "substrate-primitives 1.0.0", - "tokio 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", + "tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -2702,7 +2701,6 @@ name = "substrate-state-machine" version = "1.0.0" dependencies = [ "hash-db 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", - "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 748b5c63928c4..d2e8f5e7d32e9 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -59,7 +59,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, spec_version: 71, - impl_version: 71, + impl_version: 72, apis: RUNTIME_API_VERSIONS, }; diff --git a/node/runtime/wasm/Cargo.lock b/node/runtime/wasm/Cargo.lock index c5f7018fcc7b5..5bcbb1be192fb 100644 --- a/node/runtime/wasm/Cargo.lock +++ b/node/runtime/wasm/Cargo.lock @@ -2678,7 +2678,6 @@ dependencies = [ "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)", "hash-db 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", - "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "hex-literal 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "kvdb 0.1.0 (git+https://github.com/paritytech/parity-common?rev=b0317f649ab2c665b7987b8475878fc4d2e1f81d)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2734,7 +2733,7 @@ dependencies = [ "sr-version 1.0.0", "substrate-inherents 1.0.0", "substrate-primitives 1.0.0", - "tokio 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", + "tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -2848,7 +2847,6 @@ name = "substrate-state-machine" version = "1.0.0" dependencies = [ "hash-db 0.12.2 (registry+https://github.com/rust-lang/crates.io-index)", - "heapsize 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-codec 3.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", From 09bb8a8ef12d8a38dfe672443b46f49962d98c98 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Fri, 10 May 2019 11:34:16 +0200 Subject: [PATCH 24/24] address grumbles --- core/consensus/aura/src/lib.rs | 2 +- core/consensus/babe/src/lib.rs | 4 +++- core/consensus/common/src/select_chain.rs | 6 +++--- core/finality-grandpa/src/lib.rs | 2 +- core/finality-grandpa/src/tests.rs | 5 ----- 5 files changed, 8 insertions(+), 11 deletions(-) diff --git a/core/consensus/aura/src/lib.rs b/core/consensus/aura/src/lib.rs index 209fae29007f2..d5123feb4f17e 100644 --- a/core/consensus/aura/src/lib.rs +++ b/core/consensus/aura/src/lib.rs @@ -920,7 +920,7 @@ mod tests { let client = net.lock().peer(*peer_id).client().clone(); let select_chain = LongestChain::new( client.backend().clone(), - client.import_lock().clone() + client.import_lock().clone(), ); let environ = Arc::new(DummyFactory(client.clone())); import_notifications.push( diff --git a/core/consensus/babe/src/lib.rs b/core/consensus/babe/src/lib.rs index 1ffa247e4c290..a016e835c5fd8 100644 --- a/core/consensus/babe/src/lib.rs +++ b/core/consensus/babe/src/lib.rs @@ -851,7 +851,9 @@ fn claim_slot( } #[cfg(test)] -#[allow(dead_code, unused_imports, deprecated)] //FIXME +#[allow(dead_code, unused_imports, deprecated)] +// FIXME #2532: need to allow deprecated until refactor is done https://github.com/paritytech/substrate/issues/2532 + mod tests { use super::*; diff --git a/core/consensus/common/src/select_chain.rs b/core/consensus/common/src/select_chain.rs index c32538caa4360..47c65d1fe78e5 100644 --- a/core/consensus/common/src/select_chain.rs +++ b/core/consensus/common/src/select_chain.rs @@ -21,7 +21,7 @@ use runtime_primitives::traits::{Block as BlockT, NumberFor}; /// The SelectChain trait defines the strategy upon which the head is chosen /// if multiple forks are present for an opaque definition of "best" in the /// specific chain build. -//// +/// /// The Strategy can be customised for the two use cases of authoring new blocks /// upon the best chain or which fork to finalise. Unless implemented differently /// by default finalisation methods fall back to use authoring, so as a minimum @@ -42,8 +42,8 @@ pub trait SelectChain: Sync + Send + Clone { /// best chain to author new blocks upon and probably finalize. fn best_chain(&self) -> Result<::Header, Error>; - /// Get the most recent block hash of the best chain that contain block - /// with the given `target_hash` for finalisation + /// Get the best ancestor of `target_hash` that we should attempt + /// to finalize next. fn finality_target( &self, target_hash: ::Hash, diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index c2afeb27f2f7b..bd7bc5f9e3223 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -52,7 +52,7 @@ //! or prune any signaled changes based on whether the signaling block is //! included in the newly-finalized chain. #![forbid(warnings)] -#![allow(deprecated)] // FIXME +#![allow(deprecated)] // FIXME #2532: remove once the refactor is done https://github.com/paritytech/substrate/issues/2532 use futures::prelude::*; use log::{debug, info, warn}; diff --git a/core/finality-grandpa/src/tests.rs b/core/finality-grandpa/src/tests.rs index 52746c5592420..52b23bfcb79f2 100644 --- a/core/finality-grandpa/src/tests.rs +++ b/core/finality-grandpa/src/tests.rs @@ -418,11 +418,6 @@ fn run_to_completion_with( ) }; - // let select_chain = LongestChain::new( - // client.backend().clone(), - // client.import_lock().clone() - // ); - wait_for.push( Box::new( client.finality_notification_stream()