EIP-1363 support

[TrejGun]

🧐 Motivation

I was using ERC223 for some time with ERC998 but it seems to be obsolete and superseded by EIP-1363. It would be good to have the out-of-the-box implementation of a notification mechanism for receiving ERC20 tokens, the same as you have for ERC721 and ERC1155. We would like to use this standard for ERC998 tokens and for OZ VestingWallet contract.

📝 Details

[frangio]

There is advanced work to do this in #3017. It's been delayed due to issues around interpretation of the spec. We'll get back to it as soon as priorities allow.

[Dexaran]

@ernestognw

I did my research on EIP-1363 and I'd like to provide my feedback.

Response to the creator of the issue #3736

I was using ERC223 for some time with ERC998 but it seems to be obsolete and superseded by EIP-1363.

It is not true.

ERC-223 is not superseded by EIP-1363 - those are two different standards that define different approach of implementing tokens.

The main idea of ERC-223 is to create a token that will work exactly as Ether (native currency of Ethereum chain) works. There can be only one standard that behaves identical to Ether because Ether is Ether and it has only one logic.

The problem of ERC-20 standard is now evident:

• The standard contains known security vulnerabilities that remain in wontfix state
• There are three sub-types of ERC-20 tokens: (1) tokens that return false instead of reverting() a transaction on error during transfer like DAO token, (2) tokens that return true on successful transfer and revert() on error like it is described in the EIP-20 specification, (3) tokens that return nothing and revert() on error like USDT or BNB token. In fact that means that the most popular tokens (USDT and BNB) directly violate ERC-20 specification and these are not compatible with ERC-20 standard.
• The standard implements "approvals" mechanism which is potentially insecure and poses a threat to safety of users funds. Authorizing someone to spend funds on your behalf involves trust. It can't work in a trustless system without problems. This method was introduced to address the 1024 call stack depth problem that was fixed in 2016. Now "approvals" must be considered a deprecated method of transferring digital assets and never be used in decentralized payment systems.

I am a security expert (proof of expertise) and the main goal of ERC-223 is to create the most secure and mistake-proof token as possible. At the same time it solves the problem of uniformity by making token behavior identical to Ether behavior.

What goes wrong with ERC-223? Why is it not an adopted standard now?

This is just an issue of adoption. ERC-223 could solve all the problems of ERC-20 in 2017, it is still completely sufficient to solve this problems today.

I left the standard "as is" in 2018 and that was a mistake. I have created EIP-223, I wrote the reference implementation code and I thought "I will leave it here. Ethereum is full of smart guys, eventually they will solve the problem of token standards. My job is done!"

I was building a lot of things on EOS after 2018 and changed the focus from Ethereum. EOS token standard for example inherits the logic of ERC-223 https://github.com/EOSIO/eosio.contracts/blob/master/contracts/eosio.token/src/eosio.token.cpp

In particular this lines of code require_recipient( <name> ) are invocations of transaction handling functions: https://github.com/EOSIO/eosio.contracts/blob/master/contracts/eosio.token/src/eosio.token.cpp#L89-L90

EOS tokens don't have any approvals at all because those are not necessary if you implement transaction handling logic.

Recently I realized my job is definitely not done with Ethereum tokens - because the amount of lost funds increased significantly and there is no consensus about "what good standard looks like". A lot of new standards emerged and most of them implement some kind of transaction handling logic but they do it in so wrong way that they create even more problems without solving existing ones.

This is probably because these implementations were designed without involving any security engineers.

I also realized that ERC-223 is abandoned and considered "Draft which is not safe to work with" because nobody championed it. Now I understand that submitting a solution is far from enough - a strong champion in the community is absolutely necessary to push the adoption.

What practices should we adhere to when designing token standards?

• Pull transactions are very bad choice from security point of view. I wrote an article about Pull transactions vs Push transactions recently. Pull transactions are not applicable to decentralized trustless systems at all. They are not designed for it. We can't remove approvals right now, I understand, but at least the standard must not contain them as a musthave feature. They are supposed to be deprecated eventually.

