3

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.

2 Answers 2

6

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.

2
  • thanks for the detailed answer! but in your example you can't have contract BadERC20Token is IERC20 because it has a different return value then the transfer function defined in the interface and it would not compile. so i still don't get this check in SafeERC20 because it done on IERC20 tokens only. Oct 8, 2022 at 18:46
  • @LironAchdut although BadERC20 Token is not derived from IERC20 nor it ever will, it is still compliant from perspective of EVM when you call external contracts because the return-type is NOT part of a function selector. And in fact, there are tons of token out there which follow this pattern but still can be used on UniSwap. Look also here: medium.com/coinmonks/…
    – itinance
    Oct 13, 2022 at 16:03
1

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

3
  • thanks! still i don't get why this check is done on tokens that implement IERC20, because their transfer functions have to return a bool in order for the contract to compile Oct 8, 2022 at 18:48
  • 1
    Because you can’t know ahead if the contract does inherit from OpenZepplin IERC20, in the article I sent you can see that several contracts didn’t inherit from the transfer function that returns bool, they returned nothing. @LironAchdut
    – matank001
    Oct 8, 2022 at 18:53
  • 1
    @LironAchdut I think I understand your question, SafeERC20 is not like an ERC20 implementation. It’s like a safe API to interact with ERC20 contracts
    – matank001
    Oct 10, 2022 at 22:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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