Beefy: use VersionedFinalityProof instead of SignedCommitment (parity…

…tech#11962)

* beefy: use VersionedFinalityProof instead of SignedCommitment.

* Change the exposed RPC API to support versioned proofs.

Co-authored-by: Adrian Catangiu <adrian@parity.io>

client/beefy/rpc/src/lib.rs

@@ -35,7 +35,7 @@ use jsonrpsee::{
 };
 use log::warn;
 
-use beefy_gadget::notification::{BeefyBestBlockStream, BeefySignedCommitmentStream};
+use beefy_gadget::notification::{BeefyBestBlockStream, BeefyVersionedFinalityProofStream};
 
 mod notification;
 
@@ -101,7 +101,7 @@ pub trait BeefyApi<Notification, Hash> {
 
 /// Implements the BeefyApi RPC trait for interacting with BEEFY.
 pub struct Beefy<Block: BlockT> {
-	signed_commitment_stream: BeefySignedCommitmentStream<Block>,
+	finality_proof_stream: BeefyVersionedFinalityProofStream<Block>,
 	beefy_best_block: Arc<RwLock<Option<Block::Hash>>>,
 	executor: SubscriptionTaskExecutor,
 }
@@ -112,7 +112,7 @@ where
 {
 	/// Creates a new Beefy Rpc handler instance.
 	pub fn new(
-		signed_commitment_stream: BeefySignedCommitmentStream<Block>,
+		finality_proof_stream: BeefyVersionedFinalityProofStream<Block>,
 		best_block_stream: BeefyBestBlockStream<Block>,
 		executor: SubscriptionTaskExecutor,
 	) -> Result<Self, Error> {
@@ -126,20 +126,21 @@ where
 		});
 
 		executor.spawn("substrate-rpc-subscription", Some("rpc"), future.map(drop).boxed());
-		Ok(Self { signed_commitment_stream, beefy_best_block, executor })
+		Ok(Self { finality_proof_stream, beefy_best_block, executor })
 	}
 }
 
 #[async_trait]
