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

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

Open
1 of 2 tasks
Dexaran opened this issue Jun 20, 2024 · 6 comments · May be fixed by #13532
Open
1 of 2 tasks

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

Dexaran opened this issue Jun 20, 2024 · 6 comments · May be fixed by #13532
Assignees
Labels
awaiting PR Issue is ready for a pull request feature ✨ This is enhancing something existing or creating something new proposal 🤔 This is a proposal request for comments 🗣️ A request for comments has been made; discussion and input is encouraged Status: Stale This issue is stale because it has been open 30 days with no activity.

Comments

@Dexaran
Copy link
Contributor

Dexaran commented Jun 20, 2024

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?

  • Yes
  • No
@Dexaran Dexaran added the feature ✨ This is enhancing something existing or creating something new label Jun 20, 2024
@github-actions github-actions bot added the needs triage 📥 This issue needs triaged before being worked on label Jun 20, 2024
@wackerow wackerow added proposal 🤔 This is a proposal request for comments 🗣️ A request for comments has been made; discussion and input is encouraged and removed needs triage 📥 This issue needs triaged before being worked on labels Jun 27, 2024
@corwintines
Copy link
Member

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
Copy link
Member

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.

@wackerow wackerow added the awaiting PR Issue is ready for a pull request label Jul 11, 2024
@Pandapip1
Copy link
Member

@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
Copy link

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

@Dexaran
Copy link
Contributor Author

Dexaran commented Jul 30, 2024

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.

Copy link
Contributor

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

@github-actions github-actions bot added the Status: Stale This issue is stale because it has been open 30 days with no activity. label Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting PR Issue is ready for a pull request feature ✨ This is enhancing something existing or creating something new proposal 🤔 This is a proposal request for comments 🗣️ A request for comments has been made; discussion and input is encouraged Status: Stale This issue is stale because it has been open 30 days with no activity.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants