Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
grandpa: reduce allocations when verifying multiple messages (#4693)
Browse files Browse the repository at this point in the history
  • Loading branch information
andresilva committed Jan 21, 2020
1 parent cb9c181 commit f52ea97
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 8 deletions.
66 changes: 59 additions & 7 deletions client/finality-grandpa/src/communication/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,28 @@ impl<B: BlockT, N: Network<B>> Clone for NetworkBridge<B, N> {
}
}

pub(crate) fn localized_payload<E: Encode>(round: RoundNumber, set_id: SetIdNumber, message: &E) -> Vec<u8> {
(message, round, set_id).encode()
/// Encode round message localized to a given round and set id.
pub(crate) fn localized_payload<E: Encode>(
round: RoundNumber,
set_id: SetIdNumber,
message: &E,
) -> Vec<u8> {
let mut buf = Vec::new();
localized_payload_with_buffer(round, set_id, message, &mut buf);
buf
}

/// Encode round message localized to a given round and set id using the given
/// buffer. The given buffer will be cleared and the resulting encoded payload
/// will always be written to the start of the buffer.
pub(crate) fn localized_payload_with_buffer<E: Encode>(
round: RoundNumber,
set_id: SetIdNumber,
message: &E,
buf: &mut Vec<u8>,
) {
buf.clear();
(message, round, set_id).encode_to(buf)
}

/// Type-safe wrapper around a round number.
Expand All @@ -612,17 +632,41 @@ pub struct Round(pub RoundNumber);
#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Encode, Decode)]
pub struct SetId(pub SetIdNumber);

// check a message.
/// Check a message signature by encoding the message as a localized payload and
/// verifying the provided signature using the expected authority id.
pub(crate) fn check_message_sig<Block: BlockT>(
message: &Message<Block>,
id: &AuthorityId,
signature: &AuthoritySignature,
round: RoundNumber,
set_id: SetIdNumber,
) -> Result<(), ()> {
check_message_sig_with_buffer::<Block>(
message,
id,
signature,
round,
set_id,
&mut Vec::new(),
)
}

/// Check a message signature by encoding the message as a localized payload and
/// verifying the provided signature using the expected authority id.
/// The encoding necessary to verify the signature will be done using the given
/// buffer, the original content of the buffer will be cleared.
pub(crate) fn check_message_sig_with_buffer<Block: BlockT>(
message: &Message<Block>,
id: &AuthorityId,
signature: &AuthoritySignature,
round: RoundNumber,
set_id: SetIdNumber,
buf: &mut Vec<u8>,
) -> Result<(), ()> {
let as_public = id.clone();
let encoded_raw = localized_payload(round, set_id, message);
if AuthorityPair::verify(signature, &encoded_raw, &as_public) {
localized_payload_with_buffer(round, set_id, message, buf);

if AuthorityPair::verify(signature, buf, &as_public) {
Ok(())
} else {
debug!(target: "afg", "Bad signature on message from {:?}", id);
Expand Down Expand Up @@ -752,19 +796,21 @@ fn check_compact_commit<Block: BlockT>(
}

// check signatures on all contained precommits.
let mut buf = Vec::new();
for (i, (precommit, &(ref sig, ref id))) in msg.precommits.iter()
.zip(&msg.auth_data)
.enumerate()
{
use crate::communication::gossip::Misbehavior;
use finality_grandpa::Message as GrandpaMessage;

if let Err(()) = check_message_sig::<Block>(
if let Err(()) = check_message_sig_with_buffer::<Block>(
&GrandpaMessage::Precommit(precommit.clone()),
id,
sig,
round.0,
set_id.0,
&mut buf,
) {
debug!(target: "afg", "Bad commit message signature {}", id);
telemetry!(CONSENSUS_DEBUG; "afg.bad_commit_msg_signature"; "id" => ?id);
Expand Down Expand Up @@ -836,6 +882,7 @@ fn check_catch_up<Block: BlockT>(
round: RoundNumber,
set_id: SetIdNumber,
mut signatures_checked: usize,
buf: &mut Vec<u8>,
) -> Result<usize, ReputationChange> where
B: BlockT,
I: Iterator<Item=(Message<B>, &'a AuthorityId, &'a AuthoritySignature)>,
Expand All @@ -845,12 +892,13 @@ fn check_catch_up<Block: BlockT>(
for (msg, id, sig) in messages {
signatures_checked += 1;

if let Err(()) = check_message_sig::<B>(
if let Err(()) = check_message_sig_with_buffer::<B>(
&msg,
id,
sig,
round,
set_id,
buf,
) {
debug!(target: "afg", "Bad catch up message signature {}", id);
telemetry!(CONSENSUS_DEBUG; "afg.bad_catch_up_msg_signature"; "id" => ?id);
Expand All @@ -866,6 +914,8 @@ fn check_catch_up<Block: BlockT>(
Ok(signatures_checked)
}

let mut buf = Vec::new();

// check signatures on all contained prevotes.
let signatures_checked = check_signatures::<Block, _>(
msg.prevotes.iter().map(|vote| {
Expand All @@ -874,6 +924,7 @@ fn check_catch_up<Block: BlockT>(
msg.round_number,
set_id.0,
0,
&mut buf,
)?;

// check signatures on all contained precommits.
Expand All @@ -884,6 +935,7 @@ fn check_catch_up<Block: BlockT>(
msg.round_number,
set_id.0,
signatures_checked,
&mut buf,
)?;

Ok(())
Expand Down
4 changes: 3 additions & 1 deletion client/finality-grandpa/src/justification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,16 @@ impl<Block: BlockT> GrandpaJustification<Block> {
}
}

let mut buf = Vec::new();
let mut visited_hashes = HashSet::new();
for signed in self.commit.precommits.iter() {
if let Err(_) = communication::check_message_sig::<Block>(
if let Err(_) = communication::check_message_sig_with_buffer::<Block>(
&finality_grandpa::Message::Precommit(signed.precommit.clone()),
&signed.id,
&signed.signature,
self.round,
set_id,
&mut buf,
) {
return Err(ClientError::BadJustification(
"invalid signature for precommit in grandpa justification".to_string()).into());
Expand Down

0 comments on commit f52ea97

Please sign in to comment.