Grandpa validator set handoff justification (paritytech#1190)

* core: make block justification optional

* runtime: update wasm binaries

* core: optionally pass justification on finalize_block

* finality-grandpa: add channel to trigger authority set changes

this will allow the `BlockImport` to trigger an authority set change when
importing a change block that provides a justification (when syncing)

* finality-grandpa: move finalize_block to free function

* finality-grandpa: add GrandpaOracle for auth set liveness checking

this will be used by `BlockImport` to check whether the authority set for a
given block is still live, if the authority set isn't live then importing a
change block requires a justification.

* finality-grandpa: store justification on finalized transition blocks

* finality-grandpa: check justification on authority set change blocks

* finality-grandpa: poll grandpa liveness oracle every 10 seconds

* finality-grandpa: spawn grandpa oracle in service setup

* core: support multiple subscriptions per consensus gossip topic

* finality-grandpa: create and verify justifications

* finality-grandpa: update to local branch of grandpa

* finality-grandpa: update to finality-grandpa v0.5.0

* finality-grandpa: move grandpa oracle code

* finality-grandpa: fix canonality check

* finality-grandpa: clean up error handling

* finality-grandpa: fix canonical_at_height

* finality-grandpa: fix tests

* runtime: update wasm binaries

* core: add tests for finalizing block with justification

* finality-grandpa: improve validation of justifications

* core: remove unused IncompleteJustification block import error

* core: test multiple subscribers for same consensus gossip topic

* Revert "finality-grandpa: improve validation of justifications"

This reverts commit 51eb2c5.

* finality-grandpa: fix commit validation

* finality-grandpa: fix commit ancestry validation

* finality-grandpa: use grandpa v0.5.1

* finality-grandpa: add docs

* finality-grandpa: fix failing test

* finality-grandpa: only allow a pending authority set change per fork

* finality-grandpa: fix validator set transition test

Author: andresilva

Cargo.lock

@@ -797,14 +797,24 @@ impl<Block> client::backend::Backend<Block, Blake2Hasher> for Backend<Block> whe
 		Ok(())
 	}
 
