Skip to content

Commit

Permalink
Fix panic in case of invalid checkpoint in chain spec (#603)
Browse files Browse the repository at this point in the history
* Fix panic in case of invalid checkpoint in chain spec

* PR link

* Fix warning

* More error detections

* Ignore checkpoint if it contains less than 2 epochs instead

* Add regression test
  • Loading branch information
tomaka authored May 25, 2023
1 parent 543e1c4 commit 8f6e978
Show file tree
Hide file tree
Showing 3 changed files with 261 additions and 23 deletions.
249 changes: 239 additions & 10 deletions lib/src/chain_spec.rs

Large diffs are not rendered by default.

30 changes: 17 additions & 13 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 @@ -467,20 +464,27 @@ impl<TPlat: platform::PlatformRef, TChain> Client<TPlat, TChain> {

(Err(err), _, _) => return Err(AddChainError::InvalidGenesisStorage(err)),

(_, Some(Err(err)), _) => {
return Err(AddChainError::InvalidCheckpoint(err));
}

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

(Ok(genesis_ci), None, _) => {
(
Ok(genesis_ci),
None
| Some(Err(
chain_spec::CheckpointToChainInformationError::GenesisBlockCheckpoint,
)),
_,
) => {
let genesis_header =
header::Header::from(genesis_ci.as_ref().finalized_block_header.clone());
(genesis_ci, genesis_header, Default::default())
}

(_, Some(Err(err)), _) => {
return Err(AddChainError::InvalidCheckpoint(err));
}
}
};

Expand Down Expand Up @@ -1012,7 +1016,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::CheckpointToChainInformationError),
/// 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 is invalid, 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 zero or one Babe epochs, 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

0 comments on commit 8f6e978

Please sign in to comment.