Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERC-223: problematic description of tokenReceived #719

Open
u59149403 opened this issue Nov 16, 2024 · 2 comments
Open

ERC-223: problematic description of tokenReceived #719

u59149403 opened this issue Nov 16, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@u59149403
Copy link

Pull Request

3344c58

What happened?

In ERC-223 tokenReceived's description reads: "It works by analogy with the fallback function of Ether transactions and returns nothing". But then: "The tokenReceived function must return 0x8943ec02 after handling an incoming token transfer. The tokenReceived function call can be handled by the fallback function of the recipient contact (and in this case it may not return the magic value 0x8943ec02)".

First, this is not clear what "may not return the magic value 0x8943ec02" means. It may mean that implementations of tokenReceived are allowed to return something other than 0x8943ec02 and this will be okay. And it may mean something completely opposite: that failback function can return something other than 0x8943ec02 and for this reason we mandate tokenReceived to return 0x8943ec02 to distinguish true tokenReceived from failback function.

I'm nearly sure that the second interpretation was intended. So, please, rephrase somehow this paragraph. Ideally, this sentence about failback function should be moved to rationale.

Also, we see phrase "It works by analogy with the fallback function of Ether transactions and returns nothing". Part "returns nothing" is wrong. It returns 0x8943ec02, not nothing.

Then: we see phrase "This function can be manually called by a EOA" here. What this means? That EOAs are allowed to call tokenReceived and tokenReceived should successfully handle this? Or opposite: tokenReceived implementations should beware that EOAs may call tokenReceived and thus should reject this? I suggest simply remove this phrase, because it adds nothing.

ping @Dexaran .

Also, I think I found some other problems in ERC-223, but I didn't write them down, so I don't remember them. If you want, I can re-read ERC-223 and possibly report additional problems

Relevant log output

No response

@u59149403 u59149403 added the bug Something isn't working label Nov 16, 2024
@u59149403
Copy link
Author

Also:

ERC-20 deposit: approve ~46 gas, transferFrom ~75K gas

I think you meant 46K gas, not 46

@Dexaran
Copy link
Contributor

Dexaran commented Nov 16, 2024

I think you meant 46K gas, not 46

Yes, must be 46K.

Then: we see phrase "This function can be manually called by a EOA" here. What this means?

This is more of a security consideration to make it clear that if the tokenReceived function was invoked - it does not automatically mean that some token was deposited. A user could call it from a EOA.

Honestly this function can be called from a contract which is not a ERC-223 token, so it would be better to add that description as well.

We need to warn the implementers that the fact of tokenReceived function call does not automatically mean that tokens were deposited and they need to check that.

It works by analogy with the fallback function of Ether transactions and returns nothing

Yes, the "and returns nothing" is simply incorrect. In the old days of the standard implementation it indeed was returning nothing, but that was changed at some point and the description text was simply not updated. Needs a fix.

we mandate tokenReceived to return 0x8943ec02 to distinguish true tokenReceived from failback function.

This.

can be handled by the fallback function of the recipient contact (and in this case it may not return the magic value 0x8943ec02)

The word "may" was used because its possible to assemble a return value within fallback function by pushing something onto stack at the memory position where the returned value is expected to be stored, I'm pretty sure it was possible in solidity before 0.4.0 but that needs to be checked. The functon tokenReceived MUST return the magic value.

The implementer CAN check if the function call was handled by the tokenReceived or fallback by verifying if the returned value is magic value or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants