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

update proto definitions for ics20-v2 feature #6170

Closed
charleenfei opened this issue Apr 16, 2024 · 2 comments
Closed

update proto definitions for ics20-v2 feature #6170

charleenfei opened this issue Apr 16, 2024 · 2 comments
Assignees

Comments

@charleenfei
Copy link
Contributor

charleenfei commented Apr 16, 2024

I suggested moving the proto definition to proto pkg v3 to see what it would look like seeing as it kind of makes sense to follow some proto rules. But maybe adding FunginbleTokenPacketDataV2 or some other option is less noisy given that we are already on proto pkg v2.
When proto pkg v2 was added for packet data with amount as string we removed the v1 proto definition (ref), but this scenario is different given we want to maintain the existing packet data in parallel. There is a few different options here as @srdtrk has pointed out. I think we should avoid ics20-3 at all costs - this would add too much confusion 😅
I think another option is probably making the new packet data the proto pkg v2 FungibleTokenPacketData and renaming the existing one to have the V1 suffix or "Legacy" prefix or something. That might be a cleaner option completely, rather than introducing proto pkg v3 - I think this should be possible but will need to look into it a bit more, any thoughts on this?

Originally posted by @damiannolan in #6116 (comment)_

@charleenfei charleenfei added this to the ICS20 v2 (multi denom) milestone Apr 16, 2024
@crodriguezvega crodriguezvega added 20-transfer needs discussion Issues that need discussion before they can be worked on labels May 13, 2024
@chatton chatton removed the needs discussion Issues that need discussion before they can be worked on label May 20, 2024
@chatton chatton self-assigned this May 20, 2024
@chatton
Copy link
Contributor

chatton commented May 20, 2024

removed needs discussion, we decided to go with the original approach of a FungibleTokenPacketDataV2 in the proto v2 package.

@chatton
Copy link
Contributor

chatton commented May 21, 2024

closed by #6330

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants