Skip to content
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 JSON deserialization of abci::ResponseInfo (backport to v0.23.x) #1156

Merged

Conversation

mzabaluev
Copy link
Contributor

Backport of #1131 to v0.23.x

Should fix informalsystems/hermes#2444

The fields are annotated with json:"omitempty" in the Go source, so give them default values for Deserialize. The /abci_info endpoint is known to omit some of the fields depending on what the node software supplies.

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Added entry in .changelog/

Backport of #1131 to v0.23.x

The fields are annotated with json:"omitempty" in the Go source,
so give them default values for Deserialize. The /abci_info endpoint
is known to omit some of the fields depending on what the node
software supplies.
@mzabaluev mzabaluev added rpc backport:v0.23 To be backported to v0.23.x labels Jul 21, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2022

Codecov Report

Merging #1156 (c5e9927) into v0.23.x (ea6fd53) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           v0.23.x   #1156     +/-   ##
=========================================
- Coverage     67.4%   67.4%   -0.1%     
=========================================
  Files          212     212             
  Lines        20280   20280             
=========================================
- Hits         13680   13677      -3     
- Misses        6600    6603      +3     
Impacted Files Coverage Δ
testgen/src/apalache.rs 2.6% <0.0%> (ø)
light-client-verifier/src/types.rs 37.9% <0.0%> (-0.9%) ⬇️
testgen/src/header.rs 84.4% <0.0%> (-0.6%) ⬇️
abci/src/server.rs 9.4% <0.0%> (+0.1%) ⬆️

@mzabaluev
Copy link
Contributor Author

There's been a clippy whack-a-mole in CI, unrelated to the changes in this PR.

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Copy link
Member

@thanethomson thanethomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mzabaluev!

@thanethomson thanethomson merged commit e2f7854 into v0.23.x Jul 21, 2022
@thanethomson thanethomson deleted the mikhail/v0.23.x/allow-omitted-fields-in-abci_info branch July 21, 2022 21:32
@mzabaluev mzabaluev mentioned this pull request Jul 27, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:v0.23 To be backported to v0.23.x rpc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants