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

Commit

Permalink
removes use of sc_client::Client from sc_finality_grandpa (#5030)
Browse files Browse the repository at this point in the history
* removes use of sc_client::Client from sc_finality_grandpa

* code formatting

* code formatting

* removes use of sc_client::Client from sc_finality_grandpa
  • Loading branch information
seunlanlege authored Feb 27, 2020
1 parent d81f60e commit 6e6d06c
Show file tree
Hide file tree
Showing 13 changed files with 333 additions and 309 deletions.
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

0 comments on commit 6e6d06c

Please sign in to comment.