diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs index 612a05ffaa..a86eac35a1 100644 --- a/substrate/client/consensus/babe/src/lib.rs +++ b/substrate/client/consensus/babe/src/lib.rs @@ -347,7 +347,11 @@ impl Config { { trace!(target: "babe", "Getting slot duration"); - let best_block_id = BlockId::Hash(client.usage_info().chain.best_hash); + let mut best_block_id = BlockId::Hash(client.usage_info().chain.best_hash); + if client.usage_info().chain.finalized_state.is_none() { + debug!(target: "babe", "No finalized state is available. Reading config from genesis"); + best_block_id = BlockId::Hash(client.usage_info().chain.genesis_hash); + } let runtime_api = client.runtime_api(); let version = runtime_api.api_version::>(&best_block_id)?; diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 19766d7604..d7550ff9ae 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -477,7 +477,6 @@ impl BlockchainDb { let mut meta = self.meta.write(); if number.is_zero() { meta.genesis_hash = hash; - meta.finalized_hash = hash; } if is_best { @@ -1347,11 +1346,6 @@ impl Backend { } if number.is_zero() { - transaction.set_from_vec( - columns::META, - meta_keys::FINALIZED_BLOCK, - lookup_key.clone(), - ); transaction.set(columns::META, meta_keys::GENESIS_HASH, hash.as_ref()); if operation.commit_state { @@ -1447,14 +1441,15 @@ impl Backend { let finalized = number_u64 == 0 || pending_block.leaf_state.is_final(); finalized } else { - number.is_zero() || pending_block.leaf_state.is_final() + (number.is_zero() && last_finalized_num.is_zero()) || + pending_block.leaf_state.is_final() }; let header = &pending_block.header; let is_best = pending_block.leaf_state.is_best(); debug!(target: "db", - "DB Commit {:?} ({}), best={}, state={}, existing={}", - hash, number, is_best, operation.commit_state, existing_header, + "DB Commit {:?} ({}), best={}, state={}, existing={}, finalized={}", + hash, number, is_best, operation.commit_state, existing_header, finalized, ); self.state_usage.merge_sm(operation.old_state.usage_info()); @@ -2295,6 +2290,7 @@ pub(crate) mod tests { extrinsics_root: H256, ) -> H256 { insert_block(backend, number, parent_hash, changes, extrinsics_root, Vec::new(), None) + .unwrap() } pub fn insert_block( @@ -2305,7 +2301,7 @@ pub(crate) mod tests { extrinsics_root: H256, body: Vec>, transaction_index: Option>, - ) -> H256 { + ) -> Result { use sp_runtime::testing::Digest; let digest = Digest::default(); @@ -2329,9 +2325,9 @@ pub(crate) mod tests { if let Some(index) = transaction_index { op.update_transaction_index(index).unwrap(); } - backend.commit_operation(op).unwrap(); + backend.commit_operation(op)?; - header_hash + Ok(header_hash) } #[test] @@ -3019,7 +3015,6 @@ pub(crate) mod tests { { let header = backend.blockchain().header(BlockId::Hash(hash1)).unwrap().unwrap(); let mut op = backend.begin_operation().unwrap(); - backend.begin_state_operation(&mut op, BlockId::Hash(hash0)).unwrap(); op.set_block_data(header, None, None, None, NewBlockState::Best).unwrap(); backend.commit_operation(op).unwrap(); } @@ -3063,7 +3058,8 @@ pub(crate) mod tests { Default::default(), vec![i.into()], None, - ); + ) + .unwrap(); blocks.push(hash); prev_hash = hash; } @@ -3100,7 +3096,8 @@ pub(crate) mod tests { Default::default(), vec![i.into()], None, - ); + ) + .unwrap(); blocks.push(hash); prev_hash = hash; } @@ -3114,7 +3111,8 @@ pub(crate) mod tests { sp_core::H256::random(), vec![2.into()], None, - ); + ) + .unwrap(); insert_block( &backend, 3, @@ -3123,7 +3121,8 @@ pub(crate) mod tests { H256::random(), vec![3.into(), 11.into()], None, - ); + ) + .unwrap(); let mut op = backend.begin_operation().unwrap(); backend.begin_state_operation(&mut op, BlockId::Hash(blocks[4])).unwrap(); op.mark_head(BlockId::Hash(blocks[4])).unwrap(); @@ -3172,7 +3171,8 @@ pub(crate) mod tests { Default::default(), vec![i.into()], Some(index), - ); + ) + .unwrap(); blocks.push(hash); prev_hash = hash; } @@ -3206,7 +3206,8 @@ pub(crate) mod tests { Default::default(), vec![i.into()], None, - ); + ) + .unwrap(); blocks.push(hash); prev_hash = hash; } @@ -3220,7 +3221,8 @@ pub(crate) mod tests { sp_core::H256::random(), vec![42.into()], None, - ); + ) + .unwrap(); assert!(backend.remove_leaf_block(&best_hash).is_err()); assert!(backend.have_state_at(&prev_hash, 1)); backend.remove_leaf_block(&prev_hash).unwrap(); @@ -3290,4 +3292,21 @@ pub(crate) mod tests { assert_eq!(backend.blockchain().info().finalized_hash, block1); } + + #[test] + fn test_import_existing_state_fails() { + let backend: Backend = Backend::new_test(10, 10); + let genesis = + insert_block(&backend, 0, Default::default(), None, Default::default(), vec![], None) + .unwrap(); + + insert_block(&backend, 1, genesis, None, Default::default(), vec![], None).unwrap(); + let err = insert_block(&backend, 1, genesis, None, Default::default(), vec![], None) + .err() + .unwrap(); + match err { + sp_blockchain::Error::StateDatabase(m) if m == "Block already exists" => (), + e @ _ => panic!("Unexpected error {:?}", e), + } + } } diff --git a/substrate/client/network/src/protocol/sync.rs b/substrate/client/network/src/protocol/sync.rs index 69722dac22..af65dec1c3 100644 --- a/substrate/client/network/src/protocol/sync.rs +++ b/substrate/client/network/src/protocol/sync.rs @@ -1095,15 +1095,17 @@ impl ChainSync { } self.drain_blocks() }, - PeerSyncState::DownloadingGap(start_block) => { - let start_block = *start_block; + PeerSyncState::DownloadingGap(_) => { peer.state = PeerSyncState::Available; if let Some(gap_sync) = &mut self.gap_sync { gap_sync.blocks.clear_peer_download(who); - validate_blocks::(&blocks, who, Some(request))?; - gap_sync.blocks.insert(start_block, blocks, who.clone()); + if let Some(start_block) = + validate_blocks::(&blocks, who, Some(request))? + { + gap_sync.blocks.insert(start_block, blocks, who.clone()); + } gap = true; - gap_sync + let blocks: Vec<_> = gap_sync .blocks .drain(gap_sync.best_queued_number + One::one()) .into_iter() @@ -1126,7 +1128,9 @@ impl ChainSync { state: None, } }) - .collect() + .collect(); + debug!(target: "sync", "Drained {} gap blocks from {}", blocks.len(), gap_sync.best_queued_number); + blocks } else { debug!(target: "sync", "Unexpected gap block response from {}", who); return Err(BadPeer(who.clone(), rep::NO_BLOCK)) @@ -3169,4 +3173,44 @@ mod test { sync.peer_disconnected(&peer_id1); assert!(sync.fork_targets.len() == 0); } + + #[test] + fn can_import_response_with_missing_blocks() { + sp_tracing::try_init_simple(); + let mut client2 = Arc::new(TestClientBuilder::new().build()); + let blocks = (0..4).map(|_| build_block(&mut client2, None, false)).collect::>(); + + let empty_client = Arc::new(TestClientBuilder::new().build()); + + let mut sync = ChainSync::new( + SyncMode::Full, + empty_client.clone(), + Box::new(DefaultBlockAnnounceValidator), + 1, + None, + ) + .unwrap(); + + let peer_id1 = PeerId::random(); + let best_block = blocks[3].clone(); + sync.new_peer(peer_id1.clone(), best_block.hash(), *best_block.header().number()) + .unwrap(); + + sync.peers.get_mut(&peer_id1).unwrap().state = PeerSyncState::Available; + sync.peers.get_mut(&peer_id1).unwrap().common_number = 0; + + // Request all missing blocks and respond only with some. + let request = + get_block_request(&mut sync, FromBlock::Hash(best_block.hash()), 4, &peer_id1); + let response = + create_block_response(vec![blocks[3].clone(), blocks[2].clone(), blocks[1].clone()]); + sync.on_block_data(&peer_id1, Some(request.clone()), response).unwrap(); + assert_eq!(sync.best_queued_number, 0); + + // Request should only contain the missing block. + let request = get_block_request(&mut sync, FromBlock::Number(1), 1, &peer_id1); + let response = create_block_response(vec![blocks[0].clone()]); + sync.on_block_data(&peer_id1, Some(request), response).unwrap(); + assert_eq!(sync.best_queued_number, 4); + } } diff --git a/substrate/client/service/src/client/client.rs b/substrate/client/service/src/client/client.rs index 8769865978..7673a7b4c5 100644 --- a/substrate/client/service/src/client/client.rs +++ b/substrate/client/service/src/client/client.rs @@ -1726,9 +1726,8 @@ where .block_status(&BlockId::Hash(hash)) .map_err(|e| ConsensusError::ClientImport(e.to_string()))? { - BlockStatus::InChainWithState | BlockStatus::Queued if !import_existing => + BlockStatus::InChainWithState | BlockStatus::Queued => return Ok(ImportResult::AlreadyInChain), - BlockStatus::InChainWithState | BlockStatus::Queued => {}, BlockStatus::InChainPruned if !import_existing => return Ok(ImportResult::AlreadyInChain), BlockStatus::InChainPruned => {}, diff --git a/substrate/client/state-db/src/lib.rs b/substrate/client/state-db/src/lib.rs index b7d58bb808..74f218e88f 100644 --- a/substrate/client/state-db/src/lib.rs +++ b/substrate/client/state-db/src/lib.rs @@ -130,6 +130,8 @@ pub enum Error { InvalidPruningMode(String), /// Too many unfinalized sibling blocks inserted. TooManySiblingBlocks, + /// Trying to insert existing block. + BlockAlreadyExists, } /// Pinning error type. @@ -154,6 +156,7 @@ impl fmt::Debug for Error { Error::InvalidParent => write!(f, "Trying to insert block with unknown parent"), Error::InvalidPruningMode(e) => write!(f, "Expected pruning mode: {}", e), Error::TooManySiblingBlocks => write!(f, "Too many sibling blocks inserted"), + Error::BlockAlreadyExists => write!(f, "Block already exists"), } } } diff --git a/substrate/client/state-db/src/noncanonical.rs b/substrate/client/state-db/src/noncanonical.rs index d7c83492e5..7d6fcbced7 100644 --- a/substrate/client/state-db/src/noncanonical.rs +++ b/substrate/client/state-db/src/noncanonical.rs @@ -210,9 +210,10 @@ impl NonCanonicalOverlay { insert_values(&mut values, record.inserted); trace!( target: "state-db", - "Uncanonicalized journal entry {}.{} ({} inserted, {} deleted)", + "Uncanonicalized journal entry {}.{} ({:?}) ({} inserted, {} deleted)", block, index, + record.hash, overlay.inserted.len(), overlay.deleted.len() ); @@ -296,6 +297,9 @@ impl NonCanonicalOverlay { if level.blocks.len() >= MAX_BLOCKS_PER_LEVEL as usize { return Err(Error::TooManySiblingBlocks) } + if level.blocks.iter().any(|b| b.hash == *hash) { + return Err(Error::BlockAlreadyExists) + } let index = level.available_index(); let journal_key = to_journal_key(number, index); @@ -641,7 +645,7 @@ mod tests { use super::{to_journal_key, NonCanonicalOverlay}; use crate::{ test::{make_changeset, make_db}, - ChangeSet, CommitSet, MetaDb, + ChangeSet, CommitSet, Error, MetaDb, }; use sp_core::H256; use std::io; @@ -710,6 +714,20 @@ mod tests { .unwrap(); } + #[test] + fn insert_existing_fails() { + let db = make_db(&[]); + let h1 = H256::random(); + let mut overlay = NonCanonicalOverlay::::new(&db).unwrap(); + overlay + .insert::(&h1, 2, &H256::default(), ChangeSet::default()) + .unwrap(); + assert!(matches!( + overlay.insert::(&h1, 2, &H256::default(), ChangeSet::default()), + Err(Error::BlockAlreadyExists) + )); + } + #[test] #[should_panic] fn canonicalize_unknown_panics() { diff --git a/substrate/primitives/blockchain/src/header_metadata.rs b/substrate/primitives/blockchain/src/header_metadata.rs index 9f388dc58f..6e8dc56247 100644 --- a/substrate/primitives/blockchain/src/header_metadata.rs +++ b/substrate/primitives/blockchain/src/header_metadata.rs @@ -20,7 +20,7 @@ use lru::LruCache; use parking_lot::RwLock; -use sp_runtime::traits::{Block as BlockT, Header, NumberFor}; +use sp_runtime::traits::{Block as BlockT, Header, NumberFor, One}; /// Set to the expected max difference between `best` and `finalized` blocks at sync. const LRU_CACHE_SIZE: usize = 5_000; @@ -37,7 +37,14 @@ pub fn lowest_common_ancestor + ?Sized>( id_two: Block::Hash, ) -> Result, T::Error> { let mut header_one = backend.header_metadata(id_one)?; + if header_one.parent == id_two { + return Ok(HashAndNumber { hash: id_two, number: header_one.number - One::one() }) + } + let mut header_two = backend.header_metadata(id_two)?; + if header_two.parent == id_one { + return Ok(HashAndNumber { hash: id_one, number: header_one.number }) + } let mut orig_header_one = header_one.clone(); let mut orig_header_two = header_two.clone();