-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Re-add ERC-223 to token standards. #10854
Comments
Hello So as per our discussion in discord it have been considered an error and the arguments were provided/discussed already. Do I get it correctly? If yes then it can be reverted and there is no need to wait for something. This can be done in few minutes, isn't it? |
How long will we wait for this standard to reach its final state? What is the problem? |
The page should NOT be reinstated without a warning about the non-immutability of non-Final proposals, so a simple revert is insufficient. |
I don't think we should revert this change, and in fact I think we can just remove all the token content here and link out to eips.ethereum.org instead for users to get this content. |
You did also fail to add information here about your promotional push with having this get on ethereum.org @Dexaran. While draft was part of it, this push for promotional marketing such as https://t.co/37SFJjTYVe is a much larger factor here. |
Who makes the decision on this issue involving the whole community? |
The problem that ERC-223 solvesI discovered a problem with ERC20 token standard and reported it multiple times.
Ethereum Foundation didn't make any statement about this so far. I have created ERC-223 to solve this exact problem. While we are debating some motives behind removal of some content on ethereum.org web page - people are losing money. And I want to solve this issue. Because I think it is not good when people are losing money to a bug that can be fixed. How can we solve the problem?We need to make people stop using ERC-20 implementations that can cause monetary losses. For this reason I have developed an alternative standard (ERC-223) which is not prone to this problem. I also created EIP-7417 Token Converter that will help with the migration from ERC-20 ecosystem to ERC-223 ecosystem. And I'm working on a ERC-223 exchange because nobody will start using a token that can't be traded on an exchange. The end goal of all this activities is to solve the problem of people losing money on Ethereum due to a poor standardAnd obviously if we want to make people use a more secure token standard - we need to let them know that it exists at least. That's why I have proposed it to be added to ethereum.org web page. No other reasons. Promotion of token standardsPresence of one token standard on the ethereum.org page is a de-facto promotion. For example I requested the removal of ERC-20 from this page due to the security risks that this standard poses to the users (which resulted in a loss of $130M worth of tokens since I requested to remove it): ethereum/ethereum-org#755 If you have some standards on the page and you don't have others - then you are promoting those that are listed on the page. If you would remove the page with token standards completely - it would be similar to censoring all except the currently dominant one. I do agree that the ethereum.org page does not need a sophisticated description of the standards and EIPs will do quite well. So, I would propose to preserve a page where all existing standards will be listed and a short description like "On Ethereum we have token standards. Those describe the basic logic of the token. Here you can find a list of currently available standards:"
If you would just remove the standards page without giving any info on the standards to token developers then you would make it significantly more complicated to let them know of any alternative standards which is not a desirable effect if we are aiming to solve the problem of lost money. If you would say "They can just go read EIPs" - there are 7400 EIPs. They will not read it unless you explicitly highlight which ones are the most important to read - and I'm proposing to do this. |
Of course. BECAUSE I NEVER DID WHAT YOU SAID. Let me write it a bit more explicitly because I'm doing it for the third time and I hope this one is the last: I haven't used the fact of ERC-223 addition to the documentation at ethereum.org/docs as a confirmation of endorsement from Ethereum Foundation or Ethereum community. This is just a subjective speculation of @corwintines completely based on his personal feeling not backed by any real factsFacts
|
@corwintines feel free to find and provide any evidence of me saying that I see this as an endorsement or a sign of acceptance |
This will make it much more difficult for me to solve the problem of lost money. Is this what we want to achieve? |
ERC-1363 already exists, is peer-reviewed, and is Final (and therefore is immutable). ethereum.org should absolutely provide information about ERC-223, but it also should not not encourage users to use a non-Final, less peer-reviewed standard. Again, I repeat: I suggest that information about ERC-223 be provided. However, users and developers should also be made aware that only Final standards should be used in production, and ERC-223 is not yet Final. The change should not be reverted because the information contained in it is not sufficient for the aforementioned purposes. You should feel free to open a PR and make the needed changes. Feel free to ping me, I'll review it. |
Ethereum Foundation told me yesterday that they don't care if developers use "draft" EIPs in production or not: https://www.reddit.com/r/ethereum/comments/15jnuqq/comment/jvbmch0/?utm_source=reddit&utm_medium=web2x&context=3 Why do you keep insisting that EIPs that are in the
btw ERC-1363 does not solve the problem that ERC-223 solves. There are three types of transactions in tokens now:
If we want to solve the problem then we must remove unhandled transactions from the token standard, add push transactions and optionally remove pull txs. ERC-223 does this. ERC-1363 does not remove unhandled txs. Here is more info about tx types in existing digital assets:
What changes are needed? |
The EF is completely unrelated to the EIP project and has no power over the EIP project. The EIP editors' policy is that only Final EIPs should be used in production. I am sorry that the EF has misled you; I'll bring it up with them.
This is factually incorrect. ERC-20 doesn't introduce a security concern with unhandled transfers. It introduces a UX/UI concern/edge case. That doesn't mean it's not a concern and in no way diminishes the fact that it has caused many millions of dollars of loss, but it doesn't mean that the pattern is "insecure", it means that there are fewer safeguards. I suggest you revise this to:
I would even be fine with
But you're not going to spontaneously lose your funds if you follow all the requirements of ERC-20, unlike, say, ERC-777, which is insecure as written due to reentrancy from the pre-transfer hook. Note: ERC-223 is vulnerable to the same reentrancy attack, as it doesn't specify that the external call should be made post-transfer. You should DEFINITELY fix that.
Yes it does, it just does so at the wallet layer, prioritizing interoperability and greater flexibility, rather than the smart contract layer, prioritizing . I support the development of any standard, as long as it serves a useful purpose with unique tradeoffs. Your EIP does serve a useful purpose and has unique tradeoffs. However, as an EIP editor, as part of my support of your standard, I am required to raise concerns about:
So therefore...
Revert the commit you mentionned, then add a disclaimer at the top stating that the EIP is not Final, that it should therefore not be used in production until it reaches Final, and that it is not compatible with ERC-20 or any of its extensions. |
Would implement reentrancy protection mechanisms such as using the "checks-effects-interactions" pattern to prevent reentrancy attacks work around this issue? Here are a few more strategies:
|
Is it written anywhere? Can you give me a link to these rules?
You are wrong. The issue has nothing to do with UI/UX. Event handling is a very basic practice in programming. If two programs are intended to communicate (and in case of Ethereum tokens they MUST be intended) then they must send EVENTs to each other. Lack of this event handling model is the lack of critical feature. Similarly if you will not implement And it has nothing to do with UI/UX because:
This is how contracts work. And they work in a way that allows for unintentional loss of tokens.
Nevertheless something that can cause an unintended permanent freeze of users funds and already resulted in a loss of $130,000,000 worth of token is considered a "Critical Severity Vulnerability" by OpenZeppelin OpenZeppelin/openzeppelin-contracts#4474 And if you don't implement an
ERC-223 is as 'vulnerable' as plain ether because they work in exactly the same way. Also ERC-223 has a reference implementation where the code demonstrates how it MUST work. But you are correct - it makes sense to add some strictness to the specification. But it is not a vulnerability if you are not writing "Ether is vulnerable".
No offense but you obviously have too little expertise in security area if you think it might work. This is not a solution. It will simply not work this way. If you delegate solution of an existing problem to another layer - it is not a solution. If the problem exists at smart-contract layer - it must be fixed at the same layer. There is a good comment from a redditor that also summarizes it. I am a security expert.
So I guess I'm the only token standard author with a good background in security (and/or hacking) and that's why I see vulnerabilities where others don't and that's why I can distinguish a solution that will work from a solution that doesn't solve anything. I'm saying ERC-1363 attempts to solve a problem in a way that will not work.
I'm developing the only standard that makes tokens behave identical to ether. There can be only one standard that does this because we have only one ether logic.
Having immutable standards is a bad idea. For example the logic of ether transfers can change but for some reason the definition of the standard can not. But if it is the rule of the game - ok.
It is incorrect to say that "ERC-223 is incompatible with ERC-20".
For example if a wallet or any other UI worked with ERC-20 tokens - then it will work with ERC-223 out of the box with no modifications required. If some contract was working with ERC-20 tokens then in many cases it will work with ERC-223 in exactly the same way. Some contracts that operated with ERC-20 tokens via From UI point of view ERC-223 is much more backwards compatible with ERC-20 than ERC-777,ERC-1155,ERC-1363 because ERC-223 does not redefine the ABI unlike other standards. |
Any standard practice that works with ether reentrancy will work. ERC-223 works exactly the same as plain ether. If somebody is trying to argue that "ERC-223 is insecure because it allows for reentrancy" then he is basically saying that the native currency of Ethereum platform is insecure. |
In the ERC-223, particularly in the
In summary, the external calls to the recipient contract's |
Yup! It's just not behavior that's specified in ERC-223. It's an easy enough fix though.
...Yes, directly transferring ether is a known security issue! That's why it's recommended to implement a pull system. |
No, it's not written anywhere apparently. EIP-1 should be modified, because that is definitely the editor consensus.
Call it what you will. We're bikeshedding here.
Ah, you seem to be misunderstanding me. ERC-20 is definitely flawed and I would highly recommend that additional safeguards be implemented, whether it's ERC-223 or ERC-1363.
Yes, that is what I was getting at. When you say it's a critical security vulnerability of ERC-20, that's inaccurate, since it's not ERC-20 that has the vuln.
Yes. It would work as written. Emphasis on as written. If you devate from the standard, it might be less secure. This is well-known behavior and is in no way unique to ERC-1363. It, for example, also applies to ERC-223. If I'm so wrong, please explain exactly why it wouldn't work.
Other people can see the issue too. I'm not arguing that hundreds of millions of dollars of lost funds isn't a major issue worth fixing. It's just not a vulnerability. Definition of vulnerability: Susceptibility to injury or attack. The If you disagree with my definition of vulnerability, please provide your own.
I never said that. Please don't make up quotations. ERC-223 will not have all the extensions that ERC-20 does. That's the interop issue I'm concerned with. It's a minor one, for sure, but it exists.
ERC-777 is recommended not to be used exactly because it is insecure and redefines the ABI. ERC-1155 and ERC-20 solve different problems. There is inherently no reason it should even attempt to use the same ABI. ERC-1363 does not redefine ERC-20's ABI--it extends it! It is 100% backward compatible with ERC-20. ERC-223 does redefine ERC-20's ABI because it does not include Ergo - that statement is false in every possible way. All of this is irrelevant. For the most part, I don't care what you do with ERC-223. All I want is for you to include the disclaimer and not make false statements about other EIPs. |
It is not a known security issue because it is not a security issue at all. Recursion is not a security flaw. One function can call another function. One contract can call another contract. It is called "communication model" not "security issue". Again, EOS C++ token acts exactly as ERC-223 token and there were no EOS tokens stolen due to reentrancy attacks (because it was explicitly described in the documentation I guess). And fortunately on EOS $130M worth of tokens were not lost to lack of transaction handling.
It is definitely not recommended to implement a pull system in any trustless systems. Pull transactions are applicable in such systems as credit cards that are anything but trustless. Not in tokens.Here is a full article describing why Pull Transactions must be avoided: https://dexaran820.medium.com/erc-20-approve-transferfrom-asset-transfer-method-poses-a-threat-to-users-funds-safety-ff7195127018 No decentralized system other than Ethereum tokens implements pull transaction methods because it poses a significant security risk there. Pull system is only recommended by the guys who don't know why it was developed in the first place and have no clue "why it is there" so they keep mindlessly recommending it to each other.In reality there was a bug in EthereumVM at the time when ERC-20 was developed. Pull transacting method was introduced to make tokens not affected by this bug. It was a quirk that addressed the earlydays bug in the VM, not a smart design.
Here is a @vbuterin 's comment regarding the situation with token standards and call stack depth problem: In fact approvals are:
|
I've just said it fits in "critical vulnerability criteria" by OpenZeppelin bugbounty. Technically we can call it a software vulnerability however.
I wrote an article about it as well: https://gist.github.com/Dexaran/ddb3e89fe64bf2e06ed15fbd5679bd20 I would prefer to call it "A critical software architecture flaw" now as it describes it in a more human-friendly way.
It is written in a way that allows users to lose tokens in the same way as with ERC-20. So they will continue to lose their tokens.
ERC-1363 preserves unhandleable transactions so it is as "flawed" as ERC-20 and it will result in lost money. If something CAN go wrong then it WILL go wrong.
If something can go wrong - then it WILL go wrong. If a token standard is not secure by default then it will result in a loss of money. There is no way to preserve the insecure implementation of unhandleable Read this: https://docs.oracle.com/en/operating-systems/oracle-linux/6/security/ol_desprinsc_sec.html
Approves violate this security principle. A token standard that relies on approvals is not secure.
Read this: https://kirkpatrickprice.com/blog/secure-coding-best-practices/
ERC-20 is not secure by design. It is just insecure at all, it was developed in a wrong way in order to address an EVM bug that no longer exists. Now it must be deprecated. ERC-777 is not secure by design. ERC-1363 is not secure by design as it inherits all the ERC-20 problems (approvals & unhandleable transfer function)
Unhandleable |
I disagree on the definition of the vulnerability that you provided. The Wikipedia describes a separate class of interface faults under the software vulnerability category:
I think that your "It can be misused." is actually blaming the victim. Prompting a user to make a security decision (in our case a decision of what transacting method to use (1) For example ERC-223 does determine the method of transacting on its own (deposits to contracts and deposits to EOAs are performed in a different way) but this logic is fully abstracted from the user so that we are not prompting a user to make this decision. |
I agree. Can you recommend the best way to describe this on the page?
If you consider "inherits all the problems of the previous standard without fixing any of them" as the definition of "backwards compatibility" then yes - ERC-1363 is backwards compatible. Users will lose their tokens with ERC-1363 in exactly the same way as with ERC-20.
You didn't get what I was talking about. Example:Here is a wallet. For example this one https://dexaran.github.io/wallet/ The wallet can transfer tokens, the wallet can add new tokens as per users request. If we add a new ERC-223 token to this wallet (which is designed to work with ERC-20 tokens) then it will work just fine. You will be able to see the new ERC-223 token in your wallet, you will be able to transfer them exactly the same as ERC-20 tokens. No rework of the wallet required. On top of that - the newly added ERC-223 token will be safe for user by default i.e. it will be transferred via the If you will do the same with ERC-1363 token then it will be supported by the wallet, but the way wallets works with the ERC-1363 token is the same as it works with ERC-20 so ERC-1363 tokens will get lost in contracts. If you want the wallet to work with ERC-1363 in a secure way then the rework of the wallet code is required. Otherwise your tokens are still insecure. That's what I was talking about. |
I don't care that much about what you guys are doing with Ethereum either unless you start censoring the only viable solution to an existing problem that caused $168M loss (I recalculated it yesterday) and make your customers lose even more funds thus dealing the damage to the overall reputation of crypto industry, may be even worse than 3AC, FTX or LUNA downfall. Your bosses knew about the problem of lost money for 6 years, never attempted to solve it, allowed it to grow from $15K lost in 2017 to $168K lost today in top5% of contracts and now guys like @corwintines are censoring my attempts to solve the problem with no good reason other than his "unexplainable feeling that I'm doing it wrong". If you censor the solution, then this will lead to even more loss of money for users. For me this looks like a problem.@corwintines explicitly said that the main reason under removal of ERC-223 was "push for promotional marketing" which is just his fantasy that has nothing to do with real situation. I think that his inability to prove me wrong within 7 days quite confirms my statement. I see it as a direct censorship based on personal feelings of @corwintines
I don't want to include the disclaimer about ERC-223 being in "draft" because it will be finalized as soon as your Ethereum processes allow it. It is now moving to Also, what do you want me to add to the disclaimer in regards of ERC-20 and compatibility? Can we also add the same disclaimer to all other standards such as ERC-777 or ERC-1155? Can we add a disclaimer to ERC-20 "This standard resulted in a loss of $168M worth of tokens as of 14/08/2023" as its true and I think a security warning about an issue of such scale is required?
My statements about other EIPs are totally valid. |
@corwintines @Pandapip1 how comes it is possible to fuck up like this but not possible to undo it in 2 weeks?
|
User interface failures, such as: I'm pretty sure that you tried to tell me this was not a UI failure a while back:
What did I do wrong? I have stated over and over that I am fine with the page being reinstated--practically verbatim! Visibility for non-Final EIPs is essential to promote discussion! Again, here is what I require to have the page reinstated:
@Dexaran if you would like me to draft a PR, I would be willing to do so. |
I think that at the time this wiki page was written, there were no Ethereum tokens at all, so the wording may not be very precise, but the meaning is still true. However ERC-20 is not a user interface. There are wallets, exchange UIs or anything that users interact with - and these user interfaces interact with ERC-20 tokens. So in case of ERC-20 it's a failure of an interface that interacts with another interface.
Again, it doesn't matter that much if we call it a "critical vulnerability" or a "critical software architecture flaw" or a "feature of the standard" - the money is lost and the users suffered.
I'm asking if another banner can be placed on ERC-20? Don't you think it's illogical or even malicious to say that ERC-223 (the only properly designed solution to the problem of lost funds) must not be used in production because it's not in "Final" while ERC-20 can be used in production despite the fact it contains a known critical security flaw that resulted in a loss of at least $201,000,000 already? |
There are reasons for our policy regarding non-Final EIPs, and this is not the place to discuss them. If you feel this policy needs changing, please open an issue on the EIPs repo explaining why you feel this policy needs changing. If you feel that the ERC-20 issue needs to be added to the security considerations section, please open a PR modifying the security considerations section. If you choose to do this, you have my support, as I do believe it is entirely in the scope of that section. TL;DR: I have stated my one final requirement. If you feel that it's flawed, open an issue in the EIPs repo and we'll discuss it there. The issue with ERC-20 does need more visibility, and I have stated exactly where you should put it and how to put it there. |
ERC-223 is now Final. While there are a lot of applications that won't work with it at the moment, it is now in an immutable state and is therefore okay to use in smart contracts. Personally (i.e. not in my capacity as an EIP editor)--I slightly prefer ERC-1363 over ERC-223, since it has better compatibility with existing applications. But you should feel free to use either now, and I am okay with the ERC-223 page being reinstated without a warning. |
Transfers to contracts and transfers to EOAs are performed differently. ERC-20, ERC-1363 and other "backwards compatible with ERC-20" things place the burden of determining the transferring method on the user and if the wrong method is chosen the tokens are lost. It is critically insecure and it will always result in the lost money. Secure software design principles (fail-safe defaults, error handling, etc.) are not written just for fun. So I don't recommend using this standard. They are backwards compatible with ERC-20 but all are insecure.
Should I open a PR or can we just place it back in the same way as it was before being censored by @corwintines based on his "personal feelings about it"? |
I'm not sure what the exact content of the webpage is pre-revert. I also wouldn't assume censorship. However, unless you put something asinine in it (which I doubt), I think a revert is probably fine. |
This issue is stale because it has been open 45 days with no activity. |
Any updates? |
Will send a PR in few days. Definitely gonna work on it but not the highest priority on my tasklist right now. |
This issue is stale because it has been open 45 days with no activity. |
@corwintines @Pandapip1 PR sent #11787 |
This issue is stale because it has been open 30 days with no activity. |
Closing this as the above PR has been merged. |
Closed with #11787 |
Earlier ERC-223 was removed as it was in
draft
#10841It is not in
draft
anymore and I kindly request to place it back where it was ethereum/EIPs#7339@corwintines
The text was updated successfully, but these errors were encountered: