Signing bytes need to encoded with encode_length_delimited#69
Conversation
tendermint/src/vote.rs
Outdated
| fn sign_bytes(&self) -> Vec<u8> { | ||
| AminoMessage::bytes_vec(&self.vote.to_owned()) | ||
| let mut buf = Vec::with_capacity(self.vote.encoded_len() + 4); | ||
| self.vote.encode_length_delimited(&mut buf).unwrap(); |
There was a problem hiding this comment.
Thanks! You're right, this has to be length_delimited!
tendermint/src/vote.rs
Outdated
|
|
||
| fn sign_bytes(&self) -> Vec<u8> { | ||
| AminoMessage::bytes_vec(&self.vote.to_owned()) | ||
| let mut buf = Vec::with_capacity(self.vote.encoded_len() + 4); |
There was a problem hiding this comment.
4 bytes should be correct but it would still be cleaner to take the length and its encoded length. Prost provides a method for that. As far as I remember it was called encoded_len_varint. Let me double check.
tendermint/src/vote.rs
Outdated
|
|
||
| fn sign_bytes(&self) -> Vec<u8> { | ||
| AminoMessage::bytes_vec(&self.vote.to_owned()) | ||
| let mut buf = Vec::with_capacity(self.vote.encoded_len() + 4); |
There was a problem hiding this comment.
This should work (given the right imports):
| let mut buf = Vec::with_capacity(self.vote.encoded_len() + 4); | |
| let vote_len = self.vote.encoded_len(); | |
| let len_delimiter_len = | |
| prost_amino::encoding::encoded_len_varint(vote_len.try_into().unwrap()); | |
| let mut buf = Vec::with_capacity(self.vote.encoded_len() + len_delimiter_len); |
I think it would be better to add a method to the AminoMessage trait. Sth. like length_delimited_bytes_vec. But OK to merge with above changes 👍 (you would still need to fix the imports).
41431be to
bc4ff7c
Compare
| Self: Sized, | ||
| { | ||
| let len = self.encoded_len(); | ||
| let mut res = Vec::with_capacity(len + encoded_len_varint(len as u64)); |
There was a problem hiding this comment.
Awesome, thanks!
Why did you decide against len.try_into().unwrap() (as suggested in #69 (comment)) instead of len as u64? I think, the latter can silently overflow while the first would panic.
There was a problem hiding this comment.
Nice trick, re-commited.
bc4ff7c to
6ca48b5
Compare
So we have identical signing bytes as golang version to verify signatures.