-
Notifications
You must be signed in to change notification settings - Fork 225
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 #241 evidence JSON format #242
Conversation
Integration tests fail because of #233. Unrelated to this PR and can be ignored. |
pub enum Evidence { | ||
/// Duplicate vote evidence | ||
#[serde(rename = "tendermint/DuplicateVoteEvidence")] | ||
DuplicateVote(DuplicateVoteEvidence), |
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.
👍
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.
Thanks @yihuang 👍
/// Duplicate vote evidence | ||
#[derive(Serialize, Deserialize, Clone, Debug)] | ||
pub struct DuplicateVoteEvidence { | ||
#[serde(rename = "PubKey")] |
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.
I'm surprised to see that the JSON produces CamelCase instead of snake_case for these fields.
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.
Opened an issue to track it tendermint/tendermint#4958 !
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.
Thanks! I didn’t know that this would count as a bug. Thought it’s just a minor inconsistency.
#[derive(Serialize, Deserialize, Clone, Debug)] | ||
pub struct DuplicateVoteEvidence { | ||
#[serde(rename = "PubKey")] | ||
pub_key: PublicKey, |
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.
I just noted that in the go versions there is no pubkey field (anymore?). We need to remove this for compatibility. Tracking here: #300
Removed
new
andto_amino_bytes
methods.