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

ERC223 token standard #223

Open
Dexaran opened this Issue Mar 5, 2017 · 634 comments

Comments

Projects
None yet
@Dexaran
Copy link

Dexaran commented Mar 5, 2017

ERC: 223
Title: Token standard
Author: Dexaran, dexaran@ethereumclassic.org
Status: Draft
Type: ERC
Created: 5-03.2017
Resolution: https://github.com/Dexaran/ERC223-token-standard
Recommended implementation: https://github.com/Dexaran/ERC223-token-standard/tree/Recommended

Abstract

The following describes standard functions a token contract and contract working with specified token can implement to prevent accidentally sends of tokens to contracts and make token transactions behave like ether transactions.

Motivation

Here is a description of the ERC20 token standard problem that is solved by ERC223:

ERC20 token standard is leading to money losses for end users. The main problem is lack of possibility to handle incoming ERC20 transactions, that were performed via transfer function of ERC20 token.

If you send 100 ETH to a contract that is not intended to work with Ether, then it will reject a transaction and nothing bad will happen. If you will send 100 ERC20 tokens to a contract that is not intended to work with ERC20 tokens, then it will not reject tokens because it cant recognize an incoming transaction. As the result, your tokens will get stuck at the contracts balance.

How much ERC20 tokens are currently lost (27 Dec, 2017):

  1. QTUM, $1,204,273 lost. watch on Etherscan

  2. EOS, $1,015,131 lost. watch on Etherscan

  3. GNT, $249,627 lost. watch on Etherscan

  4. STORJ, $217,477 lost. watch on Etherscan

  5. Tronix , $201,232 lost. watch on Etherscan

  6. DGD, $151,826 lost. watch on Etherscan

  7. OMG, $149,941 lost. watch on Etherscan

NOTE: These are only 8 token contracts that I know. Each Ethereum contract is a potential token trap for ERC20 tokens, thus, there are much more losses than I showed at this example.

Another disadvantages of ERC20 that ERC223 will solve:

  1. Lack of transfer handling possibility.
  2. Loss of tokens.
  3. Token-transactions should match Ethereum ideology of uniformity. When a user wants to transfer tokens, he should always call transfer. It doesn't matter if the user is depositing to a contract or sending to an externally owned account.

Those will allow contracts to handle incoming token transactions and prevent accidentally sent tokens from being accepted by contracts (and stuck at contract's balance).

For example decentralized exchange will no more need to require users to call approve then call deposit (which is internally calling transferFrom to withdraw approved tokens). Token transaction will automatically be handled at the exchange contract.

The most important here is a call of tokenFallback when performing a transaction to a contract.

Specification

Token
Contracts that works with tokens

Methods

NOTE: An important point is that contract developers must implement tokenFallback if they want their contracts to work with the specified tokens.

If the receiver does not implement the tokenFallback function, consider the contract is not designed to work with tokens, then the transaction must fail and no tokens will be transferred. An analogy with an Ether transaction that is failing when trying to send Ether to a contract that did not implement function() payable.

totalSupply

function totalSupply() constant returns (uint256 totalSupply)

Get the total token supply

name

function name() constant returns (string _name)

Get the name of token

symbol

function symbol() constant returns (bytes32 _symbol)

Get the symbol of token

decimals

function decimals() constant returns (uint8 _decimals)

Get decimals of token

balanceOf

function balanceOf(address _owner) constant returns (uint256 balance)

Get the account balance of another account with address _owner

transfer(address, uint)

function transfer(address _to, uint _value) returns (bool)

Needed due to backwards compatibility reasons because of ERC20 transfer function doesn't have bytes parameter. This function must transfer tokens and invoke the function tokenFallback(address, uint256, bytes) in _to, if _to is a contract. If the tokenFallback function is not implemented in _to (receiver contract), then the transaction must fail and the transfer of tokens should not occur.

transfer(address, uint, bytes)

function transfer(address _to, uint _value, bytes _data) returns (bool)

function that is always called when someone wants to transfer tokens.
This function must transfer tokens and invoke the function tokenFallback (address, uint256, bytes) in _to, if _to is a contract. If the tokenFallback function is not implemented in _to (receiver contract), then the transaction must fail and the transfer of tokens should not occur.
If _to is an externally owned address, then the transaction must be sent without trying to execute tokenFallback in _to.
_data can be attached to this token transaction and it will stay in blockchain forever (requires more gas). _data can be empty.

NOTE: The recommended way to check whether the _to is a contract or an address is to assemble the code of _to. If there is no code in _to, then this is an externally owned address, otherwise it's a contract.

Events

Transfer

event Transfer(address indexed _from, address indexed _to, uint256 indexed _value, bytes _data)

Triggered when tokens are transferred.

Contract to work with tokens

function tokenFallback(address _from, uint _value, bytes _data)

A function for handling token transfers, which is called from the token contract, when a token holder sends tokens. _from is the address of the sender of the token, _value is the amount of incoming tokens, and _data is attached data similar to msg.data of Ether transactions. It works by analogy with the fallback function of Ether transactions and returns nothing.

NOTE: msg.sender will be a token-contract inside the tokenFallback function. It may be important to filter which tokens are sent (by token-contract address). The token sender (the person who initiated the token transaction) will be _from inside the tokenFallback function.

IMPORTANT: This function must be named tokenFallback and take parameters address, uint256, bytes to match the function signature 0xc0ee0b8a.

Recommended implementation

This is highly recommended implementation of ERC 223 token: https://github.com/Dexaran/ERC23-tokens/tree/Recommended

@nmushegian

This comment has been minimized.

Copy link

nmushegian commented Mar 5, 2017

Have you considered allowing this feature set by extending ERC20 with approveAndCall? Inside the "charging" contract you'll have one function call in any case (in this case transferFrom), but on the caller side entering is now an atomic operation, similar to transferToContract

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 5, 2017

The main goals of my proposal were:

  1. Prevent accidentally sent tokens from being held by contract.
  2. Allow users to deposit their tokens in contract the way simple as ether by a single transaction (i.e. transferToContract( ) call) with no need to call approval than call transferFrom.
  3. Make token transactions behave same as Ether transactions.

approveAndCall assumes that target contract will call transferFrom and its not like Ether transactions do. There is no need to allow target contract to withdraw tokens from someone (tx initiator). There is no need to fire Approval event also. From a logical point of view, we should just notify target contract that transaction appears. Also fire Transfer event with no approvals.

@Dexaran Dexaran closed this Mar 8, 2017

@Dexaran Dexaran reopened this Mar 8, 2017

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 8, 2017

Updated my ERC23 token code with a function assembling receiver address to ensure token contract if the receiver is a contract or an address.

@nmushegian

This comment has been minimized.

Copy link

nmushegian commented Mar 8, 2017

Ok I think I see where you are coming from...

What about, devs generally seem to want to replace the "execute on transfer" for ETH with a hard coded token interface with no execute on transfer, potentially just ERC20?

Is this a bad side effect or a naturally good thing? Doesn't exec-on-transfer make the simplest use cases more complex and maybe more dangerous, while not allowing anything really new?

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 8, 2017

exec-on-transfer make the simplest use cases more complex and maybe more dangerous

for contract developers you mean. From users point of view we just need to call 'transfer token' on MEW or transfer directly in contract and dont care any more about what is going on instead of calling approval then calling deposit or something like this with a chance of mistake that will cause a loss of tokens.

@nmushegian

This comment has been minimized.

Copy link

nmushegian commented Mar 8, 2017

for contract developers you mean. From users point of view we just need to call 'transfer token' on MEW or transfer directly in contract and dont care any more about what is going on instead of calling approval then calling deposit or something like this with a chance of mistake that will cause a loss of tokens.

Contract devs are not being unsympathetic to user experience by nitpicking the semantics of contract code... I understand that approve + doSomething is not optimal UX, but these are things you can abstract away at the user interface level, and this abstraction honestly has very few side effects, while making composing contracts more safe ("what stuff could happen if I transfer this token here?")

(personally I think the real base abstraction is a binary per-address approve but that is another side thread)

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 9, 2017

Well designed token contract assumes you need to trust only contract and ethereum EVM. UI level abstraction assumes you need to trust UI developers. It also requires some things to be done by UI devs. I dont see any abstraction that is required already done. So what reason is to make a lot of requirements and dependencies between contract developers,UI developers and users when there is a way to avoid it.
At the other hand the main problem of every cryptocurrency is network bandwidth right now. Transferring of ERC20 token to the contract is a couple of two different transactions in fact. While transferring ERC23 token to a contract is a single transaction.
ERC20 transfer to contract also fires Approval event then fires Transfer event. Such irrational use of blockchain can cause extra bloating. ERC23 transfer fires only Transfer event.

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 9, 2017

We were thinking about doing a similar proposal from Aragon while working on economic abstraction for companies. We finally decided approve and transferFrom was a simpler interface and more secure.

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 9, 2017

Simpler and more secure? What reasons do you have thinking so? I named my reasons.
Easier usage. Better optimization. Less requirements.

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 9, 2017

In the current implementation, if a contract doesn't implement the receiver protocol, the transfer of tokens to an address that happens to be a contract will throw https://github.com/Dexaran/ERC23-tokens/blob/master/ERC23_token.sol#L56

Also I see the problem that the receiver needs to keep a list of what tokens it supports.

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 9, 2017

Also I didn't know about approveAndCall, but it seems the way to go for me.

My two cents, I would be happy to see approveAndCall be part of the standard, but your current solution, while cool, I think it would bring too much overhead to an already very simple and versatile protocol.

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 9, 2017

There could be a standard to approveAndCall data payload that calls the similar to your so called fallback in your proposal, and the this fallback doing some delegatecall magic could actually call a function in the contract passing the sender and the value as function params. In the fallback function you could do your token accounting and then call whatever function the caller wanted.

Sorry for the ramblings, I think I will actually come up with a parallel proposal for this.

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 10, 2017

In the current implementation, if a contract doesn't implement the receiver protocol, the transfer of tokens to an address that happens to be a contract will throw

As I said earlier it's done to prevent accidentally transactions of tokens to contract address where tokens will not be accessible any more.
Contracts are throwing accidentally ether transactions if no fallback payable function is implemented. The same mechanism for accidentally token transactions not exists so Im suggesting to implement it now.

Also I didn't know about approveAndCall, but it seems the way to go for me.

approveAndCall may be a good thing or may be not but now it is not implemented in ERC20 and will not solve accidentally token transactions between address and contract so Im not suggesting to implement approveAndCall.

My proposal solves a number of problems:

  1. Accidentally transactions between address and contract that is not supporting this token will no longer cause a loss of tokens.
  2. For users: Allows not to care about contract logic of work. To transfer tokens wherever you want, you always need to call only the transfer function.
  3. For contract developers: Allows to handle incoming token transactions similar to incoming ether transaction.
  4. It also optimizes blockchain usage.

If you found my proposal too complex to accept, then I found this increase in complexity a reasonable price, which should be paid for preventing accidental loss of tokens in the entire future.

Also I see the problem that the receiver needs to keep a list of what tokens it supports.

I don't see it is a problem but if this is the only thing that prevents the token standard from being accepted I designed a light version of contract-receiver. It will accept every incoming ERC23 token and do nothing with it. It can be called "token-trap" contract.
You can browse it here: https://github.com/Dexaran/ERC23-tokens/tree/light_version

@frozeman

This comment has been minimized.

Copy link
Member

frozeman commented Mar 10, 2017

I do like it, in fact i would also suggest for wallet contracts that the fallback func fires an standard event on the receiving contract called TokenTransfer(address _from, uint _value)
Tho i would rename the fallback func to t(...) or tokenFallback(...), sounds better.

@frozeman frozeman added the ERC label Mar 10, 2017

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 10, 2017

@Dexaran those are all good points indeed. Sorry if the feedback seemed harsh, I'm indeed really interested in getting closer to economic abstraction so contracts can operate with tokens in a similar way the can with ether.

What do you think about the fallback function being something liket(address tokenSender, uint value, bytes data) and then the fallback function can do a delegatecall to itself with this data to simulate a payable function?

@frozeman

This comment has been minimized.

Copy link
Member

frozeman commented Mar 10, 2017

How you mean simulate a payable function?

Btw i don't fully understand why a contract should add supported tokens? As long as a contract fires the fallback function, it will be ERC 20 + 23

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 10, 2017

Btw i don't fully understand why a contract should add supported tokens?

Contract that is working with a specified tokens may contrain a mapping of supported tokens.For example if we are working with only Unicorns and incoming Token123 transaction appears we should reject transaction of not supported Token123.
Supported token may also be hardcoded or set inside receiving contract in any way you prefer. I just recommended addToken and removeToken functions to be in contract but they are not required.

@frozeman

This comment has been minimized.

Copy link
Member

frozeman commented Mar 10, 2017

Im not sure if i would make that into the standard, i find the fallback function useful, but as long as they are ERC 20 the contract should be able to deal with it. If he wants to reject certain tokens, than thats something they can do, but it doesn't need to be part of this standard.

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 10, 2017

By simulate a payable function I mean that after the token fallback is called, a specific function in the contract is called.

A rough, pseudo-Solidity implementation would be:

contract TokenReceiver {
  function t(address tokenSender, uint value, bytes payload) {
     if (_data) {
         delegatecall(this, payload)
     }
  }

  function foo() {}
}

So you could do transfer(contractAddress, value, '0xc2985578') in the token and have it call the function foo() in the receiving contract by sending value with a token. Maybe a couple of parameters should be added to the foo() like functions so they get the sender and the amount of tokens received.

The only thing that would need to be included in the standard is the case in which transfer has a payload parameter, as the way you handle it in the fallback could be up to every contract to decide how to do it.

Also regarding supported tokens, I think if a specific contract wants to only support a set of tokens or blacklist a specific one they can do it as part of their implementation, but I support the idea @that it shouldn't be included in the standard.

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 10, 2017

@frozeman I would recommend to "reject everything that is not marked as supported" but not "accept everything that is not marked to be rejected" because of when contract like the dao-refund is written it shouldn't accept any of incoming token except DAO. If DAO-token is not ERC23 so there is no way to accept anything ERC23 and we should place function tokenFallback(address _from, uint _amount) { throw; } inside dao-refund to make every ERC23 be rejected.
But if we need to make a DAO23 refund contract to accept ERC23 standard based tokens (DAO23) we should specify DAO23 as allowed. So any other ERC23 token will still be rejected.
Token standard is a recomendation to token developers how to develop their tokens in the best way. So I found it important not only recommend how tokens should be developed but also how token transactions should be handled.
Do you think it is not needed to be included in token standard?

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 10, 2017

What do you think about the fallback function being something liket(address tokenSender, uint value, bytes data)

@izqui as I understand it you are recommending to make token transactions behaving one step more similar to Ether transactions.
Where t(address tokenSender, uint value, bytes payload) means
address tokenSender == msg.sender
uint value == msg.value
bytes payload == msg.data

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 10, 2017

Exactly. And the actual msg.sender of t(...) is the token being used to transfer value.

So extending your comparison you would have:

address tokenSender == msg.sender
uint value == msg.value
bytes payload == msg.data
msg.sender == ether

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 10, 2017

Of course. tx.origin is now an address who is sending tokens, msg.sender is contract of sent token, msg.data is data signifying what function to call inside the token contract and payload is data signifying what function to call inside contract-reveiver of tokens.
Also tokenSender is address who is sending tokens too when only contract of tokens and contract-receiver are involved and no other contracts are called.
I found it awesome idea but I need to do some tests. So I cant say anything more specific right now.

@izqui

This comment has been minimized.

Copy link
Contributor

izqui commented Mar 10, 2017

Something to keep in mind regarding tx.origin is that in the case of usingtransferFrom(), tx.origin will be the authorized address to make the transaction and not the former token owner, which tbh I'm not really sure which one of the two should be accounted as the sender in this case.

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 10, 2017

I see we are 1 step away from creating Token-based Ethereum inside Ether-based Ethereum.
According to your idea token fallback function should handle incoming token transactions only in this manner:

function tokenFallback(address _from, uint _value, bytes payload){
  if(_data){
    delegatecall(this, payload);
   } else {
    //tokenFallback code here
  }
}

Because of Ether fallback function handles only transactions of value (ETH).
And as @frozeman said its not a part of token standard already. I found your idea cool but as for me I dont really know. So I want to get more feedback and do some more tests.
And of course there is always a possability just to ignore incoming data and handle only _from and _value as I suggested earlier.

@Dexaran Dexaran changed the title ERC token standard ERC23 token standard Mar 10, 2017

@Dexaran

This comment has been minimized.

Copy link

Dexaran commented Mar 11, 2017

I decided that token transaction must contain bytes data.

It's not a solution of any of the problems I'm aiming to solve but it may be needed for future use. As long as there is a way to attach data to Ether transactions I think there should be a way to do the same with token transactions too.
I don't care how exactly this will be used to attach HEX messages to token transactions or to encode inner functions execution but the way to attach data to the transaction (token or Ether) must exist.
As the result of this transfer function is changed and now contains bytes _data.

@hskang9

This comment has been minimized.

Copy link

hskang9 commented Jul 14, 2018

@HarryR It was a bad implementation after fixing solidity compiler's classifier warning on tokenFallback.

@KorbenDallasPewPewPuf

This comment has been minimized.

Copy link

KorbenDallasPewPewPuf commented Jul 20, 2018

You are talking using ERC223 in your comments. I just dont get it. Is this EIP223 or ERC223? And if second, then when it has became a standart? Any references? Please clarify cuz Im consused. Thank you.

@hskang9

This comment has been minimized.

Copy link

hskang9 commented Jul 20, 2018

  1. EIP is an acronym of the Ethereum Improvement Proposal and this is proposal number 223, and this is also ERC which stands for Ethereum Request for Comment. So this is both EIP223 and ERC223. I use ERC223 to refer this token contract, because somehow after ERC20, "The token standard" was uploaded, people started to call tokens that way.

  2. I do not know if ERC223 is standard now, but I am sure this is an upgrade from ERC20 since it prevents from sending tokens to unaware contract. Recently, I am fidgeting with the code if there is a potential danger or error, sometimes getting lost in a bad implementation🙈 So far I found that transfer(address _to, uint _value) function from ERC20 for backward compatibility does not support sending tokens to any contract due to its empty variable returning the value of 0x. After I set the value to any bytes4 or longer string, sending tokens to contract worked with the function. I am still investigating for the reason and whether that is intended or not, but the revision is here.

References are:
The New ERC223 Token Standard from Leonid Beder
ERC20 from wikipedia

@julesGoullee julesGoullee referenced this issue Jul 24, 2018

Closed

Recoverable token #1109

4 of 4 tasks complete
@Remi-Burgel

This comment has been minimized.

Copy link

Remi-Burgel commented Jul 25, 2018

The more I work with ERC223 the more I doubt about it's current implementation.

I'm currently working on a proxy wallet. Same logic as multisig wallets : You CAN transfer ERC223 with this wallet but it's a smart contract and must answer to the interface 'tokenFallback" and it's useless. I can't implement each interface of each proposition just to be sure it will be compatible with each safety implementation like this.

We were in a state where a smart contract could do everything that a normal user can do and by restricting their possibilites we are cutting some usecases.

@chencho777

This comment has been minimized.

Copy link

chencho777 commented Aug 16, 2018

ERC777 has been built to solve some of the shortcomings of ERC223. Please have a look at it:

http://eips.ethereum.org/EIPS/eip-777

@wuya666

This comment has been minimized.

Copy link

wuya666 commented Aug 21, 2018

I'm not sure but why do you remove the approve method? From what I understand approve is not just for contracts, it's also designed to work for people, where you can approve a fellow a certain amount of your token and then increase or decrease the allowance any time you want, instead of sending the token to him/her directly.

Also the lost token doesn't look like a real issue anyway, I think one thing about cryptocurrency is that if you send it to the wrong address then you lost it forever. Same goes for the ethers you sent to the wrong address or some wrong contract that does not implement ether withdrawal/self-destruct method, then the ethers are lost forever.

@wturyn-cf

This comment has been minimized.

Copy link

wturyn-cf commented Aug 23, 2018

Is Transfer event from ERC223 compatible with Transfer event from ERC20? The signature is different in ABI.

@bitsanity

This comment has been minimized.

Copy link

bitsanity commented Aug 24, 2018

@wturyn-cf - Ethereum does not allow to overload events, so one has to pick one Transfer definition or the other.

In order to claim ERC223-compliance one should use the ERC223 definition. But most popular services, tools and exchanges expect the ERC20 version.

@SergioDemianLerner

This comment has been minimized.

Copy link

SergioDemianLerner commented Aug 28, 2018

I think this standard lacks one more method to token receivers:

function isERC223Compatible() pure returns (bool)

The reason is that many contracts allow other contracts to register in order to receive tokens later (for example, a syndicated investment contract). This contract must detect if the registered address is able to accept future deposits in ERC223 tokens, but it cannot know this in advance. The only way to know this is to call the tokenFallback manually, and expect the contract will:

  • OOG/REVERT if it doesn't handle ERC223.
  • Returns if it does.

However the tokenFallback may interpret the caller is an ERC20 token and call back (e.g. to obtain totalSupply) to the syndicate contract. So syndicate contract must itself implement a fake ERC223 interface just be be able to call tokenFallback for testing purposes.

Also the ERC223 interface itself doesn't have any means to show it's ERC223.

It also should have a method:
function isERC223Token() pure returns (bool)

Sadly:

  • This specification doesn't say if tokenFallback can callback the msg.sender as a ERC223 token.
  • You can't add isERC223Compatiible() now without breaking compatibility.

So I guess a new standard would be required.

@GriffGreen

This comment has been minimized.

Copy link

GriffGreen commented Sep 17, 2018

@SergioDemianLerner i would check https://eips.ethereum.org/EIPS/eip-777 it's backwards compatible with ERC20 and is pretty much finalized

@yoshikazzz

This comment has been minimized.

Copy link

yoshikazzz commented Sep 24, 2018

I utilized @Dexaran 's ERC223 implementation, and the logic to check if an address is EOA or contract looks perfect:

assembly {
    // Retrieve the size of the code on target address, this needs assembly .
    codeLength := extcodesize(_to)
}

if(codeLength>0) {
    ERC223ReceivingContract receiver = ERC223ReceivingContract(_to);
    receiver.tokenFallback(msg.sender, _value, empty);
}

but, I found that transferring ERC223 token to the following code of a contract will not revert. It's successfully sent.

pragma solidity ^0.4.24;

contract BlackHole {
  constructor() public {}
  function() public payable {}
}

It obviously doesn't have tokenFallback function on it, and I debugged that the extcodesize is 396 (>0). I wonder why it got through.

@bitsanity

This comment has been minimized.

Copy link

bitsanity commented Sep 24, 2018

@yoshikazzz the reason your call succeeded is due to a "feature" of ethereum and it not being a typesafe system.

You called tokenFallback() function on BlackHole, but Ethereum could not find that function, so it scanned your contract and found the anonymous function instead. This function is always the default when Ethereum can't find an exact match.

@yoshikazzz

This comment has been minimized.

Copy link

yoshikazzz commented Sep 24, 2018

@bitsanity Thanks for the explanation. I see how it worked.
By the way, the initial motivation of this EIP is not achieved.

If you will send 100 ERC20 tokens to a contract that is not intended to work with ERC20 tokens, then it will not reject tokens because it cant recognize an incoming transaction. As the result, your tokens will get stuck at the contracts balance.

If a contract that is not intended to work with ERC20 tokens, but having no name function to take care of incoming ETH, it will accidentally receive ERC20 tokens, and the tokens will get stuck.

@buhrmi

This comment has been minimized.

Copy link

buhrmi commented Oct 26, 2018

I just came to the same conclusion as @yoshikazzz

I think in order to achieve the initial motivation of this EIP, a contract MUST revert the transaction if the receiver is a contract but tokenFallback did not return true.

I propose to update the spec to

function tokenFallback(address _from, uint _value, bytes _data) public returns (bool);

and in order to be ERC223-compliant, all calls to tokenFallback MUST revert if the function does not return true.

if (!receiver.tokenFallback(from, value, data)) revert();
@friko16

This comment has been minimized.

Copy link

friko16 commented Oct 29, 2018

is ERC-223 standard official for Ethereum ? I don't see it in https://eips.ethereum.org/all in drafts

@buhrmi

This comment has been minimized.

Copy link

buhrmi commented Oct 30, 2018

Somebody should make a PR. Ill do it by tomorrow unless @Dexaran or somebody else beats me to it.

@buhrmi

This comment has been minimized.

Copy link

buhrmi commented Oct 30, 2018

Hey guys before I open a PR could somebody check if there are any issues with this example implementation?

https://djshitcoin.github.io/shitcoinmaker

@catageek

This comment has been minimized.

Copy link

catageek commented Oct 30, 2018

As already said sometimes ago in this thread, ERC777 is more achieved to fix the ERC20 standard.

Please have a look at https://github.com/ethereum/EIPs/blob/master/EIPS/eip-777.md

@jpthor

This comment has been minimized.

Copy link

jpthor commented Nov 28, 2018

Hi @Dexaran can you add the updates to make the ERC223 Standard Solidity v0.5.0 compatible. (https://solidity.readthedocs.io/en/v0.5.0/050-breaking-changes.html)

In particular:

Conversions between bytesX and uintY of different size are now disallowed due to bytesX padding on the right and uintY padding on the left which may cause unexpected conversion results. The size must now be adjusted within the type before the conversion. For example, you can convert a bytes4 (4 bytes) to a uint64 (8 bytes) by first converting the bytes4 variable to bytes8 and then to uint64. You get the opposite padding when converting through uint32.

Breaks getSig function in the ERC22e Receiver:

    function getSig(bytes memory _data) private pure returns (bytes4 sig) {
        uint lngth = _data.length < 4 ? _data.length : 4;
        for (uint i = 0; i < lngth; i++) {
            sig = bytes4(uint(sig) + uint(_data[i]) * (2 ** (8 * (lngth - 1 - i))));
        }
    }

Also:

The functions .call(), .delegatecall(), staticcall(), keccak256(), sha256() and ripemd160() now accept only a single bytes argument. Moreover, the argument is not padded. This was changed to make more explicit and clear how the arguments are concatenated. Change every .call() (and family) to a .call("") and every .call(signature, a, b, c) to use .call(abi.encodeWithSignature(signature, a, b, c)) (the last one only works for value types). Change every keccak256(a, b, c) to keccak256(abi.encodePacked(a, b, c)). Even though it is not a breaking change, it is suggested that developers change x.call(bytes4(keccak256("f(uint256)"), a, b) to x.call(abi.encodeWithSignature("f(uint256)", a, b)).

Breaks ERC223 transfer Function:

        // ERC223 Transfer and invoke specified callback
    function transfer(address to, uint value, bytes data, string custom_fallback) public returns (bool success)
    {
        _transfer( msg.sender, to, value, data );
        if (isContract(to)) {
            ContractReceiver rx = ContractReceiver( to );
            require(
                address(rx).call.value(0)(
                    bytes4(keccak256(custom_fallback)),
                    msg.sender,
                    value,
                    data
                )
            );
        }
        return true;
    }

Thanks!

@Enelar

This comment has been minimized.

Copy link

Enelar commented Dec 3, 2018

@Dexaran I've been reading for hours whole thread, your repo, everything.
Someone, please explain what forbids anyone to call my tokenFallback and trick me with that?
I think this standard is more confusing than clarifying.

@buhrmi

This comment has been minimized.

Copy link

buhrmi commented Dec 3, 2018

Inside the tokenFallback function, you'd obviously need to check if msg.sender is the token contract

@FaysalM

This comment has been minimized.

Copy link

FaysalM commented Dec 4, 2018

@jpthor is there an audit report you guys had done on the contract? Reading into this, I can't see a valid enough reason to adopt this without more peer review.

@MrMabulous

This comment has been minimized.

Copy link

MrMabulous commented Dec 10, 2018

I think

function transfer(address to, uint value, bytes data, string custom_fallback) public returns (bool ok);

should emit a different event with the signature

event Transfer(address indexed _from, address indexed _to, uint256 indexed _value, bytes _data, string custom_fallback);

otherwise the info which function was called cannot be recovered from the emitted event.

@binary-adam

This comment has been minimized.

Copy link

binary-adam commented Dec 13, 2018

@Enelar You have to check msg.sender to find out what kind of token you have received anyway, msg.sender is authoritative on how many of that kind of token you own, so trusting it to tell you how many you've received is no increase in trust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment