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

Handle TokenContractAddress in Peggy and deal with duplicate symbols across token addresses #69

Closed
denalimarsh opened this issue Sep 25, 2019 · 4 comments
Labels

Comments

@denalimarsh
Copy link

This issue was created due a conversation over in swishlabsco/peggy: swishlabsco#4 (comment)

TokenContractAddress may also be needed in the Denom in order to guarantee coin uniqueness (see comment).

@musnit
Copy link

musnit commented Sep 25, 2019

The gist of the conversation is that Cosmos (and so Peggy) don't seem to support 2 different coins which have the same denom. This means that anyone can create an arbitrary ERC20 with the same symbol as an existing one, use Peggy to transfer it over, and so debase a real ERC20 with a fake one.

Is there any way to have two different coins with the same denom in Cosmos at the moment?

Some potential fixes:

  • Whitelist hardcoded ERC20 token addresses on the bridge and only support those
  • Whitelist the first received ERC20 token address with any given symbol and block future ones with a different address
  • Have a validator-voting based whitelist
  • Include the token address in the denom, so we have symbol_addressA and symbol_addressB as different denoms
  • Add support to Cosmos SDK for different tokens with the same denom

Can we get thoughts from someone on the team for this too?

@musnit musnit changed the title Generalize Peggy API by removing sdk.Coins Amount in favor of integer Amount, string Symbol Handle TokenContractAddress in Peggy and deal with duplicate symbols across token addresses Sep 25, 2019
@okwme
Copy link
Contributor

okwme commented Feb 27, 2020

I'm more a fan of the address prefix. It's my understanding this is similar to how IBC will work. There will be a prefix of the chain_id or some uid representing the chain with some deliminator. It could be ":" or "-", just whatever it is shouldn't be possible to include in the prefix. Client libs would just need to be careful to parse the names of the coin in order to show human readable names instead of long prefixes.

@okwme
Copy link
Contributor

okwme commented Feb 27, 2020

As mentioned in issue #147 keeping track of asset origin would also allow a more united interface.

@denalimarsh
Copy link
Author

Closing in favor of #123, #147

jkilpatr added a commit that referenced this issue Oct 16, 2020
zmanian pushed a commit that referenced this issue Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants