BlockId removal: refactor: Backend::body (#12587)

It changes the arguments of `Backend::body` method from: `BlockId<Block>` to: `&Block::Hash`

This PR is part of BlockId::Number refactoring analysis (paritytech/substrate#11292)

Co-authored-by: parity-processbot <>

substrate/bin/node/inspect/src/lib.rs

@@ -140,10 +140,11 @@ impl<TBlock: Block, TPrinter: PrettyPrinter<TBlock>> Inspector<TBlock, TPrinter>
 			BlockAddress::Bytes(bytes) => TBlock::decode(&mut &*bytes)?,
 			BlockAddress::Number(number) => {
 				let id = BlockId::number(number);
+				let hash = self.chain.expect_block_hash_from_id(&id)?;
 				let not_found = format!("Could not find block {:?}", id);
 				let body = self
 					.chain
-					.block_body(&id)?
+					.block_body(&hash)?
 					.ok_or_else(|| Error::NotFound(not_found.clone()))?;
 				let header =
 					self.chain.header(id)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;
@@ -154,7 +155,7 @@ impl<TBlock: Block, TPrinter: PrettyPrinter<TBlock>> Inspector<TBlock, TPrinter>
 				let not_found = format!("Could not find block {:?}", id);
 				let body = self
 					.chain
-					.block_body(&id)?
+					.block_body(&hash)?
 					.ok_or_else(|| Error::NotFound(not_found.clone()))?;
 				let header =
 					self.chain.header(id)?.ok_or_else(|| Error::NotFound(not_found.clone()))?;

substrate/client/api/src/client.rs

@@ -110,7 +110,7 @@ pub trait BlockBackend<Block: BlockT> {
 	/// Get block body by ID. Returns `None` if the body is not stored.
 	fn block_body(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 	) -> sp_blockchain::Result<Option<Vec<<Block as BlockT>::Extrinsic>>>;
 
 	/// Get all indexed transactions for a block,

substrate/client/api/src/in_mem.rs

@@ -405,15 +405,14 @@ impl<Block: BlockT> HeaderMetadata<Block> for Blockchain<Block> {
 impl<Block: BlockT> blockchain::Backend<Block> for Blockchain<Block> {
 	fn body(
 		&self,
-		id: BlockId<Block>,
+		hash: &Block::Hash,
 	) -> sp_blockchain::Result<Option<Vec<<Block as BlockT>::Extrinsic>>> {
-		Ok(self.id(id).and_then(|hash| {
-			self.storage
-				.read()
-				.blocks
-				.get(&hash)
-				.and_then(|b| b.extrinsics().map(|x| x.to_vec()))
-		}))
+		Ok(self
+			.storage
+			.read()
+			.blocks
+			.get(hash)
+			.and_then(|b| b.extrinsics().map(|x| x.to_vec())))
 	}
 
 	fn justifications(&self, id: BlockId<Block>) -> sp_blockchain::Result<Option<Justifications>> {

substrate/client/db/src/lib.rs

@@ -577,8 +577,10 @@ impl<Block: BlockT> sc_client_api::blockchain::HeaderBackend<Block> for Blockcha
 }
 
 impl<Block: BlockT> sc_client_api::blockchain::Backend<Block> for BlockchainDb<Block> {
-	fn body(&self, id: BlockId<Block>) -> ClientResult<Option<Vec<Block::Extrinsic>>> {
-		if let Some(body) = read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY, id)? {
+	fn body(&self, hash: &Block::Hash) -> ClientResult<Option<Vec<Block::Extrinsic>>> {
+		if let Some(body) =
+			read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY, BlockId::Hash::<Block>(*hash))?
+		{
 			// Plain body
 			match Decode::decode(&mut &body[..]) {
 				Ok(body) => return Ok(Some(body)),
@@ -590,7 +592,12 @@ impl<Block: BlockT> sc_client_api::blockchain::Backend<Block> for BlockchainDb<B
 			}
 		}
 
-		if let Some(index) = read_db(&*self.db, columns::KEY_LOOKUP, columns::BODY_INDEX, id)? {
+		if let Some(index) = read_db(
+			&*self.db,
+			columns::KEY_LOOKUP,
+			columns::BODY_INDEX,
+			BlockId::Hash::<Block>(*hash),
+		)? {
 			match Vec::<DbExtrinsic<Block>>::decode(&mut &index[..]) {
 				Ok(index) => {
 					let mut body = Vec::new();
@@ -3201,11 +3208,11 @@ pub(crate) mod tests {
 			backend.commit_operation(op).unwrap();
 		}
 		let bc = backend.blockchain();
-		assert_eq!(None, bc.body(BlockId::hash(blocks[0])).unwrap());
-		assert_eq!(None, bc.body(BlockId::hash(blocks[1])).unwrap());
-		assert_eq!(None, bc.body(BlockId::hash(blocks[2])).unwrap());
-		assert_eq!(Some(vec![3.into()]), bc.body(BlockId::hash(blocks[3])).unwrap());
-		assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap());
+		assert_eq!(None, bc.body(&blocks[0]).unwrap());
+		assert_eq!(None, bc.body(&blocks[1]).unwrap());
+		assert_eq!(None, bc.body(&blocks[2]).unwrap());
+		assert_eq!(Some(vec![3.into()]), bc.body(&blocks[3]).unwrap());
+		assert_eq!(Some(vec![4.into()]), bc.body(&blocks[4]).unwrap());
 	}
 
 	#[test]
@@ -3236,11 +3243,11 @@ pub(crate) mod tests {
 		backend.commit_operation(op).unwrap();
 
 		let bc = backend.blockchain();
-		assert_eq!(Some(vec![0.into()]), bc.body(BlockId::hash(blocks[0])).unwrap());
-		assert_eq!(Some(vec![1.into()]), bc.body(BlockId::hash(blocks[1])).unwrap());
-		assert_eq!(Some(vec![2.into()]), bc.body(BlockId::hash(blocks[2])).unwrap());
-		assert_eq!(Some(vec![3.into()]), bc.body(BlockId::hash(blocks[3])).unwrap());
-		assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap());
+		assert_eq!(Some(vec![0.into()]), bc.body(&blocks[0]).unwrap());
+		assert_eq!(Some(vec![1.into()]), bc.body(&blocks[1]).unwrap());
+		assert_eq!(Some(vec![2.into()]), bc.body(&blocks[2]).unwrap());
+		assert_eq!(Some(vec![3.into()]), bc.body(&blocks[3]).unwrap());
+		assert_eq!(Some(vec![4.into()]), bc.body(&blocks[4]).unwrap());
 	}
 
 	#[test]
@@ -3291,7 +3298,7 @@ pub(crate) mod tests {
 		backend.commit_operation(op).unwrap();
 
 		let bc = backend.blockchain();
-		assert_eq!(Some(vec![2.into()]), bc.body(BlockId::hash(fork_hash_root)).unwrap());
+		assert_eq!(Some(vec![2.into()]), bc.body(&fork_hash_root).unwrap());
 
 		for i in 1..5 {
 			let mut op = backend.begin_operation().unwrap();
@@ -3300,13 +3307,13 @@ pub(crate) mod tests {
 			backend.commit_operation(op).unwrap();
 		}
 
-		assert_eq!(Some(vec![0.into()]), bc.body(BlockId::hash(blocks[0])).unwrap());
-		assert_eq!(Some(vec![1.into()]), bc.body(BlockId::hash(blocks[1])).unwrap());
-		assert_eq!(Some(vec![2.into()]), bc.body(BlockId::hash(blocks[2])).unwrap());
-		assert_eq!(Some(vec![3.into()]), bc.body(BlockId::hash(blocks[3])).unwrap());
-		assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap());
+		assert_eq!(Some(vec![0.into()]), bc.body(&blocks[0]).unwrap());
+		assert_eq!(Some(vec![1.into()]), bc.body(&blocks[1]).unwrap());
+		assert_eq!(Some(vec![2.into()]), bc.body(&blocks[2]).unwrap());
+		assert_eq!(Some(vec![3.into()]), bc.body(&blocks[3]).unwrap());
+		assert_eq!(Some(vec![4.into()]), bc.body(&blocks[4]).unwrap());
 
-		assert_eq!(Some(vec![2.into()]), bc.body(BlockId::hash(fork_hash_root)).unwrap());
+		assert_eq!(Some(vec![2.into()]), bc.body(&fork_hash_root).unwrap());
 		assert_eq!(bc.info().best_number, 4);
 		for i in 0..5 {
 			assert!(bc.hash(i).unwrap().is_some());
@@ -3367,11 +3374,11 @@ pub(crate) mod tests {
 		}
 
 		let bc = backend.blockchain();
-		assert_eq!(None, bc.body(BlockId::hash(blocks[0])).unwrap());
-		assert_eq!(None, bc.body(BlockId::hash(blocks[1])).unwrap());
-		assert_eq!(None, bc.body(BlockId::hash(blocks[2])).unwrap());
-		assert_eq!(Some(vec![3.into()]), bc.body(BlockId::hash(blocks[3])).unwrap());
-		assert_eq!(Some(vec![4.into()]), bc.body(BlockId::hash(blocks[4])).unwrap());
+		assert_eq!(None, bc.body(&blocks[0]).unwrap());
+		assert_eq!(None, bc.body(&blocks[1]).unwrap());
+		assert_eq!(None, bc.body(&blocks[2]).unwrap());
+		assert_eq!(Some(vec![3.into()]), bc.body(&blocks[3]).unwrap());
+		assert_eq!(Some(vec![4.into()]), bc.body(&blocks[4]).unwrap());
 	}
 
 	#[test]
@@ -3408,11 +3415,12 @@ pub(crate) mod tests {
 		assert_eq!(bc.indexed_transaction(&x0_hash).unwrap().unwrap(), &x0[1..]);
 		assert_eq!(bc.indexed_transaction(&x1_hash).unwrap().unwrap(), &x1[1..]);
 
+		let hashof0 = bc.info().genesis_hash;
 		// Push one more blocks and make sure block is pruned and transaction index is cleared.
 		let block1 =
 			insert_block(&backend, 1, hash, None, Default::default(), vec![], None).unwrap();
 		backend.finalize_block(&block1, None).unwrap();
-		assert_eq!(bc.body(BlockId::Number(0)).unwrap(), None);
+		assert_eq!(bc.body(&hashof0).unwrap(), None);
 		assert_eq!(bc.indexed_transaction(&x0_hash).unwrap(), None);
 		assert_eq!(bc.indexed_transaction(&x1_hash).unwrap(), None);
 	}

substrate/client/network/sync/src/block_request_handler.rs

@@ -364,7 +364,7 @@ where
 				};
 
 			let body = if get_body {
-				match self.client.block_body(&BlockId::Hash(hash))? {
+				match self.client.block_body(&hash)? {
 					Some(mut extrinsics) =>
 						extrinsics.iter_mut().map(|extrinsic| extrinsic.encode()).collect(),
 					None => {

substrate/client/network/test/src/lib.rs

@@ -542,7 +542,7 @@ where
 	pub fn has_body(&self, hash: &H256) -> bool {
 		self.backend
 			.as_ref()
-			.map(|backend| backend.blockchain().body(BlockId::hash(*hash)).unwrap().is_some())
+			.map(|backend| backend.blockchain().body(hash).unwrap().is_some())
 			.unwrap_or(false)
 	}
 }

substrate/client/service/src/client/client.rs

@@ -1053,9 +1053,9 @@ where
 	/// Get block body by id.
 	pub fn body(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 	) -> sp_blockchain::Result<Option<Vec<<Block as BlockT>::Extrinsic>>> {
-		self.backend.blockchain().body(*id)
+		self.backend.blockchain().body(hash)
 	}
 
 	/// Gets the uncles of the block with `target_hash` going back `max_generation` ancestors.
@@ -1939,16 +1939,22 @@ where
 {
 	fn block_body(
 		&self,
-		id: &BlockId<Block>,
+		hash: &Block::Hash,
 	) -> sp_blockchain::Result<Option<Vec<<Block as BlockT>::Extrinsic>>> {
-		self.body(id)
+		self.body(hash)
 	}
 
 	fn block(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<SignedBlock<Block>>> {
-		Ok(match (self.header(id)?, self.body(id)?, self.justifications(id)?) {
-			(Some(header), Some(extrinsics), justifications) =>
-				Some(SignedBlock { block: Block::new(header, extrinsics), justifications }),
-			_ => None,
+		Ok(match self.header(id)? {
+			Some(header) => {
+				let hash = header.hash();
+				match (self.body(&hash)?, self.justifications(id)?) {
+					(Some(extrinsics), justifications) =>
+						Some(SignedBlock { block: Block::new(header, extrinsics), justifications }),
+					_ => None,
+				}
+			},
+			None => None,
 		})
 	}
 

substrate/client/service/test/src/client/mod.rs

@@ -396,16 +396,19 @@ fn block_builder_does_not_include_invalid() {
 	let block = builder.build().unwrap().block;
 	block_on(client.import(BlockOrigin::Own, block)).unwrap();
 
-	let hash0 = client
+	let hashof0 = client
 		.expect_block_hash_from_id(&BlockId::Number(0))
 		.expect("block 0 was just imported. qed");
-	let hash1 = client
+	let hashof1 = client
 		.expect_block_hash_from_id(&BlockId::Number(1))
 		.expect("block 1 was just imported. qed");
 
 	assert_eq!(client.chain_info().best_number, 1);
-	assert_ne!(client.state_at(&hash1).unwrap().pairs(), client.state_at(&hash0).unwrap().pairs());
-	assert_eq!(client.body(&BlockId::Number(1)).unwrap().unwrap().len(), 1)
+	assert_ne!(
+		client.state_at(&hashof1).unwrap().pairs(),
+		client.state_at(&hashof0).unwrap().pairs()
+	);
+	assert_eq!(client.body(&hashof1).unwrap().unwrap().len(), 1)
 }
 
 #[test]

substrate/client/tracing/src/block/mod.rs

@@ -225,7 +225,7 @@ where
 			.ok_or_else(|| Error::MissingBlockComponent("Header not found".to_string()))?;
 		let extrinsics = self
 			.client
-			.block_body(&id)
+			.block_body(&self.block)
 			.map_err(Error::InvalidBlockId)?
 			.ok_or_else(|| Error::MissingBlockComponent("Extrinsics not found".to_string()))?;
 		tracing::debug!(target: "state_tracing", "Found {} extrinsics", extrinsics.len());

substrate/client/transaction-pool/benches/basics.rs

@@ -111,7 +111,7 @@ impl ChainApi for TestApi {
 		(blake2_256(&encoded).into(), encoded.len())
 	}
 
-	fn block_body(&self, _id: &BlockId<Self::Block>) -> Self::BodyFuture {
+	fn block_body(&self, _id: &<Self::Block as BlockT>::Hash) -> Self::BodyFuture {
 		ready(Ok(None))
 	}
 

substrate/client/transaction-pool/src/api.rs

@@ -126,8 +126,8 @@ where
 		Pin<Box<dyn Future<Output = error::Result<TransactionValidity>> + Send>>;
 	type BodyFuture = Ready<error::Result<Option<Vec<<Self::Block as BlockT>::Extrinsic>>>>;
 
-	fn block_body(&self, id: &BlockId<Self::Block>) -> Self::BodyFuture {
-		ready(self.client.block_body(id).map_err(error::Error::from))
+	fn block_body(&self, hash: &Block::Hash) -> Self::BodyFuture {
+		ready(self.client.block_body(hash).map_err(error::Error::from))
 	}
 
 	fn validate_transaction(

substrate/client/transaction-pool/src/graph/pool.rs

@@ -90,8 +90,8 @@ pub trait ChainApi: Send + Sync {
 	/// Returns hash and encoding length of the extrinsic.
 	fn hash_and_length(&self, uxt: &ExtrinsicFor<Self>) -> (ExtrinsicHash<Self>, usize);
 
-	/// Returns a block body given the block id.
-	fn block_body(&self, at: &BlockId<Self::Block>) -> Self::BodyFuture;
+	/// Returns a block body given the block.
+	fn block_body(&self, at: &<Self::Block as BlockT>::Hash) -> Self::BodyFuture;
 
 	/// Returns a block header given the block id.
 	fn block_header(

substrate/client/transaction-pool/src/lib.rs

@@ -550,12 +550,12 @@ impl<N: Clone + Copy + AtLeast32Bit> RevalidationStatus<N> {
 
 /// Prune the known txs for the given block.
 async fn prune_known_txs_for_block<Block: BlockT, Api: graph::ChainApi<Block = Block>>(
-	block_id: BlockId<Block>,
+	block_hash: &Block::Hash,
 	api: &Api,
 	pool: &graph::Pool<Api>,
 ) -> Vec<ExtrinsicHash<Api>> {
 	let extrinsics = api
-		.block_body(&block_id)
+		.block_body(&block_hash)
 		.await
 		.unwrap_or_else(|e| {
 			log::warn!("Prune known transactions: error request: {}", e);
@@ -567,19 +567,21 @@ async fn prune_known_txs_for_block<Block: BlockT, Api: graph::ChainApi<Block = B
 
 	log::trace!(target: "txpool", "Pruning transactions: {:?}", hashes);
 
-	let header = match api.block_header(&block_id) {
+	let header = match api.block_header(&BlockId::Hash(*block_hash)) {
 		Ok(Some(h)) => h,
 		Ok(None) => {
-			log::debug!(target: "txpool", "Could not find header for {:?}.", block_id);
+			log::debug!(target: "txpool", "Could not find header for {:?}.", block_hash);
 			return hashes
 		},
 		Err(e) => {
-			log::debug!(target: "txpool", "Error retrieving header for {:?}: {}", block_id, e);
+			log::debug!(target: "txpool", "Error retrieving header for {:?}: {}", block_hash, e);
 			return hashes
 		},
 	};
 
-	if let Err(e) = pool.prune(&block_id, &BlockId::hash(*header.parent_hash()), &extrinsics).await
+	if let Err(e) = pool
+		.prune(&BlockId::Hash(*block_hash), &BlockId::hash(*header.parent_hash()), &extrinsics)
+		.await
 	{
 		log::error!("Cannot prune known in the pool: {}", e);
 	}
@@ -636,7 +638,7 @@ where
 			tree_route
 				.enacted()
 				.iter()
-				.map(|h| prune_known_txs_for_block(BlockId::Hash(h.hash), &*api, &*pool)),
+				.map(|h| prune_known_txs_for_block(&h.hash, &*api, &*pool)),
 		)
 		.await
 		.into_iter()
@@ -654,7 +656,7 @@ where
 				let hash = retracted.hash;
 
 				let block_transactions = api
-					.block_body(&BlockId::hash(hash))
+					.block_body(&hash)
 					.await
 					.unwrap_or_else(|e| {
 						log::warn!("Failed to fetch block body: {}", e);

substrate/client/transaction-pool/src/tests.rs

@@ -164,7 +164,7 @@ impl ChainApi for TestApi {
 		(Hashing::hash(&encoded), len)
 	}
 
-	fn block_body(&self, _id: &BlockId<Self::Block>) -> Self::BodyFuture {
+	fn block_body(&self, _id: &<Self::Block as BlockT>::Hash) -> Self::BodyFuture {
 		futures::future::ready(Ok(None))
 	}
 

substrate/primitives/blockchain/src/backend.rs

@@ -89,7 +89,7 @@ pub trait Backend<Block: BlockT>:
 	HeaderBackend<Block> + HeaderMetadata<Block, Error = Error>
 {
 	/// Get block body. Returns `None` if block is not found.
-	fn body(&self, id: BlockId<Block>) -> Result<Option<Vec<<Block as BlockT>::Extrinsic>>>;
+	fn body(&self, hash: &Block::Hash) -> Result<Option<Vec<<Block as BlockT>::Extrinsic>>>;
 	/// Get block justifications. Returns `None` if no justification exists.
 	fn justifications(&self, id: BlockId<Block>) -> Result<Option<Justifications>>;
 	/// Get last finalized block hash.

substrate/test-utils/runtime/transaction-pool/src/lib.rs

@@ -315,13 +315,13 @@ impl sc_transaction_pool::ChainApi for TestApi {
 		Self::hash_and_length_inner(ex)
 	}
 
-	fn block_body(&self, id: &BlockId<Self::Block>) -> Self::BodyFuture {
-		futures::future::ready(Ok(match id {
-			BlockId::Number(num) =>
-				self.chain.read().block_by_number.get(num).map(|b| b[0].0.extrinsics().to_vec()),
-			BlockId::Hash(hash) =>
-				self.chain.read().block_by_hash.get(hash).map(|b| b.extrinsics().to_vec()),
-		}))
+	fn block_body(&self, hash: &<Self::Block as BlockT>::Hash) -> Self::BodyFuture {
+		futures::future::ready(Ok(self
+			.chain
+			.read()
+			.block_by_hash
+			.get(hash)
+			.map(|b| b.extrinsics().to_vec())))
 	}
 
 	fn block_header(