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

General Amino audit / cleanup / IAVL changes #6194

Closed
2 of 3 tasks
aaronc opened this issue May 11, 2020 · 7 comments
Closed
2 of 3 tasks

General Amino audit / cleanup / IAVL changes #6194

aaronc opened this issue May 11, 2020 · 7 comments
Assignees
Labels
C:Encoding Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Comments

@aaronc
Copy link
Member

aaronc commented May 11, 2020

Summary

In the process of migrating to protobuf, several pieces of the code outside of the core tx/query/state logic are also relying on amino in addition to

This issue is to track a task of auditing and migrating all the remaining bits:

In general:

  • all data types should be proto-marshalable
  • all code that does marshaling should use codec.Marshaler instead of amino directly (IAVL may need to implement its own marshaler interface)
@aaronc aaronc changed the title General Amino Audit / cleanup (IAVL) General Amino audit / cleanup / IAVL changes May 11, 2020
@tac0turtle
Copy link
Member

for iavl: cosmos/iavl#242

@clevinson clevinson added this to the v0.39 milestone May 29, 2020
@tac0turtle
Copy link
Member

is the plan to remove amino from IAVL before 0.39 release of the sdk?

cc @aaronc @clevinson

@aaronc
Copy link
Member Author

aaronc commented Jun 11, 2020

That would be desirable

@alexanderbez
Copy link
Contributor

alexanderbez commented Jun 12, 2020

From @erikgrinaker:

So about IAVL and Amino: it would actually be trivial to strip out Amino (the library)
but retain  the same encoding. The only thing it actually uses is varints
(which are included in the Go stdlib), and varint-prefixed byte slices.

I’m not convinced that moving to Protobuf schemas has any significant benefit over
this, especially since the Protobuf encoding is basically the same (varints everywhere).

I agree in that IAVL can be simplified and just use binary varint encoding. No need for anything special here.

@fedekunze fedekunze added Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. C:Encoding labels Jun 13, 2020
@fedekunze fedekunze mentioned this issue Jun 13, 2020
8 tasks
@erikgrinaker
Copy link
Contributor

Submitted an initial IAVL PR here: cosmos/iavl#265

Need to figure out what to do with encoding of proofs.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@aaronc
Copy link
Member Author

aaronc commented Oct 23, 2020

This is done right? @blushi? Closing.

@aaronc aaronc closed this as completed Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:Encoding Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

No branches or pull requests

7 participants