Skip to content

Commit

Permalink
Introduce 'intermediate_insert' method to hide implementation details (
Browse files Browse the repository at this point in the history
…paritytech#12215)

Renaming from 'intermediate_take' to 'intermediate_remove'
  • Loading branch information
davxy authored and ark0f committed Feb 27, 2023
1 parent cc1f422 commit cfe5388
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 46 deletions.
8 changes: 4 additions & 4 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ mod tests {
RuntimeAppPublic,
};
use sp_timestamp;
use std::{borrow::Cow, sync::Arc};
use std::sync::Arc;

type AccountPublic = <Signature as Verify>::Signer;

Expand Down Expand Up @@ -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::<Block> { epoch_descriptor }) as Box<_>,
params.insert_intermediate(
INTERMEDIATE_KEY,
BabeIntermediate::<Block> { epoch_descriptor },
);
params.fork_choice = Some(ForkChoiceStrategy::LongestChain);

Expand Down
17 changes: 7 additions & 10 deletions client/consensus/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
#![warn(missing_docs)]

use std::{
borrow::Cow,
collections::{HashMap, HashSet},
future::Future,
pin::Pin,
Expand Down Expand Up @@ -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::<B> { epoch_descriptor }) as Box<_>,
);
import_block
.insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate::<B> { epoch_descriptor });

Ok(import_block)
}
Expand Down Expand Up @@ -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::<Block> { epoch_descriptor }) as Box<_>,
block.insert_intermediate(
INTERMEDIATE_KEY,
BabeIntermediate::<Block> { epoch_descriptor },
);
block.post_hash = Some(hash);

Expand Down Expand Up @@ -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::<BabeIntermediate<Block>>(INTERMEDIATE_KEY);
let _ = block.remove_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY);
block.fork_choice = Some(ForkChoiceStrategy::Custom(false));
return self.inner.import_block(block, new_cache).await.map_err(Into::into)
},
Expand Down Expand Up @@ -1495,7 +1492,7 @@ where
};

let intermediate =
block.take_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY)?;
block.remove_intermediate::<BabeIntermediate<Block>>(INTERMEDIATE_KEY)?;

let epoch_descriptor = intermediate.epoch_descriptor;
let first_in_epoch = parent_slot < epoch_descriptor.start_slot();
Expand Down
6 changes: 2 additions & 4 deletions client/consensus/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,10 +642,8 @@ fn propose_and_import_block<Transaction: Send + 'static>(
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::<TestBlock> { epoch_descriptor }) as Box<_>,
);
import
.insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate::<TestBlock> { epoch_descriptor });
import.fork_choice = Some(ForkChoiceStrategy::LongestChain);
let import_result = block_on(block_import.import_block(import, Default::default())).unwrap();

Expand Down
15 changes: 10 additions & 5 deletions client/consensus/common/src/block_import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,18 +294,23 @@ impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> {
}
}

/// Take intermediate by given key, and remove it from the processing list.
pub fn take_intermediate<T: 'static>(&mut self, key: &[u8]) -> Result<Box<T>, Error> {
/// Insert intermediate by given key.
pub fn insert_intermediate<T: 'static + Send>(&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<T: 'static>(&mut self, key: &[u8]) -> Result<T, Error> {
let (k, v) = self.intermediates.remove_entry(key).ok_or(Error::NoIntermediate)?;

v.downcast::<T>().map_err(|v| {
v.downcast::<T>().map(|v| *v).map_err(|v| {
self.intermediates.insert(k, v);
Error::InvalidIntermediate
})
}

/// Get a reference to a given intermediate.
pub fn intermediate<T: 'static>(&self, key: &[u8]) -> Result<&T, Error> {
pub fn get_intermediate<T: 'static>(&self, key: &[u8]) -> Result<&T, Error> {
self.intermediates
.get(key)
.ok_or(Error::NoIntermediate)?
Expand All @@ -314,7 +319,7 @@ impl<Block: BlockT, Transaction> BlockImportParams<Block, Transaction> {
}

/// Get a mutable reference to a given intermediate.
pub fn intermediate_mut<T: 'static>(&mut self, key: &[u8]) -> Result<&mut T, Error> {
pub fn get_intermediate_mut<T: 'static>(&mut self, key: &[u8]) -> Result<&mut T, Error> {
self.intermediates
.get_mut(key)
.ok_or(Error::NoIntermediate)?
Expand Down
13 changes: 4 additions & 9 deletions client/consensus/manual-seal/src/consensus/babe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -125,10 +125,8 @@ where
// drop the lock
drop(epoch_changes);

import_params.intermediates.insert(
Cow::from(INTERMEDIATE_KEY),
Box::new(BabeIntermediate::<B> { epoch_descriptor }) as Box<_>,
);
import_params
.insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate::<B> { epoch_descriptor });

Ok((import_params, None))
}
Expand Down Expand Up @@ -315,10 +313,7 @@ where
};
}

params.intermediates.insert(
Cow::from(INTERMEDIATE_KEY),
Box::new(BabeIntermediate::<B> { epoch_descriptor }) as Box<_>,
);
params.insert_intermediate(INTERMEDIATE_KEY, BabeIntermediate::<B> { epoch_descriptor });

Ok(())
}
Expand Down
13 changes: 4 additions & 9 deletions client/consensus/pow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<B: BlockT> {
Expand Down Expand Up @@ -358,8 +355,8 @@ where

let inner_seal = fetch_seal::<B>(block.post_digests.last(), block.header.hash())?;

let intermediate =
block.take_intermediate::<PowIntermediate<Algorithm::Difficulty>>(INTERMEDIATE_KEY)?;
let intermediate = block
.remove_intermediate::<PowIntermediate<Algorithm::Difficulty>>(INTERMEDIATE_KEY)?;

let difficulty = match intermediate.difficulty {
Some(difficulty) => difficulty,
Expand Down Expand Up @@ -455,9 +452,7 @@ where
let intermediate = PowIntermediate::<Algorithm::Difficulty> { 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))
Expand Down
6 changes: 1 addition & 5 deletions client/consensus/pow/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use sp_runtime::{
DigestItem,
};
use std::{
borrow::Cow,
collections::HashMap,
pin::Pin,
sync::{
Expand Down Expand Up @@ -212,10 +211,7 @@ where
let intermediate = PowIntermediate::<Algorithm::Difficulty> {
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();
Expand Down

0 comments on commit cfe5388

Please sign in to comment.