Skip to content

Commit

Permalink
Further reduce boilerplate code by leveraging generics
Browse files Browse the repository at this point in the history
  • Loading branch information
liamsi committed Nov 9, 2019
1 parent 3758cf4 commit 8bbe23f
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 36 deletions.
1 change: 1 addition & 0 deletions tendermint/src/amino_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

pub mod block_id;
pub mod ed25519;
pub mod message;
pub mod ping;
pub mod proposal;
pub mod remote_error;
Expand Down
39 changes: 39 additions & 0 deletions tendermint/src/amino_types/message.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/// A wrapper whose sole purpose is to extend the original
/// prost::Message trait with a few helper functions in order to reduce boiler-plate code
/// (and without modifying the prost-amino dependency).
pub struct AminoMessage<M>(M); // Note: we have to introduce a new type in order to add methods like bytes_vec or impl Into<Vec<u8>>

impl<M> AminoMessage<M>
where
M: prost_amino::Message,
{
/// Returns a wrapper for the given prost-amino message
pub fn new(m: M) -> AminoMessage<M> {
AminoMessage(m)
}
/// Directly amino encode a prost-amino message into a freshly created Vec<u8>.
/// This can be useful when passing those bytes directly to a hasher, or,
/// to reduce boiler plate code when working with the encoded bytes.
///
/// Warning: Only use this method, if you are in control what will be encoded.
/// If there is an encoding error, this method will panic.
pub fn bytes_vec(m: M) -> Vec<u8> {
AminoMessage(m).into()
}
}

