search

ethereum / ethereum-org-website

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

Re-add ERC-223 to token standards. #10854

Closed Dexaran closed 5 months ago

Dexaran commented 1 year ago

Earlier ERC-223 was removed as it was in draft https://github.com/ethereum/ethereum-org-website/pull/10841

It is not in draft anymore and I kindly request to place it back where it was https://github.com/ethereum/EIPs/pull/7339

@corwintines

Dexaran commented 1 year ago

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?

@corwintines @wackerow @pettinarip @joshjwelsh

theRomanMercury commented 1 year ago

How long will we wait for this standard to reach its final state? What is the problem?

Pandapip1 commented 1 year ago

The page should NOT be reinstated without a warning about the non-immutability of non-Final proposals, so a simple revert is insufficient.

corwintines commented 1 year ago

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.

corwintines commented 1 year ago

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.

theRomanMercury commented 1 year ago

Who makes the decision on this issue involving the whole community?

Dexaran commented 1 year ago

The problem that ERC-223 solves

I 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 standard

And 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 standards

Presence 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): https://github.com/ethereum/ethereum-org/issues/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.

Dexaran commented 1 year ago

You did also fail to add information here about your promotional push with having this get on ethereum.org @Dexaran.

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 facts

Facts

Dexaran commented 1 year ago

@corwintines feel free to find and provide any evidence of me saying that I see this as an endorsement or a sign of acceptance

Dexaran commented 1 year ago

I don't think we should revert this change

This will make it much more difficult for me to solve the problem of lost money. Is this what we want to achieve?

Pandapip1 commented 1 year ago

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.

Dexaran commented 1 year ago

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.

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 final are somehow better than those that are not in final? Who said it? Is this your personal point of view or are there any arguments behind it? At first I thought it was Ethereum Foundation policy, but they told me they don't care.

ERC-1363 already exists, is peer-reviewed, and is Final (and therefore is immutable).

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:

Transaction type ERC-223 Ether ERC-20 ERC-721 (NFT) ERC-777 ERC-1155 ERC-1363 EOS C++ token
Push tx + + - + + + + +
Pull tx (risky) - - + + + + + -
Unhandled (insecure) - - + - - + + -

You should feel free to open a PR and make the needed changes.

What changes are needed?

Pandapip1 commented 1 year ago

Why do you keep insisting that EIPs that are in the final are somehow better than those that are not in final? Who said it? Is this your personal point of view or are there any arguments behind it? At first I thought it was Ethereum Foundation policy, but they told me they don't care.

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.

unhandled transactions (straight up insecure)

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.

btw ERC-1363 does not solve the problem that ERC-223 solves.

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...

What changes are needed?

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.

theRomanMercury commented 1 year ago

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.

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:

  1. Mutex Locks: Implementing mutex locks using a boolean flag to restrict the reentrant execution of a specific function. The flag prevents the function from being called again until the first execution completes.

  2. Guard Modifiers: Use guard modifiers to prevent functions from being called again if they're already in progress. Similar to mutex locks, these modifiers set a flag before executing the function and reset it afterward.

  3. Withdraw Pattern: Implement a pattern where the user initiates a withdrawal, and the contract holds the funds for a specific period before transferring them. This gives users time to cancel the withdrawal if needed.

  4. State Reversal: Reverse state changes in case an external call fails. This ensures that the contract's state remains unchanged if an interaction with another contract fails.

  5. Gas Limits: Implement a gas limit on external calls to restrict their execution. This limits the impact of reentrancy attacks by preventing excessive gas consumption during the reentrant call.

  6. Use of Send Method: In Ethereum, using the send method to transfer Ether provides a limited amount of gas and prevents further execution if the gas runs out, helping to mitigate reentrancy attacks.

  7. Delegatecall Limitation: If your contract uses delegate calls to interact with other contracts, ensure that the called contract cannot call back into your contract using delegate calls.

Dexaran commented 1 year ago

The EIP editors' policy is that only Final EIPs should be used in production.

Is it written anywhere? Can you give me a link to these rules?

This is factually incorrect. ERC-20 doesn't introduce a security concern with unhandled transfers. It introduces a UX/UI concern/edge case.

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 onlyOwner modifier on a function that is supposed to be invoked only by the owner of the contract - it will be a vulnerability. This is a major program architecture flaw that resulted in a loss of $130,000,000.

And it has nothing to do with UI/UX because:

  1. If you send ether via geth console (without any UI) to a contract that doesn't implement a receive() function then your ether will not be delivered.

  2. If you send ERC-20 via geth console (which is also possible) to a contrac thtat doesn't explicitly declare that it is supposed to receive tokens - then the tokens will still be delivered and get permanently frozen in the contract.

This is how contracts work. And they work in a way that allows for unintentional loss of tokens.

But you're not going to spontaneously lose your funds if you follow all the requirements of ERC-20

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 https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4474

And if you don't implement an onlyOwner restriction on a governance function then it will be a critical severity security vulnerability.

ERC-223 is vulnerable to the same reentrancy attack, as it doesn't specify that the external call should be made post-transfer.

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".