-	fn finalize_block(&self, block: BlockId<Block>) -> Result<(), client::error::Error> {
+	fn finalize_block(&self, block: BlockId<Block>, justification: Option<Justification>)
+		-> Result<(), client::error::Error>
+	{
 		use runtime_primitives::traits::Header;
 
 		if let Some(header) = ::client::blockchain::HeaderBackend::header(&self.blockchain, block)? {
 			let mut transaction = DBTransaction::new();
 			// TODO: ensure best chain contains this block.
 			let hash = header.hash();
 			self.note_finalized(&mut transaction, &header, hash.clone())?;
+			if let Some(justification) = justification {
+				let number = header.number().clone();
+				transaction.put(
+					columns::JUSTIFICATION,
+					&::utils::number_and_hash_to_lookup_key(number, hash.clone()),
+					&justification.encode(),
+				);
+			}
 			self.storage.db.write(transaction).map_err(db_err)?;
 			self.blockchain.update_meta(hash, header.number().clone(), false, true);
 			Ok(())
@@ -1196,8 +1206,8 @@ mod tests {
 			assert!(backend.storage.db.get(::columns::STATE, key.as_bytes()).unwrap().is_none());
 		}
 
-		backend.finalize_block(BlockId::Number(1)).unwrap();
-		backend.finalize_block(BlockId::Number(2)).unwrap();
+		backend.finalize_block(BlockId::Number(1), None).unwrap();
+		backend.finalize_block(BlockId::Number(2), None).unwrap();
 		assert!(backend.storage.db.get(::columns::STATE, key.as_bytes()).unwrap().is_none());
 	}
 
@@ -1499,4 +1509,39 @@ mod tests {
 		backend.insert_aux(&[], &[&b"test"[..]]).unwrap();
 		assert!(backend.get_aux(b"test").unwrap().is_none());
 	}
+
+	#[test]
+	fn test_finalize_block_with_justification() {
+		use client::blockchain::{Backend as BlockChainBackend};
+
+		let backend = Backend::<Block>::new_test(0, 0);
+
+		{
+			let mut op = backend.begin_operation(BlockId::Hash(Default::default())).unwrap();
+			let header = Header {
+				number: 0,
+				parent_hash: Default::default(),
+				state_root: Default::default(),
+				digest: Default::default(),
+				extrinsics_root: Default::default(),
+			};
+
+			op.set_block_data(
+				header,
+				Some(vec![]),
+				None,
+				NewBlockState::Best,
+			).unwrap();
+
+			backend.commit_operation(op).unwrap();
+		}
+
+		let justification = Some(vec![1, 2, 3]);
+		backend.finalize_block(BlockId::Number(0), justification.clone()).unwrap();
+
+		assert_eq!(
+			backend.blockchain().justification(BlockId::Number(0)).unwrap(),
+			justification,
+		);
+	}
 }

core/client/db/src/lib.rs

@@ -107,7 +107,7 @@ pub trait Backend<Block, H>: Send + Sync where
 	fn commit_operation(&self, transaction: Self::BlockImportOperation) -> error::Result<()>;
 	/// Finalize block with given Id. This should only be called if the parent of the given
 	/// block has been finalized.
-	fn finalize_block(&self, block: BlockId<Block>) -> error::Result<()>;
+	fn finalize_block(&self, block: BlockId<Block>, justification: Option<Justification>) -> error::Result<()>;
 	/// Returns reference to blockchain backend.
 	fn blockchain(&self) -> &Self::Blockchain;
 	/// Returns reference to changes trie storage.

core/client/src/backend.rs

@@ -593,7 +593,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		origin: BlockOrigin,
 		hash: Block::Hash,
 		import_headers: PrePostHeader<Block::Header>,
-		justification: Justification,
+		justification: Option<Justification>,
 		body: Option<Vec<Block::Extrinsic>>,
 		authorities: Option<Vec<AuthorityId>>,
 		finalized: bool,
@@ -624,7 +624,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		// ensure parent block is finalized to maintain invariant that
 		// finality is called sequentially.
 		if finalized {
-			self.apply_finality(parent_hash, last_best, make_notifications)?;
+			self.apply_finality(parent_hash, None, last_best, make_notifications)?;
 		}
 
 		let tags = self.transaction_tags(parent_hash, &body)?;
@@ -678,7 +678,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		transaction.set_block_data(
 			import_headers.post().clone(),
 			body,
-			Some(justification),
+			justification,
 			leaf_state,
 		)?;
 
@@ -727,8 +727,16 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		Ok(ImportResult::Queued)
 	}
 
-	/// Finalizes all blocks up to given.
-	fn apply_finality(&self, block: Block::Hash, best_block: Block::Hash, notify: bool) -> error::Result<()> {
+	/// Finalizes all blocks up to given. If a justification is provided it is
+	/// stored with the given finalized block (any other finalized blocks are
+	/// left unjustified).
+	fn apply_finality(
+		&self,
+		block: Block::Hash,
+		justification: Option<Justification>,
+		best_block: Block::Hash,
+		notify: bool,
+	) -> error::Result<()> {
 		// find tree route from last finalized to given block.
 		let last_finalized = self.backend.blockchain().last_finalized()?;
 
@@ -759,10 +767,15 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 			// `block`.
 		}
 
-		for finalize_new in route_from_finalized.enacted() {
-			self.backend.finalize_block(BlockId::Hash(finalize_new.hash))?;
+		let enacted = route_from_finalized.enacted();
+		assert!(enacted.len() > 0);
+		for finalize_new in &enacted[..enacted.len() - 1] {
+			self.backend.finalize_block(BlockId::Hash(finalize_new.hash), None)?;
 		}
 
+		assert_eq!(enacted.last().map(|e| e.hash), Some(block));
+		self.backend.finalize_block(BlockId::Hash(block), justification)?;
+
 		if notify {
 			// sometimes when syncing, tons of blocks can be finalized at once.
 			// we'll send notifications spuriously in that case.
@@ -791,15 +804,15 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 	/// Pass a flag to indicate whether finality notifications should be propagated.
 	/// This is usually tied to some synchronization state, where we don't send notifications
 	/// while performing major synchronization work.
-	pub fn finalize_block(&self, id: BlockId<Block>, notify: bool) -> error::Result<()> {
+	pub fn finalize_block(&self, id: BlockId<Block>, justification: Option<Justification>, notify: bool) -> error::Result<()> {
 		let last_best = self.backend.blockchain().info()?.best_hash;
 		let to_finalize_hash = match id {
 			BlockId::Hash(h) => h,
 			BlockId::Number(n) => self.backend.blockchain().hash(n)?
 				.ok_or_else(|| error::ErrorKind::UnknownBlock(format!("No block with number {:?}", n)))?,
 		};
 
-		self.apply_finality(to_finalize_hash, last_best, notify)
+		self.apply_finality(to_finalize_hash, justification, last_best, notify)
 	}
 
 	/// Attempts to revert the chain by `n` blocks. Returns the number of blocks that were
@@ -858,7 +871,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		-> error::Result<Option<SignedBlock<Block>>>
 	{
 		Ok(match (self.header(id)?, self.body(id)?, self.justification(id)?) {
-			(Some(header), Some(extrinsics), Some(justification)) =>
+			(Some(header), Some(extrinsics), justification) =>
 				Some(SignedBlock { block: Block::new(header, extrinsics), justification }),
 			_ => None,
 		})
@@ -1064,7 +1077,8 @@ impl<B, E, Block, RA> consensus::BlockImport<Block> for Client<B, E, Block, RA>
 {
 	type Error = Error;
 
-	/// Import a checked and validated block
+	/// Import a checked and validated block. If a justification is provided in
+	/// `ImportBlock` then `finalized` *must* be true.
 	fn import_block(
 		&self,
 		import_block: ImportBlock<Block>,
@@ -1081,6 +1095,9 @@ impl<B, E, Block, RA> consensus::BlockImport<Block> for Client<B, E, Block, RA>
 			finalized,
 			auxiliary,
 		} = import_block;
+
+		assert!(justification.is_some() && finalized || justification.is_none());
+
 		let parent_hash = header.parent_hash().clone();
 
 		match self.backend.blockchain().status(BlockId::Hash(parent_hash))? {
@@ -1254,7 +1271,7 @@ pub(crate) mod tests {
 					nonce: *nonces.entry(from).and_modify(|n| { *n = *n + 1 }).or_default(),
 				}).unwrap();
 			}
-			remote_client.justify_and_import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
+			remote_client.import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
 
 			let header = remote_client.header(&BlockId::Number(i as u64 + 1)).unwrap().unwrap();
 			let trie_root = header.digest().log(DigestItem::as_changes_trie_root)
@@ -1334,7 +1351,7 @@ pub(crate) mod tests {
 
 		let builder = client.new_block().unwrap();
 
-		client.justify_and_import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
+		client.import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
 
 		assert_eq!(client.info().unwrap().chain.best_number, 1);
 	}
@@ -1352,7 +1369,7 @@ pub(crate) mod tests {
 			nonce: 0,
 		}).unwrap();
 
-		client.justify_and_import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
+		client.import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
 
 		assert_eq!(client.info().unwrap().chain.best_number, 1);
 		assert!(client.state_at(&BlockId::Number(1)).unwrap() != client.state_at(&BlockId::Number(0)).unwrap());
@@ -1404,7 +1421,7 @@ pub(crate) mod tests {
 			nonce: 0,
 		}).is_err());
 
-		client.justify_and_import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
+		client.import(BlockOrigin::Own, builder.bake().unwrap()).unwrap();
 
 		assert_eq!(client.info().unwrap().chain.best_number, 1);
 		assert!(client.state_at(&BlockId::Number(1)).unwrap() != client.state_at(&BlockId::Number(0)).unwrap());
@@ -1444,11 +1461,11 @@ pub(crate) mod tests {
 
 		// G -> A1
 		let a1 = client.new_block().unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a1.clone()).unwrap();
+		client.import(BlockOrigin::Own, a1.clone()).unwrap();
 
 		// A1 -> A2
 		let a2 = client.new_block().unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a2.clone()).unwrap();
+		client.import(BlockOrigin::Own, a2.clone()).unwrap();
 
 		let genesis_hash = client.info().unwrap().chain.genesis_hash;
 
@@ -1473,23 +1490,23 @@ pub(crate) mod tests {
 
 		// G -> A1
 		let a1 = client.new_block().unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a1.clone()).unwrap();
+		client.import(BlockOrigin::Own, a1.clone()).unwrap();
 
 		// A1 -> A2
 		let a2 = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a2.clone()).unwrap();
+		client.import(BlockOrigin::Own, a2.clone()).unwrap();
 
 		// A2 -> A3
 		let a3 = client.new_block_at(&BlockId::Hash(a2.hash())).unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a3.clone()).unwrap();
+		client.import(BlockOrigin::Own, a3.clone()).unwrap();
 
 		// A3 -> A4
 		let a4 = client.new_block_at(&BlockId::Hash(a3.hash())).unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a4.clone()).unwrap();
+		client.import(BlockOrigin::Own, a4.clone()).unwrap();
 
 		// A4 -> A5
 		let a5 = client.new_block_at(&BlockId::Hash(a4.hash())).unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, a5.clone()).unwrap();
+		client.import(BlockOrigin::Own, a5.clone()).unwrap();
 
 		// A1 -> B2
 		let mut builder = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap();
@@ -1501,15 +1518,15 @@ pub(crate) mod tests {
 			nonce: 0,
 		}).unwrap();
 		let b2 = builder.bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, b2.clone()).unwrap();
+		client.import(BlockOrigin::Own, b2.clone()).unwrap();
 
 		// B2 -> B3
 		let b3 = client.new_block_at(&BlockId::Hash(b2.hash())).unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, b3.clone()).unwrap();
+		client.import(BlockOrigin::Own, b3.clone()).unwrap();
 
 		// B3 -> B4
 		let b4 = client.new_block_at(&BlockId::Hash(b3.hash())).unwrap().bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, b4.clone()).unwrap();
+		client.import(BlockOrigin::Own, b4.clone()).unwrap();
 
 		// // B2 -> C3
 		let mut builder = client.new_block_at(&BlockId::Hash(b2.hash())).unwrap();
@@ -1521,7 +1538,7 @@ pub(crate) mod tests {
 			nonce: 1,
 		}).unwrap();
 		let c3 = builder.bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, c3.clone()).unwrap();
+		client.import(BlockOrigin::Own, c3.clone()).unwrap();
 
 		// A1 -> D2
 		let mut builder = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap();
@@ -1533,7 +1550,7 @@ pub(crate) mod tests {
 			nonce: 0,
 		}).unwrap();
 		let d2 = builder.bake().unwrap();
-		client.justify_and_import(BlockOrigin::Own, d2.clone()).unwrap();
+		client.import(BlockOrigin::Own, d2.clone()).unwrap();
 
 		assert_eq!(client.info().unwrap().chain.best_hash, a5.hash());
 
@@ -1686,4 +1703,44 @@ pub(crate) mod tests {
 			}
 		}
 	}
+
+	#[test]
+	fn import_with_justification() {
+		use test_client::blockchain::Backend;
+
+		let client = test_client::new();
+
+		// G -> A1
+		let a1 = client.new_block().unwrap().bake().unwrap();
+		client.import(BlockOrigin::Own, a1.clone()).unwrap();
+
+		// A1 -> A2
+		let a2 = client.new_block_at(&BlockId::Hash(a1.hash())).unwrap().bake().unwrap();
+		client.import(BlockOrigin::Own, a2.clone()).unwrap();
+
+		// A2 -> A3
+		let justification = vec![1, 2, 3];
+		let a3 = client.new_block_at(&BlockId::Hash(a2.hash())).unwrap().bake().unwrap();
+		client.import_justified(BlockOrigin::Own, a3.clone(), justification.clone()).unwrap();
+
+		assert_eq!(
+			client.backend().blockchain().last_finalized().unwrap(),
+			a3.hash(),
+		);
+
+		assert_eq!(
+			client.backend().blockchain().justification(BlockId::Hash(a3.hash())).unwrap(),
+			Some(justification),
+		);
+
+		assert_eq!(
+			client.backend().blockchain().justification(BlockId::Hash(a1.hash())).unwrap(),
+			None,
+		);
+
+		assert_eq!(
+			client.backend().blockchain().justification(BlockId::Hash(a2.hash())).unwrap(),
+			None,
+		);
+	}
 }