ethereum / ethereum-org-website

Ethereum.org is a primary online resource for the Ethereum community.
https://ethereum.org/
MIT License
5.04k stars 4.78k forks source link

Highlight the severity of the ERC-20 known issue in the token standard description. #13218

Open Dexaran opened 3 months ago

Dexaran commented 3 months ago

Is your feature request related to a problem? Please describe.

With 8.7.0 a "Known issues" section was added to ERC-20 description https://ethereum.org/en/developers/docs/standards/tokens/erc-20/#erc20-issues

However, the representation looks like it is a known issue so a token developer has nothing to worry about. It mustn't look like this because the amount of damage that this issue is dealing to the ecosystem is still growing over time and if there will be no active countermeasures people will keep losing money because of this known issue.

I would propose to add two paragraphs here: ERC-20_losses_on_ethereum-org

Describe the solution you'd like

  1. The severity of the issue must be highlighted so that devs wouldn't think that they don't need to worry about it.
  2. There are few ways to mitigate a significant part of potential damage that this issue can deal so this options must be proposed.

1. Highlight the severity of the issue

I would recommend to add a phrase "As of 06/20/2024 at least $83,656,418 worth of ERC-20 tokens were lost due to this issue."

We have built a script that calculates the amount of lost tokens (https://dexaran.github.io/erc20-losses/) and there are over $230M worth of lost tokens on Ethereum mainnet currently and this amount is growing every day as users keep losing tokens and those tokens that were already lost are never recovered.

My team did a due diligence on the tokens that were reported as "lost" by this script and we were able to manually verify that at least $83M worth of tokens are clearly unrecoverably lost. All the data is transparently available via Etherscan where you can observe the tokens on the balances of contracts with verified source codes that have no functions to extract these tokens.

2. Propose solution options

  1. The most common case is when a user is sending tokens to the address of that exact tokens smart-contract. Implementing a simple check within the logic of the transfer function would prevent this require(address _to != address(this), "Sending to token contract");
  2. When deploying smart-contracts on Ethereum always assume that your contract can receive ERC-20 tokens even if it is not supposed to. It is a good practice to implement an extraction function that would allow tokens to be extracted.
  3. Consider using alternative standards.

Describe alternatives you've considered

It is possible to restrict the logic of transfer function so that it wouldn't be possible to deliver tokens to smart-contracts with it but this would break backwards compatibility with some multisig wallets that are supposed to be ERC-20 compatible as well as Uniswap pools.

Additional context

No response

Would you like to work on this issue?

corwintines commented 2 months ago

Thanks @Dexaran

I think overall this could probably make sense to add. Just want to get a bit more feedback.

@Pandapip1 @minimalsm @konopkja @wackerow do you have any opinions on this?

wackerow commented 2 months ago

I think this is good... not sure if there would be any value in having some of this in the design-and-ux page (ie front-end design patterns to pre-empt inappropriate transfers), but here with ERC-20 would probably make the most sense. Open to thoughts from others.

Overall though I'm in favor of shining light on these challenges though.

Pandapip1 commented 2 months ago

@Pandapip1 @minimalsm @konopkja @wackerow do you have any opinions on this?

I approve of this. Once my concerns with the automated script are addressed (I do believe Dexaran's working on them), I approve of that too.

gorbatiukcom commented 2 months ago

Hello everyone, I have prepared a PR with the addition of the description described above. @corwintines @Pandapip1 @wackerow @konopkja @minimalsm please take a look at it — https://github.com/ethereum/ethereum-org-website/pull/13532

Dexaran commented 2 months ago

Hello everyone, I have prepared a PR with the addition of the description described above. @corwintines @Pandapip1 @wackerow @konopkja @minimalsm please take a look at it — #13532

These are the changes that I recommended.

A note regarding automated calculation script: it's quite difficult to verify the losses without disassembling the contract source codes and we still didn't came to an automated method. We will send a separate PR once it's ready.

github-actions[bot] commented 4 weeks ago

This issue is stale because it has been open 30 days with no activity.