diff --git a/client/api/src/backend.rs b/client/api/src/backend.rs index 77a9da3535655..df9c92b626ecc 100644 --- a/client/api/src/backend.rs +++ b/client/api/src/backend.rs @@ -27,7 +27,6 @@ use sp_blockchain; use sp_consensus::BlockOrigin; use sp_core::offchain::OffchainStorage; use sp_runtime::{ - generic::BlockId, traits::{Block as BlockT, HashFor, NumberFor}, Justification, Justifications, StateVersion, Storage, }; @@ -485,12 +484,12 @@ pub trait Backend: AuxStore + Send + Sync { justification: Option, ) -> sp_blockchain::Result<()>; - /// Append justification to the block with the given Id. + /// Append justification to the block with the given `hash`. /// /// This should only be called for blocks that are already finalized. fn append_justification( &self, - block: BlockId, + hash: &Block::Hash, justification: Justification, ) -> sp_blockchain::Result<()>; diff --git a/client/api/src/in_mem.rs b/client/api/src/in_mem.rs index 29f7eade9fa6f..4028b99d6fb37 100644 --- a/client/api/src/in_mem.rs +++ b/client/api/src/in_mem.rs @@ -295,10 +295,9 @@ impl Blockchain { fn append_justification( &self, - id: BlockId, + hash: &Block::Hash, justification: Justification, ) -> sp_blockchain::Result<()> { - let hash = self.expect_block_hash_from_id(&id)?; let mut storage = self.storage.write(); let block = storage @@ -745,10 +744,10 @@ where fn append_justification( &self, - block: BlockId, + hash: &Block::Hash, justification: Justification, ) -> sp_blockchain::Result<()> { - self.blockchain.append_justification(block, justification) + self.blockchain.append_justification(hash, justification) } fn blockchain(&self) -> &Self::Blockchain { @@ -865,26 +864,27 @@ mod tests { fn append_and_retrieve_justifications() { let blockchain = test_blockchain(); let last_finalized = blockchain.last_finalized().unwrap(); - let block = BlockId::Hash(last_finalized); - blockchain.append_justification(block, (ID2, vec![4])).unwrap(); + blockchain.append_justification(&last_finalized, (ID2, vec![4])).unwrap(); let justifications = { let mut just = Justifications::from((ID1, vec![3])); just.append((ID2, vec![4])); just }; - assert_eq!(blockchain.justifications(block).unwrap(), Some(justifications)); + assert_eq!( + blockchain.justifications(BlockId::Hash(last_finalized)).unwrap(), + Some(justifications) + ); } #[test] fn store_duplicate_justifications_is_forbidden() { let blockchain = test_blockchain(); let last_finalized = blockchain.last_finalized().unwrap(); - let block = BlockId::Hash(last_finalized); - blockchain.append_justification(block, (ID2, vec![0])).unwrap(); + blockchain.append_justification(&last_finalized, (ID2, vec![0])).unwrap(); assert!(matches!( - blockchain.append_justification(block, (ID2, vec![1])), + blockchain.append_justification(&last_finalized, (ID2, vec![1])), Err(sp_blockchain::Error::BadJustification(_)), )); } diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 9b331b73ed093..305b92d696975 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -500,20 +500,22 @@ where self.on_demand_justifications.cancel_requests_older_than(block_num); - if let Err(e) = self.backend.append_justification( - BlockId::Number(block_num), - (BEEFY_ENGINE_ID, finality_proof.encode()), - ) { + if let Err(e) = self + .backend + .blockchain() + .expect_block_hash_from_id(&BlockId::Number(block_num)) + .and_then(|hash| { + self.links + .to_rpc_best_block_sender + .notify(|| Ok::<_, ()>(hash)) + .expect("forwards closure result; the closure always returns Ok; qed."); + + self.backend + .append_justification(&hash, (BEEFY_ENGINE_ID, finality_proof.encode())) + }) { error!(target: "beefy", "🥩 Error {:?} on appending justification: {:?}", e, finality_proof); } - self.backend.blockchain().hash(block_num).ok().flatten().map(|hash| { - self.links - .to_rpc_best_block_sender - .notify(|| Ok::<_, ()>(hash)) - .expect("forwards closure result; the closure always returns Ok; qed.") - }); - self.links .to_rpc_justif_sender .notify(|| Ok::<_, ()>(finality_proof)) @@ -1546,8 +1548,10 @@ pub(crate) mod tests { commitment, signatures: vec![None], }); + let hashof10 = + backend.blockchain().expect_block_hash_from_id(&BlockId::Number(10)).unwrap(); backend - .append_justification(BlockId::Number(10), (BEEFY_ENGINE_ID, justif.encode())) + .append_justification(&hashof10, (BEEFY_ENGINE_ID, justif.encode())) .unwrap(); // initialize voter at block 13, expect rounds initialized at last beefy finalized 10 @@ -1580,8 +1584,10 @@ pub(crate) mod tests { commitment, signatures: vec![None], }); + let hashof12 = + backend.blockchain().expect_block_hash_from_id(&BlockId::Number(12)).unwrap(); backend - .append_justification(BlockId::Number(12), (BEEFY_ENGINE_ID, justif.encode())) + .append_justification(&hashof12, (BEEFY_ENGINE_ID, justif.encode())) .unwrap(); // initialize voter at block 13, expect rounds initialized at last beefy finalized 12 diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index e212b382716f1..4a7159208c2c8 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -2013,12 +2013,11 @@ impl sc_client_api::backend::Backend for Backend { fn append_justification( &self, - block: BlockId, + hash: &Block::Hash, justification: Justification, ) -> ClientResult<()> { let mut transaction: Transaction = Transaction::new(); - let hash = self.blockchain.expect_block_hash_from_id(&block)?; - let header = self.blockchain.expect_header(block)?; + let header = self.blockchain.expect_header(BlockId::Hash(*hash))?; let number = *header.number(); // Check if the block is finalized first. @@ -2027,13 +2026,13 @@ impl sc_client_api::backend::Backend for Backend { // We can do a quick check first, before doing a proper but more expensive check if number > self.blockchain.info().finalized_number || - (hash != last_finalized && !is_descendent_of(&hash, &last_finalized)?) + (*hash != last_finalized && !is_descendent_of(hash, &last_finalized)?) { return Err(ClientError::NotInFinalizedChain) } let justifications = if let Some(mut stored_justifications) = - self.blockchain.justifications(block)? + self.blockchain.justifications(BlockId::Hash(*hash))? { if !stored_justifications.append(justification) { return Err(ClientError::BadJustification("Duplicate consensus engine ID".into())) @@ -2045,7 +2044,7 @@ impl sc_client_api::backend::Backend for Backend { transaction.set_from_vec( columns::JUSTIFICATIONS, - &utils::number_and_hash_to_lookup_key(number, hash)?, + &utils::number_and_hash_to_lookup_key(number, *hash)?, justifications.encode(), ); @@ -3029,11 +3028,11 @@ pub(crate) mod tests { backend.finalize_block(&block1, Some(just0.clone().into())).unwrap(); let just1 = (CONS1_ENGINE_ID, vec![4, 5]); - backend.append_justification(BlockId::Number(1), just1.clone()).unwrap(); + backend.append_justification(&block1, just1.clone()).unwrap(); let just2 = (CONS1_ENGINE_ID, vec![6, 7]); assert!(matches!( - backend.append_justification(BlockId::Number(1), just2), + backend.append_justification(&block1, just2), Err(ClientError::BadJustification(_)) ));