Clean up the basic-authorship crate (#3206)

* Switch consensus-common to new futures

* Fix tests

* More tests fixing

* Make Proposer, OnSlot and SyncOracle mut

* Make the Environment mut as well

* Fix test

* Fix Babe tests

* Babe fixes

* Fix CLI service tests

* Fix Babe tests

* Remove unnecessary trait bound

* Inline the code of BlockBuilder and AuthoringApi

* Remove warning lint

* Bounds simplification

* Imports simplification

* Don't panic on bad generated block

* Code style

* Add doc example

* Remove dependency on aura

* Order dependencies alphabetically

* Minor style
This commit is contained in:
Pierre Krieger
2019-07-28 02:24:51 +02:00
committed by DemiMarie-parity
parent ba55d31d44
commit 9370a4a6b6
13 changed files with 203 additions and 219 deletions
@@ -18,101 +18,25 @@
// FIXME #1021 move this into substrate-consensus-common
//
use std::{self, time, sync::Arc};
use log::{info, debug, trace};
use std::{time, sync::Arc};
use client::{
self, error, Client as SubstrateClient, CallExecutor,
block_builder::api::BlockBuilder as BlockBuilderApi, runtime_api::Core,
error, Client as SubstrateClient, CallExecutor,
block_builder::api::BlockBuilder as BlockBuilderApi,
};
use codec::Decode;
use consensus_common::{self, evaluation};
use consensus_common::{evaluation};
use inherents::InherentData;
use log::{error, info, debug, trace};
use primitives::{H256, Blake2Hasher, ExecutionContext};
use runtime_primitives::{ApplyError, generic::BlockId};
use runtime_primitives::traits::{
Block as BlockT, Hash as HashT, Header as HeaderT, ProvideRuntimeApi,
DigestFor,
DigestFor, BlakeTwo256,
};
use runtime_primitives::generic::BlockId;
use runtime_primitives::ApplyError;
use transaction_pool::txpool::{self, Pool as TransactionPool};
use inherents::InherentData;
use substrate_telemetry::{telemetry, CONSENSUS_INFO};
/// Build new blocks.
pub trait BlockBuilder<Block: BlockT> {
/// Push an extrinsic onto the block. Fails if the extrinsic is invalid.
fn push_extrinsic(&mut self, extrinsic: <Block as BlockT>::Extrinsic) -> Result<(), error::Error>;
}
/// Local client abstraction for the consensus.
pub trait AuthoringApi: Send + Sync + ProvideRuntimeApi where
<Self as ProvideRuntimeApi>::Api: Core<Self::Block>
{
/// The block used for this API type.
type Block: BlockT;
/// The error used by this API type.
type Error: std::error::Error;
/// Build a block on top of the given block, with inherent extrinsics and
/// inherent digests pre-pushed.
fn build_block<F: FnMut(&mut dyn BlockBuilder<Self::Block>) -> ()>(
&self,
at: &BlockId<Self::Block>,
inherent_data: InherentData,
inherent_digests: DigestFor<Self::Block>,
build_ctx: F,
) -> Result<Self::Block, error::Error>;
}
impl<'a, B, E, Block, RA> BlockBuilder<Block>
for client::block_builder::BlockBuilder<'a, Block, SubstrateClient<B, E, Block, RA>>
where
B: client::backend::Backend<Block, Blake2Hasher> + 'static,
E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
Block: BlockT<Hash=H256>,
RA: Send + Sync + 'static,
SubstrateClient<B, E, Block, RA> : ProvideRuntimeApi,
<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
{
fn push_extrinsic(&mut self, extrinsic: <Block as BlockT>::Extrinsic) -> Result<(), error::Error> {
client::block_builder::BlockBuilder::push(self, extrinsic).map_err(Into::into)
}
}
impl<B, E, Block, RA> AuthoringApi for SubstrateClient<B, E, Block, RA> where
B: client::backend::Backend<Block, Blake2Hasher> + Send + Sync + 'static,
E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
Block: BlockT<Hash=H256>,
RA: Send + Sync + 'static,
SubstrateClient<B, E, Block, RA> : ProvideRuntimeApi,
<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
{
type Block = Block;
type Error = client::error::Error;
fn build_block<F: FnMut(&mut dyn BlockBuilder<Self::Block>) -> ()>(
&self,
at: &BlockId<Self::Block>,
inherent_data: InherentData,
inherent_digests: DigestFor<Self::Block>,
mut build_ctx: F,
) -> Result<Self::Block, error::Error> {
let mut block_builder = self.new_block_at(at, inherent_digests)?;
let runtime_api = self.runtime_api();
// We don't check the API versions any further here since the dispatch compatibility
// check should be enough.
runtime_api.inherent_extrinsics_with_context(at, ExecutionContext::BlockConstruction, inherent_data)?
.into_iter().try_for_each(|i| block_builder.push(i))?;
build_ctx(&mut block_builder);
block_builder.bake().map_err(Into::into)
}
}
/// Proposer factory.
pub struct ProposerFactory<C, A> where A: txpool::ChainApi {
/// The client instance.
@@ -121,19 +45,23 @@ pub struct ProposerFactory<C, A> where A: txpool::ChainApi {
pub transaction_pool: Arc<TransactionPool<A>>,
}
impl<C, A> consensus_common::Environment<<C as AuthoringApi>::Block> for ProposerFactory<C, A> where
C: AuthoringApi,
<C as ProvideRuntimeApi>::Api: BlockBuilderApi<<C as AuthoringApi>::Block>,
A: txpool::ChainApi<Block=<C as AuthoringApi>::Block>,
client::error::Error: From<<C as AuthoringApi>::Error>,
Proposer<<C as AuthoringApi>::Block, C, A>: consensus_common::Proposer<<C as AuthoringApi>::Block>,
impl<B, E, Block, RA, A> consensus_common::Environment<Block> for
ProposerFactory<SubstrateClient<B, E, Block, RA>, A>
where
A: txpool::ChainApi<Block=Block>,
B: client::backend::Backend<Block, Blake2Hasher> + Send + Sync + 'static,
E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
Block: BlockT<Hash=H256>,
RA: Send + Sync + 'static,
SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi,
<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
{
type Proposer = Proposer<<C as AuthoringApi>::Block, C, A>;
type Proposer = Proposer<Block, SubstrateClient<B, E, Block, RA>, A>;
type Error = error::Error;
fn init(
&self,
parent_header: &<<C as AuthoringApi>::Block as BlockT>::Header,
&mut self,
parent_header: &<Block as BlockT>::Header,
) -> Result<Self::Proposer, error::Error> {
let parent_hash = parent_header.hash();
@@ -164,18 +92,22 @@ pub struct Proposer<Block: BlockT, C, A: txpool::ChainApi> {
now: Box<dyn Fn() -> time::Instant>,
}
impl<Block, C, A> consensus_common::Proposer<<C as AuthoringApi>::Block> for Proposer<Block, C, A> where
Block: BlockT,
C: AuthoringApi<Block=Block>,
<C as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
impl<B, E, Block, RA, A> consensus_common::Proposer<Block> for
Proposer<Block, SubstrateClient<B, E, Block, RA>, A>
where
A: txpool::ChainApi<Block=Block>,
client::error::Error: From<<C as AuthoringApi>::Error>
B: client::backend::Backend<Block, Blake2Hasher> + Send + Sync + 'static,
E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
Block: BlockT<Hash=H256>,
RA: Send + Sync + 'static,
SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi,
<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
{
type Create = futures::future::Ready<Result<<C as AuthoringApi>::Block, error::Error>>;
type Create = futures::future::Ready<Result<Block, error::Error>>;
type Error = error::Error;
fn propose(
&self,
&mut self,
inherent_data: InherentData,
inherent_digests: DigestFor<Block>,
max_duration: time::Duration,
@@ -186,80 +118,89 @@ impl<Block, C, A> consensus_common::Proposer<<C as AuthoringApi>::Block> for Pro
}
}
impl<Block, C, A> Proposer<Block, C, A> where
Block: BlockT,
C: AuthoringApi<Block=Block>,
<C as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
impl<Block, B, E, RA, A> Proposer<Block, SubstrateClient<B, E, Block, RA>, A> where
A: txpool::ChainApi<Block=Block>,
client::error::Error: From<<C as AuthoringApi>::Error>,
B: client::backend::Backend<Block, Blake2Hasher> + Send + Sync + 'static,
E: CallExecutor<Block, Blake2Hasher> + Send + Sync + Clone + 'static,
Block: BlockT<Hash=H256>,
RA: Send + Sync + 'static,
SubstrateClient<B, E, Block, RA>: ProvideRuntimeApi,
<SubstrateClient<B, E, Block, RA> as ProvideRuntimeApi>::Api: BlockBuilderApi<Block>,
{
fn propose_with(
&self,
inherent_data: InherentData,
inherent_digests: DigestFor<Block>,
deadline: time::Instant,
) -> Result<<C as AuthoringApi>::Block, error::Error>
{
use runtime_primitives::traits::BlakeTwo256;
) -> Result<Block, error::Error> {
/// If the block is full we will attempt to push at most
/// this number of transactions before quitting for real.
/// It allows us to increase block utilization.
const MAX_SKIPPED_TRANSACTIONS: usize = 8;
let block = self.client.build_block(
&self.parent_id,
inherent_data,
inherent_digests.clone(),
|block_builder| {
// proceed with transactions
let mut is_first = true;
let mut skipped = 0;
let mut unqueue_invalid = Vec::new();
let pending_iterator = self.transaction_pool.ready();
let mut block_builder = self.client.new_block_at(&self.parent_id, inherent_digests)?;
debug!("Attempting to push transactions from the pool.");
for pending in pending_iterator {
if (self.now)() > deadline {
debug!("Consensus deadline reached when pushing block transactions, proceeding with proposing.");
// We don't check the API versions any further here since the dispatch compatibility
// check should be enough.
for extrinsic in self.client.runtime_api()
.inherent_extrinsics_with_context(
&self.parent_id,
ExecutionContext::BlockConstruction,
inherent_data
)?
{
block_builder.push(extrinsic)?;
}
// proceed with transactions
let mut is_first = true;
let mut skipped = 0;
let mut unqueue_invalid = Vec::new();
let pending_iterator = self.transaction_pool.ready();
debug!("Attempting to push transactions from the pool.");
for pending in pending_iterator {
if (self.now)() > deadline {
debug!("Consensus deadline reached when pushing block transactions, proceeding with proposing.");
break;
}
trace!("[{:?}] Pushing to the block.", pending.hash);
match client::block_builder::BlockBuilder::push(&mut block_builder, pending.data.clone()) {
Ok(()) => {
debug!("[{:?}] Pushed to the block.", pending.hash);
}
Err(error::Error::ApplyExtrinsicFailed(ApplyError::FullBlock)) => {
if is_first {
debug!("[{:?}] Invalid transaction: FullBlock on empty block", pending.hash);
unqueue_invalid.push(pending.hash.clone());
} else if skipped < MAX_SKIPPED_TRANSACTIONS {
skipped += 1;
debug!(
"Block seems full, but will try {} more transactions before quitting.",
MAX_SKIPPED_TRANSACTIONS - skipped
);
} else {
debug!("Block is full, proceed with proposing.");
break;
}
trace!("[{:?}] Pushing to the block.", pending.hash);
match block_builder.push_extrinsic(pending.data.clone()) {
Ok(()) => {
debug!("[{:?}] Pushed to the block.", pending.hash);
}
Err(error::Error::ApplyExtrinsicFailed(ApplyError::FullBlock)) => {
if is_first {
debug!("[{:?}] Invalid transaction: FullBlock on empty block", pending.hash);
unqueue_invalid.push(pending.hash.clone());
} else if skipped < MAX_SKIPPED_TRANSACTIONS {
skipped += 1;
debug!(
"Block seems full, but will try {} more transactions before quitting.",
MAX_SKIPPED_TRANSACTIONS - skipped
);
} else {
debug!("Block is full, proceed with proposing.");
break;
}
}
Err(e) => {
debug!("[{:?}] Invalid transaction: {}", pending.hash, e);
unqueue_invalid.push(pending.hash.clone());
}
}
is_first = false;
}
Err(e) => {
debug!("[{:?}] Invalid transaction: {}", pending.hash, e);
unqueue_invalid.push(pending.hash.clone());
}
}
self.transaction_pool.remove_invalid(&unqueue_invalid);
})?;
is_first = false;
}
self.transaction_pool.remove_invalid(&unqueue_invalid);
let block = block_builder.bake()?;
info!("Prepared block for proposing at {} [hash: {:?}; parent_hash: {}; extrinsics: [{}]]",
block.header().number(),
<<C as AuthoringApi>::Block as BlockT>::Hash::from(block.header().hash()),
<Block as BlockT>::Hash::from(block.header().hash()),
block.header().parent_hash(),
block.extrinsics()
.iter()
@@ -269,19 +210,18 @@ impl<Block, C, A> Proposer<Block, C, A> where
);
telemetry!(CONSENSUS_INFO; "prepared_block_for_proposing";
"number" => ?block.header().number(),
"hash" => ?<<C as AuthoringApi>::Block as BlockT>::Hash::from(block.header().hash()),
"hash" => ?<Block as BlockT>::Hash::from(block.header().hash()),
);
let substrate_block = Decode::decode(&mut block.encode().as_slice())
.expect("blocks are defined to serialize to substrate blocks correctly; qed");
if Decode::decode(&mut block.encode().as_slice()).as_ref() != Some(&block) {
error!("Failed to verify block encoding/decoding");
}
assert!(evaluation::evaluate_initial(
&substrate_block,
&self.parent_hash,
self.parent_number,
).is_ok());
if let Err(err) = evaluation::evaluate_initial(&block, &self.parent_hash, self.parent_number) {
error!("Failed to evaluate authored block: {:?}", err);
}
Ok(substrate_block)
Ok(block)
}
}
@@ -311,7 +251,7 @@ mod tests {
txpool.submit_at(&BlockId::number(0), vec![extrinsic(0), extrinsic(1)]).unwrap();
let proposer_factory = ProposerFactory {
let mut proposer_factory = ProposerFactory {
client: client.clone(),
transaction_pool: txpool.clone(),
};
+38 -3
View File
@@ -15,9 +15,44 @@
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.
//! Basic implementation of block-authoring logic.
#![warn(unused_extern_crates)]
//!
//! # Example
//!
//! ```
//! # use substrate_basic_authorship::ProposerFactory;
//! # use consensus_common::{Environment, Proposer};
//! # use runtime_primitives::generic::BlockId;
//! # use std::{sync::Arc, time::Duration};
//! # use test_client::{self, runtime::{Extrinsic, Transfer}, AccountKeyring};
//! # use transaction_pool::txpool::{self, Pool as TransactionPool};
//! # let client = Arc::new(test_client::new());
//! # let chain_api = transaction_pool::ChainApi::new(client.clone());
//! # let txpool = Arc::new(TransactionPool::new(Default::default(), chain_api));
//! // The first step is to create a `ProposerFactory`.
//! let mut proposer_factory = ProposerFactory {
//! client: client.clone(),
//! transaction_pool: txpool.clone(),
//! };
//!
//! // From this factory, we create a `Proposer`.
//! let mut proposer = proposer_factory.init(
//! &client.header(&BlockId::number(0)).unwrap().unwrap(),
//! ).unwrap();
//!
//! // This `Proposer` allows us to create a block proposition.
//! // The proposer will grab transactions from the transaction pool, and put them into the block.
//! let future = proposer.propose(
//! Default::default(),
//! Default::default(),
//! Duration::from_secs(2)
//! );
//!
//! // We wait until the proposition is performed.
//! let block = futures::executor::block_on(future).unwrap();
//! println!("Generated block: {:?}", block);
//! ```
//!
mod basic_authorship;
pub use crate::basic_authorship::{ProposerFactory, BlockBuilder, AuthoringApi, Proposer};
pub use crate::basic_authorship::{ProposerFactory, Proposer};