Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
31 changes: 16 additions & 15 deletions client/beefy/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ use jsonrpsee::{
};
use log::warn;

use beefy_gadget::notification::{BeefyBestBlockStream, BeefySignedCommitmentStream};
use beefy_gadget::notification::{BeefyBestBlockStream, BeefyVersionedFinalityProofStream,};
use beefy_primitives::VersionedFinalityProof;

mod notification;

Expand Down Expand Up @@ -101,7 +102,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,
}
Expand All @@ -112,7 +113,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> {
Expand All @@ -126,7 +127,7 @@ 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 })
}
}

Expand All @@ -136,10 +137,10 @@ where
Block: BlockT,
{
fn subscribe_justifications(&self, pending: PendingSubscription) {
let stream = self
.signed_commitment_stream
.subscribe()
.map(|sc| notification::EncodedSignedCommitment::new::<Block>(sc));
let stream = self.signed_commitment_stream.subscribe().map(|vfp| match vfp {
VersionedFinalityProof::V1(sc) =>
notification::EncodedSignedCommitment::new::<Block>(sc),
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would actually change the exposed RPCs too. User facing API should support versioning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the exposed RPCs now.


let fut = async move {
if let Some(mut sink) = pending.accept() {
Expand All @@ -166,30 +167,30 @@ mod tests {

use beefy_gadget::{
justification::BeefySignedCommitment,
notification::{BeefyBestBlockStream, BeefySignedCommitmentSender},
notification::{BeefyBestBlockStream, BeefyVersionedFinalityProofSender},
};
use beefy_primitives::{known_payload_ids, Payload};
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]
Expand Down
21 changes: 10 additions & 11 deletions client/beefy/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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.
Expand All @@ -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> {
Expand All @@ -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 }
}
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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(
Expand All @@ -116,11 +117,9 @@ where
}
// 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
BeefyVersionedFinalityProof::<Block>::V1(signed_commitment) => self
.justification_sender
.notify(|| Ok::<_, ()>(signed_commitment))
.notify(|| Ok::<_, ()>(BeefyVersionedFinalityProof::<Block>::V1(signed_commitment)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to unpack here just to pack it back in V1, just directly send the justification

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I made confused here. 😄

.expect("forwards closure result; the closure always returns Ok; qed."),
};
}
Expand Down
53 changes: 32 additions & 21 deletions client/beefy/src/justification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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) => {
Expand Down Expand Up @@ -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]
Expand All @@ -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
Expand All @@ -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);
}
}
14 changes: 7 additions & 7 deletions client/beefy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ mod tests;
use crate::{
import::BeefyBlockImport,
notification::{
BeefyBestBlockSender, BeefyBestBlockStream, BeefySignedCommitmentSender,
BeefySignedCommitmentStream,
BeefyBestBlockSender, BeefyBestBlockStream, BeefyVersionedFinalityProofSender,
BeefyVersionedFinalityProofStream,
},
};

Expand Down Expand Up @@ -121,11 +121,11 @@ where
pub struct BeefyVoterLinks<B: Block> {
// BlockImport -> Voter links
/// Stream of BEEFY signed commitments from block import to voter.
pub from_block_import_justif_stream: BeefySignedCommitmentStream<B>,
pub from_block_import_justif_stream: BeefyVersionedFinalityProofStream<B>,

// Voter -> RPC links
/// Sends BEEFY signed commitments from voter to RPC.
pub to_rpc_justif_sender: BeefySignedCommitmentSender<B>,
pub to_rpc_justif_sender: BeefyVersionedFinalityProofSender<B>,
/// Sends BEEFY best block hashes from voter to RPC.
pub to_rpc_best_block_sender: BeefyBestBlockSender<B>,
}
Expand All @@ -134,7 +134,7 @@ pub struct BeefyVoterLinks<B: Block> {
#[derive(Clone)]
pub struct BeefyRPCLinks<B: Block> {
/// Stream of signed commitments coming from the voter.
pub from_voter_justif_stream: BeefySignedCommitmentStream<B>,
pub from_voter_justif_stream: BeefyVersionedFinalityProofStream<B>,
/// Stream of BEEFY best block hashes coming from the voter.
pub from_voter_best_beefy_stream: BeefyBestBlockStream<B>,
}
Expand All @@ -156,13 +156,13 @@ where
{
// Voter -> RPC links
let (to_rpc_justif_sender, from_voter_justif_stream) =
notification::BeefySignedCommitmentStream::<B>::channel();
notification::BeefyVersionedFinalityProofStream::<B>::channel();
let (to_rpc_best_block_sender, from_voter_best_beefy_stream) =
notification::BeefyBestBlockStream::<B>::channel();

// BlockImport -> Voter links
let (to_voter_justif_sender, from_block_import_justif_stream) =
notification::BeefySignedCommitmentStream::<B>::channel();
notification::BeefyVersionedFinalityProofStream::<B>::channel();

// BlockImport
let import =
Expand Down
21 changes: 11 additions & 10 deletions client/beefy/src/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use sc_utils::notification::{NotificationSender, NotificationStream, TracingKeyStr};
use sp_runtime::traits::Block as BlockT;

use crate::justification::BeefySignedCommitment;
use crate::justification::BeefyVersionedFinalityProof;

/// The sending half of the notifications channel(s) used to send
/// notifications about best BEEFY block from the gadget side.
Expand All @@ -31,13 +31,14 @@ pub type BeefyBestBlockStream<Block> =
NotificationStream<<Block as BlockT>::Hash, BeefyBestBlockTracingKey>;

/// The sending half of the notifications channel(s) used to send notifications
/// about signed commitments generated at the end of a BEEFY round.
pub type BeefySignedCommitmentSender<Block> = NotificationSender<BeefySignedCommitment<Block>>;
/// about versioned finality proof generated at the end of a BEEFY round.
pub type BeefyVersionedFinalityProofSender<Block> =
NotificationSender<BeefyVersionedFinalityProof<Block>>;

/// The receiving half of a notifications channel used to receive notifications
/// about signed commitments generated at the end of a BEEFY round.
pub type BeefySignedCommitmentStream<Block> =
NotificationStream<BeefySignedCommitment<Block>, BeefySignedCommitmentTracingKey>;
/// about versioned finality proof generated at the end of a BEEFY round.
pub type BeefyVersionedFinalityProofStream<Block> =
NotificationStream<BeefyVersionedFinalityProof<Block>, BeefyVersionedFinalityProofTracingKey>;

/// Provides tracing key for BEEFY best block stream.
#[derive(Clone)]
Expand All @@ -46,9 +47,9 @@ impl TracingKeyStr for BeefyBestBlockTracingKey {
const TRACING_KEY: &'static str = "mpsc_beefy_best_block_notification_stream";
}

/// Provides tracing key for BEEFY signed commitments stream.
/// Provides tracing key for BEEFY versioned finality proof stream.
#[derive(Clone)]
pub struct BeefySignedCommitmentTracingKey;
impl TracingKeyStr for BeefySignedCommitmentTracingKey {
const TRACING_KEY: &'static str = "mpsc_beefy_signed_commitments_notification_stream";
pub struct BeefyVersionedFinalityProofTracingKey;
impl TracingKeyStr for BeefyVersionedFinalityProofTracingKey {
const TRACING_KEY: &'static str = "mpsc_beefy_versioned_finality_proof_notification_stream";
}
Loading