From d26d6eb2687109c39112d02294cf970c56124555 Mon Sep 17 00:00:00 2001 From: Nazar Mokrynskyi Date: Thu, 16 Dec 2021 12:01:30 +0200 Subject: [PATCH] Async block import params (#10488) * Make `SimpleSlotWorker::block_import_params()` return function that returns a future * Simplify `SimpleSlotWorker::block_import_params()` to just async method --- substrate/client/consensus/aura/src/lib.rs | 85 +++++++++---------- substrate/client/consensus/babe/src/lib.rs | 94 ++++++++++----------- substrate/client/consensus/slots/src/lib.rs | 51 +++++------ 3 files changed, 104 insertions(+), 126 deletions(-) diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs index 0d79e3c4ec..ec577b0844 100644 --- a/substrate/client/consensus/aura/src/lib.rs +++ b/substrate/client/consensus/aura/src/lib.rs @@ -382,58 +382,51 @@ where vec![>::aura_pre_digest(slot)] } - fn block_import_params( + async fn block_import_params( &self, - ) -> Box< - dyn Fn( - B::Header, - &B::Hash, - Vec, - StorageChanges, B>, - Self::Claim, - Self::EpochData, - ) -> Result< - sc_consensus::BlockImportParams>, - sp_consensus::Error, - > + Send - + 'static, + header: B::Header, + header_hash: &B::Hash, + body: Vec, + storage_changes: StorageChanges<>::Transaction, B>, + public: Self::Claim, + _epoch: Self::EpochData, + ) -> Result< + sc_consensus::BlockImportParams>::Transaction>, + sp_consensus::Error, > { - let keystore = self.keystore.clone(); - Box::new(move |header, header_hash, body, storage_changes, public, _epoch| { - // sign the pre-sealed hash of the block and then - // add it to a digest item. - let public_type_pair = public.to_public_crypto_pair(); - let public = public.to_raw_vec(); - let signature = SyncCryptoStore::sign_with( - &*keystore, - as AppKey>::ID, - &public_type_pair, - header_hash.as_ref(), + // sign the pre-sealed hash of the block and then + // add it to a digest item. + let public_type_pair = public.to_public_crypto_pair(); + let public = public.to_raw_vec(); + let signature = SyncCryptoStore::sign_with( + &*self.keystore, + as AppKey>::ID, + &public_type_pair, + header_hash.as_ref(), + ) + .map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))? + .ok_or_else(|| { + sp_consensus::Error::CannotSign( + public.clone(), + "Could not find key in keystore.".into(), ) - .map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))? - .ok_or_else(|| { - sp_consensus::Error::CannotSign( - public.clone(), - "Could not find key in keystore.".into(), - ) - })?; - let signature = signature - .clone() - .try_into() - .map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?; + })?; + let signature = signature + .clone() + .try_into() + .map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?; - let signature_digest_item = - >::aura_seal(signature); + let signature_digest_item = + >::aura_seal(signature); - let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); - import_block.post_digests.push(signature_digest_item); - import_block.body = Some(body); - import_block.state_action = - StateAction::ApplyChanges(sc_consensus::StorageChanges::Changes(storage_changes)); - import_block.fork_choice = Some(ForkChoiceStrategy::LongestChain); + let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); + import_block.post_digests.push(signature_digest_item); + import_block.body = Some(body); + import_block.state_action = + StateAction::ApplyChanges(sc_consensus::StorageChanges::Changes(storage_changes)); + import_block.fork_choice = Some(ForkChoiceStrategy::LongestChain); - Ok(import_block) - }) + Ok(import_block) } fn force_authoring(&self) -> bool { diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 168cdff43c..21c3c883a3 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -772,60 +772,52 @@ where vec![::babe_pre_digest(claim.0.clone())] } - fn block_import_params( + async fn block_import_params( &self, - ) -> Box< - dyn Fn( - B::Header, - &B::Hash, - Vec, - StorageChanges, - Self::Claim, - Self::EpochData, - ) -> Result, sp_consensus::Error> - + Send - + 'static, + header: B::Header, + header_hash: &B::Hash, + body: Vec, + storage_changes: StorageChanges<>::Transaction, B>, + (_, public): Self::Claim, + epoch_descriptor: Self::EpochData, + ) -> Result< + sc_consensus::BlockImportParams>::Transaction>, + sp_consensus::Error, > { - let keystore = self.keystore.clone(); - Box::new( - move |header, header_hash, body, storage_changes, (_, public), epoch_descriptor| { - // sign the pre-sealed hash of the block and then - // add it to a digest item. - let public_type_pair = public.clone().into(); - let public = public.to_raw_vec(); - let signature = SyncCryptoStore::sign_with( - &*keystore, - ::ID, - &public_type_pair, - header_hash.as_ref(), - ) - .map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))? - .ok_or_else(|| { - sp_consensus::Error::CannotSign( - public.clone(), - "Could not find key in keystore.".into(), - ) - })?; - let signature: AuthoritySignature = signature - .clone() - .try_into() - .map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?; - let digest_item = ::babe_seal(signature.into()); - - let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); - import_block.post_digests.push(digest_item); - import_block.body = Some(body); - import_block.state_action = StateAction::ApplyChanges( - sc_consensus::StorageChanges::Changes(storage_changes), - ); - import_block.intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, - ); - - Ok(import_block) - }, + // sign the pre-sealed hash of the block and then + // add it to a digest item. + let public_type_pair = public.clone().into(); + let public = public.to_raw_vec(); + let signature = SyncCryptoStore::sign_with( + &*self.keystore, + ::ID, + &public_type_pair, + header_hash.as_ref(), ) + .map_err(|e| sp_consensus::Error::CannotSign(public.clone(), e.to_string()))? + .ok_or_else(|| { + sp_consensus::Error::CannotSign( + public.clone(), + "Could not find key in keystore.".into(), + ) + })?; + let signature: AuthoritySignature = signature + .clone() + .try_into() + .map_err(|_| sp_consensus::Error::InvalidSignature(signature, public))?; + let digest_item = ::babe_seal(signature.into()); + + let mut import_block = BlockImportParams::new(BlockOrigin::Own, header); + import_block.post_digests.push(digest_item); + import_block.body = Some(body); + import_block.state_action = + StateAction::ApplyChanges(sc_consensus::StorageChanges::Changes(storage_changes)); + import_block.intermediates.insert( + Cow::from(INTERMEDIATE_KEY), + Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, + ); + + Ok(import_block) } fn force_authoring(&self) -> bool { diff --git a/substrate/client/consensus/slots/src/lib.rs b/substrate/client/consensus/slots/src/lib.rs index ead209ef86..3174eacaff 100644 --- a/substrate/client/consensus/slots/src/lib.rs +++ b/substrate/client/consensus/slots/src/lib.rs @@ -144,24 +144,17 @@ pub trait SimpleSlotWorker { fn pre_digest_data(&self, slot: Slot, claim: &Self::Claim) -> Vec; /// Returns a function which produces a `BlockImportParams`. - fn block_import_params( + async fn block_import_params( &self, - ) -> Box< - dyn Fn( - B::Header, - &B::Hash, - Vec, - StorageChanges<>::Transaction, B>, - Self::Claim, - Self::EpochData, - ) -> Result< - sc_consensus::BlockImportParams< - B, - >::Transaction, - >, - sp_consensus::Error, - > + Send - + 'static, + header: B::Header, + header_hash: &B::Hash, + body: Vec, + storage_changes: StorageChanges<>::Transaction, B>, + public: Self::Claim, + epoch: Self::EpochData, + ) -> Result< + sc_consensus::BlockImportParams>::Transaction>, + sp_consensus::Error, >; /// Whether to force authoring if offline. @@ -342,23 +335,23 @@ pub trait SimpleSlotWorker { }, }; - let block_import_params_maker = self.block_import_params(); - let block_import = self.block_import(); - let (block, storage_proof) = (proposal.block, proposal.proof); let (header, body) = block.deconstruct(); let header_num = *header.number(); let header_hash = header.hash(); let parent_hash = *header.parent_hash(); - let block_import_params = match block_import_params_maker( - header, - &header_hash, - body.clone(), - proposal.storage_changes, - claim, - epoch_data, - ) { + let block_import_params = match self + .block_import_params( + header, + &header_hash, + body.clone(), + proposal.storage_changes, + claim, + epoch_data, + ) + .await + { Ok(bi) => bi, Err(err) => { warn!(target: logging_target, "Failed to create block import params: {:?}", err); @@ -385,7 +378,7 @@ pub trait SimpleSlotWorker { ); let header = block_import_params.post_header(); - match block_import.import_block(block_import_params, Default::default()).await { + match self.block_import().import_block(block_import_params, Default::default()).await { Ok(res) => { res.handle_justification( &header.hash(),