• Minimalistic design. Less code. The more code and features a program has - the harder it is for a developer to understand it correctly and implement without mistakes. We must not focus on multitool solutions that have 30 versions of functions for everything - we must focus on implementations that are viable with as few code as possible to reduce the number of hacks in the industry. Elon said "write less code". Elon is correct because he works with rockets and "security" makes sense there: https://twitter.com/elonmusk/status/1211557592125857793

• Error handling / transaction handling.

• There is no point in redefining existing function signatures or adding new functions that do exactly the same as existing ones. What needs to be redefined is transferring method logic, not function signatures. If a standards does not redefine the logic of transfer ERC-20 function then it is a security vulnerability. If it does redefine the logic of transfer function similar to how ERC-223 does this - then there is no point in adding new functions like transferAndCall as the basic transfer() will be sufficient.

ERC-1363 will not be a good solution

• ERC-1363 logic is inherently misleading. This token standard declares 10 functions that can transfer tokens. EOS token has only one transferring function and there is only one method of transferring a token. ERC-223 has only one method of transferring a token (but it is overloaded for backwards compatibility with ERC-20 UIs). Ether has only one way of transferring.

• Because it requires a champion to be adopted as I said earlier. And the champion of ERC-1363 is not here it seems.

• It has approvals as a core part of the standard. It is a security flaw (that I described in the article above). Approvals must be an optional feature because they are totally unnecessary if you implement transaction handling. Eventually approvals must be deprecated as they pose a threat to safety of users funds.

• It redefines transferring function names and ABI. This will be extremely hard to convince wallet devs and UI devs to restandardize their services. With ERC-223 you don't need to do anything because its ABI is the same as ERC-20 and every UI that works with ERC-20 will work with ERC-223 by default and it will be already auto-secure because the ERC-223 solves most problems of ERC-20 tokens on standard level.

ERC-1155 will not be a good solution

- Too much unnecessary code. Batch transfers can be a decent feature but we are talking about digital assets. Bitcoin exists. Ether exists. Tesla stock ($TSLA) exist. All those classes of assets do not require any batch transferring features to work. What ERC-1155 introduces is a very specific set of extensions that may be helpful in very specific situations but it is not a "standard for everyone to use".

• Overcomplicated logic. If you understand how Ether transfer works and you look at the token contract code and it doesn't work the same as Ether - that's bad design. The idea of ERC-223 was to create a token that would be intuitive for developers even BEFORE THEY LOOK AT THE CODE.

• Pull transactions = security threat for users.

NOTE: I'm not saying ERC-1155 is a bad standard. Among other standards it is the one that has some real utility behind it that other standards are missing. I'm saying that it is not suitable to become the global standard. My position regarding ERC-1155 is that we must aim for cross-standard operability by creating Token Converters - contracts that exchange 1:1 tokens of one standard for another and vice versa.

ERC-777 will not be a good solution

• It doesn't solve the problem of "frozen ERC-20 tokens" to start with and it is affected by the same exact problem.

• No champion.

• Unnecessary logic that is standardized and everyone has to support it to be compatible with the standard.

ERC-223 authors comment

I don't see any standard that solves the existing problems better than the one that I created 6 years ago. Most of the standard rely on deprecated transferring methods that should have been removed in 2016.

I am planning to champion the ERC-223 and bring it to the final status and pursue its adoption because I don't see any viable alternatives emerging in the past 6 years.

• I have resurrected EIP-223 and resubmitted it according to the current Ethereum standards: https://eips.ethereum.org/EIPS/eip-223

• I am updating Ethereum documentation to include it as an "official" standard and PR is merged: Add ERC-223 to token standards. ethereum/ethereum-org-website#9651

• ERC-223 is moving to review Update EIP-223: Move to Review ethereum/EIPs#7339

• I have created a ERC20-to-223 Token Converter contract. I will introduce a clear migration process that will allow to seamlessly swap ERC-20 tokens for ERC-223 tokens and back to ERC-20 at any time. This will be submitted as a new EIP as soon as I will test the code: https://github.com/Dexaran/TokenStandardConverter

• This time we will have a full-scale campaign for token standard adoption. I will create an open organization to coordinate the efforts of developing the ERC-223 ecosystem. We will write the required guidelines, tutorials, explanations, code samples and templates.

• I will put Token Converter in action.

• I will create a ERC-223 compatible DEX. For example there is one: https://app.soy.finance/swap but it lacks the proper ERC-223 support in its UI.

ERC-223 Compatible DEX

ERC-20 swap

Here is approve: https://explorer.callisto.network/tx/0xa20d2838ea371759f92e7d4ae9700d2de96cf65de738b518dea1753db7180377

Here is swapExactTokenForCLO: https://explorer.callisto.network/tx/0xedf726375e86b2e1df80a614049ab5e1a797174fb762d81471e3379e98497d36

ERC-223 swap

• A transfer of ERC-223 token that encodes swapExactTokenForCLO function invocation in its _data: https://explorer.callisto.network/tx/0x8cf1d1454723c2c4e0d57b1f7d202bccd47d780de1ffb1482de377a4ae1bef9b

It takes only one transaction to swap ERC-223 tokens without approvals. And there is no need to worry about revoking approvals that were issued before.

It should be noted that this is exactly the same contract that works with both ERC-20 and ERC-223 tokens at the same time and supports both methods of swap function invocation.

Disclaimer

"Dex, you are advocating ERC-223 because you are the author of this standard and you want your standard to be adopted."

If I wanted “my standard to be recognized”, then I would have done it in 2017. I don't get paid for it, I don't earn anything from it other than headache. I am a dedicated security expert who wants the problem solved. People are losing money.

And a lot of guys proposed their versions of "token standards" that do not solve any problems for the above mentioned reasons.

References

1. Known problems of ERC-20 standard

2. Why approvals are a threat for the safety of users funds and why approvals exist

3. ERC-223 original discussion thread

4. I was talking to developers in Ethereum discord. There is a good illustration of how misleading the specification of ERC-20 is for most of the token developers: https://discord.com/channels/435685690936786944/435685690936786946/1129853369654186044

The most common questions are:

• How can a contract receive ERC-20?
• Why it is not the same as it receives Ether?
• How to prevent a contract from receiving ERC-20 that it is not intended to receive?
• Why it doesn't work the same as with Ether?

5. Another good example of how people perceive crypto security level and why I think minimalistic contracts are much more secure https://discord.com/channels/435685690936786944/435685690936786946/1132722420311134359

[TrejGun]

Hey @Dexaran I'm very excited to see you here

First of all, let me point out some facts:

1. your repository had no commits for two years until recently
2. people need easy-to-use, proven, and compatible solutions
3. there is unmerged PR for ERC1363 #3017

using these statements it's easy to come to the conclusion that people will use standard at the final stage integrated into their favorite framework.

while all of your arguments seem to be reasonable, the fact of the existence of erc1363 (and eip4524) means they are not enough

on the other hand:

1. erc998 is far from the final stage too (greetings to @mudgen, man you are ahead of the curve with your technology, the thing is - it is just too complicated for 99% of developers)
2. DEXes does not support any of those standards
3. USDT is the major exception giving me a headache
4. nobody can stop me from using whatever I like

This basically means you should not worry about my custom implementation of erc998. I can easily switch back to erc223 when:

1. ERC223 is at the final stage
2. ERC223 integrated into OZ framework

So I wish you luck, strength, and lots of community support to finish what was started back in the days

[Dexaran]

while all of your arguments seem to be reasonable, the fact of the existence of erc1363 (and eip4524) means they are not enough

I have pointed out what was not enough - marketing component of advertising the new standard.

And I also pointed out why ERC-1363 and other existing standards are not a solution to the problem that ERC-223 solves.

This basically means you should not worry about my custom implementation of erc998.

