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 contract addresses that don't implement ERC20 interface for addToken method #33

Open
0xGabi opened this issue Jun 8, 2019 · 2 comments
Labels
enhancement New feature or request redemptions-app smartcontracts smartcontracts

Comments

@0xGabi
Copy link
Member

0xGabi commented Jun 8, 2019

When we add an address with the addToken method that don't implement the ERC20 interface the Redemptions app will broke until this address is removed from the token list.

Two possible solutions are:

  1. We could handle this in the frontend to prevent the app to stop working.
  2. We could improve the smart contract check to prevent this kind of address to be added.
@0xGabi 0xGabi added enhancement New feature or request smartcontracts smartcontracts redemptions-app labels Jun 8, 2019
@fabriziovigevani fabriziovigevani removed this from the 1 milestone Jul 18, 2019
@fabriziovigevani
Copy link
Member

From keybase:

@lkngtn :

I don’t think it’s a huge deal given the security model around adding and removing tokens. One other possible solution could be to allow users to exclude tokens when redeeming, so the redeem function cannot be blocked by an invalid token.

@yeqbfgxjiq
Copy link
Contributor

While this might not be a security issue as the org can vote to add/remove tokens at will, it could be a UX issue. It should be as easy as possible for people to use our apps, esp if we're targeting open source communities who are already unfamiliar or skeptical of crypto. With that in mind, seems like encoding a check in the contract would be best since it would eliminate any potential user errors in setting up or using the app. Is there any reason not to add it as a feature to the contract other than gas fees?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request redemptions-app smartcontracts smartcontracts
Projects
None yet
Development

No branches or pull requests

4 participants