Document more TODOs as tickets (#1418)

Went through the TODOs, removed a bunch, which are outdated or nothing more than a regular comment, documented a bunch more as actual tickets and made them FIXMEs and unified their structure (`FIXME #TICKETNO DESC` for local tickets, `FIXME: DESC LINK` for external tickets) for easier in-editor support. Further more remove unnecessary remarks and related old code that I noticed in that instance.

substrate/core/client/db/src/lib.rs

@@ -307,7 +307,6 @@ where Block: BlockT<Hash=H256>,
 	}
 
 	fn reset_storage(&mut self, mut top: StorageMap, children: ChildrenStorageMap) -> Result<H256, client::error::Error> {
-		// TODO: wipe out existing trie.
 
 		if top.iter().any(|(k, _)| well_known_keys::is_child_storage_key(k)) {
 			return Err(client::error::ErrorKind::GenesisInvalid.into());
@@ -384,7 +383,7 @@ struct DbGenesisStorage(pub H256);
 impl DbGenesisStorage {
 	pub fn new() -> Self {
 		let mut root = H256::default();
-		let mut mdb = MemoryDB::<Blake2Hasher>::default();	// TODO: use new() to make it more correct
+		let mut mdb = MemoryDB::<Blake2Hasher>::default();
 		state_machine::TrieDBMut::<Blake2Hasher>::new(&mut mdb, &mut root);
 		DbGenesisStorage(root)
 	}
@@ -1024,7 +1023,7 @@ mod tests {
 
 	fn prepare_changes(changes: Vec<(Vec<u8>, Vec<u8>)>) -> (H256, MemoryDB<Blake2Hasher>) {
 		let mut changes_root = H256::default();
-		let mut changes_trie_update = MemoryDB::<Blake2Hasher>::default();		// TODO: change to new() to make more correct
+		let mut changes_trie_update = MemoryDB::<Blake2Hasher>::default();
 		{
 			let mut trie = TrieDBMut::<Blake2Hasher>::new(
 				&mut changes_trie_update,

substrate/core/client/db/src/light.rs

@@ -341,7 +341,6 @@ impl<Block> LightBlockchainStorage<Block> for LightStorage<Block>
 					// update block number to hash lookup entries.
 					for retracted in tree_route.retracted() {
 						if retracted.hash == meta.finalized_hash {
-							// TODO: can we recover here?
 							warn!("Safety failure: reverting finalized block {:?}",
 								(&retracted.number, &retracted.hash));
 						}
@@ -438,7 +437,6 @@ impl<Block> LightBlockchainStorage<Block> for LightStorage<Block>
 	fn finalize_header(&self, id: BlockId<Block>) -> ClientResult<()> {
 		if let Some(header) = self.header(id)? {
 			let mut transaction = DBTransaction::new();
-			// TODO: ensure best chain contains this block.
 			let hash = header.hash();
 			let number = *header.number();
 			self.note_finalized(&mut transaction, &header, hash.clone())?;

substrate/core/client/db/src/storage_cache.rs

@@ -45,7 +45,7 @@ pub type SharedCache<B, H> = Arc<Mutex<Cache<B, H>>>;
 
 /// Create new shared cache instance with given max memory usage.
 pub fn new_shared_cache<B: Block, H: Hasher>(shared_cache_size: usize) -> SharedCache<B, H> {
-	let cache_items = shared_cache_size / 100; // Estimated average item size. TODO: more accurate tracking
+	let cache_items = shared_cache_size / 100; // Guestimate, potentially inaccurate
 	Arc::new(Mutex::new(Cache {
 		storage: LruCache::new(cache_items),
 		hashes: LruCache::new(cache_items),

substrate/core/client/src/call_executor.rs

@@ -205,7 +205,6 @@ where
 		native_call: Option<NC>,
 	) -> Result<NativeOrEncoded<R>, error::Error> where ExecutionManager<EM>: Clone {
 		let state = self.backend.state_at(*at)?;
-		//TODO: Find a better way to prevent double block initialization
 		if method != "Core_initialise_block" && initialised_block.map(|id| id != *at).unwrap_or(true) {
 			let header = prepare_environment_block()?;
 			state_machine::execute_using_consensus_failure_handler::<

substrate/core/client/src/client.rs

@@ -130,7 +130,6 @@ pub trait BlockBody<Block: BlockT> {
 }
 
 /// Client info
-// TODO: split queue info from chain info and amalgamate into single struct.
 #[derive(Debug)]
 pub struct ClientInfo<Block: BlockT> {
 	/// Best block hash.
@@ -325,7 +324,6 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 
 	/// Get the RuntimeVersion at a given block.
 	pub fn runtime_version_at(&self, id: &BlockId<Block>) -> error::Result<RuntimeVersion> {
-		// TODO: Post Poc-2 return an error if version is missing
 		self.executor.runtime_version(id)
 	}
 
@@ -738,11 +736,9 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			self.apply_finality_with_block_hash(operation, parent_hash, None, last_best, make_notifications)?;
 		}
 
-		// TODO: correct path logic for when to execute this function
-		// https://github.com/paritytech/substrate/issues/1232
+		// FIXME #1232: correct path logic for when to execute this function
 		let (storage_update,changes_update,storage_changes) = self.block_execution(&operation.op, &import_headers, origin, hash, body.clone())?;
 
-		// TODO: non longest-chain rule.
 		let is_new_best = finalized || match fork_choice {
 			ForkChoiceStrategy::LongestChain => import_headers.post().number() > &last_best_number,
 			ForkChoiceStrategy::Custom(v) => v,
@@ -880,7 +876,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		// if the block is not a direct ancestor of the current best chain,
 		// then some other block is the common ancestor.
 		if route_from_best.common_block().hash != block {
-			// TODO: reorganize best block to be the best chain containing
+			// FIXME: #1442 reorganize best block to be the best chain containing
 			// `block`.
 		}
 
@@ -1020,7 +1016,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 
 	/// Get block status.
 	pub fn block_status(&self, id: &BlockId<Block>) -> error::Result<BlockStatus> {
-		// TODO: more efficient implementation
+		// this can probably be implemented more efficiently
 		if let BlockId::Hash(ref h) = id {
 			if self.importing_block.read().as_ref().map_or(false, |importing| h == importing) {
 				return Ok(BlockStatus::Queued);
@@ -1073,10 +1069,9 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	/// If `maybe_max_block_number` is `Some(max_block_number)`
 	/// the search is limited to block `numbers <= max_block_number`.
 	/// in other words as if there were no blocks greater `max_block_number`.
-	///
-	/// TODO [snd] possibly implement this on blockchain::Backend and just redirect here
+	/// TODO : we want to move this implement to `blockchain::Backend`, see [#1443](https://github.com/paritytech/substrate/issues/1443)
 	/// Returns `Ok(None)` if `target_hash` is not found in search space.
-	/// TODO [snd] write down time complexity
+	/// TODO: document time complexity of this, see [#1444](https://github.com/paritytech/substrate/issues/1444)
 	pub fn best_containing(&self, target_hash: Block::Hash, maybe_max_number: Option<NumberFor<Block>>)
 		-> error::Result<Option<Block::Hash>>
 	{
@@ -1140,7 +1135,6 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			// waiting until we are <= max_number
 			if let Some(max_number) = maybe_max_number {
 				loop {
-					// TODO [snd] this should be a panic
 					let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))?
 						.ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?;
 
@@ -1160,7 +1154,6 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 					return Ok(Some(best_hash));
 				}
 
-				// TODO [snd] this should be a panic
 				let current_header = self.backend.blockchain().header(BlockId::Hash(current_hash.clone()))?
 					.ok_or_else(|| error::Error::from(format!("failed to get header for hash {}", current_hash)))?;
 
@@ -1176,8 +1169,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		// header may be on a dead fork -- the only leaves that are considered are
 		// those which can still be finalized.
 		//
-		// TODO: only issue this warning when not on a dead fork
-		// part of https://github.com/paritytech/substrate/issues/1558
+		// FIXME #1558 only issue this warning when not on a dead fork
 		warn!(
 			"Block {:?} exists in chain but not found when following all \
 			leaves backwards. Number limit = {:?}",

substrate/core/client/src/light/blockchain.rs

@@ -141,7 +141,7 @@ impl<S, F, Block> BlockchainHeaderBackend<Block> for Blockchain<S, F> where Bloc
 
 impl<S, F, Block> BlockchainBackend<Block> for Blockchain<S, F> where Block: BlockT, S: Storage<Block>, F: Fetcher<Block> {
 	fn body(&self, _id: BlockId<Block>) -> ClientResult<Option<Vec<Block::Extrinsic>>> {
-		// TODO [light]: fetch from remote node
+		// TODO: #1445 fetch from remote node
 		Ok(None)
 	}