My comment was mostly for OpenZeppelin staff as we discussed a critical vulnerability of ERC-20.sol implementation and they pointed me here stating that this proposal can be a solution. And I'm saying that in my opinion it is not a viable solution in its current state.

[Dexaran]

I personally have no objections against ERC-1363 but if it is going to be supported I would recommend to re-define the logic of transfer function of ERC-20 so that in a basic ERC-1363 implementation it would not allow transferring to contract addresses.

[ernestognw]

This thread is about EIP-1363 support and it's best to keep it that way. The standard is already finalized so there's no way to make changes to it, even by the original author.

The EIP process is about ecosystem consensus and there seems to be demand for EIP-1363 while others are not yet finalized. We can see a significant amount of verified contracts including "ERC1363"

The current issue with EIP-1363 is that the return value of onTransferReceived can't be returned by an EOA. We'd like to asses community consensus around this particular ambiguity.

[Dexaran]

The EIP process is about ecosystem consensus and there seems to be demand for EIP-1363 while others are not yet finalized.

That's great but it's as insecure as ERC-20. So there must be a clear restriction on transfer function that would prevent it from sending tokens to contracts. Otherwise it will be compatible with the (insecure) standard and will inevitably result in a permanent freeze of tokens in exactly the same way as it can happen with ERC-20.

So this standard inherits all the security problems of ERC-20 and I recommend:

• highlighting it in the documentation
• implement a restriction on transfer function

The current issue with EIP-1363 is that the return value of onTransferReceived can't be returned by an EOA.

Can't speak on the behalf of the "majority" but it is logical to examine the recipient and not to send via transferAndCall to EOA. If the expected return of onTransferReceived is not returned - consider transfer invalid and revert() the transaction.

The specification of the EIP-1363 does not provide reference implementation or any description that declares token behavior in such scenarios.

[TrejGun]

@Dexaran if you are serious about reviving erc223 please open a new thread, make an RP with implementation and I promise to test it against my codebase and give you feedback

meanwhile, you can check how erc1363 is used in my system.

bytes4 constant IERC1363_RECEIVER_ID = 0x88a7ca5c;
bytes4 constant IERC1363_ID = 0xb0202a11;

library ExchangeUtils {
  using Address for address;
  using SafeERC20 for IERC20;
  
  function spendFrom(
    address token,
    uint256 amount,
    address spender,
    address receiver
  ) internal {
    if (_isERC1363Supported(receiver, token)) {
      IERC1363(token).transferFromAndCall(spender, receiver, amount);
    } else {
      SafeERC20.safeTransferFrom(IERC20(token), spender, receiver, amount);
    }
  }

  function _isERC1363Supported(address receiver, address token) internal view returns (bool) {
    return
      (receiver == address(this) ||
        (receiver.isContract() && _tryGetSupportedInterface(receiver, IERC1363_RECEIVER_ID))) &&
      _tryGetSupportedInterface(token, IERC1363_ID);
  }

  function _tryGetSupportedInterface(address account, bytes4 interfaceId) internal view returns (bool) {
    try IERC165(account).supportsInterface(interfaceId) returns (bool isSupported) {
      return isSupported;
    } catch (bytes memory) {
      return false;
    }
  }
}

@ernestognw that comment about EOA, man you just can't prevent people from shooting their foot

[Dexaran]

@TrejGun can you show the code of isContract()? Is it the function from Address lib? I recall there was a change of its logic and I'd like to review which implementation is used in your case

[TrejGun]

yes this function comes from OZ framework

[Dexaran]

this one doesn't have .isContract() implemented https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Address.sol it seems

[TrejGun]

well you are right it was recently removed
c5d040b

[Dexaran]

Initially we were using a method for identifying which address is a EOA and which one is a contract that I proposed in 2017. We were assemblying the code and if it's length was non-zero then it was considered that examined address is a contract:

https://github.com/Dexaran/ERC223-token-standard/blob/development/utils/Address.sol#L24

    function isContract(address account) internal view returns (bool) {
        // This method relies in extcodesize, which returns 0 for contracts in
        // construction, since the code is only stored at the end of the
        // constructor execution.

        uint256 size;
        // solhint-disable-next-line no-inline-assembly
        assembly { size := extcodesize(account) }
        return size > 0;
    }

