diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 4452d5dcb3f29..7c2635f843418 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -1213,9 +1213,8 @@ impl Backend { let meta = self.blockchain.meta.read(); - if meta.best_number > best_number && - (meta.best_number - best_number).saturated_into::() > - self.canonicalization_delay + if meta.best_number.saturating_sub(best_number).saturated_into::() > + self.canonicalization_delay { return Err(sp_blockchain::Error::SetHeadTooOld) } @@ -1316,42 +1315,43 @@ impl Backend { fn force_delayed_canonicalize( &self, transaction: &mut Transaction, - hash: Block::Hash, - number: NumberFor, ) -> ClientResult<()> { - let number_u64 = number.saturated_into::(); - if number_u64 > self.canonicalization_delay { - let new_canonical = number_u64 - self.canonicalization_delay; + let best_canonical = || self.storage.state_db.best_canonical().unwrap_or(0); + let info = self.blockchain.info(); + let best_number: u64 = self.blockchain.info().best_number.saturated_into(); - if new_canonical <= self.storage.state_db.best_canonical().unwrap_or(0) { - return Ok(()) - } - let hash = if new_canonical == number_u64 { - hash - } else { - sc_client_api::blockchain::HeaderBackend::hash( - &self.blockchain, - new_canonical.saturated_into(), - )? - .ok_or_else(|| { - sp_blockchain::Error::Backend(format!( - "Can't canonicalize missing block number #{} when importing {:?} (#{})", - new_canonical, hash, number, - )) - })? - }; - if !sc_client_api::Backend::have_state_at(self, hash, new_canonical.saturated_into()) { + while best_number.saturating_sub(best_canonical()) > self.canonicalization_delay { + let to_canonicalize = best_canonical() + 1; + + let hash_to_canonicalize = sc_client_api::blockchain::HeaderBackend::hash( + &self.blockchain, + to_canonicalize.saturated_into(), + )? + .ok_or_else(|| { + let best_hash = info.best_hash; + + sp_blockchain::Error::Backend(format!( + "Can't canonicalize missing block number #{to_canonicalize} when for best block {best_hash:?} (#{best_number})", + )) + })?; + + if !sc_client_api::Backend::have_state_at( + self, + hash_to_canonicalize, + to_canonicalize.saturated_into(), + ) { return Ok(()) } - trace!(target: "db", "Canonicalize block #{} ({:?})", new_canonical, hash); - let commit = self.storage.state_db.canonicalize_block(&hash).map_err( + trace!(target: "db", "Canonicalize block #{} ({:?})", to_canonicalize, hash_to_canonicalize); + let commit = self.storage.state_db.canonicalize_block(&hash_to_canonicalize).map_err( sp_blockchain::Error::from_state_db::< sc_state_db::Error, >, )?; apply_state_commit(transaction, commit); } + Ok(()) } @@ -1559,7 +1559,7 @@ impl Backend { )?; } else { // canonicalize blocks which are old enough, regardless of finality. - self.force_delayed_canonicalize(&mut transaction, hash, *header.number())? + self.force_delayed_canonicalize(&mut transaction)? } if !existing_header { @@ -2759,6 +2759,33 @@ pub(crate) mod tests { .into(); let hash = header.hash(); + op.set_block_data(header, Some(vec![]), None, None, NewBlockState::Best) + .unwrap(); + + backend.commit_operation(op).unwrap(); + hash + }; + + let hashof4 = { + let mut op = backend.begin_operation().unwrap(); + backend.begin_state_operation(&mut op, hashof3).unwrap(); + let mut header = Header { + number: 4, + parent_hash: hashof3, + state_root: Default::default(), + digest: Default::default(), + extrinsics_root: Default::default(), + }; + + let storage: Vec<(_, _)> = vec![]; + + header.state_root = op + .old_state + .storage_root(storage.iter().cloned().map(|(x, y)| (x, Some(y))), state_version) + .0 + .into(); + let hash = header.hash(); + op.set_block_data(header, Some(vec![]), None, None, NewBlockState::Best) .unwrap(); @@ -2774,6 +2801,7 @@ pub(crate) mod tests { backend.finalize_block(hashof1, None).unwrap(); backend.finalize_block(hashof2, None).unwrap(); backend.finalize_block(hashof3, None).unwrap(); + backend.finalize_block(hashof4, None).unwrap(); assert!(backend .storage .db @@ -3801,4 +3829,105 @@ pub(crate) mod tests { assert_eq!(backend.blockchain().leaves().unwrap(), vec![block2]); assert_eq!(backend.blockchain().info().best_hash, block2); } + + #[test] + fn force_delayed_canonicalize_waiting_for_blocks_to_be_finalized() { + let backend = Backend::::new_test(10, 1); + + let genesis = + insert_block(&backend, 0, Default::default(), None, Default::default(), vec![], None) + .unwrap(); + + let block1 = { + let mut op = backend.begin_operation().unwrap(); + backend.begin_state_operation(&mut op, genesis).unwrap(); + let header = Header { + number: 1, + parent_hash: genesis, + state_root: BlakeTwo256::trie_root(Vec::new(), StateVersion::V1), + digest: Default::default(), + extrinsics_root: Default::default(), + }; + + op.set_block_data(header.clone(), Some(Vec::new()), None, None, NewBlockState::Normal) + .unwrap(); + + backend.commit_operation(op).unwrap(); + + header.hash() + }; + + assert_eq!(0, backend.storage.state_db.best_canonical().unwrap()); + + // This should not trigger any forced canonicalization as we didn't have imported any best + // block by now. + let block2 = { + let mut op = backend.begin_operation().unwrap(); + backend.begin_state_operation(&mut op, block1).unwrap(); + let header = Header { + number: 2, + parent_hash: block1, + state_root: BlakeTwo256::trie_root(Vec::new(), StateVersion::V1), + digest: Default::default(), + extrinsics_root: Default::default(), + }; + + op.set_block_data(header.clone(), Some(Vec::new()), None, None, NewBlockState::Normal) + .unwrap(); + + backend.commit_operation(op).unwrap(); + + header.hash() + }; + + assert_eq!(0, backend.storage.state_db.best_canonical().unwrap()); + + // This should also not trigger it yet, because we import a best block, but the best block + // from the POV of the db is still at `0`. + let block3 = { + let mut op = backend.begin_operation().unwrap(); + backend.begin_state_operation(&mut op, block2).unwrap(); + let header = Header { + number: 3, + parent_hash: block2, + state_root: BlakeTwo256::trie_root(Vec::new(), StateVersion::V1), + digest: Default::default(), + extrinsics_root: Default::default(), + }; + + op.set_block_data(header.clone(), Some(Vec::new()), None, None, NewBlockState::Best) + .unwrap(); + + backend.commit_operation(op).unwrap(); + + header.hash() + }; + + // Now it should kick in. + let block4 = { + let mut op = backend.begin_operation().unwrap(); + backend.begin_state_operation(&mut op, block3).unwrap(); + let header = Header { + number: 4, + parent_hash: block3, + state_root: BlakeTwo256::trie_root(Vec::new(), StateVersion::V1), + digest: Default::default(), + extrinsics_root: Default::default(), + }; + + op.set_block_data(header.clone(), Some(Vec::new()), None, None, NewBlockState::Best) + .unwrap(); + + backend.commit_operation(op).unwrap(); + + header.hash() + }; + + assert_eq!(2, backend.storage.state_db.best_canonical().unwrap()); + + assert_eq!(block1, backend.blockchain().hash(1).unwrap().unwrap()); + assert_eq!(block2, backend.blockchain().hash(2).unwrap().unwrap()); + assert_eq!(block3, backend.blockchain().hash(3).unwrap().unwrap()); + assert_eq!(block4, backend.blockchain().hash(4).unwrap().unwrap()); + } }