Skip to content

json: add Amino-compatible encoder/decoder#4955

Merged
mergify[bot] merged 34 commits intomasterfrom
erik/migrate-json-amino
Jun 8, 2020
Merged

json: add Amino-compatible encoder/decoder#4955
mergify[bot] merged 34 commits intomasterfrom
erik/migrate-json-amino

Conversation

@erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Jun 4, 2020

Amino-compatible JSON encoder/decoder, including bug compatibility. Interface types must be registered via json.RegisterType(). Unlike Amino, this allows floats to be encoded/decoded.

Partial fix for #4828, needs code migration.

@erikgrinaker erikgrinaker added the T:encoding Type: Amino, ProtoBuf label Jun 4, 2020
@auto-comment
Copy link

auto-comment bot commented Jun 4, 2020

👋 Thanks for creating a PR!

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Wrote tests
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer
  • Applied Appropriate Labels

Thank you for your contribution to Tendermint! 🚀

@erikgrinaker erikgrinaker self-assigned this Jun 4, 2020
@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #4955 into master will increase coverage by 0.20%.
The diff coverage is 72.34%.

@@            Coverage Diff             @@
##           master    #4955      +/-   ##
==========================================
+ Coverage   63.26%   63.46%   +0.20%     
==========================================
  Files         187      176      -11     
  Lines       19647    16509    -3138     
==========================================
- Hits        12429    10477    -1952     
+ Misses       6225     5150    -1075     
+ Partials      993      882     -111     
Impacted Files Coverage Δ
libs/json/encoder.go 62.01% <62.01%> (ø)
libs/json/decoder.go 73.97% <73.97%> (ø)
libs/json/types.go 78.26% <78.26%> (ø)
libs/json/structs.go 94.59% <94.59%> (ø)
crypto/sr25519/pubkey.go 40.74% <0.00%> (-7.41%) ⬇️
p2p/pex/pex_reactor.go 82.01% <0.00%> (-0.53%) ⬇️
consensus/reactor.go
rpc/jsonrpc/server/http_uri_handler.go
consensus/wal_generator.go
p2p/fuzz.go
... and 49 more

@tac0turtle tac0turtle requested a review from liamsi June 5, 2020 12:58
Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

utACK but would like to get some more eyes on it.

Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

I really like this approach 🙌 Left a few nits, mostly about the necessity to highlight a few edge-cases in a package documentation.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

great work @erikgrinaker 👍

Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

🎉 👍

@erikgrinaker erikgrinaker changed the title add Amino-compatible JSON encoder/decoder json: add Amino-compatible encoder/decoder Jun 8, 2020
@erikgrinaker erikgrinaker added the S:automerge Automatically merge PR when requirements pass label Jun 8, 2020
@mergify mergify bot merged commit ccc9904 into master Jun 8, 2020
@mergify mergify bot deleted the erik/migrate-json-amino branch June 8, 2020 11:42
mergify bot pushed a commit that referenced this pull request Jun 8, 2020
Migrates the `rpc` package to use new JSON encoder in #4955. Branched off of that PR.

Tests pass, but I haven't done any manual testing beyond that. This should be handled as part of broader 0.34 testing.
mergify bot pushed a commit that referenced this pull request Jun 8, 2020
Uses new JSON encoder in #4955 for all JSON. Branched off of #4968.
@tessr tessr mentioned this pull request Oct 29, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S:automerge Automatically merge PR when requirements pass T:encoding Type: Amino, ProtoBuf

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants