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

Rename IBC Height's "Version" to a unique name for IBC #7709

Closed
4 tasks
colin-axner opened this issue Oct 28, 2020 · 5 comments · Fixed by #8020
Closed
4 tasks

Rename IBC Height's "Version" to a unique name for IBC #7709

colin-axner opened this issue Oct 28, 2020 · 5 comments · Fixed by #8020
Assignees
Labels
T: Dev UX UX for SDK developers (i.e. how to call our code)

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Oct 28, 2020

Summary

There continues to be confusion around the IBC height which originally used "epoch" and now uses "version". Version collides with the IAVL tree usage of version. We propose renaming the name to something that can be more uniquely identifier with IBC to avoid naming collision and confusion

@cwgoes proposed possibly using:

  • edition
  • revision

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@colin-axner colin-axner added x/ibc T: Dev UX UX for SDK developers (i.e. how to call our code) labels Oct 28, 2020
@cwgoes
Copy link
Contributor

cwgoes commented Nov 17, 2020

Let's choose "revision" here.

@ValarDragon
Copy link
Contributor

ValarDragon commented Nov 17, 2020

I'm still in favor of prefixing LiteClient to whatever name is chosen, so that the variable / concept name is self-contained.

@colin-axner
Copy link
Contributor Author

I'm still in favor of prefixing LiteClient to whatever name is chosen, so that the variable / concept name is self-contained.

I don't think that would help in this case since a revision number is actually referencing the revision of a chain not a light client. ie cosmos-hub-4 is the 4th revision.

From the implementation side, I think using Revision will be pretty well self-contained. It will be embedded in the height struct which is contained in x/ibc/core/02-client/types/height.go and only used in IBC. Furthermore the Height type is only used within the ClientState struct for light clients and as the key when referencing the ConsensusState of a light client

@colin-axner colin-axner self-assigned this Nov 18, 2020
@colin-axner
Copy link
Contributor Author

This should be tackled after we merge the upgrade consensus state change to avoid merge conflicts

@AdityaSripal
Copy link
Member

Decided upon Revision

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Dev UX UX for SDK developers (i.e. how to call our code)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants