Merge a3540ae into 9854a66

.changelog/unreleased/breaking-changes/1323-encode-vec-no-error.md

@@ -0,0 +1,6 @@
+- `[tendermint-proto]` Align the return signature of the `encode_vec` and
+  `encode_length_delimited_vec` methods in the `Protobuf` trait with
+  `prost::Message` by directly returning `Vec<u8>`.
+  ([\#1323](https://github.com/informalsystems/tendermint-rs/issues/1323))
+  * Remove mandatory cloning in `Protobuf` methods and let callers decide on
+    clone beforehand for original value access

proto/src/lib.rs

@@ -22,14 +22,11 @@ mod error;
 #[allow(warnings)]
 mod tendermint;
 
-use core::{
-    convert::{TryFrom, TryInto},
-    fmt::Display,
-};
+use core::{convert::TryFrom, fmt::Display};
 
 use bytes::{Buf, BufMut};
 pub use error::Error;
-use prost::{encoding::encoded_len_varint, Message};
+use prost::Message;
 pub use tendermint::*;
 
 pub mod serializers;
@@ -124,10 +121,8 @@ where
     /// Protobuf data structure.
     ///
     /// [`prost::Message::encode`]: https://docs.rs/prost/*/prost/trait.Message.html#method.encode
-    fn encode<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> {
-        T::from(self.clone())
-            .encode(buf)
-            .map_err(Error::encode_message)
+    fn encode<B: BufMut>(self, buf: &mut B) -> Result<(), Error> {
+        T::from(self).encode(buf).map_err(Error::encode_message)
     }
 
     /// Encode with a length-delimiter to a buffer in Protobuf format.
@@ -138,8 +133,8 @@ where
     /// its counterpart Protobuf data structure.
     ///
     /// [`prost::Message::encode_length_delimited`]: https://docs.rs/prost/*/prost/trait.Message.html#method.encode_length_delimited
-    fn encode_length_delimited<B: BufMut>(&self, buf: &mut B) -> Result<(), Error> {
-        T::from(self.clone())
+    fn encode_length_delimited<B: BufMut>(self, buf: &mut B) -> Result<(), Error> {
+        T::from(self)
             .encode_length_delimited(buf)
             .map_err(Error::encode_message)
     }
@@ -179,14 +174,13 @@ where
     /// counterpart Protobuf data structure.
     ///
     /// [`prost::Message::encoded_len`]: https://docs.rs/prost/*/prost/trait.Message.html#method.encoded_len
-    fn encoded_len(&self) -> usize {
-        T::from(self.clone()).encoded_len()
+    fn encoded_len(self) -> usize {
+        T::from(self).encoded_len()
     }
 
     /// Encodes 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)
+    fn encode_vec(self) -> Vec<u8> {
+        T::from(self).encode_to_vec()
     }
 
     /// Constructor that attempts to decode a Protobuf-encoded instance from a
@@ -196,11 +190,8 @@ where
     }
 
     /// Encode 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(Error::parse_length)?;
-        let mut wire = Vec::with_capacity(len + encoded_len_varint(lenu64));
-        self.encode_length_delimited(&mut wire).map(|_| wire)
+    fn encode_length_delimited_vec(self) -> Vec<u8> {
+        T::from(self).encode_length_delimited_to_vec()
     }
 
     /// Constructor that attempts to decode a Protobuf-encoded instance with a

proto/tests/unit.rs

@@ -56,7 +56,7 @@ pub fn protobuf_struct_example() {
     };
 
     let mut wire = vec![];
-    my_domain_type.encode(&mut wire).unwrap();
+    my_domain_type.clone().encode(&mut wire).unwrap();
     assert_eq!(
         wire,
         vec![10, 12, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]
@@ -93,15 +93,15 @@ pub fn protobuf_struct_conveniences_example() {
         part_set_header_exists: false,
     };
 
-    let wire = my_domain_type.encode_vec().unwrap();
+    let wire = my_domain_type.clone().encode_vec();
     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();
+    let wire = my_domain_type.clone().encode_length_delimited_vec();
     assert_eq!(
         wire,
         vec![14, 10, 12, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]

tendermint/src/block/header.rs

@@ -84,26 +84,20 @@ impl Header {
         // https://github.com/tendermint/tendermint/blob/134fe2896275bb926b49743c1e25493f6b24cc31/types/encoding_helper.go#L9:6
 
         let fields_bytes = vec![
-            Protobuf::<RawConsensusVersion>::encode_vec(&self.version).unwrap(),
-            self.chain_id.encode_vec().unwrap(),
-            self.height.encode_vec().unwrap(),
-            self.time.encode_vec().unwrap(),
-            Protobuf::<RawBlockId>::encode_vec(&self.last_block_id.unwrap_or_default()).unwrap(),
-            self.last_commit_hash
-                .unwrap_or_default()
-                .encode_vec()
-                .unwrap(),
-            self.data_hash.unwrap_or_default().encode_vec().unwrap(),
-            self.validators_hash.encode_vec().unwrap(),
-            self.next_validators_hash.encode_vec().unwrap(),
-            self.consensus_hash.encode_vec().unwrap(),
-            self.app_hash.encode_vec().unwrap(),
-            self.last_results_hash
-                .unwrap_or_default()
-                .encode_vec()
-                .unwrap(),
-            self.evidence_hash.unwrap_or_default().encode_vec().unwrap(),
-            self.proposer_address.encode_vec().unwrap(),
+            Protobuf::<RawConsensusVersion>::encode_vec(self.version),
+            self.chain_id.clone().encode_vec(),
+            self.height.encode_vec(),
+            self.time.encode_vec(),
+            Protobuf::<RawBlockId>::encode_vec(self.last_block_id.unwrap_or_default()),
+            self.last_commit_hash.unwrap_or_default().encode_vec(),
+            self.data_hash.unwrap_or_default().encode_vec(),
+            self.validators_hash.encode_vec(),
+            self.next_validators_hash.encode_vec(),
+            self.consensus_hash.encode_vec(),
+            self.app_hash.clone().encode_vec(),
+            self.last_results_hash.unwrap_or_default().encode_vec(),
+            self.evidence_hash.unwrap_or_default().encode_vec(),
+            self.proposer_address.encode_vec(),
         ];
 
         Hash::Sha256(merkle::simple_hash_from_byte_vectors::<H>(&fields_bytes))

tendermint/src/proposal.rs

@@ -94,14 +94,14 @@ impl Proposal {
         B: BufMut,
     {
         let canonical = CanonicalProposal::new(self.clone(), chain_id);
-        Protobuf::<RawCanonicalProposal>::encode_length_delimited(&canonical, sign_bytes)?;
+        Protobuf::<RawCanonicalProposal>::encode_length_delimited(canonical, sign_bytes)?;
         Ok(true)
     }
 
     /// Create signable vector from Proposal.
-    pub fn to_signable_vec(&self, chain_id: ChainId) -> Result<Vec<u8>, ProtobufError> {
+    pub fn to_signable_vec(&self, chain_id: ChainId) -> Vec<u8> {
         let canonical = CanonicalProposal::new(self.clone(), chain_id);
-        Protobuf::<RawCanonicalProposal>::encode_length_delimited_vec(&canonical)
+        Protobuf::<RawCanonicalProposal>::encode_length_delimited_vec(canonical)
     }
 
     /// Consensus state from this proposal - This doesn't seem to be used anywhere.

tendermint/src/proposal/sign_proposal.rs

@@ -24,7 +24,7 @@ impl SignProposalRequest {
     }
 
     /// Create signable vector from Proposal.
-    pub fn to_signable_vec(&self) -> Result<Vec<u8>, ProtobufError> {
+    pub fn to_signable_vec(&self) -> Vec<u8> {
         self.proposal.to_signable_vec(self.chain_id.clone())
     }
 }

tendermint/src/public_key.rs

@@ -512,7 +512,7 @@ mod tests {
                 ),
                 error: None,
             };
-            let got = Protobuf::<RawPubKeyResponse>::encode_vec(&msg).unwrap();
+            let got = Protobuf::<RawPubKeyResponse>::encode_vec(msg.clone());
 
             assert_eq!(got, encoded);
             let decoded = <PubKeyResponse as Protobuf<RawPubKeyResponse>>::decode_vec(

tendermint/src/public_key/pub_key_request.rs

@@ -62,7 +62,7 @@ mod tests {
                 chain_id: ChainId::from_str("A").unwrap(),
             };
             let mut got = vec![];
-            Protobuf::<RawPubKeyRequest>::encode(&msg, &mut got).unwrap();
+            Protobuf::<RawPubKeyRequest>::encode(msg.clone(), &mut got).unwrap();
 
             assert_eq!(got, want);
 

tendermint/src/validator.rs

@@ -194,7 +194,7 @@ impl Info {
     /// Returns the bytes to be hashed into the Merkle tree -
     /// the leaves of the tree.
     pub fn hash_bytes(&self) -> Vec<u8> {
-        Protobuf::<RawSimpleValidator>::encode_vec(&SimpleValidator::from(self)).unwrap()
+        Protobuf::<RawSimpleValidator>::encode_vec(SimpleValidator::from(self))
     }
 }
 

tendermint/src/vote.rs

@@ -260,14 +260,14 @@ impl Vote {
         B: BufMut,
     {
         let canonical = CanonicalVote::new(self.clone(), chain_id);
-        Protobuf::<RawCanonicalVote>::encode_length_delimited(&canonical, sign_bytes)?;
+        Protobuf::<RawCanonicalVote>::encode_length_delimited(canonical, sign_bytes)?;
         Ok(true)
     }
 
     /// Create signable vector from Vote.
-    pub fn to_signable_vec(&self, chain_id: ChainId) -> Result<Vec<u8>, ProtobufError> {
+    pub fn to_signable_vec(&self, chain_id: ChainId) -> Vec<u8> {
         let canonical = CanonicalVote::new(self.clone(), chain_id);
-        Protobuf::<RawCanonicalVote>::encode_length_delimited_vec(&canonical)
+        Protobuf::<RawCanonicalVote>::encode_length_delimited_vec(canonical)
     }
 
     /// Consensus state from this vote - This doesn't seem to be used anywhere.
@@ -326,7 +326,7 @@ impl SignedVote {
 
     /// Return the bytes (of the canonicalized vote) that were signed.
     pub fn sign_bytes(&self) -> Vec<u8> {
-        Protobuf::<RawCanonicalVote>::encode_length_delimited_vec(&self.vote).unwrap()
+        Protobuf::<RawCanonicalVote>::encode_length_delimited_vec(self.vote.clone())
     }
 
     /// Return the actual signature on the canonicalized vote.

tendermint/src/vote/sign_vote.rs

@@ -23,7 +23,7 @@ impl SignVoteRequest {
     }
 
     /// Create signable vector from Vote.
-    pub fn to_signable_vec(&self) -> Result<Vec<u8>, ProtobufError> {
+    pub fn to_signable_vec(&self) -> Vec<u8> {
         self.vote.to_signable_vec(self.chain_id.clone())
     }
 }
@@ -159,7 +159,7 @@ mod tests {
         // Option 1 using bytes:
         let _have = request.to_signable_bytes(&mut got);
         // Option 2 using Vec<u8>:
-        let got2 = request.to_signable_vec().unwrap();
+        let got2 = request.to_signable_vec();
 
         // the following vector is generated via:
         // import (
@@ -241,7 +241,7 @@ mod tests {
             chain_id: ChainId::from_str("test_chain_id").unwrap(),
         };
 
-        let got = request.to_signable_vec().unwrap();
+        let got = request.to_signable_vec();
 
         // the following vector is generated via:
         // import (
@@ -311,7 +311,7 @@ mod tests {
             let cv = CanonicalVote::new(dummy_vote(), ChainId::try_from("A").unwrap());
             let mut got = vec![];
             // SignBytes are encoded using MarshalBinary and not MarshalBinaryBare
-            Protobuf::<RawCanonicalVote>::encode_length_delimited(&cv, &mut got).unwrap();
+            Protobuf::<RawCanonicalVote>::encode_length_delimited(cv, &mut got).unwrap();
             let want = vec![
                 0x10, 0x8, 0x1, 0x11, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2a, 0x0, 0x32, 0x1,
                 0x41,
@@ -328,7 +328,7 @@ mod tests {
                 };
                 println!("{vt_precommit:?}");
                 let cv_precommit = CanonicalVote::new(vt_precommit, ChainId::try_from("A").unwrap());
-                let got = Protobuf::<RawCanonicalVote>::encode_vec(&cv_precommit).unwrap();
+                let got = Protobuf::<RawCanonicalVote>::encode_vec(cv_precommit);
                 let want = vec![
                     0x8,  // (field_number << 3) | wire_type
                     0x2,  // PrecommitType
@@ -355,7 +355,7 @@ mod tests {
 
                 let cv_prevote = CanonicalVote::new(vt_prevote, ChainId::try_from("A").unwrap());
 
-                let got = Protobuf::<RawCanonicalVote>::encode_vec(&cv_prevote).unwrap();
+                let got = Protobuf::<RawCanonicalVote>::encode_vec(cv_prevote);
 
                 let want = vec![
                     0x8,  // (field_number << 3) | wire_type
@@ -462,7 +462,7 @@ mod tests {
                 extension: vec![],
                 extension_signature: None,
             };
-            let got = Protobuf::<pb::types::Vote>::encode_vec(&vote).unwrap();
+            let got = Protobuf::<pb::types::Vote>::encode_vec(vote.clone());
             let v = <Vote as Protobuf::<pb::types::Vote>>::decode_vec(&got).unwrap();
 
             assert_eq!(v, vote);
@@ -473,7 +473,7 @@ mod tests {
                     chain_id: ChainId::from_str("test_chain_id").unwrap(),
                 };
                 let mut got = vec![];
-                let _have = Protobuf::<pb::privval::SignVoteRequest>::encode(&svr, &mut got);
+                let _have = Protobuf::<pb::privval::SignVoteRequest>::encode(svr.clone(), &mut got);
 
                 let svr2 = <SignVoteRequest as Protobuf<pb::privval::SignVoteRequest>>::decode(
                     got.as_ref()