Storing multiple Justifications per block

client/api/src/backend.rs

@@ -21,8 +21,8 @@
 use std::sync::Arc;
 use std::collections::{HashMap, HashSet};
 use sp_core::ChangesTrieConfigurationRange;
-use sp_core::offchain::OffchainStorage;
-use sp_runtime::{generic::BlockId, Justification, Storage};
+use sp_core::offchain::{OffchainStorage};
+use sp_runtime::{generic::BlockId, Justification, Justifications, Storage};
 use sp_runtime::traits::{Block as BlockT, NumberFor, HashFor};
 use sp_state_machine::{
 	ChangesTrieState, ChangesTrieStorage as StateChangesTrieStorage, ChangesTrieTransaction,
@@ -148,7 +148,7 @@ pub trait BlockImportOperation<Block: BlockT> {
 		&mut self,
 		header: Block::Header,
 		body: Option<Vec<Block::Extrinsic>>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 		state: NewBlockState,
 	) -> sp_blockchain::Result<()>;
 
@@ -195,8 +195,9 @@ pub trait BlockImportOperation<Block: BlockT> {
 	fn mark_finalized(
 		&mut self,
 		id: BlockId<Block>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 	) -> sp_blockchain::Result<()>;
+
 	/// Mark a block as new head. If both block import and set head are specified, set head
 	/// overrides block import's best block rule.
 	fn mark_head(&mut self, id: BlockId<Block>) -> sp_blockchain::Result<()>;
@@ -226,11 +227,10 @@ pub trait Finalizer<Block: BlockT, B: Backend<Block>> {
 		&self,
 		operation: &mut ClientImportOperation<Block, B>,
 		id: BlockId<Block>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 		notify: bool,
 	) -> sp_blockchain::Result<()>;
 
-
 	/// Finalize a block.
 	///
 	/// This will implicitly finalize all blocks up to it and
@@ -247,10 +247,9 @@ pub trait Finalizer<Block: BlockT, B: Backend<Block>> {
 	fn finalize_block(
 		&self,
 		id: BlockId<Block>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 		notify: bool,
 	) -> sp_blockchain::Result<()>;
-
 }
 
 /// Provides access to an auxiliary database.
@@ -429,7 +428,16 @@ pub trait Backend<Block: BlockT>: AuxStore + Send + Sync {
 	fn finalize_block(
 		&self,
 		block: BlockId<Block>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
+	) -> sp_blockchain::Result<()>;
+
+	/// Append justification to the block with the given Id.
+	///
+	/// This should only be called for blocks that are already finalized.
+	fn append_justification(
+		&self,
+		block: BlockId<Block>,
+		justification: Justification,
 	) -> sp_blockchain::Result<()>;
 
 	/// Returns reference to blockchain backend.

client/api/src/client.rs

@@ -23,7 +23,7 @@ use sp_core::storage::StorageKey;
 use sp_runtime::{
 	traits::{Block as BlockT, NumberFor},
 	generic::{BlockId, SignedBlock},
-	Justification,
+	Justifications,
 };
 use sp_consensus::BlockOrigin;
 
@@ -90,8 +90,8 @@ pub trait BlockBackend<Block: BlockT> {
 	/// Get block status.
 	fn block_status(&self, id: &BlockId<Block>) -> sp_blockchain::Result<sp_consensus::BlockStatus>;
 
-	/// Get block justification set by id.
-	fn justification(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<Justification>>;
+	/// Get block justifications set by id.
+	fn justifications(&self, id: &BlockId<Block>) -> sp_blockchain::Result<Option<Justifications>>;
 
 	/// Get block hash by number.
 	fn block_hash(&self, number: NumberFor<Block>) -> sp_blockchain::Result<Option<Block::Hash>>;

client/api/src/in_mem.rs

@@ -27,7 +27,7 @@ use sp_core::{
 };
 use sp_runtime::generic::BlockId;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT, Zero, NumberFor, HashFor};
-use sp_runtime::{Justification, Storage};
+use sp_runtime::{Justification, Justifications, Storage};
 use sp_state_machine::{
 	ChangesTrieTransaction, InMemoryBackend, Backend as StateBackend, StorageCollection,
 	ChildStorageCollection,
@@ -51,12 +51,12 @@ struct PendingBlock<B: BlockT> {
 
 #[derive(PartialEq, Eq, Clone)]
 enum StoredBlock<B: BlockT> {
-	Header(B::Header, Option<Justification>),
-	Full(B, Option<Justification>),
+	Header(B::Header, Option<Justifications>),
+	Full(B, Option<Justifications>),
 }
 
 impl<B: BlockT> StoredBlock<B> {
-	fn new(header: B::Header, body: Option<Vec<B::Extrinsic>>, just: Option<Justification>) -> Self {
+	fn new(header: B::Header, body: Option<Vec<B::Extrinsic>>, just: Option<Justifications>) -> Self {
 		match body {
 			Some(body) => StoredBlock::Full(B::new(header, body), just),
 			None => StoredBlock::Header(header, just),
@@ -70,7 +70,7 @@ impl<B: BlockT> StoredBlock<B> {
 		}
 	}
 
-	fn justification(&self) -> Option<&Justification> {
+	fn justifications(&self) -> Option<&Justifications> {
 		match *self {
 			StoredBlock::Header(_, ref j) | StoredBlock::Full(_, ref j) => j.as_ref()
 		}
@@ -83,7 +83,7 @@ impl<B: BlockT> StoredBlock<B> {
 		}
 	}
 
-	fn into_inner(self) -> (B::Header, Option<Vec<B::Extrinsic>>, Option<Justification>) {
+	fn into_inner(self) -> (B::Header, Option<Vec<B::Extrinsic>>, Option<Justifications>) {
 		match self {
 			StoredBlock::Header(header, just) => (header, None, just),
 			StoredBlock::Full(block, just) => {
@@ -164,7 +164,7 @@ impl<Block: BlockT> Blockchain<Block> {
 		&self,
 		hash: Block::Hash,
 		header: <Block as BlockT>::Header,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 		body: Option<Vec<<Block as BlockT>::Extrinsic>>,
 		new_state: NewBlockState,
 	) -> sp_blockchain::Result<()> {
@@ -176,7 +176,7 @@ impl<Block: BlockT> Blockchain<Block> {
 		{
 			let mut storage = self.storage.write();
 			storage.leaves.import(hash.clone(), number.clone(), header.parent_hash().clone());
-			storage.blocks.insert(hash.clone(), StoredBlock::new(header, body, justification));
+			storage.blocks.insert(hash.clone(), StoredBlock::new(header, body, justifications));
 
 			if let NewBlockState::Final = new_state {
 				storage.finalized_hash = hash;
@@ -272,7 +272,7 @@ impl<Block: BlockT> Blockchain<Block> {
 		Ok(())
 	}
 
-	fn finalize_header(&self, id: BlockId<Block>, justification: Option<Justification>) -> sp_blockchain::Result<()> {
+	fn finalize_header(&self, id: BlockId<Block>, justifications: Option<Justifications>) -> sp_blockchain::Result<()> {
 		let hash = match self.header(id)? {
 			Some(h) => h.hash(),
 			None => return Err(sp_blockchain::Error::UnknownBlock(format!("{}", id))),
@@ -281,20 +281,48 @@ impl<Block: BlockT> Blockchain<Block> {
 		let mut storage = self.storage.write();
 		storage.finalized_hash = hash;
 
-		if justification.is_some() {
+		if justifications.is_some() {
 			let block = storage.blocks.get_mut(&hash)
 				.expect("hash was fetched from a block in the db; qed");
 
-			let block_justification = match block {
+			let block_justifications = match block {
 				StoredBlock::Header(_, ref mut j) | StoredBlock::Full(_, ref mut j) => j
 			};
 
-			*block_justification = justification;
+			*block_justifications = justifications;
 		}
 
 		Ok(())
 	}
 
+	fn append_justification(&self, id: BlockId<Block>, justification: Justification)
+		-> sp_blockchain::Result<()>
+	{
+		let hash = self.expect_block_hash_from_id(&id)?;
+		let mut storage = self.storage.write();
+
+		let block = storage
+			.blocks
+			.get_mut(&hash)
+			.expect("hash was fetched from a block in the db; qed");
+
+		let block_justification = match block {
+			StoredBlock::Header(_, ref mut j) | StoredBlock::Full(_, ref mut j) => j
+		};
+
+		if let Some(stored_justifications) = block_justification {
+			if !stored_justifications.append(justification) {
+				return Err(sp_blockchain::Error::BadJustification(
+					"Duplicate consensus engine ID".into()
+				));
+			}
+		} else {
+			*block_justification = Some(Justifications::from(justification));
+		};
+
+		Ok(())
+	}
+
 	fn write_aux(&self, ops: Vec<(Vec<u8>, Option<Vec<u8>>)>) {
 		let mut storage = self.storage.write();
 		for (k, v) in ops {
@@ -365,9 +393,9 @@ impl<Block: BlockT> blockchain::Backend<Block> for Blockchain<Block> {
 		}))
 	}
 
-	fn justification(&self, id: BlockId<Block>) -> sp_blockchain::Result<Option<Justification>> {
+	fn justifications(&self, id: BlockId<Block>) -> sp_blockchain::Result<Option<Justifications>> {
 		Ok(self.id(id).and_then(|hash| self.storage.read().blocks.get(&hash).and_then(|b|
-			b.justification().map(|x| x.clone()))
+			b.justifications().map(|x| x.clone()))
 		))
 	}
 
@@ -492,7 +520,7 @@ pub struct BlockImportOperation<Block: BlockT> {
 	old_state: InMemoryBackend<HashFor<Block>>,
 	new_state: Option<<InMemoryBackend<HashFor<Block>> as StateBackend<HashFor<Block>>>::Transaction>,
 	aux: Vec<(Vec<u8>, Option<Vec<u8>>)>,
-	finalized_blocks: Vec<(BlockId<Block>, Option<Justification>)>,
+	finalized_blocks: Vec<(BlockId<Block>, Option<Justifications>)>,
 	set_head: Option<BlockId<Block>>,
 }
 
@@ -509,12 +537,12 @@ impl<Block: BlockT> backend::BlockImportOperation<Block> for BlockImportOperatio
 		&mut self,
 		header: <Block as BlockT>::Header,
 		body: Option<Vec<<Block as BlockT>::Extrinsic>>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 		state: NewBlockState,
 	) -> sp_blockchain::Result<()> {
 		assert!(self.pending_block.is_none(), "Only one block per operation is allowed");
 		self.pending_block = Some(PendingBlock {
-			block: StoredBlock::new(header, body, justification),
+			block: StoredBlock::new(header, body, justifications),
 			state,
 		});
 		Ok(())
@@ -577,9 +605,9 @@ impl<Block: BlockT> backend::BlockImportOperation<Block> for BlockImportOperatio
 	fn mark_finalized(
 		&mut self,
 		block: BlockId<Block>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 	) -> sp_blockchain::Result<()> {
-		self.finalized_blocks.push((block, justification));
+		self.finalized_blocks.push((block, justifications));
 		Ok(())
 	}
 
@@ -695,9 +723,17 @@ impl<Block: BlockT> backend::Backend<Block> for Backend<Block> where Block::Hash
 	fn finalize_block(
 		&self,
 		block: BlockId<Block>,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 	) -> sp_blockchain::Result<()> {
-		self.blockchain.finalize_header(block, justification)
+		self.blockchain.finalize_header(block, justifications)
+	}
+
+	fn append_justification(
+		&self,
+		block: BlockId<Block>,
+		justification: Justification,
+	) -> sp_blockchain::Result<()> {
+		self.blockchain.append_justification(block, justification)
 	}
 
 	fn blockchain(&self) -> &Self::Blockchain {
@@ -770,3 +806,64 @@ pub fn check_genesis_storage(storage: &Storage) -> sp_blockchain::Result<()> {
 
 	Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+	use crate::{NewBlockState, in_mem::Blockchain};
+	use sp_api::{BlockId, HeaderT};
+	use sp_runtime::{ConsensusEngineId, Justifications};
+	use sp_blockchain::Backend;
+	use substrate_test_runtime::{Block, Header, H256};
+
+	pub const ID1: ConsensusEngineId = *b"TST1";
+	pub const ID2: ConsensusEngineId = *b"TST2";
+
+	fn header(number: u64) -> Header {
+		let parent_hash = match number {
+			0 => Default::default(),
+			_ => header(number - 1).hash(),
+		};
+		Header::new(number, H256::from_low_u64_be(0), H256::from_low_u64_be(0), parent_hash, Default::default())
+	}
+
+	fn test_blockchain() -> Blockchain<Block> {
+		let blockchain = Blockchain::<Block>::new();
+		let just0 = Some(Justifications::from((ID1, vec![0])));
+		let just1 = Some(Justifications::from((ID1, vec![1])));
+		let just2 = None;
+		let just3 = Some(Justifications::from((ID1, vec![3])));
+		blockchain.insert(header(0).hash(), header(0), just0, None, NewBlockState::Final).unwrap();
+		blockchain.insert(header(1).hash(), header(1), just1, None, NewBlockState::Final).unwrap();
+		blockchain.insert(header(2).hash(), header(2), just2, None, NewBlockState::Best).unwrap();
+		blockchain.insert(header(3).hash(), header(3), just3, None, NewBlockState::Final).unwrap();
+		blockchain
+	}
+
+	#[test]
+	fn append_and_retreive_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();
+		let justifications = {
+			let mut just = Justifications::from((ID1, vec![3]));
+			just.append((ID2, vec![4]));
+			just
+		};
+		assert_eq!(blockchain.justifications(block).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();
+		assert!(matches!(
+			blockchain.append_justification(block, (ID2, vec![1])),
+			Err(sp_blockchain::Error::BadJustification(_)),
+		));
+	}
+}

client/consensus/aura/src/lib.rs

@@ -57,7 +57,7 @@ use sp_blockchain::{
 use sp_block_builder::BlockBuilder as BlockBuilderApi;
 use sp_core::crypto::Public;
 use sp_application_crypto::{AppKey, AppPublic};
-use sp_runtime::{generic::{BlockId, OpaqueDigestItemId}, traits::NumberFor, Justification};
+use sp_runtime::{generic::{BlockId, OpaqueDigestItemId}, traits::NumberFor, Justifications};
 use sp_runtime::traits::{Block as BlockT, Header, DigestItemFor, Zero, Member};
 use sp_api::ProvideRuntimeApi;
 use sp_core::crypto::Pair;
@@ -589,7 +589,7 @@ impl<B: BlockT, C, P, CAW> Verifier<B> for AuraVerifier<C, P, CAW> where
 		&mut self,
 		origin: BlockOrigin,
 		header: B::Header,
-		justification: Option<Justification>,
+		justifications: Option<Justifications>,
 		mut body: Option<Vec<B::Extrinsic>>,
 	) -> Result<(BlockImportParams<B, ()>, Option<Vec<(CacheKeyId, Vec<u8>)>>), String> {
 		let mut inherent_data = self.inherent_data_providers
@@ -662,7 +662,7 @@ impl<B: BlockT, C, P, CAW> Verifier<B> for AuraVerifier<C, P, CAW> where
 				let mut import_block = BlockImportParams::new(origin, pre_header);
 				import_block.post_digests.push(seal);
 				import_block.body = body;
-				import_block.justification = justification;
+				import_block.justifications = justifications;
 				import_block.fork_choice = Some(ForkChoiceStrategy::LongestChain);
 				import_block.post_hash = Some(hash);