impl<M> Into<Vec<u8>> for AminoMessage<M>
where
M: prost_amino::Message,
{
/// Convert a wrapped prost-amino message directly into a byte vector which contains the amino
/// encoding.
///
/// Warning: Only use this method, if you are in control what will be encoded. If there is an
/// encoding error, this method will panic.
fn into(self) -> Vec<u8> {
let mut res = Vec::with_capacity(self.0.encoded_len());
self.0.encode(&mut res).unwrap();
res
}

This comment has been minimized.

Copy link
@liamsi

liamsi Nov 11, 2019

Author Member

The above code snippet is repeated a lot throughout the tendermint_rs codebase. If we agree on this approach we can replace many occurrences where we do actually operate on the encoded bytes directly, e.g., when hashing, generating sign-bytes etc (instead of just writing them into a BufMut).

See hash_bytes and the changes in the tests as an example, or, in simple_merkle:
8bbe23f#diff-a1644bb72a1c83c54d82738ef71ce095L97-L111

This comment has been minimized.

Copy link
@tarcieri

tarcieri Nov 11, 2019

Contributor

Would suggest either of the following:

  • Creating a trait for it with a blanket impl
  • Adding the method upstream to prost_amino

If the goal is to eventually move to prost proper, the trait option probably makes the most sense

This comment has been minimized.

Copy link
@liamsi

liamsi Nov 11, 2019

Author Member

The blanket impl is a great suggestion (I sometimes forget the how great the Rust type-system is...).
Thanks @tarcieri! Done in 6f8c7f7.

}
13 changes: 5 additions & 8 deletions tendermint/src/amino_types/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ impl ConsensusMessage for Vote {
mod tests {
use super::super::PartsSetHeader;
use super::*;
use crate::amino_types::message::AminoMessage;
use crate::amino_types::SignedMsgType;
use chrono::{DateTime, Utc};
use prost::Message;
Expand Down Expand Up @@ -333,8 +334,7 @@ mod tests {
vt_precommit.vote_type = SignedMsgType::PreCommit.to_u32(); // precommit
println!("{:?}", vt_precommit);
let cv_precommit = CanonicalVote::new(vt_precommit, "");
got = vec![];
cv_precommit.encode(&mut got).unwrap();
let got = AminoMessage::bytes_vec(cv_precommit);
let want = vec![
0x8, // (field_number << 3) | wire_type
0x2, // PrecommitType
Expand All @@ -355,10 +355,9 @@ mod tests {
vt_prevote.round = 1;
vt_prevote.vote_type = SignedMsgType::PreVote.to_u32();

got = vec![];
let cv_prevote = CanonicalVote::new(vt_prevote, "");

cv_prevote.encode(&mut got).unwrap();
let got = AminoMessage::bytes_vec(cv_prevote);

let want = vec![
0x8, // (field_number << 3) | wire_type
Expand All @@ -379,9 +378,8 @@ mod tests {
vt_no_type.height = 1;
vt_no_type.round = 1;

got = vec![];
let cv = CanonicalVote::new(vt_no_type, "");
cv.encode(&mut got).unwrap();
let got = AminoMessage::bytes_vec(cv);

let want = vec![
0x11, // (field_number << 3) | wire_type
Expand All @@ -400,8 +398,7 @@ mod tests {
no_vote_type2.round = 1;

let with_chain_id = CanonicalVote::new(no_vote_type2, "test_chain_id");
got = vec![];
with_chain_id.encode(&mut got).unwrap();
got = AminoMessage::bytes_vec(with_chain_id);
let want = vec![
0x11, // (field_number << 3) | wire_type
0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, // height
Expand Down
29 changes: 9 additions & 20 deletions tendermint/src/block/header.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Block headers
use crate::merkle::simple_hash_from_byte_slices;
use crate::{account, amino_types, block, chain, lite, Hash, Time};
use prost::Message;
use amino_types::{message::AminoMessage, BlockId, ConsensusVersion, TimeMsg};
use {
crate::serializers,
serde::{Deserialize, Serialize},
Expand Down Expand Up @@ -89,32 +89,21 @@ impl lite::Header for Header {
}

fn hash(&self) -> Hash {
let mut version_enc = vec![];
// TODO: if there is an encoding problem this will
// Note that if there is an encoding problem this will
// panic (as the golang code would):
// https://github.com/tendermint/tendermint/blob/134fe2896275bb926b49743c1e25493f6b24cc31/types/block.go#L393
// https://github.com/tendermint/tendermint/blob/134fe2896275bb926b49743c1e25493f6b24cc31/types/encoding_helper.go#L9:6
// Instead, handle errors gracefully here.
amino_types::ConsensusVersion::from(&self.version)
.encode(&mut version_enc)
.unwrap();
let mut time_enc = vec![];
amino_types::TimeMsg::from(self.time)
.encode(&mut time_enc)
.unwrap();
let mut last_block_id_enc = vec![];
amino_types::BlockId::from(&self.last_block_id)
.encode(&mut last_block_id_enc)
.unwrap();

let mut byteslices: Vec<Vec<u8>> = vec![];
byteslices.push(version_enc);

let mut byteslices: Vec<Vec<u8>> = Vec::with_capacity(16);
byteslices.push(AminoMessage::bytes_vec(ConsensusVersion::from(
&self.version,
)));
byteslices.push(bytes_enc(self.chain_id.as_bytes()));
byteslices.push(encode_varint(self.height.value()));
byteslices.push(time_enc);
byteslices.push(AminoMessage::bytes_vec(TimeMsg::from(self.time)));
byteslices.push(encode_varint(self.num_txs));
byteslices.push(encode_varint(self.total_txs));
byteslices.push(last_block_id_enc);
byteslices.push(AminoMessage::bytes_vec(BlockId::from(&self.last_block_id)));
byteslices.push(encode_hash(self.last_commit_hash));
byteslices.push(encode_hash(self.data_hash));
byteslices.push(encode_hash(self.validators_hash));
Expand Down
6 changes: 2 additions & 4 deletions tendermint/src/validator.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Tendermint validators

use crate::amino_types::message::AminoMessage;
use crate::validator::signatory::{Signature, Verifier};
use crate::{account, lite, merkle, vote, Hash, PublicKey};
use prost::Message;
use serde::{de::Error as _, Deserialize, Deserializer, Serialize, Serializer};
use signatory;
use signatory::ed25519;
Expand Down Expand Up @@ -147,9 +147,7 @@ impl From<&Info> for InfoHashable {
// pubkey and voting power, so it includes the pubkey's amino prefix.
impl Info {
fn hash_bytes(&self) -> Vec<u8> {
let mut bytes: Vec<u8> = Vec::new();
InfoHashable::from(self).encode(&mut bytes).unwrap();
bytes
AminoMessage::bytes_vec(InfoHashable::from(self))
}
}

Expand Down
6 changes: 2 additions & 4 deletions tendermint/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
mod power;

pub use self::power::Power;
use crate::amino_types::message::AminoMessage;
use crate::amino_types::vote::CanonicalVote;
use crate::prost::Message;
use crate::{account, block, lite, Signature, Time};
use {
crate::serializers,
Expand Down Expand Up @@ -99,9 +99,7 @@ impl lite::Vote for SignedVote {
}

fn sign_bytes(&self) -> Vec<u8> {
let mut sign_bytes = vec![];
self.vote.encode(&mut sign_bytes).unwrap();
sign_bytes
AminoMessage::bytes_vec(self.vote.to_owned())
}
fn signature(&self) -> &[u8] {
match &self.signature {
Expand Down

0 comments on commit 8bbe23f

Please sign in to comment.