There is a caveat that must be taken into account: if you will call extcodesize on an account in the deployment transaction - it will say it is not a contract even though the contract will be deployed at this address.

[TrejGun]

is there any difference in

return address(this).code.length == 0

and

uint256 size;
assembly { size := extcodesize(this) }
return size > 0;

rather than a few saved gas units

[Dexaran]

I don't think there is any significant difference

[ernestognw]

@ernestognw that comment about EOA, man you just can't prevent people from shooting their foot

It's true we can't prevent people's mistakes, but we would prefer providing the simplest secure implementation.
The fact that ERC-1363 does not specify the EOA behavior leaves two alternatives:

On one hand, if transferAndCall doesn't revert implementers will need to check whether the specific transferAndCall implementation they're calling reverts or not with EOAs. This is because some implementations out there (including #3017 and #4631) already do that and is highly likely that somebody has already deployed versions like this (see @vittominacori's erc-contract-payable dependants)

The pro of this approach is that reverting when calling EOAs is a behavior that can be implemented on top:

if (target.code.length == 0) revert SomeError();
token.transferAndCall(target, amount);

On the other side, if transferAndCall reverts, implementers will need to check if the callee is an EOA anyway to use transfer/transferAndCall accordingly:

if (target.code.length == 0) {
    token.transfer(target, amount);
} else {
    token.transferAndCall(target, amount);
}

I agree with @Amxx comment's that not reverting is the most efficient alternative because it leaves the target.code.length == 0 check to the contract calling a token.transferCall.

However, because this approach still requires user checking, we may want to provide a safeTransferAndCall (or similar) utility to deal with transferAndCall implementations that revert, but that may add an extra code.length == 0 check. This is how we've dealt with EIP ambiguities in the past but I'm not a fan of these utilities.

Since there are EOA-reverting implementations out there, I'd think it's safer to just revert for EOAs and let the users check if it's an EOA or not. They'll most likely do it anyway for safety when interacting with untrusted tokens and the savings of avoiding the target.code.length == 0 aren't in a high order value (perhaps a few gas units).

[vittominacori]

@ernestognw thanks for pointing out.
I agree with your considerations and understand that you need to implement something to be included in a mass adopted library.

The EIP was developed to add functionalities to ERC20 in order to be able to perform actions after transfers or approvals.
The *andCall methods were not intended to replace the transfer or approve behaviors or to be used in their place or having different behaviors dealing with contracts or EOA. While it might seem ambiguous, the original specifications say that A contract that wants to accept token payments via *andCall ... and doesn't mention EOA since they are not included in (my) proposal.

Users who simply want to transfer tokens can continue to use the transfer method or implement the contract using approve and transferFrom. But users who want to be sure to perform an action after a transfer or approval can use the *andCall methods. This was my purpose.

[ernestognw]

I see your intention was to revert, which in my opinion is what the EIP attempted to state. However, I think we've agreed the EIP is finalized and that should be our source of truth. It doesn't mention the EOA case at all, thus is an undefined behavior.

Given a regular user can expect both behaviors, I think the best way to implement ERC1363 in OZ Contracts will be to pick one and provide a utility along with it to deal with the selected scenario:

• If reverting on EOAs (as you originally intended): Users will need to implement custom routing between transfer/approve and *andCall
• If not reverting on EOAs: Users will need to catch errors for tokens that do revert.

Personally, I'd not revert to avoid the extra check @Amxx pointed out but given that there are multiple reverting implementations out there, I'd go for reverting and include a routing function as a utility (or maybe just docs recommendations).

[ernestognw]

Probably later. I predict that OZ staff will reply "we analyzed the chain and there are no ERC-223 contracts so we are not interested" as they did before.

So it's better to deploy the token converter first.

Basically, yes. Can you create an issue specific to ERC-223?
We're overpopulating this one. Just hid the comments as off-topic.