Skip to content

Commit

Permalink
imp(Protobuf): remove mandatory cloning in default method impls
Browse files Browse the repository at this point in the history
  • Loading branch information
Farhad-Shabani committed Jul 12, 2023
1 parent 67a28a3 commit c8d5fc2
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 33 deletions.
22 changes: 10 additions & 12 deletions proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,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.
Expand All @@ -135,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)
}
Expand Down Expand Up @@ -176,13 +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) -> Vec<u8> {
T::from(self.clone()).encode_to_vec()
fn encode_vec(self) -> Vec<u8> {
T::from(self).encode_to_vec()
}

/// Constructor that attempts to decode a Protobuf-encoded instance from a
Expand All @@ -192,8 +190,8 @@ where
}

/// Encode with a length-delimiter to a `Vec<u8>` Protobuf-encoded message.
fn encode_length_delimited_vec(&self) -> Vec<u8> {
T::from(self.clone()).encode_length_delimited_to_vec()
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
Expand Down
11 changes: 7 additions & 4 deletions proto/tests/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -75,7 +75,10 @@ pub fn protobuf_struct_length_delimited_example() {
};

let mut wire = vec![];
my_domain_type.encode_length_delimited(&mut wire).unwrap();
my_domain_type
.clone()

Check failure on line 79 in proto/tests/unit.rs

View workflow job for this annotation

GitHub Actions / clippy-results

redundant clone

error: redundant clone --> proto/tests/unit.rs:79:9 | 79 | .clone() | ^^^^^^^^ help: remove this | note: this value is dropped without further use --> proto/tests/unit.rs:78:5 | 78 | / my_domain_type 79 | | .clone() | |________^ = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone = note: `-D clippy::redundant-clone` implied by `-D warnings`
.encode_length_delimited(&mut wire)
.unwrap();
assert_eq!(
wire,
vec![14, 10, 12, 72, 101, 108, 108, 111, 32, 119, 111, 114, 108, 100, 33]
Expand All @@ -93,15 +96,15 @@ pub fn protobuf_struct_conveniences_example() {
part_set_header_exists: false,
};

let wire = my_domain_type.encode_vec();
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();
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]
Expand Down
8 changes: 4 additions & 4 deletions tendermint/src/block/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,17 @@ 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),
self.chain_id.encode_vec(),
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()),
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.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(),
Expand Down
4 changes: 2 additions & 2 deletions tendermint/src/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> 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.
Expand Down
2 changes: 1 addition & 1 deletion tendermint/src/public_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ mod tests {
),
error: None,
};
let got = Protobuf::<RawPubKeyResponse>::encode_vec(&msg);
let got = Protobuf::<RawPubKeyResponse>::encode_vec(msg.clone());

assert_eq!(got, encoded);
let decoded = <PubKeyResponse as Protobuf<RawPubKeyResponse>>::decode_vec(
Expand Down
2 changes: 1 addition & 1 deletion tendermint/src/public_key/pub_key_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion tendermint/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Protobuf::<RawSimpleValidator>::encode_vec(SimpleValidator::from(self))
}
}

Expand Down
6 changes: 3 additions & 3 deletions tendermint/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> 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.
Expand Down Expand Up @@ -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)
Protobuf::<RawCanonicalVote>::encode_length_delimited_vec(self.vote.clone())
}

/// Return the actual signature on the canonicalized vote.
Expand Down
10 changes: 5 additions & 5 deletions tendermint/src/vote/sign_vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
let got = Protobuf::<RawCanonicalVote>::encode_vec(cv_precommit);
let want = vec![
0x8, // (field_number << 3) | wire_type
0x2, // PrecommitType
Expand All @@ -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);
let got = Protobuf::<RawCanonicalVote>::encode_vec(cv_prevote);

let want = vec![
0x8, // (field_number << 3) | wire_type
Expand Down Expand Up @@ -462,7 +462,7 @@ mod tests {
extension: vec![],
extension_signature: None,
};
let got = Protobuf::<pb::types::Vote>::encode_vec(&vote);
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);
Expand All @@ -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()
Expand Down

0 comments on commit c8d5fc2

Please sign in to comment.