Skip to content

Commit

Permalink
Block verification shouldn't require &self, but this is not support…
Browse files Browse the repository at this point in the history
…ed fully by some consensus engines, so introduce `Verifier::supports_stateless_verification()` that can be used to check this
  • Loading branch information
nazar-pc committed Sep 17, 2023
1 parent f1dc9d5 commit a64e29d
Show file tree
Hide file tree
Showing 16 changed files with 47 additions and 30 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cumulus/client/consensus/aura/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ edition.workspace = true
async-trait = "0.1.73"
codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive" ] }
futures = "0.3.28"
parking_lot = "0.12.1"
tracing = "0.1.37"
schnellru = "0.2.1"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
/// should be thrown out and which ones should be kept.
use codec::Codec;
use cumulus_client_consensus_common::ParachainBlockImportMarker;
use parking_lot::Mutex;
use schnellru::{ByLength, LruMap};

use sc_consensus::{
Expand Down Expand Up @@ -71,7 +72,7 @@ struct Verifier<P, Client, Block, CIDP> {
client: Arc<Client>,
create_inherent_data_providers: CIDP,
slot_duration: SlotDuration,
defender: NaiveEquivocationDefender,
defender: Mutex<NaiveEquivocationDefender>,
telemetry: Option<TelemetryHandle>,
_phantom: std::marker::PhantomData<fn() -> (Block, P)>,
}
Expand All @@ -89,7 +90,7 @@ where
CIDP: CreateInherentDataProviders<Block, ()>,
{
async fn verify(
&mut self,
&self,
mut block_params: BlockImportParams<Block>,
) -> Result<BlockImportParams<Block>, String> {
// Skip checks that include execution, if being told so, or when importing only state.
Expand Down Expand Up @@ -132,7 +133,7 @@ where
block_params.post_hash = Some(post_hash);

// Check for and reject egregious amounts of equivocations.
if self.defender.insert_and_check(slot) {
if self.defender.lock().insert_and_check(slot) {
return Err(format!(
"Rejecting block {:?} due to excessive equivocations at slot",
post_hash,
Expand Down Expand Up @@ -239,7 +240,7 @@ where
let verifier = Verifier::<P, _, _, _> {
client,
create_inherent_data_providers,
defender: NaiveEquivocationDefender::default(),
defender: Mutex::new(NaiveEquivocationDefender::default()),
slot_duration,
telemetry,
_phantom: std::marker::PhantomData,
Expand Down
2 changes: 1 addition & 1 deletion cumulus/client/consensus/common/src/import_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub struct VerifyNothing;
#[async_trait::async_trait]
impl<Block: BlockT> Verifier<Block> for VerifyNothing {
async fn verify(
&mut self,
&self,
params: BlockImportParams<Block>,
) -> Result<BlockImportParams<Block>, String> {
Ok(params)
Expand Down
2 changes: 1 addition & 1 deletion cumulus/client/consensus/relay-chain/src/import_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ where
CIDP: CreateInherentDataProviders<Block, ()>,
{
async fn verify(
&mut self,
&self,
mut block_params: BlockImportParams<Block>,
) -> Result<BlockImportParams<Block>, String> {
// Skip checks that include execution, if being told so, or when importing only state.
Expand Down
8 changes: 4 additions & 4 deletions cumulus/polkadot-parachain/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,7 @@ where

struct Verifier<Client, AuraId> {
client: Arc<Client>,
aura_verifier: BuildOnAccess<Box<dyn VerifierT<Block>>>,
aura_verifier: Mutex<BuildOnAccess<Box<dyn VerifierT<Block>>>>,
relay_chain_verifier: Box<dyn VerifierT<Block>>,
_phantom: PhantomData<AuraId>,
}
Expand All @@ -1035,7 +1035,7 @@ where
AuraId: Send + Sync + Codec,
{
async fn verify(
&mut self,
&self,
block_import: BlockImportParams<Block>,
) -> Result<BlockImportParams<Block>, String> {
if self
Expand All @@ -1044,7 +1044,7 @@ where
.has_api::<dyn AuraApi<Block, AuraId>>(*block_import.header.parent_hash())
.unwrap_or(false)
{
self.aura_verifier.get_mut().verify(block_import).await
self.aura_verifier.lock().await.get_mut().verify(block_import).await
} else {
self.relay_chain_verifier.verify(block_import).await
}
Expand Down Expand Up @@ -1104,7 +1104,7 @@ where
let verifier = Verifier {
client,
relay_chain_verifier,
aura_verifier: BuildOnAccess::Uninitialized(Some(Box::new(aura_verifier))),
aura_verifier: Mutex::new(BuildOnAccess::Uninitialized(Some(Box::new(aura_verifier)))),
_phantom: PhantomData,
};

Expand Down
2 changes: 1 addition & 1 deletion substrate/client/consensus/aura/src/import_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ where
CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
{
async fn verify(
&mut self,
&self,
mut block: BlockImportParams<B>,
) -> Result<BlockImportParams<B>, String> {
// Skip checks that include execution, if being told so or when importing only state.
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/consensus/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,7 @@ where
CIDP::InherentDataProviders: InherentDataProviderExt + Send + Sync,
{
async fn verify(
&mut self,
&self,
mut block: BlockImportParams<Block>,
) -> Result<BlockImportParams<Block>, String> {
trace!(
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ impl Verifier<TestBlock> for TestVerifier {
/// new set of validators to import. If not, err with an Error-Message
/// presented to the User in the logs.
async fn verify(
&mut self,
&self,
mut block: BlockImportParams<TestBlock>,
) -> Result<BlockImportParams<TestBlock>, String> {
// apply post-sealing mutations (i.e. stripping seal, if desired).
Expand Down
17 changes: 14 additions & 3 deletions substrate/client/consensus/common/src/import_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,22 @@ pub struct IncomingBlock<B: BlockT> {

/// Verify a justification of a block
#[async_trait::async_trait]
pub trait Verifier<B: BlockT>: Send {
pub trait Verifier<B: BlockT>: Send + Sync {
/// Whether verifier supports stateless verification.
///
/// Stateless verification means that verification on blocks can be done in arbitrary order,
/// doesn't expect parent block to be imported first, etc.
///
/// Verifiers that support stateless verification can verify multiple blocks concurrently,
/// significantly improving sync speed.
fn supports_stateless_verification(&self) -> bool {
// Unless re-defined by verifier is assumed to not support stateless verification.
false
}

/// Verify the given block data and return the `BlockImportParams` to
/// continue the block import process.
async fn verify(&mut self, block: BlockImportParams<B>)
-> Result<BlockImportParams<B>, String>;
async fn verify(&self, block: BlockImportParams<B>) -> Result<BlockImportParams<B>, String>;
}

/// Blocks import queue API.
Expand Down
16 changes: 11 additions & 5 deletions substrate/client/consensus/common/src/import_queue/basic_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,16 @@ impl<B: BlockT> BasicQueue<B> {
/// Instantiate a new basic queue, with given verifier.
///
/// This creates a background task, and calls `on_start` on the justification importer.
pub fn new<V: 'static + Verifier<B>>(
pub fn new<V>(
verifier: V,
block_import: BoxBlockImport<B>,
justification_import: Option<BoxJustificationImport<B>>,
spawner: &impl sp_core::traits::SpawnEssentialNamed,
prometheus_registry: Option<&Registry>,
) -> Self {
) -> Self
where
V: Verifier<B> + 'static,
{
let (result_sender, result_port) = buffered_link::buffered_link(100_000);

let metrics = prometheus_registry.and_then(|r| {
Expand Down Expand Up @@ -252,7 +255,7 @@ struct BlockImportWorker<B: BlockT> {
}

impl<B: BlockT> BlockImportWorker<B> {
fn new<V: 'static + Verifier<B>>(
fn new<V>(
result_sender: BufferedLinkSender<B>,
verifier: V,
block_import: BoxBlockImport<B>,
Expand All @@ -262,7 +265,10 @@ impl<B: BlockT> BlockImportWorker<B> {
impl Future<Output = ()> + Send,
TracingUnboundedSender<worker_messages::ImportJustification<B>>,
TracingUnboundedSender<worker_messages::ImportBlocks<B>>,
) {
)
where
V: Verifier<B> + 'static,
{
use worker_messages::*;

let (justification_sender, mut justification_port) =
Expand Down Expand Up @@ -501,7 +507,7 @@ mod tests {
#[async_trait::async_trait]
impl Verifier<Block> for () {
async fn verify(
&mut self,
&self,
block: BlockImportParams<Block>,
) -> Result<BlockImportParams<Block>, String> {
Ok(BlockImportParams::new(block.origin, block.header))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ where
C: HeaderBackend<B> + HeaderMetadata<B, Error = sp_blockchain::Error>,
{
async fn verify(
&mut self,
&self,
mut import_params: BlockImportParams<B>,
) -> Result<BlockImportParams<B>, String> {
import_params.finalized = false;
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/consensus/manual-seal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct ManualSealVerifier;
#[async_trait::async_trait]
impl<B: BlockT> Verifier<B> for ManualSealVerifier {
async fn verify(
&mut self,
&self,
mut block: BlockImportParams<B>,
) -> Result<BlockImportParams<B>, String> {
block.finalized = false;
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/consensus/pow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ where
Algorithm::Difficulty: 'static + Send,
{
async fn verify(
&mut self,
&self,
mut block: BlockImportParams<B>,
) -> Result<BlockImportParams<B>, String> {
let hash = block.header.hash();
Expand Down
7 changes: 2 additions & 5 deletions substrate/client/network/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl PassThroughVerifier {
#[async_trait::async_trait]
impl<B: BlockT> Verifier<B> for PassThroughVerifier {
async fn verify(
&mut self,
&self,
mut block: BlockImportParams<B>,
) -> Result<BlockImportParams<B>, String> {
if block.fork_choice.is_none() {
Expand Down Expand Up @@ -608,10 +608,7 @@ struct VerifierAdapter<B: BlockT> {

#[async_trait::async_trait]
impl<B: BlockT> Verifier<B> for VerifierAdapter<B> {
async fn verify(
&mut self,
block: BlockImportParams<B>,
) -> Result<BlockImportParams<B>, String> {
async fn verify(&self, block: BlockImportParams<B>) -> Result<BlockImportParams<B>, String> {
let hash = block.header.hash();
self.verifier.lock().await.verify(block).await.map_err(|e| {
self.failed_verifications.lock().insert(hash, e.clone());
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/network/test/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl TestNetworkBuilder {
#[async_trait::async_trait]
impl<B: BlockT> sc_consensus::Verifier<B> for PassThroughVerifier {
async fn verify(
&mut self,
&self,
mut block: sc_consensus::BlockImportParams<B>,
) -> Result<sc_consensus::BlockImportParams<B>, String> {
block.finalized = self.0;
Expand Down

0 comments on commit a64e29d

Please sign in to comment.