walleth / walleth

free (libre) native Android Ethereum wallet
https://walleth.org
GNU General Public License v3.0
610 stars 177 forks source link

Evaluate on-chain token-registry #78

Open ligi opened 7 years ago

ligi commented 7 years ago

Currently WALLETH is using https://github.com/ligi/ethereum-lists to resolve tokens

@5chdn suggested after the WALLETH intro-talk at DevCon3 to use:

https://etherscan.io/address/0x5f0281910af44bfb5fc7e86a404d0304b0e042f1#code

I love the idea of decentralized on-chain resolution - but have to evaluate the dangers of using this contract. Seems everyone can register tokens without any governance - I think this is a bit dangerous - will think about it a bit more after DevCon3 - but opened this issue to not forget and be open to other input.

5chdn commented 7 years ago

So far we had not experienced any issues with the token registry regarding security, but I'm happy to discuss any questions you may have.

The only thing that happened is that someone created ERC-20 tokens with fake balances, mimicking an air-drop without actually distributing the tokens. But that's easy to filter by checking random address balances.

ligi commented 7 years ago

The attack-vector I see:

1 someone registers a token that is not yet registered

2 sells someone these worthless tokens pretending they are tokens with value

3 scam-profit (undesired)

4 unhappy/angry users, crying and other things I do not want

cc @tayvano , @409H

409H commented 7 years ago

This is the first time I've come across this contract - maybe because I'm not deploying my own tokens - so maybe you guys know, is there a defacto-standard of registering your token with this contract?

The attack vectors that @ligi highlighted above is something I can't argue or counter with anything tangible to mitigate against.

The contract also allows for governance from address 0x3ff047e5e803e20f5ef55ea1029adb89618047db to unregister the token, which could help, but damage could have also been done before unregister() is called, right? (Unless I've read this contract wrong, I'm still getting into Solidity)

What's the end-goal? To have a verified trusted token registry list on-chain?

ligi commented 7 years ago

Yes - I was also hearing about this contract for the first time - not yet sure about this to be honest - like the idea of having it decentralized on chain - but think this implementation is a bit to naive to use in production - will have to think about this deeper after DevCon3 - mainly added this issue to not forget about it (easy with all the input here) - and gather some other opinions on this ..-)

tayvano commented 7 years ago

I believe this is being used in parity and recommended for tokens (e.g. there is a tutorial on how to deploy your own token and adding it here)

I like it conceptually and think it's a great way to introduce people to smart contracts and tokens, but we will remain semi-centralized on this front as this is an example of how a bit of curation / moderation is a good thing and that decentralization for decentralization sake is not always good for the end user.

For example, requiring support emails and or considering only including tokens with a certain level of distribution (ethplorer.io has good data on this) could be worthwhile and a more useful list than the one provided by this contract.

These are all things we are thinking about as we rework the UX and flow for v4 surrounding tokens. It may be effective to query this contract is no results are returned for a users search in our list.

5chdn commented 7 years ago

The contract also allows for governance from address 0x3ff047e5e803e20f5ef55ea1029adb89618047db to unregister the token, which could help, but damage could have also been done before unregister() is called, right? (Unless I've read this contract wrong, I'm still getting into Solidity)

Yes, we can remove malformed or malicious entries. It's been in production for more than a year now and I'm not aware of any security-relevant incidents. The only notable things that happened so far are:

I'm not aware of anyone selling fake-tokens, this is not that easy because they would have to convince users not to use exchanges. Not saying it's not possible, but also not very likely. In general I recommend - if using this contract is desired - enabling token-whitelisting in the wallet/client. This means, the users can control which token balances they want to see, and which not.

Related Parity Docs https://github.com/paritytech/parity/wiki/token-registry

Next-Generation Registry https://github.com/paritytech/contracts/issues/83 by @rstormsf

PhABC commented 7 years ago

We implemented our own token registry for tokens (ERC20 only for now) that we currently maintain ourselves, but this is clearly not a sustainable approach for the long term. It's unclear how many tokens there will be in the future, but more and more tools allowing easy token creation (for good or bad) will come and it will be practically impossible to manage centrally. It make sense to have a more filtered list for platforms like MEW and Etherscan (don't want to track balances of all existing tokens), but it does not make sense for a protocol like 0x.

We are currently working on a suite of smart contracts that will allow community maintained token-registry. Our TokenRegistry V2 allows anyone to submit proposals, but the approval is restricted to the TokenRegistry owners. Version 2 or 3 will replace the owners by an incentivized community voting scheme that should ensure each proposal is verified correctly (no profit is or will be made by contract creators).