Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic in case of invalid checkpoint in chain spec #603

Merged
merged 6 commits into from
May 25, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 39 additions & 10 deletions lib/src/chain_spec.rs
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, ValidChainInformation,
ChainInformationFinality, ValidChainInformation, ValidityError,
},
executor, libp2p, trie,
};
Expand Down Expand Up @@ -420,7 +420,9 @@ fn convert_epoch(epoch: &light_sync_state::BabeEpoch) -> BabeEpochInformation {
}

impl LightSyncState {
pub fn to_chain_information(&self) -> ChainInformation {
pub fn to_chain_information(&self) -> Result<ValidChainInformation, InvalidCheckpointError> {
// TODO: this code is a bit of a shitshow when it comes to corner cases and should be cleaned up after https://github.com/paritytech/substrate/issues/11184

// Create a sorted list of all regular epochs that haven't been pruned from the sync state.
let mut epochs: Vec<_> = self
.inner
Expand All @@ -442,14 +444,22 @@ impl LightSyncState {
epochs.dedup_by_key(|(_, epoch)| epoch.epoch_index);

// Get the latest two epochs.
let current_epoch = &epochs[epochs.len() - 2].1;
let next_epoch = &epochs[epochs.len() - 1].1;
let finalized_block_epoch_information = if epochs.len() >= 2 {
Some(convert_epoch(epochs[epochs.len() - 2].1))
} else {
None
};
if epochs.is_empty() {
return Err(InvalidCheckpointError::EmptyBabeEpochsList);
}
let next_epoch = epochs[epochs.len() - 1].1;

ChainInformation {
finalized_block_header: self.inner.finalized_block_header.clone(),
consensus: ChainInformationConsensus::Babe {
slots_per_epoch: NonZeroU64::new(current_epoch.duration).unwrap(),
finalized_block_epoch_information: Some(convert_epoch(current_epoch)),
slots_per_epoch: NonZeroU64::new(next_epoch.duration)
.ok_or(InvalidCheckpointError::InvalidBabeSlotsPerEpoch)?,
finalized_block_epoch_information,
finalized_next_epoch_transition: convert_epoch(next_epoch),
},
finality: ChainInformationFinality::Grandpa {
Expand All @@ -459,15 +469,20 @@ impl LightSyncState {
.grandpa_authority_set
.current_authorities
.iter()
.map(|authority| crate::header::GrandpaAuthority {
public_key: authority.public_key,
weight: NonZeroU64::new(authority.weight).unwrap(),
.map(|authority| {
Ok(crate::header::GrandpaAuthority {
public_key: authority.public_key,
weight: NonZeroU64::new(authority.weight)
.ok_or(InvalidCheckpointError::InvalidGrandpaAuthorityWeight)?,
})
})
.collect()
.collect::<Result<_, _>>()?
},
finalized_scheduled_change: None, // TODO: unimplemented
},
}
.try_into()
.map_err(InvalidCheckpointError::InvalidData)
}
}

Expand Down Expand Up @@ -500,6 +515,20 @@ pub enum FromGenesisStorageError {
UnknownStorageItems,
}

/// Error when building the chain information from the genesis storage.
#[derive(Debug, derive_more::Display)]
pub enum InvalidCheckpointError {
/// The list of Babe epochs is empty.
EmptyBabeEpochsList,
/// Found a value of 0 for the number of Babe slots per epoch.
InvalidBabeSlotsPerEpoch,
/// Found a Grandpa authority with a weight of 0.
InvalidGrandpaAuthorityWeight,
/// Information found in the checkpoint is invalid.
#[display(fmt = "{_0}")]
InvalidData(ValidityError),
}

#[cfg(test)]
mod tests {
use super::{Bootnode, ChainSpec};
Expand Down
13 changes: 5 additions & 8 deletions light-base/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ use futures_util::{future, FutureExt as _};
use hashbrown::{hash_map::Entry, HashMap};
use itertools::Itertools as _;
use smoldot::{
chain::{self, chain_information},
chain_spec, header,
chain, chain_spec, header,
informant::HashDisplay,
libp2p::{connection, multiaddr, peer_id},
};
Expand Down Expand Up @@ -363,11 +362,9 @@ impl<TPlat: platform::PlatformRef, TChain> Client<TPlat, TChain> {
let (chain_information, genesis_block_header, checkpoint_nodes) = {
match (
chain_spec.to_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.to_chain_information(),
)
}),
chain_spec
.light_sync_state()
.map(|s| s.to_chain_information()),
database::decode_database(
config.database_content,
chain_spec.block_number_bytes().into(),
Expand Down Expand Up @@ -1012,7 +1009,7 @@ pub enum AddChainError {
ChainSpecNeitherGenesisStorageNorCheckpoint,
/// Checkpoint provided in the chain specification is invalid.
#[display(fmt = "Invalid checkpoint in chain specification: {_0}")]
InvalidCheckpoint(chain_information::ValidityError),
InvalidCheckpoint(chain_spec::InvalidCheckpointError),
/// Failed to build the information about the chain from the genesis storage. This indicates
/// invalid data in the genesis storage.
#[display(fmt = "Failed to build genesis chain information: {_0}")]
Expand Down
5 changes: 5 additions & 0 deletions wasm-node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## Unreleased

### Fixed

- Fix panic when the checkpoint in the chain specification contains an empty list of Babe epochs, which can normally only happen if the checkpoint was modified manually. ([#603](https://github.com/smol-dot/smoldot/pull/603))
- Fix panic when the checkpoint in the chain specification contains only one Babe epoch, which can happen if the checkpoint was generated before any block was authored. ([#603](https://github.com/smol-dot/smoldot/pull/603))

## 1.0.6 - 2023-05-09

### Changed
Expand Down