diff --git a/substrate/bin/node/inspect/src/lib.rs b/substrate/bin/node/inspect/src/lib.rs index aacae0ff7a0d91f9ce6fee1c9a8a3209f4ea3807..1e44cec914995c4ad83a95fd36760877d1fccad9 100644 --- a/substrate/bin/node/inspect/src/lib.rs +++ b/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()))?; diff --git a/substrate/client/api/src/client.rs b/substrate/client/api/src/client.rs index b809e0ee610328e033ef50e3e9d16c9ec14f9035..670c1711db948d447ff36297913fd805e29f1651 100644 --- a/substrate/client/api/src/client.rs +++ b/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, diff --git a/substrate/client/api/src/in_mem.rs b/substrate/client/api/src/in_mem.rs index 4028b99d6fb3784186bfe22961188ca8954504a4..7b0e6f7b725759a72f8e45ab9871e572bd63e576 100644 --- a/substrate/client/api/src/in_mem.rs +++ b/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>> { diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index 4a7159208c2c811cbc9ee02fc0422783650dc1e5..9d9a8d268dd8108f85fab2c666acba5e1ee6c5dd 100644 --- a/substrate/client/db/src/lib.rs +++ b/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); } diff --git a/substrate/client/network/sync/src/block_request_handler.rs b/substrate/client/network/sync/src/block_request_handler.rs index a6b39591e79454d7e61f793abfa097fe8962f7ce..dbde5fc5d8c22c50570706343b449da222a5a9d0 100644 --- a/substrate/client/network/sync/src/block_request_handler.rs +++ b/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 => { diff --git a/substrate/client/network/test/src/lib.rs b/substrate/client/network/test/src/lib.rs index c9b99fbc6af10771715cefa654c8c8743abe86c2..a6d73b53efdf0684cf2c069c2d214bd7f479fef5 100644 --- a/substrate/client/network/test/src/lib.rs +++ b/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) } } diff --git a/substrate/client/service/src/client/client.rs b/substrate/client/service/src/client/client.rs index eb946436671e4d568e19169cde7b3b47e365de1f..b5ab06a0318c54f20a04a6d66e4cbc049a6f0e0a 100644 --- a/substrate/client/service/src/client/client.rs +++ b/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, }) } diff --git a/substrate/client/service/test/src/client/mod.rs b/substrate/client/service/test/src/client/mod.rs index 329bffae03e7b484a4f34cafbab35ee4a43a5fcd..b9aec421802c97c368e7eb589652a4709fa7dc10 100644 --- a/substrate/client/service/test/src/client/mod.rs +++ b/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] diff --git a/substrate/client/tracing/src/block/mod.rs b/substrate/client/tracing/src/block/mod.rs index 2a034f06ce8a838bbe98ab1731d3f0c5f0473310..ee524f5f7290204a10c5be2b6f3fbcff66cebe54 100644 --- a/substrate/client/tracing/src/block/mod.rs +++ b/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()); diff --git a/substrate/client/transaction-pool/benches/basics.rs b/substrate/client/transaction-pool/benches/basics.rs index 2632f8fb6aab56518d7b81bce9a18f6448b07c9f..bc6f2f7d5e947436ac360a02c1cc62c92ff49f6d 100644 --- a/substrate/client/transaction-pool/benches/basics.rs +++ b/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)) } diff --git a/substrate/client/transaction-pool/src/api.rs b/substrate/client/transaction-pool/src/api.rs index 110647b8cb3b0573539c8bc4fad918f2d611312e..f162a02ddb643455beacca10d3f160620378f663 100644 --- a/substrate/client/transaction-pool/src/api.rs +++ b/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( diff --git a/substrate/client/transaction-pool/src/graph/pool.rs b/substrate/client/transaction-pool/src/graph/pool.rs index d311e0d0c8f2f229fcbec1014253f4df2501f3ed..99119ac8fa8ab1e386bc3833712262105cb72468 100644 --- a/substrate/client/transaction-pool/src/graph/pool.rs +++ b/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( diff --git a/substrate/client/transaction-pool/src/lib.rs b/substrate/client/transaction-pool/src/lib.rs index 410ab662e3601c13409c5b3752a1edfe0a880a33..9e0a2e74bf421eea3e0432a45173fe3c6ab482ef 100644 --- a/substrate/client/transaction-pool/src/lib.rs +++ b/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); diff --git a/substrate/client/transaction-pool/src/tests.rs b/substrate/client/transaction-pool/src/tests.rs index ce2c7872e32bb81ec4e73c753af1d56c2807e7c6..8775dfb6e9e8838d0d8345bcb1500916bd1abf51 100644 --- a/substrate/client/transaction-pool/src/tests.rs +++ b/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)) } diff --git a/substrate/primitives/blockchain/src/backend.rs b/substrate/primitives/blockchain/src/backend.rs index 79c05aec8adcae7dd651eed85576d2dd4d19e647..610ceb0cf93234cf3b148d2419d7457c3947f892 100644 --- a/substrate/primitives/blockchain/src/backend.rs +++ b/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. diff --git a/substrate/test-utils/runtime/transaction-pool/src/lib.rs b/substrate/test-utils/runtime/transaction-pool/src/lib.rs index dc8be78aa739750e1dce8bfd66f677e37b9ac1fa..e2d6efccea424002a3a9dbed35b01a5021e7bc77 100644 --- a/substrate/test-utils/runtime/transaction-pool/src/lib.rs +++ b/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(