grandpa: reduce allocations when verifying multiple messages (#4693)

This commit is contained in:
André Silva
2020-01-21 11:45:36 +00:00
committed by GitHub
parent 85aa632278
commit b7a63e3b77
2 changed files with 62 additions and 8 deletions
@@ -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> { /// Encode round message localized to a given round and set id.
(message, round, set_id).encode() 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. /// Type-safe wrapper around a round number.
@@ -612,17 +632,41 @@ pub struct Round(pub RoundNumber);
#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Encode, Decode)] #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Encode, Decode)]
pub struct SetId(pub SetIdNumber); 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>( pub(crate) fn check_message_sig<Block: BlockT>(
message: &Message<Block>, message: &Message<Block>,
id: &AuthorityId, id: &AuthorityId,
signature: &AuthoritySignature, signature: &AuthoritySignature,
round: RoundNumber, round: RoundNumber,
set_id: SetIdNumber, 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<(), ()> { ) -> Result<(), ()> {
let as_public = id.clone(); let as_public = id.clone();
let encoded_raw = localized_payload(round, set_id, message); localized_payload_with_buffer(round, set_id, message, buf);
if AuthorityPair::verify(signature, &encoded_raw, &as_public) {
if AuthorityPair::verify(signature, buf, &as_public) {
Ok(()) Ok(())
} else { } else {
debug!(target: "afg", "Bad signature on message from {:?}", id); debug!(target: "afg", "Bad signature on message from {:?}", id);
@@ -752,6 +796,7 @@ fn check_compact_commit<Block: BlockT>(
} }
// check signatures on all contained precommits. // check signatures on all contained precommits.
let mut buf = Vec::new();
for (i, (precommit, &(ref sig, ref id))) in msg.precommits.iter() for (i, (precommit, &(ref sig, ref id))) in msg.precommits.iter()
.zip(&msg.auth_data) .zip(&msg.auth_data)
.enumerate() .enumerate()
@@ -759,12 +804,13 @@ fn check_compact_commit<Block: BlockT>(
use crate::communication::gossip::Misbehavior; use crate::communication::gossip::Misbehavior;
use finality_grandpa::Message as GrandpaMessage; 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()), &GrandpaMessage::Precommit(precommit.clone()),
id, id,
sig, sig,
round.0, round.0,
set_id.0, set_id.0,
&mut buf,
) { ) {
debug!(target: "afg", "Bad commit message signature {}", id); debug!(target: "afg", "Bad commit message signature {}", id);
telemetry!(CONSENSUS_DEBUG; "afg.bad_commit_msg_signature"; "id" => ?id); telemetry!(CONSENSUS_DEBUG; "afg.bad_commit_msg_signature"; "id" => ?id);
@@ -836,6 +882,7 @@ fn check_catch_up<Block: BlockT>(
round: RoundNumber, round: RoundNumber,
set_id: SetIdNumber, set_id: SetIdNumber,
mut signatures_checked: usize, mut signatures_checked: usize,
buf: &mut Vec<u8>,
) -> Result<usize, ReputationChange> where ) -> Result<usize, ReputationChange> where
B: BlockT, B: BlockT,
I: Iterator<Item=(Message<B>, &'a AuthorityId, &'a AuthoritySignature)>, I: Iterator<Item=(Message<B>, &'a AuthorityId, &'a AuthoritySignature)>,
@@ -845,12 +892,13 @@ fn check_catch_up<Block: BlockT>(
for (msg, id, sig) in messages { for (msg, id, sig) in messages {
signatures_checked += 1; signatures_checked += 1;
if let Err(()) = check_message_sig::<B>( if let Err(()) = check_message_sig_with_buffer::<B>(
&msg, &msg,
id, id,
sig, sig,
round, round,
set_id, set_id,
buf,
) { ) {
debug!(target: "afg", "Bad catch up message signature {}", id); debug!(target: "afg", "Bad catch up message signature {}", id);
telemetry!(CONSENSUS_DEBUG; "afg.bad_catch_up_msg_signature"; "id" => ?id); telemetry!(CONSENSUS_DEBUG; "afg.bad_catch_up_msg_signature"; "id" => ?id);
@@ -866,6 +914,8 @@ fn check_catch_up<Block: BlockT>(
Ok(signatures_checked) Ok(signatures_checked)
} }
let mut buf = Vec::new();
// check signatures on all contained prevotes. // check signatures on all contained prevotes.
let signatures_checked = check_signatures::<Block, _>( let signatures_checked = check_signatures::<Block, _>(
msg.prevotes.iter().map(|vote| { msg.prevotes.iter().map(|vote| {
@@ -874,6 +924,7 @@ fn check_catch_up<Block: BlockT>(
msg.round_number, msg.round_number,
set_id.0, set_id.0,
0, 0,
&mut buf,
)?; )?;
// check signatures on all contained precommits. // check signatures on all contained precommits.
@@ -884,6 +935,7 @@ fn check_catch_up<Block: BlockT>(
msg.round_number, msg.round_number,
set_id.0, set_id.0,
signatures_checked, signatures_checked,
&mut buf,
)?; )?;
Ok(()) Ok(())
@@ -132,14 +132,16 @@ impl<Block: BlockT> GrandpaJustification<Block> {
} }
} }
let mut buf = Vec::new();
let mut visited_hashes = HashSet::new(); let mut visited_hashes = HashSet::new();
for signed in self.commit.precommits.iter() { 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()), &finality_grandpa::Message::Precommit(signed.precommit.clone()),
&signed.id, &signed.id,
&signed.signature, &signed.signature,
self.round, self.round,
set_id, set_id,
&mut buf,
) { ) {
return Err(ClientError::BadJustification( return Err(ClientError::BadJustification(
"invalid signature for precommit in grandpa justification".to_string()).into()); "invalid signature for precommit in grandpa justification".to_string()).into());