Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

removes use of sc_client::Client from sc_finality_grandpa #5030

Merged
merged 5 commits into from
Feb 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions bin/node-template/node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ macro_rules! new_full_start {
.ok_or_else(|| sc_service::Error::SelectChainRequired)?;

let (grandpa_block_import, grandpa_link) =
grandpa::block_import::<_, _, _, node_template_runtime::RuntimeApi, _>(
client.clone(), &*client, select_chain
)?;
grandpa::block_import(client.clone(), &*client, select_chain)?;

let aura_block_import = sc_consensus_aura::AuraBlockImport::<_, _, _, AuraPair>::new(
grandpa_block_import.clone(), client.clone(),
Expand Down Expand Up @@ -202,7 +200,7 @@ pub fn new_light(config: Configuration<GenesisConfig>)
let fetch_checker = fetcher
.map(|fetcher| fetcher.checker().clone())
.ok_or_else(|| "Trying to start light import queue without active fetch checker")?;
let grandpa_block_import = grandpa::light_block_import::<_, _, _, RuntimeApi>(
let grandpa_block_import = grandpa::light_block_import(
client.clone(), backend, &*client.clone(), Arc::new(fetch_checker),
)?;
let finality_proof_import = grandpa_block_import.clone();
Expand Down
2 changes: 1 addition & 1 deletion bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ pub fn new_light(config: NodeConfiguration)
let fetch_checker = fetcher
.map(|fetcher| fetcher.checker().clone())
.ok_or_else(|| "Trying to start light import queue without active fetch checker")?;
let grandpa_block_import = grandpa::light_block_import::<_, _, _, RuntimeApi>(
let grandpa_block_import = grandpa::light_block_import(
client.clone(),
backend,
&*client,
Expand Down
9 changes: 9 additions & 0 deletions client/api/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,15 @@ pub trait BlockImportOperation<Block: BlockT> {
fn mark_head(&mut self, id: BlockId<Block>) -> sp_blockchain::Result<()>;
}

/// Interface for performing operations on the backend.
pub trait LockImportRun<Block: BlockT, B: Backend<Block>> {
/// Lock the import lock, and run operations inside.
fn lock_import_and_run<R, Err, F>(&self, f: F) -> Result<R, Err>
where
F: FnOnce(&mut ClientImportOperation<Block, B>) -> Result<R, Err>,
Err: From<sp_blockchain::Error>;
}

/// Finalize Facilities
pub trait Finalizer<Block: BlockT, B: Backend<Block>> {
/// Mark all blocks up to given as finalized in operation.
Expand Down
3 changes: 2 additions & 1 deletion client/finality-grandpa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ assert_matches = "1.3.0"
parity-scale-codec = { version = "1.0.0", features = ["derive"] }
sp-arithmetic = { version = "2.0.0-alpha.2", path = "../../primitives/arithmetic" }
sp-runtime = { version = "2.0.0-alpha.2", path = "../../primitives/runtime" }
sp-consensus = { version = "0.8.0-alpha.2", path = "../../primitives/consensus/common" }
sp-consensus = { version = "0.8.0-alpha.1", path = "../../primitives/consensus/common" }
sp-core = { version = "2.0.0-alpha.2", path = "../../primitives/core" }
sp-api = { version = "2.0.0-alpha.2", path = "../../primitives/api" }
sc-telemetry = { version = "2.0.0-alpha.2", path = "../telemetry" }
sc-keystore = { version = "2.0.0-alpha.2", path = "../keystore" }
serde_json = "1.0.41"
Expand Down
79 changes: 36 additions & 43 deletions client/finality-grandpa/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,14 @@ use parity_scale_codec::{Decode, Encode};
use futures::prelude::*;
use futures_timer::Delay;
use parking_lot::RwLock;
use sp_blockchain::{HeaderBackend, Error as ClientError};
use sp_blockchain::{HeaderBackend, Error as ClientError, HeaderMetadata};
use std::marker::PhantomData;

use sc_client_api::{
BlockchainEvents,
backend::{AuxStore, Backend},
Finalizer,
call_executor::CallExecutor,
backend::Backend,
utils::is_descendent_of,
};
use sc_client::{
apply_aux, Client,
};
use sc_client::apply_aux;
use finality_grandpa::{
BlockNumberOps, Equivocation, Error as GrandpaError, round::State as RoundState,
voter, voter_set::VoterSet,
Expand Down Expand Up @@ -377,8 +373,8 @@ impl<Block: BlockT> SharedVoterSetState<Block> {
}

/// The environment we run GRANDPA in.
pub(crate) struct Environment<B, E, Block: BlockT, N: NetworkT<Block>, RA, SC, VR> {
pub(crate) client: Arc<Client<B, E, Block, RA>>,
pub(crate) struct Environment<Backend, Block: BlockT, C, N: NetworkT<Block>, SC, VR> {
pub(crate) client: Arc<C>,
pub(crate) select_chain: SC,
pub(crate) voters: Arc<VoterSet<AuthorityId>>,
pub(crate) config: Config,
Expand All @@ -388,9 +384,10 @@ pub(crate) struct Environment<B, E, Block: BlockT, N: NetworkT<Block>, RA, SC, V
pub(crate) set_id: SetId,
pub(crate) voter_set_state: SharedVoterSetState<Block>,
pub(crate) voting_rule: VR,
pub(crate) _phantom: PhantomData<Backend>,
}

impl<B, E, Block: BlockT, N: NetworkT<Block>, RA, SC, VR> Environment<B, E, Block, N, RA, SC, VR> {
impl<Backend, Block: BlockT, C, N: NetworkT<Block>, SC, VR> Environment<Backend, Block, C, N, SC, VR> {
/// 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.
Expand All @@ -406,17 +403,16 @@ impl<B, E, Block: BlockT, N: NetworkT<Block>, RA, SC, VR> Environment<B, E, Bloc
}
}

impl<Block: BlockT, B, E, N, RA, SC, VR>
impl<BE, Block: BlockT, C, N, SC, VR>
finality_grandpa::Chain<Block::Hash, NumberFor<Block>>
for Environment<B, E, Block, N, RA, SC, VR>
for Environment<BE, Block, C, N, SC, VR>
where
Block: 'static,
B: Backend<Block> + 'static,
E: CallExecutor<Block> + Send + Sync,
BE: Backend<Block>,
C: crate::ClientForGrandpa<Block, BE>,
N: NetworkT<Block> + 'static + Send,
SC: SelectChain<Block> + 'static,
VR: VotingRule<Block, Client<B, E, Block, RA>>,
RA: Send + Sync,
VR: VotingRule<Block, C>,
NumberFor<Block>: BlockNumberOps,
{
fn ancestry(&self, base: Block::Hash, block: Block::Hash) -> Result<Vec<Block::Hash>, GrandpaError> {
Expand All @@ -432,7 +428,7 @@ where
return None;
}

let base_header = match self.client.header(&BlockId::Hash(block)).ok()? {
let base_header = match self.client.header(BlockId::Hash(block)).ok()? {
Some(h) => h,
None => {
debug!(target: "afg", "Encountered error finding best chain containing {:?}: couldn't find base block", block);
Expand All @@ -450,7 +446,7 @@ where

match self.select_chain.finality_target(block, None) {
Ok(Some(best_hash)) => {
let best_header = self.client.header(&BlockId::Hash(best_hash)).ok()?
let best_header = self.client.header(BlockId::Hash(best_hash)).ok()?
.expect("Header known to exist after `finality_target` call; qed");

// check if our vote is currently being limited due to a pending change
Expand All @@ -474,7 +470,7 @@ where
break;
}

target_header = self.client.header(&BlockId::Hash(*target_header.parent_hash())).ok()?
target_header = self.client.header(BlockId::Hash(*target_header.parent_hash())).ok()?
.expect("Header known to exist after `finality_target` call; qed");
}

Expand Down Expand Up @@ -519,17 +515,16 @@ where
}


pub(crate) fn ancestry<B, Block: BlockT, E, RA>(
client: &Client<B, E, Block, RA>,
pub(crate) fn ancestry<Block: BlockT, Client>(
client: &Arc<Client>,
base: Block::Hash,
block: Block::Hash,
) -> Result<Vec<Block::Hash>, GrandpaError> where
B: Backend<Block>,
E: CallExecutor<Block>,
Client: HeaderMetadata<Block, Error = sp_blockchain::Error>,
{
if base == block { return Err(GrandpaError::NotDescendent) }

let tree_route_res = sp_blockchain::tree_route(client, block, base);
let tree_route_res = sp_blockchain::tree_route(&**client, block, base);

let tree_route = match tree_route_res {
Ok(tree_route) => tree_route,
Expand All @@ -550,19 +545,17 @@ pub(crate) fn ancestry<B, Block: BlockT, E, RA>(
Ok(tree_route.retracted().iter().skip(1).map(|e| e.hash).collect())
}

impl<B, E, Block: BlockT, N, RA, SC, VR>
impl<B, Block: BlockT, C, N, SC, VR>
voter::Environment<Block::Hash, NumberFor<Block>>
for Environment<B, E, Block, N, RA, SC, VR>
for Environment<B, Block, C, N, SC, VR>
where
Block: 'static,
B: Backend<Block> + 'static,
E: CallExecutor<Block> + 'static + Send + Sync,
B: Backend<Block>,
C: crate::ClientForGrandpa<Block, B> + 'static,
N: NetworkT<Block> + 'static + Send,
RA: 'static + Send + Sync,
SC: SelectChain<Block> + 'static,
VR: VotingRule<Block, Client<B, E, Block, RA>>,
VR: VotingRule<Block, C>,
NumberFor<Block>: BlockNumberOps,
Client<B, E, Block, RA>: AuxStore,
{
type Timer = Pin<Box<dyn Future<Output = Result<(), Self::Error>> + Send>>;
type Id = AuthorityId;
Expand Down Expand Up @@ -882,7 +875,7 @@ where
commit: Commit<Block>,
) -> Result<(), Self::Error> {
finalize_block(
&*self.client,
self.client.clone(),
&self.authority_set,
&self.consensus_changes,
Some(self.config.justification_period.into()),
Expand Down Expand Up @@ -940,25 +933,25 @@ impl<Block: BlockT> From<GrandpaJustification<Block>> for JustificationOrCommit<
/// authority set change is enacted then a justification is created (if not
/// given) and stored with the block when finalizing it.
/// This method assumes that the block being finalized has already been imported.
pub(crate) fn finalize_block<B, Block: BlockT, E, RA>(
client: &Client<B, E, Block, RA>,
pub(crate) fn finalize_block<BE, Block, Client>(
client: Arc<Client>,
authority_set: &SharedAuthoritySet<Block::Hash, NumberFor<Block>>,
consensus_changes: &SharedConsensusChanges<Block::Hash, NumberFor<Block>>,
justification_period: Option<NumberFor<Block>>,
hash: Block::Hash,
number: NumberFor<Block>,
justification_or_commit: JustificationOrCommit<Block>,
) -> Result<(), CommandOrError<Block::Hash, NumberFor<Block>>> where
B: Backend<Block>,
E: CallExecutor<Block> + Send + Sync,
RA: Send + Sync,
Block: BlockT,
BE: Backend<Block>,
Client: crate::ClientForGrandpa<Block, BE>,
{
// NOTE: lock must be held through writing to DB to avoid race. this lock
// also implicitly synchronizes the check for last finalized number
// below.
let mut authority_set = authority_set.inner().write();

let status = client.chain_info();
let status = client.info();
if number <= status.finalized_number && client.hash(number)? == Some(hash) {
// This can happen after a forced change (triggered by the finality tracker when finality is stalled), since
// the voter will be restarted at the median last finalized block, which can be lower than the local best
Expand All @@ -981,14 +974,14 @@ pub(crate) fn finalize_block<B, Block: BlockT, E, RA>(
let mut consensus_changes = consensus_changes.lock();
let canon_at_height = |canon_number| {
// "true" because the block is finalized
canonical_at_height(client, (hash, number), true, canon_number)
canonical_at_height(&*client, (hash, number), true, canon_number)
};

let update_res: Result<_, Error> = client.lock_import_and_run(|import_op| {
let status = authority_set.apply_standard_changes(
hash,
number,
&is_descendent_of::<Block, _>(client, None),
&is_descendent_of::<Block, _>(&*client, None),
).map_err(|e| Error::Safety(e.to_string()))?;

// check if this is this is the first finalization of some consensus changes
Expand Down Expand Up @@ -1031,7 +1024,7 @@ pub(crate) fn finalize_block<B, Block: BlockT, E, RA>(
// finalization to remote nodes
if !justification_required {
if let Some(justification_period) = justification_period {
let last_finalized_number = client.chain_info().finalized_number;
let last_finalized_number = client.info().finalized_number;
justification_required =
(!last_finalized_number.is_zero() || number - last_finalized_number == justification_period) &&
(last_finalized_number / justification_period != number / justification_period);
Expand All @@ -1040,7 +1033,7 @@ pub(crate) fn finalize_block<B, Block: BlockT, E, RA>(

if justification_required {
let justification = GrandpaJustification::from_commit(
client,
&client,
round_number,
commit,
)?;
Expand Down
Loading