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

Remove all custom JSON marshaling #7010

Closed
aaronc opened this issue Aug 11, 2020 · 5 comments
Closed

Remove all custom JSON marshaling #7010

aaronc opened this issue Aug 11, 2020 · 5 comments
Labels
T:tech debt Tech debt that should be cleaned up

Comments

@aaronc
Copy link
Member

aaronc commented Aug 11, 2020

This just causes problems with proto JSON. All of the vesting account custom JSON was removed in #6859. We should probably do an audit and remove any other custom JSON marshaling we can find (and probably YAML too).

This may unfortunately cause some breakage for uses of the legacy REST endpoints, but I don't really see a way around it and it is part of the path towards stable formats.

What do you think @alexanderbez ?

@alexanderbez
Copy link
Contributor

What exact problems does it cause? I don't think we have much custom JSON serialization, so that should be fine. I also don't think we have any custom YAML, but if we do, that's OK to remove IMHO.

@aaronc
Copy link
Member Author

aaronc commented Aug 12, 2020

It appears to be causing some of the sim export issues in #6859. Basically jsonpb does some custom stuff because of it but only on one side of marshal/unmarshal. So exported genesis json can't be imported.

@tac0turtle
Copy link
Member

I believe the yaml will need to be removed in the near future anyways. The gogoproto extension to add it will not be supported in the new generator

@tac0turtle tac0turtle added the T:tech debt Tech debt that should be cleaned up label Oct 21, 2022
@tac0turtle
Copy link
Member

we need to look at removing all calls in the proto files to (gogoproto.jsontag) . With the amino work there may not be a need for these anymore

@tac0turtle
Copy link
Member

closing in favour of #8952

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:tech debt Tech debt that should be cleaned up
Projects
None yet
Development

No branches or pull requests

3 participants