-
Notifications
You must be signed in to change notification settings - Fork 227
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
Fix serializer and deserializer not symmetric #98
Conversation
let commit = TmpCommit::deserialize(deserializer)?; | ||
if commit.block_id.is_none() || commit.precommits.is_none() { | ||
Ok(None) | ||
if let Some(commit) = <Option<TmpCommit>>::deserialize(deserializer)? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks! I want to double check if the tendermint rpc will return an empty commit (likely), or, if this can be treated as invalid input instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An empty commit happens in the last_commit
field in the first block.
Thanks for more fixes. Do you think you could add some tests to help illustrate the issue better? Would probably be good to get a bunch of json data for blocks so we can tests all these encoding issues for first block, blocks with/without other hashes. Maybe in |
I've added the unit tests which reveal the issue, and rebased the PR against current master. |
Some optional types encode to null but don't parse null. - Block.last_commit - CommitSig.validator_address
1524: Problem (Fix #1523): sync state deserialization fails when nil vote happens r=tomtau a=yihuang Solution: - Make parse_non_empty_id parse null in tendermint-rs Related upstream PR: informalsystems/tendermint-rs#98 Co-authored-by: yihuang <huang@crypto.com>
* Symmetric serialization tests * CHANGES.md update * Fix serializer and deserializer not symmetric - reimpl from yihuang PR #98
These deserializers intents to parse some empty data as
None
, because golang version of tendermint would encode these empty values as some default values.But since the serializers would encode
None
asnull
, which makes them not symmetric.So the solution is make these deserializers also support decoding
null
asNone
.