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

Change packet key to support efficient iteration in v2 #7132

Closed
3 tasks
colin-axner opened this issue Aug 12, 2024 · 4 comments
Closed
3 tasks

Change packet key to support efficient iteration in v2 #7132

colin-axner opened this issue Aug 12, 2024 · 4 comments
Assignees
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

Change packet keys to use big endian encodings on the sequence to support efficient iteration

Problem Definition

Packet keys which have sequences which are lexographically ordered are not efficient to iterate and cause issues later on which are hard to workaround. See #5413 for example. This will also be problematic if we tried to create a pruning solution as we cannot easily iterate. We would need to do individual deletions or queries to the store.

Proposal

The "Eureka" version of IBC introduces a V2 to the packet commitment structure. Given that we are changing how we are committing to packets, we might as well change how we are storing these packet commitments, so that we can future proof the protocol and make future features, like pruning, much easier to implement

I propose changing the packet key structure to be:

clients/<client-ID/sequences/<big-endian-encoded-sequence>

Currently we use:
ports/<port-ID>/channels/<channel-id>/sequences/<sequence>

I think the port ID should be moved to the commitment value (this would be in-line with multi-packetdata) and I think we should be more explicit in the future approach that we are using clientIDs not channelIDs, but I haven't given these additional points much thought, so I am happy to exclude them. The big endian encoding of the sequence should definitely be done


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega
Copy link
Contributor

Note: if we decide to use the proposed new path key we should also document this in the specs.

@DimitrisJim
Copy link
Contributor

#5752 umbrella issue on some level

@AdityaSripal
Copy link
Member

This has been changed in the spec: cosmos/ibc#1144

@DimitrisJim
Copy link
Contributor

sequence in keys formatted using Big Endian in #7378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 🥳
Status: Done
Development

No branches or pull requests

5 participants