From cfe5388a0a45947a9fa5754798315c70cdac42cc Mon Sep 17 00:00:00 2001 From: Davide Galassi Date: Wed, 21 Sep 2022 11:42:12 +0200 Subject: [PATCH] Introduce 'intermediate_insert' method to hide implementation details (#12215) Renaming from 'intermediate_take' to 'intermediate_remove' --- bin/node/cli/src/service.rs | 8 ++++---- client/consensus/babe/src/lib.rs | 17 +++++++---------- client/consensus/babe/src/tests.rs | 6 ++---- client/consensus/common/src/block_import.rs | 15 ++++++++++----- .../consensus/manual-seal/src/consensus/babe.rs | 13 ++++--------- client/consensus/pow/src/lib.rs | 13 ++++--------- client/consensus/pow/src/worker.rs | 6 +----- 7 files changed, 32 insertions(+), 46 deletions(-) diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index c058e22fa7a97..f2b4feb58075f 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -587,7 +587,7 @@ mod tests { RuntimeAppPublic, }; use sp_timestamp; - use std::{borrow::Cow, sync::Arc}; + use std::sync::Arc; type AccountPublic = ::Signer; @@ -733,9 +733,9 @@ mod tests { let mut params = BlockImportParams::new(BlockOrigin::File, new_header); params.post_digests.push(item); params.body = Some(new_body); - params.intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, + params.insert_intermediate( + INTERMEDIATE_KEY, + BabeIntermediate:: { epoch_descriptor }, ); params.fork_choice = Some(ForkChoiceStrategy::LongestChain); diff --git a/client/consensus/babe/src/lib.rs b/client/consensus/babe/src/lib.rs index eca6654f6e4dc..aef4785b7bb81 100644 --- a/client/consensus/babe/src/lib.rs +++ b/client/consensus/babe/src/lib.rs @@ -67,7 +67,6 @@ #![warn(missing_docs)] use std::{ - borrow::Cow, collections::{HashMap, HashSet}, future::Future, pin::Pin, @@ -857,10 +856,8 @@ where 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<_>, - ); + import_block + .insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate:: { epoch_descriptor }); Ok(import_block) } @@ -1272,9 +1269,9 @@ where block.header = pre_header; block.post_digests.push(verified_info.seal); - block.intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, + block.insert_intermediate( + INTERMEDIATE_KEY, + BabeIntermediate:: { epoch_descriptor }, ); block.post_hash = Some(hash); @@ -1426,7 +1423,7 @@ where match self.client.status(BlockId::Hash(hash)) { Ok(sp_blockchain::BlockStatus::InChain) => { // When re-importing existing block strip away intermediates. - let _ = block.take_intermediate::>(INTERMEDIATE_KEY); + let _ = block.remove_intermediate::>(INTERMEDIATE_KEY); block.fork_choice = Some(ForkChoiceStrategy::Custom(false)); return self.inner.import_block(block, new_cache).await.map_err(Into::into) }, @@ -1495,7 +1492,7 @@ where }; let intermediate = - block.take_intermediate::>(INTERMEDIATE_KEY)?; + block.remove_intermediate::>(INTERMEDIATE_KEY)?; let epoch_descriptor = intermediate.epoch_descriptor; let first_in_epoch = parent_slot < epoch_descriptor.start_slot(); diff --git a/client/consensus/babe/src/tests.rs b/client/consensus/babe/src/tests.rs index ab3805138482c..a3467e0020200 100644 --- a/client/consensus/babe/src/tests.rs +++ b/client/consensus/babe/src/tests.rs @@ -642,10 +642,8 @@ fn propose_and_import_block( let mut import = BlockImportParams::new(BlockOrigin::Own, block.header); import.post_digests.push(seal); import.body = Some(block.extrinsics); - import.intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, - ); + import + .insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate:: { epoch_descriptor }); import.fork_choice = Some(ForkChoiceStrategy::LongestChain); let import_result = block_on(block_import.import_block(import, Default::default())).unwrap(); diff --git a/client/consensus/common/src/block_import.rs b/client/consensus/common/src/block_import.rs index b8fc815cc5c5c..f888176addd2d 100644 --- a/client/consensus/common/src/block_import.rs +++ b/client/consensus/common/src/block_import.rs @@ -294,18 +294,23 @@ impl BlockImportParams { } } - /// Take intermediate by given key, and remove it from the processing list. - pub fn take_intermediate(&mut self, key: &[u8]) -> Result, Error> { + /// Insert intermediate by given key. + pub fn insert_intermediate(&mut self, key: &'static [u8], value: T) { + self.intermediates.insert(Cow::from(key), Box::new(value)); + } + + /// Remove and return intermediate by given key. + pub fn remove_intermediate(&mut self, key: &[u8]) -> Result { let (k, v) = self.intermediates.remove_entry(key).ok_or(Error::NoIntermediate)?; - v.downcast::().map_err(|v| { + v.downcast::().map(|v| *v).map_err(|v| { self.intermediates.insert(k, v); Error::InvalidIntermediate }) } /// Get a reference to a given intermediate. - pub fn intermediate(&self, key: &[u8]) -> Result<&T, Error> { + pub fn get_intermediate(&self, key: &[u8]) -> Result<&T, Error> { self.intermediates .get(key) .ok_or(Error::NoIntermediate)? @@ -314,7 +319,7 @@ impl BlockImportParams { } /// Get a mutable reference to a given intermediate. - pub fn intermediate_mut(&mut self, key: &[u8]) -> Result<&mut T, Error> { + pub fn get_intermediate_mut(&mut self, key: &[u8]) -> Result<&mut T, Error> { self.intermediates .get_mut(key) .ok_or(Error::NoIntermediate)? diff --git a/client/consensus/manual-seal/src/consensus/babe.rs b/client/consensus/manual-seal/src/consensus/babe.rs index 300a96695c90a..206f5163a13cd 100644 --- a/client/consensus/manual-seal/src/consensus/babe.rs +++ b/client/consensus/manual-seal/src/consensus/babe.rs @@ -30,7 +30,7 @@ use sc_consensus_epochs::{ descendent_query, EpochHeader, SharedEpochChanges, ViableEpochDescriptor, }; use sp_keystore::SyncCryptoStorePtr; -use std::{borrow::Cow, marker::PhantomData, sync::Arc}; +use std::{marker::PhantomData, sync::Arc}; use sc_consensus::{BlockImportParams, ForkChoiceStrategy, Verifier}; use sp_api::{ProvideRuntimeApi, TransactionFor}; @@ -125,10 +125,8 @@ where // drop the lock drop(epoch_changes); - import_params.intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, - ); + import_params + .insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate:: { epoch_descriptor }); Ok((import_params, None)) } @@ -315,10 +313,7 @@ where }; } - params.intermediates.insert( - Cow::from(INTERMEDIATE_KEY), - Box::new(BabeIntermediate:: { epoch_descriptor }) as Box<_>, - ); + params.insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate:: { epoch_descriptor }); Ok(()) } diff --git a/client/consensus/pow/src/lib.rs b/client/consensus/pow/src/lib.rs index 9b3ef501d2396..dcf069d617bab 100644 --- a/client/consensus/pow/src/lib.rs +++ b/client/consensus/pow/src/lib.rs @@ -65,10 +65,7 @@ use sp_runtime::{ traits::{Block as BlockT, Header as HeaderT}, RuntimeString, }; -use std::{ - borrow::Cow, cmp::Ordering, collections::HashMap, marker::PhantomData, sync::Arc, - time::Duration, -}; +use std::{cmp::Ordering, collections::HashMap, marker::PhantomData, sync::Arc, time::Duration}; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -358,8 +355,8 @@ where let inner_seal = fetch_seal::(block.post_digests.last(), block.header.hash())?; - let intermediate = - block.take_intermediate::>(INTERMEDIATE_KEY)?; + let intermediate = block + .remove_intermediate::>(INTERMEDIATE_KEY)?; let difficulty = match intermediate.difficulty { Some(difficulty) => difficulty, @@ -455,9 +452,7 @@ where let intermediate = PowIntermediate:: { difficulty: None }; block.header = checked_header; block.post_digests.push(seal); - block - .intermediates - .insert(Cow::from(INTERMEDIATE_KEY), Box::new(intermediate) as Box<_>); + block.insert_intermediate(INTERMEDIATE_KEY, intermediate); block.post_hash = Some(hash); Ok((block, None)) diff --git a/client/consensus/pow/src/worker.rs b/client/consensus/pow/src/worker.rs index 750e78cd9a038..a00da6e7022fb 100644 --- a/client/consensus/pow/src/worker.rs +++ b/client/consensus/pow/src/worker.rs @@ -32,7 +32,6 @@ use sp_runtime::{ DigestItem, }; use std::{ - borrow::Cow, collections::HashMap, pin::Pin, sync::{ @@ -212,10 +211,7 @@ where let intermediate = PowIntermediate:: { difficulty: Some(build.metadata.difficulty), }; - - import_block - .intermediates - .insert(Cow::from(INTERMEDIATE_KEY), Box::new(intermediate) as Box<_>); + import_block.insert_intermediate(INTERMEDIATE_KEY, intermediate); let header = import_block.post_header(); let mut block_import = self.block_import.lock();