Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blanket implementation for DomainType #571

Merged
merged 4 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 2 additions & 34 deletions proto-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,41 +55,9 @@ fn expand_domaintype(input: &syn::DeriveInput) -> TokenStream {
}
};

// This is what all the hubbub is about.
Copy link
Member

Choose a reason for hiding this comment

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

Not entirely. I was also wondering if we need the whole proc_macro at all.
You can replace all these two lines that look like this

#[derive(DomainType)]
#[rawtype(RawPartSetHeader)]

with:
impl DomainType<MyRawType> for ActualType {}.
As a consequence you can delete the whole macro code:

#[proc_macro_derive(DomainType, attributes(rawtype))]
pub fn domaintype(input: TokenStream) -> TokenStream {
let input = syn::parse_macro_input!(input as syn::DeriveInput);
expand_domaintype(&input)
}
fn expand_domaintype(input: &syn::DeriveInput) -> TokenStream {
let ident = &input.ident;
// Todo: Make this function more robust and easier to read.
let rawtype_attributes = &input
.attrs
.iter()
.filter(|&attr| attr.path.is_ident("rawtype"))
.collect::<Vec<&syn::Attribute>>();
if rawtype_attributes.len() != 1 {
return syn::Error::new(
rawtype_attributes.first().span(),
"exactly one #[rawtype(RawType)] expected",
)
.to_compile_error()
.into();
}
let rawtype_tokens = rawtype_attributes[0]
.tokens
.clone()
.into_iter()
.collect::<Vec<quote::__private::TokenTree>>();
if rawtype_tokens.len() != 1 {
return syn::Error::new(rawtype_attributes[0].span(), "#[rawtype(RawType)] expected")
.to_compile_error()
.into();
}
let rawtype = match &rawtype_tokens[0] {
proc_macro2::TokenTree::Group(group) => group.stream(),
_ => {
return syn::Error::new(
rawtype_tokens[0].span(),
"#[rawtype(RawType)] group expected",
)
.to_compile_error()
.into()
}
};

And in fact the whole proto-derive crate can be deleted, without losing any functionality or any further consequences. I would argue whole code becomes easier to navigate as the macro creates some unnecessary indirection for a single line that explains what is happing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, true. Annotations seem cleaner to me, because I don't expect to look at the derive function. Serde does the same thing and it worked out pretty well there.

Also, we need to do some kind of JSON serialization for these domain types too and I didn't have time to get into the details of how we're going to do it. Maybe the derive macro helps, maybe not. Well, currently not really, it's just syntactic sugar.

I'll open this up to the team, I'm happy to do it either way, I just really wanted to do annotations. :)

let gen = quote! {
impl ::tendermint_proto::DomainType<#rawtype> for #ident {

fn encode<B: ::tendermint_proto::bytes::BufMut>(self, buf: &mut B) -> ::std::result::Result<(), ::tendermint_proto::Error> {
use ::tendermint_proto::prost::Message;
#rawtype::from(self).encode(buf).map_err(|e| ::tendermint_proto::Kind::EncodeMessage.context(e).into())
}

fn encode_length_delimited<B: ::tendermint_proto::bytes::BufMut>(self, buf: &mut B) -> ::std::result::Result<(), ::tendermint_proto::Error> {
use ::tendermint_proto::prost::Message;
#rawtype::from(self).encode_length_delimited(buf).map_err(|e| ::tendermint_proto::Kind::EncodeMessage.context(e).into())
}

fn decode<B: ::tendermint_proto::bytes::Buf>(buf: B) -> Result<Self, ::tendermint_proto::Error> {
use ::tendermint_proto::prost::Message;
#rawtype::decode(buf).map_or_else(
|e| ::std::result::Result::Err(::tendermint_proto::Kind::DecodeMessage.context(e).into()),
|t| Self::try_from(t).map_err(|e| ::tendermint_proto::Kind::TryIntoDomainType.context(e).into())
)
}

fn decode_length_delimited<B: ::tendermint_proto::bytes::Buf>(buf: B) -> Result<Self, ::tendermint_proto::Error> {
use ::tendermint_proto::prost::Message;
#rawtype::decode_length_delimited(buf).map_or_else(
|e| ::std::result::Result::Err(::tendermint_proto::Kind::DecodeMessage.context(e).into()),
|t| Self::try_from(t).map_err(|e| ::tendermint_proto::Kind::TryIntoDomainType.context(e).into())
)
}

fn encoded_len(self) -> usize {
use ::tendermint_proto::prost::Message;
#rawtype::from(self).encoded_len()
}

}
impl ::tendermint_proto::DomainType<#rawtype> for #ident {}
};
gen.into()
}
89 changes: 77 additions & 12 deletions proto/src/domaintype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@
//! Note that the Prost library and the TryFrom method have their own set of errors. These are
//! merged into a custom Error type defined in this crate for easier handling.
//!
//! Requirements:
//! * The DomainType trait requires the struct to implement the Clone trait.
//! * The DomainType trait requires the struct to have a #[rawtype(MYRAWTYPE)] attribute set where
//! the MYRAWTYPE is a structure that has the prost::Message trait implemented. (protobuf type)
//! * The DomainType trait requires that the TryFrom<RawType> implemented on the structure has an
//! error type that implements Into<BoxError>. (The current implementations with anomaly are
//! fine.)
Copy link
Member

Choose a reason for hiding this comment

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

Actually some of the comments need updating now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll do that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Minor updates in the documentation: removed references to the DomainType macro. I hope this is it. :)

//!
//! How to implement a DomainType struct:
//! 1. Implement your struct based on your expectations for the developer
Expand All @@ -43,37 +50,95 @@
//!
//! Note: the `[rawtype()]` parameter is similar to how `serde` implements serialization through a
//! `[serde(with="")]` interim type.
//!

use crate::Error;
use crate::{Error, Kind};
use anomaly::BoxError;
use bytes::{Buf, BufMut};
use prost::Message;
use prost::{encoding::encoded_len_varint, Message};
use std::convert::{TryFrom, TryInto};

/// DomainType trait allows protobuf encoding and decoding for domain types
pub trait DomainType<T: Message + From<Self>>: Sized {
pub trait DomainType<T: Message + From<Self> + Default>
where
Self: Sized + Clone + TryFrom<T>,
<Self as TryFrom<T>>::Error: Into<BoxError>,
{
/// Encodes the DomainType into a buffer.
///
/// The DomainType will be consumed.
fn encode<B: BufMut>(self, buf: &mut B) -> Result<(), Error>;
/// This function replaces the Prost::Message encode() function for DomainTypes.
fn encode<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> {
T::from(self.clone())
.encode(buf)
.map_err(|e| Kind::EncodeMessage.context(e).into())
}

/// Encodes the DomainType with a length-delimiter to a buffer.
///
/// The DomainType will be consumed.
/// An error will be returned if the buffer does not have sufficient capacity.
fn encode_length_delimited<B: BufMut>(self, buf: &mut B) -> Result<(), Error>;
///
/// This function replaces the Prost::Message encode_length_delimited() function for
/// DomainTypes.
fn encode_length_delimited<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> {
T::from(self.clone())
.encode_length_delimited(buf)
.map_err(|e| Kind::EncodeMessage.context(e).into())
}

/// Decodes an instance of the message from a buffer and then converts it into DomainType.
///
/// The entire buffer will be consumed.
fn decode<B: Buf>(buf: B) -> Result<Self, Error>;
///
/// This function replaces the Prost::Message decode() function for DomainTypes.
fn decode<B: Buf>(buf: B) -> Result<Self, Error> {
T::decode(buf).map_or_else(
|e| Err(Kind::DecodeMessage.context(e).into()),
|t| Self::try_from(t).map_err(|e| Kind::TryIntoDomainType.context(e).into()),
)
}

/// Decodes a length-delimited instance of the message from the buffer.
///
/// The entire buffer will be consumed.
fn decode_length_delimited<B: Buf>(buf: B) -> Result<Self, Error>;
///
/// This function replaces the Prost::Message decode_length_delimited() function for
/// DomainTypes.
fn decode_length_delimited<B: Buf>(buf: B) -> Result<Self, Error> {
T::decode_length_delimited(buf).map_or_else(
|e| Err(Kind::DecodeMessage.context(e).into()),
|t| Self::try_from(t).map_err(|e| Kind::TryIntoDomainType.context(e).into()),
)
}

/// Returns the encoded length of the message without a length delimiter.
///
/// The DomainType will be consumed.
fn encoded_len(self) -> usize;
/// This function replaces the Prost::Message encoded_len() function for DomainTypes.
fn encoded_len(&self) -> usize {
T::from(self.clone()).encoded_len()
}

/// Encodes the DomainType into a protobuf-encoded Vec<u8>
fn encode_vec(&self) -> Result<Vec<u8>, Error> {
let mut wire = Vec::with_capacity(self.encoded_len());
self.encode(&mut wire).map(|_| wire)
}

/// Decodes a protobuf-encoded instance of the message from a Vec<u8> and then converts it into
/// DomainType.
fn decode_vec(v: &[u8]) -> Result<Self, Error> {
Self::decode(v)
}

/// Encodes the DomainType with a length-delimiter to a Vec<u8> protobuf-encoded message.
fn encode_length_delimited_vec(&self) -> Result<Vec<u8>, Error> {
let len = self.encoded_len();
let lenu64 = len.try_into().map_err(|e| Kind::EncodeMessage.context(e))?;
let mut wire = Vec::with_capacity(len + encoded_len_varint(lenu64));
self.encode_length_delimited(&mut wire).map(|_| wire)
}

/// Decodes a protobuf-encoded instance of the message with a length-delimiter from a Vec<u8>
/// and then converts it into DomainType.
fn decode_length_delimited_vec(v: &[u8]) -> Result<Self, Error> {
Self::decode_length_delimited(v)
}
}
6 changes: 0 additions & 6 deletions proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,6 @@ pub use domaintype::DomainType;
mod error;
pub use error::{Error, Kind};

// Re-export the bytes and prost crates for use within derived code.
#[doc(hidden)]
pub use bytes;
#[doc(hidden)]
pub use prost;

// Re-export the DomainType derive macro #[derive(DomainType)]
#[cfg(feature = "tendermint-proto-derive")]
#[doc(hidden)]
Expand Down
35 changes: 33 additions & 2 deletions proto/tests/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use tendermint_proto::types::PartSetHeader as RawPartSetHeader;
use tendermint_proto::DomainType;

// Example implementation of a protobuf struct using DomainType.
#[derive(DomainType, Clone)]
#[derive(DomainType, Clone, Debug)]
#[rawtype(RawBlockId)]
pub struct BlockId {
hash: String,
Expand Down Expand Up @@ -40,6 +40,13 @@ impl From<BlockId> for RawBlockId {
}
}

// Do any custom implementation for your type
impl PartialEq for BlockId {
fn eq(&self, other: &Self) -> bool {
self.part_set_header_exists == other.part_set_header_exists && self.hash == other.hash
}
}

#[test]
pub fn domaintype_struct_example() {
let my_domain_type = BlockId {
Expand All @@ -48,7 +55,7 @@ pub fn domaintype_struct_example() {
};

let mut wire = vec![];
my_domain_type.clone().encode(&mut wire).unwrap();
my_domain_type.encode(&mut wire).unwrap();
assert_eq!(
wire,
vec![10, 12, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]
Expand Down Expand Up @@ -77,3 +84,27 @@ pub fn domaintype_struct_length_delimited_example() {
assert_eq!(new_domain_type.hash, "Hello world!".to_string());
assert_eq!(new_domain_type.part_set_header_exists, false);
}

#[test]
pub fn domaintype_struct_conveniences_example() {
let my_domain_type = BlockId {
hash: "Hello world!".to_string(),
part_set_header_exists: false,
};

let wire = my_domain_type.encode_vec().unwrap();
assert_eq!(
wire,
vec![10, 12, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]
);
let new_domain_type = BlockId::decode_vec(&wire).unwrap();
assert_eq!(my_domain_type, new_domain_type);

let wire = my_domain_type.encode_length_delimited_vec().unwrap();
assert_eq!(
wire,
vec![14, 10, 12, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]
);
let new_domain_type = BlockId::decode_length_delimited_vec(&wire).unwrap();
assert_eq!(my_domain_type, new_domain_type);
}
4 changes: 2 additions & 2 deletions tendermint/src/amino_types/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ mod tests {
chain_id: "".to_string(),
};
let mut got = vec![];
let _have = msg.clone().encode(&mut got);
let _have = msg.encode(&mut got);

assert_eq!(got, want);

Expand Down Expand Up @@ -190,7 +190,7 @@ mod tests {
error: None,
};
let mut got = vec![];
let _have = msg.clone().encode(&mut got);
let _have = msg.encode(&mut got);

assert_eq!(got, encoded);

Expand Down
4 changes: 2 additions & 2 deletions tendermint/src/amino_types/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ mod tests {
],
};
let mut got = vec![];
let _have = vote.clone().encode(&mut got);
let _have = vote.encode(&mut got);
let v = Vote::decode(got.as_ref()).unwrap();

assert_eq!(v, vote);
Expand All @@ -587,7 +587,7 @@ mod tests {
chain_id: "test_chain_id".to_string(),
};
let mut got = vec![];
let _have = svr.clone().encode(&mut got);
let _have = svr.encode(&mut got);

let svr2 = SignVoteRequest::decode(got.as_ref()).unwrap();
assert_eq!(svr, svr2);
Expand Down