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.

Sign up for GitHub

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

Jump to bottom

Remove HybridCodec #6837

Closed
15 of 16 tasks
aaronc opened this issue Jul 24, 2020 · 3 comments · Fixed by #6859
Closed
15 of 16 tasks

Remove HybridCodec #6837

aaronc opened this issue Jul 24, 2020 · 3 comments · Fixed by #6859
Assignees
@aaronc
Labels
C:Encoding
Milestone
v0.40 [Stargate]

Comments

@aaronc
Copy link
Member

aaronc commented Jul 24, 2020 •
edited by sahith-narahari
Loading

Motivation

HybridCodec provides hybrid protobuf binary marshaling and amino JSON marshaling in a single type. It seemed like a good idea to me when we first did the state migration because it allows queriers to still use amino JSON. But it's just messy in other ways because the CLI and genesis should use the same encoding as everything else (either proto or amino). And it just feels wrong.

So I propose we remove HybridCodec and just always pass an amino JSON marshaler into the legacy querier and REST endpoints and let everything else that is non-legacy (genesis and CLI) just use either ProtoCodec or AminoCodec.

Needed for #5917.

Details
@aaronc aaronc added this to the v0.40 [Stargate] milestone Jul 24, 2020
@alexanderbez
Copy link
Contributor

ACK

@zmanian
Copy link
Member

zmanian commented Jul 24, 2020

Ack

@aaronc aaronc mentioned this issue Jul 24, 2020
Merged
9 tasks
@anilcse anilcse mentioned this issue Jul 24, 2020
Merged
9 tasks
@aaronc aaronc mentioned this issue Jul 27, 2020
Merged
9 tasks
@clevinson
Copy link
Contributor

clevinson commented Jul 29, 2020 •
edited
Loading

We should also make sure to remove references to HybridCodec from ADR019, and make sure the changes here are reflected in the ADR.

@aaronc aaronc mentioned this issue Jul 30, 2020
Merged
9 tasks
@alpe alpe mentioned this issue Aug 12, 2020
Closed
6 tasks
@mergify mergify bot closed this as completed in #6859 Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aaronc aaronc

Labels
C:Encoding
Projects
None yet
Milestone
v0.40 [Stargate]
Development

Successfully merging a pull request may close this issue.

Enable proto JSON generally and remove HybridCodec cosmos/cosmos-sdk
4 participants
@aaronc @zmanian @clevinson @alexanderbez