1

According to the ERC-20 Token Standard, functions transfer and transferFrom should return a boolean value indicating success or failure:

interface IERC20Token {
    function transfer(address _to, uint256 _value) public returns (bool);
    function transferFrom(address _from, address _to, uint256 _value) public returns (bool);
}

Nevertheless, there are quite a few ERC-20 Token contracts which were deployed on mainnet without conforming to this requirement, i.e., without returning anything.

Suppose I use this interface in order to call these functions on the address of such contract:

IERC20Token token = IERC20Token(someAddress);
token.transfer(someAccount, someAmount);

And of course, because I'm not sure whether or not it returns a bool, I simply ignore the return-value in my code.

The compiler, however, possibly allocates a slot on the stack for this return-value.

Could this possibly yield any sort of unexpected behavior?

Of course, I can force my code to assume that this function does not return anything, by declaring this interface:

interface INonStandardERC20Token {
    function transfer(address _to, uint256 _value) public;
    function transferFrom(address _from, address _to, uint256 _value) public;
}

And then using it:

INonStandardERC20Token token = INonStandardERC20Token(someAddress);
token.transfer(someAccount, someAmount);

But the thing is, I have no knowledge as to which one of these interfaces is implemented by the ERC-20 Token deployed at someAddress.

So I'm even more worried about unexpected behavior in my second example, since in this case the compiler surely doesn't allocate any slot for the return-value, yet the function can possibly return something.

Any idea which approach of the two given above is safe?

Could the returndatasize instruction possibly be useful here?

Thank you very much!

1

OK, just found this very useful post which refers exactly to this specific problem!

The proposed solution does indeed make use of the returndatasize instruction (that's how I found it, but then again, I've only become aware of this instruction while tagging this question, so I guess I had to go through the process of writig it in order to get to the answer).

In any case, the solution in that post is given as an image rather than a copy-pastable code, so I tracked it down in this GitHub repository, should anyone find it useful.


UPDATE:

I just realized that I haven't actually answered the question of which approach is safe to use.

So I conducted a small experiment.

For the given contract:

pragma solidity 0.4.25; 

contract GoodToken {
    event Transfer(address indexed _from, address indexed _to, uint256 _value);
    function transfer(address _to, uint256 _value) public returns (bool) {
        emit Transfer(msg.sender, _to, _value);
        return true;
    }
}

contract BadToken {
    event Transfer(address indexed _from, address indexed _to, uint256 _value);
    function transfer(address _to, uint256 _value) public {
        emit Transfer(msg.sender, _to, _value);
    }
}

contract Caller {
    function transferGoodToken(address _token, address _to, uint256 _value) public {
        GoodToken(_token).transfer(_to, _value);
    }
    function transferBadToken(address _token, address _to, uint256 _value) public {
        BadToken(_token).transfer(_to, _value);
    }
}

The following test:

contract("test", function(accounts) {
    it("test", async function() {
        const goodToken = await artifacts.require("GoodToken").new();
        const badToken  = await artifacts.require("BadToken" ).new();
        const caller    = await artifacts.require("Caller"   ).new();
        console.log();
        await test("cast good token to good token:", caller.transferGoodToken, goodToken);
        await test("cast bad  token to good token:", caller.transferGoodToken, badToken );
        await test("cast good token to bad  token:", caller.transferBadToken , goodToken);
        await test("cast bad  token to bad  token:", caller.transferBadToken , badToken );
    });

    async function test(title, func, token) {
        try {
            await func(token.address, accounts[0], 0);
            console.log(title, "passed");
        }
        catch (error) {
            console.log(title, error.message);
        }
    }
});

Prints:

cast good token to good token: passed
cast bad  token to good token: VM Exception while processing transaction: revert
cast good token to bad  token: passed
cast bad  token to bad  token: passed

So calling a transfer function which possibly doesn't return anything using an interface of it which returns bool is most certainly a risk (though it's not a risk of unexpected behavior, but simply a risk of failure).

The other approach (calling a transfer function which possibly returns bool using an interface of it which doesn't return anything) seems to complete without reverting, but of course, this is not a proof of safety.

  • Newer version of solidity compiler will enforce that calls should return the expected data size (since solc 0.5 I think). So for example if you call transfer in a "bad" token using standard ERC20 interface it will revert since it expects 32 bytes (for the bool) and it receives 0 instead. Returning more data than expected should be OK, the extra data will be ignored. – Ismael Dec 19 '19 at 5:02
  • @Ismael: Thank you! I just realized that I haven't actually answered the question of which approach is safe to use. Can you please see my updated answer and tell me if it aligns with what you know? It does seem to align with what you wrote, but I'd still like to know if the opposite approach (calling transfer of a "good" token using a "bad" interface) is indeed safe. – goodvibration Dec 19 '19 at 5:31
  • Yes, it matches my experience if you use the "old" interface (without returns (bool)) you can call good and bad tokens. If you use the "new" interface you can only call the good tokens. A problem using the "old" interface with good tokens is that if it returns false you will be ignoring the result. Since solc 0.5 call returns two values the status of the call and the returned data as a byte array so you can checks returned data length and convert it to bool using abi.decode. It is a bit easier than calling RETURNDATASIZE from assembly. – Ismael Dec 19 '19 at 14:27
  • 1
    @Ismael: That would not solve the problem of not having a return value (success or failure) to begin with. But it's nice to know that I won't need to use assembly on 0.5 onward. Thanks!!! – goodvibration Dec 19 '19 at 14:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.