Why does SafeERC20 assume that the token's transfer and transferFrom return value is optional?

Question

In openZeppelin's SafeERC20, the functions safeTransfer and safeTransferFrom call an internal function called _callOptionalReturn, which makes a low-level call to the token's transfer and transferFrom functions and checks that the return value is true, if there is a return value.

This is the code:

/**
 * @dev Imitates a Solidity high-level call (i.e. a regular function call to a contract), relaxing the requirement
 * on the return value: the return value is optional (but if data is returned, it must not be false).
 * @param token The token targeted by the call.
 * @param data The call data (encoded using abi.encode or one of its variants).
 */
function _callOptionalReturn(IERC20 token, bytes memory data) private {
    // We need to perform a low level call here, to bypass Solidity's return data size checking mechanism, since
    // we're implementing it ourselves. We use {Address-functionCall} to perform this call, which verifies that
    // the target address contains contract code and also asserts for success in the low-level call.

    bytes memory returndata = address(token).functionCall(data, "SafeERC20: low-level call failed");
    if (returndata.length > 0) {
        // Return data is optional
        require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed");
    }
}

Why is there a check for the return value? Isn't there always a Boolean return value for ERC20 transfer and transferFrom?

This is what is defined in the ERC20 standard, and also in the IERC20 interface that this contract references.

Answer 1

Quick answer: there is NOT always a return value.

Long answer:

Although it is not fully ERC20-compliant, there are lots of ERC20-tokens out there which don't return anything. Instead, they just are built like BadERC20Token in the following examples:

contract FullCompliantERC20Token {
  function transfer() returns (bool) { return true; }
}

contract BadERC20Token {
  function transfer() {}
}

Let's say you perform an external function call to a transfer-function in your DEX, and you would expect the function to return a boolean value, then there might be some garbage in this return value that has been existed before that call in memory if this function is not explicitly return a value. This is possible because a return value of a function is not part of the actual function selector! So a function transfer() without a return value and a function transfer() returns(bool) have the same function selector although they are still different. They will be executed the same way, but in the version of non-return-value the return value will contain random garbage and leads to errors.

Actually it is a big problem since Byzantium hard fork in 2017. Before that the return value was a constant value from the function call itself, but since Byzantinum the return value is sort of random and would lead to errors and even security issues.

In order to really check wether the transfer-function really intentionally returned a boolean value or not, the only chance is to check the RETURNDATASIZE, which is an EVM opcode which was invented with Byzantinum fork.

And this is happening in SafeERC20: it simply checks, wether a boolean return value exists or not. Only if a return value exists, it will evaluate this value and either revert or not. This way it gives the calling contract more reliability and safety.

Answer 2

Well, as you can see the function is called _callOptionalReturn. Even though the standard specifies that a return value is needed some contracts might think that ERC20 tokens are like a jungle that you can do whatever you want, And: don’t return bool value in their transfer function.

So, if you expect a bool to return but the function returns nothing, bad things might happen like accessing memory you didn’t intend to access.

For further reading you can look here: https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca