diff --git a/substrate/client/api/src/light.rs b/substrate/client/api/src/light.rs index b3e2e686ea..fb3aeeab7c 100644 --- a/substrate/client/api/src/light.rs +++ b/substrate/client/api/src/light.rs @@ -259,19 +259,19 @@ pub trait Storage: AuxStore + HeaderBackend + HeaderMetada /// Get last finalized header. fn last_finalized(&self) -> ClientResult; - /// Get headers CHT root for given block. Fails if the block is not pruned (not a part of any CHT). + /// Get headers CHT root for given block. Returns None if the block is not pruned (not a part of any CHT). fn header_cht_root( &self, cht_size: NumberFor, block: NumberFor, - ) -> ClientResult; + ) -> ClientResult>; - /// Get changes trie CHT root for given block. Fails if the block is not pruned (not a part of any CHT). + /// Get changes trie CHT root for given block. Returns None if the block is not pruned (not a part of any CHT). fn changes_trie_cht_root( &self, cht_size: NumberFor, block: NumberFor, - ) -> ClientResult; + ) -> ClientResult>; /// Get storage cache. fn cache(&self) -> Option>>; diff --git a/substrate/client/db/src/light.rs b/substrate/client/db/src/light.rs index 2885ee5046..f6f176ae79 100644 --- a/substrate/client/db/src/light.rs +++ b/substrate/client/db/src/light.rs @@ -365,14 +365,22 @@ impl LightStorage { cht_type: u8, cht_size: NumberFor, block: NumberFor - ) -> ClientResult { - let no_cht_for_block = || ClientError::Backend(format!("CHT for block {} not exists", block)); + ) -> ClientResult> { + let no_cht_for_block = || ClientError::Backend(format!("Missing CHT for block {}", block)); + let meta = self.meta.read(); + let max_cht_number = cht::max_cht_number(cht_size, meta.finalized_number); let cht_number = cht::block_to_cht_number(cht_size, block).ok_or_else(no_cht_for_block)?; + match max_cht_number { + Some(max_cht_number) if cht_number <= max_cht_number => (), + _ => return Ok(None), + } + let cht_start = cht::start_number(cht_size, cht_number); self.db.get(columns::CHT, &cht_key(cht_type, cht_start)?).map_err(db_err)? .ok_or_else(no_cht_for_block) .and_then(|hash| Block::Hash::decode(&mut &*hash).map_err(|_| no_cht_for_block())) + .map(Some) } } @@ -505,7 +513,7 @@ impl LightBlockchainStorage for LightStorage &self, cht_size: NumberFor, block: NumberFor, - ) -> ClientResult { + ) -> ClientResult> { self.read_cht_root(HEADER_CHT_PREFIX, cht_size, block) } @@ -513,7 +521,7 @@ impl LightBlockchainStorage for LightStorage &self, cht_size: NumberFor, block: NumberFor, - ) -> ClientResult { + ) -> ClientResult> { self.read_cht_root(CHANGES_TRIE_CHT_PREFIX, cht_size, block) } @@ -763,20 +771,20 @@ pub(crate) mod tests { assert_eq!(db.db.iter(columns::KEY_LOOKUP).count(), (2 * (1 + cht_size + 1)) as usize); assert_eq!(db.db.iter(columns::CHT).count(), 1); assert!((0..cht_size as _).all(|i| db.header(BlockId::Number(1 + i)).unwrap().is_none())); - assert!(db.header_cht_root(cht_size, cht_size / 2).is_ok()); - assert!(db.header_cht_root(cht_size, cht_size + cht_size / 2).is_err()); + assert!(db.header_cht_root(cht_size, cht_size / 2).unwrap().is_some()); + assert!(db.header_cht_root(cht_size, cht_size + cht_size / 2).unwrap().is_none()); assert!(db.changes_trie_cht_root(cht_size, cht_size / 2).is_err()); - assert!(db.changes_trie_cht_root(cht_size, cht_size + cht_size / 2).is_err()); + assert!(db.changes_trie_cht_root(cht_size, cht_size + cht_size / 2).unwrap().is_none()); // when headers are created with changes tries roots let db = insert_headers(header_with_changes_trie); assert_eq!(db.db.iter(columns::HEADER).count(), (1 + cht_size + 1) as usize); assert_eq!(db.db.iter(columns::CHT).count(), 2); assert!((0..cht_size as _).all(|i| db.header(BlockId::Number(1 + i)).unwrap().is_none())); - assert!(db.header_cht_root(cht_size, cht_size / 2).is_ok()); - assert!(db.header_cht_root(cht_size, cht_size + cht_size / 2).is_err()); - assert!(db.changes_trie_cht_root(cht_size, cht_size / 2).is_ok()); - assert!(db.changes_trie_cht_root(cht_size, cht_size + cht_size / 2).is_err()); + assert!(db.header_cht_root(cht_size, cht_size / 2).unwrap().is_some()); + assert!(db.header_cht_root(cht_size, cht_size + cht_size / 2).unwrap().is_none()); + assert!(db.changes_trie_cht_root(cht_size, cht_size / 2).unwrap().is_some()); + assert!(db.changes_trie_cht_root(cht_size, cht_size + cht_size / 2).unwrap().is_none()); } #[test] @@ -787,7 +795,7 @@ pub(crate) mod tests { #[test] fn get_cht_fails_for_non_existant_cht() { let cht_size: u64 = cht::size(); - assert!(LightStorage::::new_test().header_cht_root(cht_size, cht_size / 2).is_err()); + assert!(LightStorage::::new_test().header_cht_root(cht_size, cht_size / 2).unwrap().is_none()); } #[test] @@ -803,15 +811,18 @@ pub(crate) mod tests { db.finalize_header(BlockId::Hash(prev_hash)).unwrap(); } - let cht_root_1 = db.header_cht_root(cht_size, cht::start_number(cht_size, 0)).unwrap(); - let cht_root_2 = db.header_cht_root(cht_size, cht::start_number(cht_size, 0) + cht_size / 2).unwrap(); - let cht_root_3 = db.header_cht_root(cht_size, cht::end_number(cht_size, 0)).unwrap(); + let cht_root_1 = db.header_cht_root(cht_size, cht::start_number(cht_size, 0)).unwrap().unwrap(); + let cht_root_2 = db.header_cht_root(cht_size, cht::start_number(cht_size, 0) + cht_size / 2).unwrap().unwrap(); + let cht_root_3 = db.header_cht_root(cht_size, cht::end_number(cht_size, 0)).unwrap().unwrap(); assert_eq!(cht_root_1, cht_root_2); assert_eq!(cht_root_2, cht_root_3); - let cht_root_1 = db.changes_trie_cht_root(cht_size, cht::start_number(cht_size, 0)).unwrap(); - let cht_root_2 = db.changes_trie_cht_root(cht_size, cht::start_number(cht_size, 0) + cht_size / 2).unwrap(); - let cht_root_3 = db.changes_trie_cht_root(cht_size, cht::end_number(cht_size, 0)).unwrap(); + let cht_root_1 = db.changes_trie_cht_root(cht_size, cht::start_number(cht_size, 0)).unwrap().unwrap(); + let cht_root_2 = db.changes_trie_cht_root( + cht_size, + cht::start_number(cht_size, 0) + cht_size / 2, + ).unwrap().unwrap(); + let cht_root_3 = db.changes_trie_cht_root(cht_size, cht::end_number(cht_size, 0)).unwrap().unwrap(); assert_eq!(cht_root_1, cht_root_2); assert_eq!(cht_root_2, cht_root_3); } diff --git a/substrate/client/src/cht.rs b/substrate/client/src/cht.rs index 86c2ca756b..7189360174 100644 --- a/substrate/client/src/cht.rs +++ b/substrate/client/src/cht.rs @@ -62,6 +62,19 @@ pub fn is_build_required(cht_size: N, block_num: N) -> Option Some(block_cht_num - two) } +/// Returns Some(max_cht_number) if CHT has ever been built given maximal canonical block number. +pub fn max_cht_number(cht_size: N, max_canonical_block: N) -> Option + where + N: Clone + SimpleArithmetic, +{ + let max_cht_number = block_to_cht_number(cht_size, max_canonical_block)?; + let two = N::one() + N::one(); + if max_cht_number < two { + return None; + } + Some(max_cht_number - two) +} + /// Compute a CHT root from an iterator of block hashes. Fails if shorter than /// SIZE items. The items are assumed to proceed sequentially from `start_number(cht_num)`. /// Discards the trie's nodes. @@ -329,8 +342,24 @@ mod tests { assert_eq!(is_build_required(SIZE, SIZE + 1), None); assert_eq!(is_build_required(SIZE, 2 * SIZE), None); assert_eq!(is_build_required(SIZE, 2 * SIZE + 1), Some(0)); + assert_eq!(is_build_required(SIZE, 2 * SIZE + 2), None); assert_eq!(is_build_required(SIZE, 3 * SIZE), None); assert_eq!(is_build_required(SIZE, 3 * SIZE + 1), Some(1)); + assert_eq!(is_build_required(SIZE, 3 * SIZE + 2), None); + } + + #[test] + fn max_cht_number_works() { + assert_eq!(max_cht_number(SIZE, 0u32.into()), None); + assert_eq!(max_cht_number(SIZE, 1u32.into()), None); + assert_eq!(max_cht_number(SIZE, SIZE), None); + assert_eq!(max_cht_number(SIZE, SIZE + 1), None); + assert_eq!(max_cht_number(SIZE, 2 * SIZE), None); + assert_eq!(max_cht_number(SIZE, 2 * SIZE + 1), Some(0)); + assert_eq!(max_cht_number(SIZE, 2 * SIZE + 2), Some(0)); + assert_eq!(max_cht_number(SIZE, 3 * SIZE), Some(0)); + assert_eq!(max_cht_number(SIZE, 3 * SIZE + 1), Some(1)); + assert_eq!(max_cht_number(SIZE, 3 * SIZE + 2), Some(1)); } #[test] diff --git a/substrate/client/src/in_mem.rs b/substrate/client/src/in_mem.rs index 5a54960aa6..a65084c17c 100644 --- a/substrate/client/src/in_mem.rs +++ b/substrate/client/src/in_mem.rs @@ -433,18 +433,20 @@ impl sc_client_api::light::Storage for Blockchain &self, _cht_size: NumberFor, block: NumberFor, - ) -> sp_blockchain::Result { + ) -> sp_blockchain::Result> { self.storage.read().header_cht_roots.get(&block).cloned() .ok_or_else(|| sp_blockchain::Error::Backend(format!("Header CHT for block {} not exists", block))) + .map(Some) } fn changes_trie_cht_root( &self, _cht_size: NumberFor, block: NumberFor, - ) -> sp_blockchain::Result { + ) -> sp_blockchain::Result> { self.storage.read().changes_trie_cht_roots.get(&block).cloned() .ok_or_else(|| sp_blockchain::Error::Backend(format!("Changes trie CHT for block {} not exists", block))) + .map(Some) } fn cache(&self) -> Option>> { diff --git a/substrate/client/src/light/blockchain.rs b/substrate/client/src/light/blockchain.rs index 1ea4987078..756147c941 100644 --- a/substrate/client/src/light/blockchain.rs +++ b/substrate/client/src/light/blockchain.rs @@ -164,7 +164,10 @@ impl RemoteBlockchain for Blockchain } Ok(LocalOrRemote::Remote(RemoteHeaderRequest { - cht_root: self.storage.header_cht_root(cht::size(), number)?, + cht_root: match self.storage.header_cht_root(cht::size(), number)? { + Some(cht_root) => cht_root, + None => return Ok(LocalOrRemote::Unknown), + }, block: number, retry_count: None, })) @@ -298,17 +301,18 @@ pub mod tests { Err(ClientError::Backend("Test error".into())) } - fn header_cht_root(&self, _cht_size: u64, _block: u64) -> ClientResult { + fn header_cht_root(&self, _cht_size: u64, _block: u64) -> ClientResult> { Err(ClientError::Backend("Test error".into())) } - fn changes_trie_cht_root(&self, cht_size: u64, block: u64) -> ClientResult { + fn changes_trie_cht_root(&self, cht_size: u64, block: u64) -> ClientResult> { cht::block_to_cht_number(cht_size, block) .and_then(|cht_num| self.changes_tries_cht_roots.get(&cht_num)) .cloned() .ok_or_else(|| ClientError::Backend( format!("Test error: CHT for block #{} not found", block) ).into()) + .map(Some) } fn cache(&self) -> Option>> { diff --git a/substrate/client/src/light/fetcher.rs b/substrate/client/src/light/fetcher.rs index df99897e6d..3dd0b344b8 100644 --- a/substrate/client/src/light/fetcher.rs +++ b/substrate/client/src/light/fetcher.rs @@ -153,7 +153,7 @@ impl> LightDataChecker { // all the checks are sharing the same storage let storage = create_proof_check_backend_storage(remote_roots_proof); - // we remote_roots.keys() are sorted => we can use this to group changes tries roots + // remote_roots.keys() are sorted => we can use this to group changes tries roots // that are belongs to the same CHT let blocks = remote_roots.keys().cloned(); cht::for_each_cht_group::(cht_size, blocks, |mut storage, _, cht_blocks| { @@ -162,7 +162,8 @@ impl> LightDataChecker { // when required header has been pruned (=> replaced with CHT) let first_block = cht_blocks.first().cloned() .expect("for_each_cht_group never calls callback with empty groups"); - let local_cht_root = self.blockchain.storage().changes_trie_cht_root(cht_size, first_block)?; + let local_cht_root = self.blockchain.storage().changes_trie_cht_root(cht_size, first_block)? + .ok_or(ClientError::InvalidCHTProof)?; // check changes trie root for every block within CHT range for block in cht_blocks {