Fix JSON deserialization of abci::ResponseInfo #1131
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 the app_version field in some recent revisions of Gaia.
mzabaluev
commented
May 24, 2022
Comment on lines
+91
to
+100
| (".tendermint.abci.ResponseInfo.data", DEFAULT), | ||
| (".tendermint.abci.ResponseInfo.version", DEFAULT), | ||
| ( | ||
| ".tendermint.abci.ResponseInfo.app_version", | ||
| QUOTED_WITH_DEFAULT, | ||
| ), | ||
| ( | ||
| ".tendermint.abci.ResponseInfo.last_block_height", | ||
| QUOTED_WITH_DEFAULT, | ||
| ), |
There was a problem hiding this comment.
Not sure if it's best to fix it here with compiler tweaks, or maybe move the serde annotations to the AbciInfo struct, where the serde traits are really used, while currently being delegated to tendermint_proto::abci::ResponseInfo.
We have to customize serde for every field of the involved struct in either case, so doing it directly in source would be cleaner.
6 tasks
greg-szabo
previously approved these changes
May 24, 2022
The only field of abci::ResponseInfo where it remained missing, to match the json:"omitempty" flag in Go.
Codecov Report
@@ Coverage Diff @@
## master #1131 +/- ##
======================================
Coverage 64.7% 64.7%
======================================
Files 231 231
Lines 15637 15637
======================================
+ Hits 10121 10124 +3
+ Misses 5516 5513 -3
Continue to review full report at Codecov.
|
thanethomson
approved these changes
Jun 9, 2022
5 tasks
mzabaluev
added a commit
that referenced
this pull request
Jul 21, 2022
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.
5 tasks
thanethomson
added a commit
that referenced
this pull request
Jul 21, 2022
…1156) * Fix JSON deserialization of abci::ResponseInfo 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. * Fix a clippy lint * Correct fmt * Use writeln! instead of write! Co-authored-by: Thane Thomson <connect@thanethomson.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #1132
The fields of the tendermint-go struct equivalent to
abci::ResponseInfoare annotated withjson:"omitempty", so give them default values forDeserialize. The/abci_infoendpoint is known to omit theapp_versionfield in some recent revisions of Gaia..changelog/