Yes it does, it just does so at the wallet layer, prioritizing interoperability and greater flexibility, rather than the smart contract layer, prioritizing.

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.

Standards proliferation

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.

Interoperability;

See https://github.com/ethereum/EIPs/pull/7418

The use and promotion of non-immutable standards

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 not compatible with ERC-20 or any of its extensions.

It is incorrect to say that "ERC-223 is incompatible with ERC-20".

Backward compatibility (sometimes known as backwards compatibility) is a property of an operating system, software, real-world product, or technology that allows for interoperability with an older legacy system, or with input designed for such a system, especially in telecommunications and computing. [Wiki: Backwards compatibility](https://en.wikipedia.org/wiki/Backward_compatibility#:~:text=Backward%20compatibility%20(sometimes%20known%20as,especially%20in%20telecommunications%20and%20computing.)

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 approve + transferFrom however will need to redefine the process of depositing tokens. So, compatibility is a big paragraph of text not just a statement "ERC-223 is not compatible with ERC-20". Also I think it is a good idea to link EIP-7417 there as it is focused on interoperability of this standards and addresses compatibility concerns.

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.

Dexaran commented 1 year ago

Would implement reentrancy protection mechanisms such as using the "checks-effects-interactions" pattern to prevent reentrancy attacks work around this issue?

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.

theRomanMercury commented 1 year ago

In the ERC-223, particularly in the ERC223Token contract, there are external calls made to another contract (IERC223Recipient) after the token transfer in the transfer functions. These external calls are designed to execute the tokenReceived function in the recipient contract. Here's a breakdown of the flow:

  1. transfer Function:

    • The transfer functions are responsible for transferring tokens from the sender's balance to the recipient's balance.
    • After transferring the tokens, if the recipient address is a contract (checked using Address.isContract(_to)), an external call is made to the recipient contract's tokenReceived function.
    • The external call includes the sender's address, transferred value, and optional transaction metadata (_data).

  2. IERC223Recipient Contract:

    • The tokenReceived function in the IERC223Recipient contract is designed to handle incoming token transfers.
    • It receives information about the sender's address, the transferred value, and any additional transaction metadata.
    • The function allows the recipient contract to perform actions based on the received tokens and metadata.

  3. tkn Structure:

    • Inside the IERC223Recipient contract, a structure named tkn is used to store information about the token transfer.
    • This structure is then populated with data like the token contract address, sender's address, transferred value, and transaction metadata.
    • The purpose of this structure seems to be to provide the recipient contract with information about the token transfer.

In summary, the external calls to the recipient contract's tokenReceived function are made after the token transfer to allow the recipient contract to react to the received tokens and metadata, if needed. The recipient contract can use this information to perform specific actions or logic based on the received tokens.

Pandapip1 commented 1 year ago

Would implement reentrancy protection mechanisms such as using the "checks-effects-interactions" pattern to prevent reentrancy attacks work around this issue?

Yup! It's just not behavior that's specified in ERC-223. It's an easy enough fix though.

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.

...Yes, directly transferring ether is a known security issue! That's why it's recommended to implement a pull system.

Pandapip1 commented 1 year ago

Is it written anywhere? Can you give me a link to these rules?

No, it's not written anywhere apparently. EIP-1 should be modified, because that is definitely the editor consensus.

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.

Call it what you will. We're bikeshedding here.

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.

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.

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 https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4474

And if you don't implement an onlyOwner restriction on a governance function then it will be a critical severity security vulnerability.

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.

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.

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.

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.

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 transfer function is NOT succeptible to attack. It can be misused.

If you disagree with my definition of vulnerability, please provide your own.

It is incorrect to say that "ERC-223 is incompatible with ERC-20".

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.

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.

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 approve or transferFrom functions.

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.

Dexaran commented 1 year ago

@Pandapip1

...Yes, directly transferring ether is a known security issue!

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.

That's why it's recommended to implement a pull system.

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: 67496b44-afc0-4acc-bc1b-bafb9be43e5c

In fact approvals are:

  1. A pair of transactions. You can't deposit tokens in one TX via pull method. This bloats blockchain with unnecessary data. This burns more gas than actually needed.
  2. Needed for nothing. They are a deprecated quirk that addresses a bug that no longer exists
  3. Designed for credit cards, not tokens. Credit cards are anything but trustless
  4. Not used in new systems built from scratch (such as EOS). They remain a vestige of early solidity contracts.
  5. Approves introduce a new way of insecurely operating with tokens: unlimited approvals. UIs often promot users to call approval only once and issue one huge approval that will cover all token deposits for a user. This is a cruel violation of security principles. Here is Design Principles for Secure Coding. The first rule says "A process should be given only those privileges that are necessary to complete a task." The smart-contract must NEVER have unlimited access to your tokens. It is definitely insecure. Like.. we never seen a contract hack?
  6. We witnessed multiple approval-related hacks and monetary losses. This is no less of a problem than lack of transaction handling.
Dexaran commented 1 year ago

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 https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4474

And if you don't implement an onlyOwner restriction on a governance function then it will be a critical severity security vulnerability.

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.

I've just said it fits in "critical vulnerability criteria" by OpenZeppelin bugbounty.

Technically we can call it a software vulnerability however.

User interface failures, such as: Blaming the Victim prompting a user to make a security decision without giving the user enough information to answer it Source:https://en.wikipedia.org/wiki/Vulnerability_(computing)#Software_vulnerabilities

Interface_failure

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.

Yes. It would work as written. Emphasis on as written.

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. I already said: if we want to fix the problem then we need:

  1. Remove unhandled transactions from the standard.

  2. Possibly remove approvals or at least remove the explicit need to use them.

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 I'm so wrong, please explain exactly why it wouldn't work.

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 transfer function as in ERC-20 and solve the problem of lost money at the same time. That's why ERC-223 overrides the logic of transfer.

Read this: https://docs.oracle.com/en/operating-systems/oracle-linux/6/security/ol_desprinsc_sec.html

Least privilege A process or user should be given only those privileges that are necessary to complete a task.

Approves violate this security principle. A token standard that relies on approvals is not secure.

Fail-safe defaults The default action should be to deny access to an operation.

transfer function is not secure by default.

Read this: https://kirkpatrickprice.com/blog/secure-coding-best-practices/

Security by Design Security needs to be a priority as you develop code, not an afterthought. Organizations may have competing priorities where software engineering and coding are concerned. Following software security best practices can conflict with optimizing for development speed. However, a “security by design” approach that puts security first tends to pay off in the long run.

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)