-impl<Block> BeefyApiServer<notification::EncodedSignedCommitment, Block::Hash> for Beefy<Block>
+impl<Block> BeefyApiServer<notification::EncodedVersionedFinalityProof, Block::Hash>
+	for Beefy<Block>
 where
 	Block: BlockT,
 {
 	fn subscribe_justifications(&self, mut sink: SubscriptionSink) -> SubscriptionResult {
 		let stream = self
-			.signed_commitment_stream
+			.finality_proof_stream
 			.subscribe()
-			.map(|sc| notification::EncodedSignedCommitment::new::<Block>(sc));
+			.map(|vfp| notification::EncodedVersionedFinalityProof::new::<Block>(vfp));
 
 		let fut = async move {
 			sink.pipe_from_stream(stream).await;
@@ -164,31 +165,31 @@ mod tests {
 	use super::*;
 
 	use beefy_gadget::{
-		justification::BeefySignedCommitment,
-		notification::{BeefyBestBlockStream, BeefySignedCommitmentSender},
+		justification::BeefyVersionedFinalityProof,
+		notification::{BeefyBestBlockStream, BeefyVersionedFinalityProofSender},
 	};
-	use beefy_primitives::{known_payload_ids, Payload};
+	use beefy_primitives::{known_payload_ids, Payload, SignedCommitment};
 	use codec::{Decode, Encode};
 	use jsonrpsee::{types::EmptyParams, RpcModule};
 	use sp_runtime::traits::{BlakeTwo256, Hash};
 	use substrate_test_runtime_client::runtime::Block;
 
-	fn setup_io_handler() -> (RpcModule<Beefy<Block>>, BeefySignedCommitmentSender<Block>) {
+	fn setup_io_handler() -> (RpcModule<Beefy<Block>>, BeefyVersionedFinalityProofSender<Block>) {
 		let (_, stream) = BeefyBestBlockStream::<Block>::channel();
 		setup_io_handler_with_best_block_stream(stream)
 	}
 
 	fn setup_io_handler_with_best_block_stream(
 		best_block_stream: BeefyBestBlockStream<Block>,
-	) -> (RpcModule<Beefy<Block>>, BeefySignedCommitmentSender<Block>) {
-		let (commitment_sender, commitment_stream) =
-			BeefySignedCommitmentStream::<Block>::channel();
+	) -> (RpcModule<Beefy<Block>>, BeefyVersionedFinalityProofSender<Block>) {
+		let (finality_proof_sender, finality_proof_stream) =
+			BeefyVersionedFinalityProofStream::<Block>::channel();
 
 		let handler =
-			Beefy::new(commitment_stream, best_block_stream, sc_rpc::testing::test_executor())
+			Beefy::new(finality_proof_stream, best_block_stream, sc_rpc::testing::test_executor())
 				.expect("Setting up the BEEFY RPC handler works");
 
-		(handler.into_rpc(), commitment_sender)
+		(handler.into_rpc(), finality_proof_sender)
 	}
 
 	#[tokio::test]
@@ -262,38 +263,38 @@ mod tests {
 		assert_eq!(response.result, expected);
 	}
 
-	fn create_commitment() -> BeefySignedCommitment<Block> {
+	fn create_finality_proof() -> BeefyVersionedFinalityProof<Block> {
 		let payload = Payload::new(known_payload_ids::MMR_ROOT_ID, "Hello World!".encode());
-		BeefySignedCommitment::<Block> {
+		BeefyVersionedFinalityProof::<Block>::V1(SignedCommitment {
 			commitment: beefy_primitives::Commitment {
 				payload,
 				block_number: 5,
 				validator_set_id: 0,
 			},
 			signatures: vec![],
-		}
+		})
 	}
 
 	#[tokio::test]
 	async fn subscribe_and_listen_to_one_justification() {
-		let (rpc, commitment_sender) = setup_io_handler();
+		let (rpc, finality_proof_sender) = setup_io_handler();
 
 		// Subscribe
 		let mut sub = rpc
 			.subscribe("beefy_subscribeJustifications", EmptyParams::new())
 			.await
 			.unwrap();
 
-		// Notify with commitment
-		let commitment = create_commitment();
-		let r: Result<(), ()> = commitment_sender.notify(|| Ok(commitment.clone()));
+		// Notify with finality_proof
+		let finality_proof = create_finality_proof();
+		let r: Result<(), ()> = finality_proof_sender.notify(|| Ok(finality_proof.clone()));
 		r.unwrap();
 
 		// Inspect what we received
 		let (bytes, recv_sub_id) = sub.next::<sp_core::Bytes>().await.unwrap().unwrap();
-		let recv_commitment: BeefySignedCommitment<Block> =
+		let recv_finality_proof: BeefyVersionedFinalityProof<Block> =
 			Decode::decode(&mut &bytes[..]).unwrap();
 		assert_eq!(&recv_sub_id, sub.subscription_id());
-		assert_eq!(recv_commitment, commitment);
+		assert_eq!(recv_finality_proof, finality_proof);
 	}
 }

client/beefy/rpc/src/notification.rs

@@ -21,19 +21,19 @@ use serde::{Deserialize, Serialize};
 
 use sp_runtime::traits::Block as BlockT;
 
-/// An encoded signed commitment proving that the given header has been finalized.
+/// An encoded finality proof proving that the given header has been finalized.
 /// The given bytes should be the SCALE-encoded representation of a
-/// `beefy_primitives::SignedCommitment`.
+/// `beefy_primitives::VersionedFinalityProof`.
 #[derive(Clone, Serialize, Deserialize)]
-pub struct EncodedSignedCommitment(sp_core::Bytes);
+pub struct EncodedVersionedFinalityProof(sp_core::Bytes);
 
-impl EncodedSignedCommitment {
+impl EncodedVersionedFinalityProof {
 	pub fn new<Block>(
-		signed_commitment: beefy_gadget::justification::BeefySignedCommitment<Block>,
+		finality_proof: beefy_gadget::justification::BeefyVersionedFinalityProof<Block>,
 	) -> Self
 	where
 		Block: BlockT,
 	{
-		EncodedSignedCommitment(signed_commitment.encode().into())
+		EncodedVersionedFinalityProof(finality_proof.encode().into())
 	}
 }

client/beefy/src/import.rs

@@ -16,7 +16,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-use beefy_primitives::{crypto::Signature, BeefyApi, VersionedFinalityProof, BEEFY_ENGINE_ID};
+use beefy_primitives::{BeefyApi, BEEFY_ENGINE_ID};
 use codec::Encode;
 use log::error;
 use std::{collections::HashMap, sync::Arc};
@@ -34,7 +34,8 @@ use sc_client_api::backend::Backend;
 use sc_consensus::{BlockCheckParams, BlockImport, BlockImportParams, ImportResult};
 
 use crate::{
-	justification::decode_and_verify_commitment, notification::BeefySignedCommitmentSender,
+	justification::{decode_and_verify_finality_proof, BeefyVersionedFinalityProof},
+	notification::BeefyVersionedFinalityProofSender,
 };
 
 /// A block-import handler for BEEFY.
@@ -47,7 +48,7 @@ pub struct BeefyBlockImport<Block: BlockT, Backend, RuntimeApi, I> {
 	backend: Arc<Backend>,
 	runtime: Arc<RuntimeApi>,
 	inner: I,
-	justification_sender: BeefySignedCommitmentSender<Block>,
+	justification_sender: BeefyVersionedFinalityProofSender<Block>,
 }
 
 impl<Block: BlockT, BE, Runtime, I: Clone> Clone for BeefyBlockImport<Block, BE, Runtime, I> {
@@ -67,7 +68,7 @@ impl<Block: BlockT, BE, Runtime, I> BeefyBlockImport<Block, BE, Runtime, I> {
 		backend: Arc<BE>,
 		runtime: Arc<Runtime>,
 		inner: I,
-		justification_sender: BeefySignedCommitmentSender<Block>,
+		justification_sender: BeefyVersionedFinalityProofSender<Block>,
 	) -> BeefyBlockImport<Block, BE, Runtime, I> {
 		BeefyBlockImport { backend, runtime, inner, justification_sender }
 	}
@@ -85,7 +86,7 @@ where
 		encoded: &EncodedJustification,
 		number: NumberFor<Block>,
 		hash: <Block as BlockT>::Hash,
-	) -> Result<VersionedFinalityProof<NumberFor<Block>, Signature>, ConsensusError> {
+	) -> Result<BeefyVersionedFinalityProof<Block>, ConsensusError> {
 		let block_id = BlockId::hash(hash);
 		let validator_set = self
 			.runtime
@@ -94,7 +95,7 @@ where
 			.map_err(|e| ConsensusError::ClientImport(e.to_string()))?
 			.ok_or_else(|| ConsensusError::ClientImport("Unknown validator set".to_string()))?;
 
-		decode_and_verify_commitment::<Block>(&encoded[..], number, &validator_set)
+		decode_and_verify_finality_proof::<Block>(&encoded[..], number, &validator_set)
 	}
 
 	/// Import BEEFY justification: Send it to worker for processing and also append it to backend.
@@ -105,7 +106,7 @@ where
 	fn import_beefy_justification_unchecked(
 		&self,
 		number: NumberFor<Block>,
-		justification: VersionedFinalityProof<NumberFor<Block>, Signature>,
+		justification: BeefyVersionedFinalityProof<Block>,
 	) {
 		// Append the justification to the block in the backend.
 		if let Err(e) = self.backend.append_justification(
@@ -115,14 +116,9 @@ where
 			error!(target: "beefy", "🥩 Error {:?} on appending justification: {:?}", e, justification);
 		}
 		// Send the justification to the BEEFY voter for processing.
-		match justification {
-			// TODO #11838: Should not unpack, these channels should also use
-			// `VersionedFinalityProof`.
-			VersionedFinalityProof::V1(signed_commitment) => self
-				.justification_sender
-				.notify(|| Ok::<_, ()>(signed_commitment))
-				.expect("forwards closure result; the closure always returns Ok; qed."),
-		};
+		self.justification_sender
+			.notify(|| Ok::<_, ()>(justification))
+			.expect("forwards closure result; the closure always returns Ok; qed.");
 	}
 }
 

client/beefy/src/justification.rs

@@ -25,17 +25,17 @@ use codec::{Decode, Encode};
 use sp_consensus::Error as ConsensusError;
 use sp_runtime::traits::{Block as BlockT, NumberFor};
 
-/// A commitment with matching BEEFY authorities' signatures.
-pub type BeefySignedCommitment<Block> =
-	beefy_primitives::SignedCommitment<NumberFor<Block>, beefy_primitives::crypto::Signature>;
+/// A finality proof with matching BEEFY authorities' signatures.
+pub type BeefyVersionedFinalityProof<Block> =
+	beefy_primitives::VersionedFinalityProof<NumberFor<Block>, Signature>;
 
-/// Decode and verify a Beefy SignedCommitment.
-pub(crate) fn decode_and_verify_commitment<Block: BlockT>(
+/// Decode and verify a Beefy FinalityProof.
+pub(crate) fn decode_and_verify_finality_proof<Block: BlockT>(
 	encoded: &[u8],
 	target_number: NumberFor<Block>,
 	validator_set: &ValidatorSet<AuthorityId>,
-) -> Result<VersionedFinalityProof<NumberFor<Block>, Signature>, ConsensusError> {
-	let proof = <VersionedFinalityProof<NumberFor<Block>, Signature>>::decode(&mut &*encoded)
+) -> Result<BeefyVersionedFinalityProof<Block>, ConsensusError> {
+	let proof = <BeefyVersionedFinalityProof<Block>>::decode(&mut &*encoded)
 		.map_err(|_| ConsensusError::InvalidJustification)?;
 	verify_with_validator_set::<Block>(target_number, validator_set, &proof).map(|_| proof)
 }
@@ -44,7 +44,7 @@ pub(crate) fn decode_and_verify_commitment<Block: BlockT>(
 fn verify_with_validator_set<Block: BlockT>(
 	target_number: NumberFor<Block>,
 	validator_set: &ValidatorSet<AuthorityId>,
-	proof: &VersionedFinalityProof<NumberFor<Block>, Signature>,
+	proof: &BeefyVersionedFinalityProof<Block>,
 ) -> Result<(), ConsensusError> {
 	match proof {
 		VersionedFinalityProof::V1(signed_commitment) => {
@@ -80,25 +80,27 @@ fn verify_with_validator_set<Block: BlockT>(
 
 #[cfg(test)]
 pub(crate) mod tests {
-	use beefy_primitives::{known_payload_ids, Commitment, Payload, SignedCommitment};
+	use beefy_primitives::{
+		known_payload_ids, Commitment, Payload, SignedCommitment, VersionedFinalityProof,
+	};
 	use substrate_test_runtime_client::runtime::Block;
 
 	use super::*;
 	use crate::{keystore::tests::Keyring, tests::make_beefy_ids};
 
-	pub(crate) fn new_signed_commitment(
+	pub(crate) fn new_finality_proof(
 		block_num: NumberFor<Block>,
 		validator_set: &ValidatorSet<AuthorityId>,
 		keys: &[Keyring],
-	) -> BeefySignedCommitment<Block> {
+	) -> BeefyVersionedFinalityProof<Block> {
 		let commitment = Commitment {
 			payload: Payload::new(known_payload_ids::MMR_ROOT_ID, vec![]),
 			block_number: block_num,
 			validator_set_id: validator_set.id(),
 		};
 		let message = commitment.encode();
 		let signatures = keys.iter().map(|key| Some(key.sign(&message))).collect();
-		SignedCommitment { commitment, signatures }
+		VersionedFinalityProof::V1(SignedCommitment { commitment, signatures })
 	}
 
 	#[test]
@@ -108,7 +110,7 @@ pub(crate) mod tests {
 
 		// build valid justification
 		let block_num = 42;
-		let proof = new_signed_commitment(block_num, &validator_set, keys);
+		let proof = new_finality_proof(block_num, &validator_set, keys);
 
 		let good_proof = proof.clone().into();
 		// should verify successfully
@@ -132,46 +134,55 @@ pub(crate) mod tests {
 		// wrong signatures length -> should fail verification
 		let mut bad_proof = proof.clone();
 		// change length of signatures
-		bad_proof.signatures.pop().flatten().unwrap();
+		let bad_signed_commitment = match bad_proof {
+			VersionedFinalityProof::V1(ref mut sc) => sc,
+		};
+		bad_signed_commitment.signatures.pop().flatten().unwrap();
 		match verify_with_validator_set::<Block>(block_num + 1, &validator_set, &bad_proof.into()) {
 			Err(ConsensusError::InvalidJustification) => (),
 			_ => assert!(false, "Expected Err(ConsensusError::InvalidJustification)"),
 		};
 
 		// not enough signatures -> should fail verification
 		let mut bad_proof = proof.clone();
+		let bad_signed_commitment = match bad_proof {
+			VersionedFinalityProof::V1(ref mut sc) => sc,
+		};
 		// remove a signature (but same length)
-		*bad_proof.signatures.first_mut().unwrap() = None;
+		*bad_signed_commitment.signatures.first_mut().unwrap() = None;
 		match verify_with_validator_set::<Block>(block_num + 1, &validator_set, &bad_proof.into()) {
 			Err(ConsensusError::InvalidJustification) => (),
 			_ => assert!(false, "Expected Err(ConsensusError::InvalidJustification)"),
 		};
 
 		// not enough _correct_ signatures -> should fail verification
 		let mut bad_proof = proof.clone();
+		let bad_signed_commitment = match bad_proof {
+			VersionedFinalityProof::V1(ref mut sc) => sc,
+		};
 		// change a signature to a different key
-		*bad_proof.signatures.first_mut().unwrap() =
-			Some(Keyring::Dave.sign(&proof.commitment.encode()));
+		*bad_signed_commitment.signatures.first_mut().unwrap() =
+			Some(Keyring::Dave.sign(&bad_signed_commitment.commitment.encode()));
 		match verify_with_validator_set::<Block>(block_num + 1, &validator_set, &bad_proof.into()) {
 			Err(ConsensusError::InvalidJustification) => (),
 			_ => assert!(false, "Expected Err(ConsensusError::InvalidJustification)"),
 		};
 	}
 
 	#[test]
-	fn should_decode_and_verify_commitment() {
+	fn should_decode_and_verify_finality_proof() {
 		let keys = &[Keyring::Alice, Keyring::Bob];
 		let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap();
 		let block_num = 1;
 
 		// build valid justification
-		let proof = new_signed_commitment(block_num, &validator_set, keys);
-		let versioned_proof: VersionedFinalityProof<NumberFor<Block>, Signature> = proof.into();
+		let proof = new_finality_proof(block_num, &validator_set, keys);
+		let versioned_proof: BeefyVersionedFinalityProof<Block> = proof.into();
 		let encoded = versioned_proof.encode();
 
 		// should successfully decode and verify
 		let verified =
-			decode_and_verify_commitment::<Block>(&encoded, block_num, &validator_set).unwrap();
+			decode_and_verify_finality_proof::<Block>(&encoded, block_num, &validator_set).unwrap();
 		assert_eq!(verified, versioned_proof);
 	}
 }