Skip to content

Commit

Permalink
Return a ValidChainInformation when building the chain spec info (#2979)
Browse files Browse the repository at this point in the history 
The "chain information building process" is done in two steps: fetching
all the info, then validating that the values are coherent together.

In the past, the `chain_spec` module used to do only the first step,
leaving the second step to the API user of this module. Then I've
changed the implementation to produce a `ValidChainInformation` and turn
it back into an (not-validated) `ChainInformation`.

This PR changes the return type to `ValidChainInformation`, and thus the
API user doesn't need to handle that.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@tomaka @mergify
tomaka and mergify[bot] authored Nov 11, 2022
1 parent becc0a3 commit 150bc4d
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 23 deletions.
18 changes: 10 additions & 8 deletions bin/full-node/src/run.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {

let (db, existed) = open_database(
&chain_spec,
&genesis_chain_information,
genesis_chain_information.as_ref(),
db_path,
matches!(cli_output, cli::Output::Informant),
)
Expand All @@ -191,7 +191,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
Some(Arc::new(database_thread::DatabaseThread::from(
open_database(
relay_chain_spec,
relay_genesis_chain_information.as_ref().unwrap(),
relay_genesis_chain_information.as_ref().unwrap().as_ref(),
relay_db_path,
matches!(cli_output, cli::Output::Informant),
)
Expand Down Expand Up @@ -267,6 +267,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
peer_id::PublicKey::Ed25519(*noise_key.libp2p_public_ed25519_key()).into_peer_id();

let genesis_block_hash = genesis_chain_information
.as_ref()
.finalized_block_header
.hash(chain_spec.block_number_bytes().into());

Expand All @@ -287,8 +288,8 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
block_number_bytes: usize::from(chain_spec.block_number_bytes()),
database: database.clone(),
has_grandpa_protocol: matches!(
genesis_chain_information.finality,
chain::chain_information::ChainInformationFinality::Grandpa { .. }
genesis_chain_information.as_ref().finality,
chain::chain_information::ChainInformationFinalityRef::Grandpa { .. }
),
genesis_block_hash,
best_block: {
Expand Down Expand Up @@ -340,13 +341,13 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
block_number_bytes: usize::from(relay_chains_specs.block_number_bytes()),
database: relay_chain_database.clone().unwrap(),
has_grandpa_protocol: matches!(
relay_genesis_chain_information.as_ref().unwrap().finality,
chain::chain_information::ChainInformationFinality::Grandpa { .. }
relay_genesis_chain_information.as_ref().unwrap().as_ref().finality,
chain::chain_information::ChainInformationFinalityRef::Grandpa { .. }
),
genesis_block_hash: relay_genesis_chain_information
.as_ref()
.unwrap()
.finalized_block_header
.as_ref().finalized_block_header
.hash(chain_spec.block_number_bytes().into(),),
best_block: relay_chain_database
.as_ref()
Expand Down Expand Up @@ -437,6 +438,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
genesis_block_hash: relay_genesis_chain_information
.as_ref()
.unwrap()
.as_ref()
.finalized_block_header
.hash(usize::from(
relay_chain_spec.as_ref().unwrap().block_number_bytes(),
Expand Down Expand Up @@ -672,7 +674,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
#[tracing::instrument(level = "trace", skip(chain_spec, show_progress))]
async fn open_database(
chain_spec: &chain_spec::ChainSpec,
genesis_chain_information: &chain::chain_information::ChainInformation,
genesis_chain_information: chain::chain_information::ChainInformationRef<'_>,
db_path: Option<PathBuf>,
show_progress: bool,
) -> (full_sqlite::SqliteFullDatabase, bool) {
Expand Down
14 changes: 4 additions & 10 deletions bin/light-base/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,7 @@ impl<TPlat: platform::Platform, TChain> Client<TPlat, TChain> {
// TODO: clean up that block
let (chain_information, genesis_block_header, checkpoint_nodes) = {
match (
chain_spec
.as_chain_information()
.map(|(ci, _)| chain::chain_information::ValidChainInformation::try_from(ci)), // TODO: don't just throw away the runtime
chain_spec.as_chain_information().map(|(ci, _)| ci), // TODO: don't just throw away the runtime
chain_spec.light_sync_state().map(|s| {
chain::chain_information::ValidChainInformation::try_from(
s.as_chain_information(),
Expand All @@ -368,7 +366,7 @@ impl<TPlat: platform::Platform, TChain> Client<TPlat, TChain> {
),
) {
// Use the database if it contains a more recent block than the chain spec checkpoint.
(Ok(Ok(genesis_ci)), checkpoint, Ok(database_content))
(Ok(genesis_ci), checkpoint, Ok(database_content))
if database_content.genesis_block_hash
== genesis_ci
.as_ref()
Expand Down Expand Up @@ -469,23 +467,19 @@ impl<TPlat: platform::Platform, TChain> Client<TPlat, TChain> {
));
}

(Ok(Err(err)), _, _) => {
return Err(format!("Invalid genesis chain information: {}", err));
}

(_, Some(Err(err)), _) => {
return Err(format!(
"Invalid checkpoint in chain specification: {}",
err
));
}

(Ok(Ok(genesis_ci)), Some(Ok(checkpoint)), _) => {
(Ok(genesis_ci), Some(Ok(checkpoint)), _) => {
let genesis_header = genesis_ci.as_ref().finalized_block_header.clone();
(checkpoint, genesis_header.into(), Default::default())
}

(Ok(Ok(genesis_ci)), None, _) => {
(Ok(genesis_ci), None, _) => {
let genesis_header =
header::Header::from(genesis_ci.as_ref().finalized_block_header.clone());
(genesis_ci, genesis_header, Default::default())
Expand Down
2 changes: 1 addition & 1 deletion src/author/runtime/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn block_building_works() {
let genesis_storage = chain_specs.genesis_storage().into_genesis_items().unwrap();

let (chain_info, genesis_runtime) = chain_specs.as_chain_information().unwrap();
let genesis_hash = chain_info.finalized_block_header.hash(4);
let genesis_hash = chain_info.as_ref().finalized_block_header.hash(4);

let mut builder = super::build_block(super::Config {
block_number_bytes: 4,
Expand Down
8 changes: 4 additions & 4 deletions src/chain_spec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
use crate::{
chain::chain_information::{
build, BabeEpochInformation, ChainInformation, ChainInformationConsensus,
ChainInformationFinality,
ChainInformationFinality, ValidChainInformation,
},
executor, libp2p, trie,
};
Expand Down Expand Up @@ -86,7 +86,8 @@ impl ChainSpec {
/// genesis block.
pub fn as_chain_information(
&self,
) -> Result<(ChainInformation, executor::host::HostVmPrototype), FromGenesisStorageError> {
) -> Result<(ValidChainInformation, executor::host::HostVmPrototype), FromGenesisStorageError>
{
let genesis_storage = match self.genesis_storage() {
GenesisStorage::Items(items) => items,
GenesisStorage::TrieRootHash(_) => {
Expand Down Expand Up @@ -173,8 +174,7 @@ impl ChainSpec {
}
};

// TODO: ! we return a ChainInformation while we have a ValidChainInformation
Ok((chain_info.into(), vm_prototype))
Ok((chain_info, vm_prototype))
}

/// Returns the name of the chain. Meant to be displayed to the user.
Expand Down

0 comments on commit 150bc4d

Please sign in to comment.