Error Handling and Logging

Unhandleable transfer function (in any standard, be it ERC-20 or ERC-1363) does not allow for error handling.

Dexaran commented 1 year ago

@Pandapip1

Definition of vulnerability: Susceptibility to injury or attack. The transfer function is NOT succeptible to attack. It can be misused.

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:

User interface failures, such as: Blaming the Victim prompting a user to make a security decision without giving the user enough information to answer it Source:https://en.wikipedia.org/wiki/Vulnerability_(computing)#Software_vulnerabilities

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) transfer or (2) approve & transferFrom) is unnecessary and the contract must determine which method to use on its own.

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.

Dexaran commented 1 year ago

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.

I agree. Can you recommend the best way to describe this on the page?

ERC-1363 does not redefine ERC-20's ABI--it extends it! It is 100% backward compatible with ERC-20.

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.

ERC-223 does redefine ERC-20's ABI because it does not include approve or transferFrom functions. Ergo - that statement is false in every possible way.

You didn't get what I was talking about.

Example:

Here is a wallet. For example this one https://dexaran.github.io/wallet/ This wallet is a UI that works with tokens.

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 transfer function and it will not be prone to ERC-20 problems. Again, secure by default, no modifications of the wallet required.

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.

Dexaran commented 1 year ago

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.

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

include the disclaimer

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 last call already https://github.com/ethereum/EIPs/pull/7483

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?

not make false statements about other EIPs

My statements about other EIPs are totally valid.

sgitt-vassky commented 1 year ago

@corwintines @Pandapip1 how comes it is possible to fuck up like this but not possible to undo it in 2 weeks?

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 facts

Pandapip1 commented 1 year ago

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:

User interface failures, such as: Blaming the Victim prompting a user to make a security decision without giving the user enough information to answer it Source:https://en.wikipedia.org/wiki/Vulnerability_(computing)#Software_vulnerabilities

I'm pretty sure that you tried to tell me this was not a UI failure a while back:

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.

@corwintines @Pandapip1 how comes it is possible to https://github.com/ethereum/ethereum-org-website/issues/10854#issuecomment-1670628057 but not possible to undo it in 2 weeks?

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:

  1. Have a banner that states that non-Final EIPs should not be used in production.
  2. That's it.

@Dexaran if you would like me to draft a PR, I would be willing to do so.

Dexaran commented 1 year ago

@Pandapip1

I'm pretty sure that you tried to tell me this was not a UI failure a while back

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.

ERC-20 is an interface. ERC-20_is_an_interface

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.

Again, here is what I require to have the page reinstated:

  1. Have a banner that states that non-Final EIPs should not be used in production.

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?

Pandapip1 commented 1 year ago

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.

Pandapip1 commented 1 year ago

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.

Dexaran commented 1 year ago

I slightly prefer ERC-1363 over ERC-223

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.

I am okay with the ERC-223 page being reinstated without a warning.

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"?

Pandapip1 commented 1 year ago

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.

github-actions[bot] commented 1 year ago

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

Pandapip1 commented 1 year ago

Any updates?

Dexaran commented 1 year ago

Will send a PR in few days. Definitely gonna work on it but not the highest priority on my tasklist right now.

github-actions[bot] commented 11 months ago

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

Dexaran commented 11 months ago

@corwintines @Pandapip1 PR sent https://github.com/ethereum/ethereum-org-website/pull/11787

github-actions[bot] commented 5 months ago

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

corwintines commented 5 months ago

Closing this as the above PR has been merged.

wackerow commented 5 months ago

Closed with #11787