Solidity < 0.8.0 doesn't revert for ERC20 tokens which returns uint instead of bool for token transfer (transfer and transferfrom)

Hello, Everyone, I'm a little confused over an issue, it's regarding BAD ERC20 tokens.
So, I have a token which returns uint value instead of bool for transfer functions.

reference:

    function transfer(address recipient, uint256 amount) public virtual override returns(uint256){
        _transfer(_msgSender(), recipient, amount);
        return amount;
    }

    function transferFrom(
        address sender,
        address recipient,
        uint256 amount
    ) public virtual override returns(uint256){
        _transfer(sender, recipient, amount);

        uint256 currentAllowance = _allowances[sender][_msgSender()];
        require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance");
        unchecked {
            _approve(sender, _msgSender(), currentAllowance - amount);
        }
        
        return amount;

    }
    function _transfer(
        address sender,
        address recipient,
        uint256 amount
    ) internal virtual {
        require(sender != address(0), "ERC20: transfer from the zero address");
        require(recipient != address(0), "ERC20: transfer to the zero address");

        _beforeTokenTransfer(sender, recipient, amount);

        uint256 senderBalance = _balances[sender];
        require(senderBalance >= amount, "ERC20: transfer amount exceeds balance");
        unchecked {
            _balances[sender] = senderBalance - amount;
        }
        _balances[recipient] += amount;

        emit Transfer(sender, recipient, amount);

        _afterTokenTransfer(sender, recipient, amount);
    }

And now this is giving me different results for compiler < 0.8 and >=0.8.
For <0.8, the transfer still works fine in the testing contract(ref. below).
But >=0.8 rightly reverts.

Ref, Testing Contract:

interface IERC20 {
    /**
     * @dev Returns the amount of tokens in existence.
     */
    function totalSupply() external view returns (uint256);

    /**
     * @dev Returns the amount of tokens owned by `account`.
     */
    function balanceOf(address account) external view returns (uint256);

    /**
     * @dev Moves `amount` tokens from the caller's account to `recipient`.
     *
     * Returns a boolean value indicating whether the operation succeeded.
     *
     * Emits a {Transfer} event.
     */
    function transfer(address recipient, uint256 amount) external returns (bool);

    /**
     * @dev Returns the remaining number of tokens that `spender` will be
     * allowed to spend on behalf of `owner` through {transferFrom}. This is
     * zero by default.
     *
     * This value changes when {approve} or {transferFrom} are called.
     */
    function allowance(address owner, address spender) external view returns (uint256);

    /**
     * @dev Sets `amount` as the allowance of `spender` over the caller's tokens.
     *
     * Returns a boolean value indicating whether the operation succeeded.
     *
     * IMPORTANT: Beware that changing an allowance with this method brings the risk
     * that someone may use both the old and the new allowance by unfortunate
     * transaction ordering. One possible solution to mitigate this race
     * condition is to first reduce the spender's allowance to 0 and set the
     * desired value afterwards:
     * https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
     *
     * Emits an {Approval} event.
     */
    function approve(address spender, uint256 amount) external returns (bool);

    /**
     * @dev Moves `amount` tokens from `sender` to `recipient` using the
     * allowance mechanism. `amount` is then deducted from the caller's
     * allowance.
     *
     * Returns a boolean value indicating whether the operation succeeded.
     *
     * Emits a {Transfer} event.
     */
    function transferFrom(
        address sender,
        address recipient,
        uint256 amount
    ) external returns (bool);

    /**
     * @dev Emitted when `value` tokens are moved from one account (`from`) to
     * another (`to`).
     *
     * Note that `value` may be zero.
     */
    event Transfer(address indexed from, address indexed to, uint256 value);

    /**
     * @dev Emitted when the allowance of a `spender` for an `owner` is set by
     * a call to {approve}. `value` is the new allowance.
     */
    event Approval(address indexed owner, address indexed spender, uint256 value);
}

contract TokenTest{
  
    
    function testTransfer(address tokenAdd, address from, uint256 amount) external{
       bool xfer = IERC20(tokenAdd).transferFrom(from, address(this), amount);
       require(xfer, "ERC20_FALSE");

    }
}

Kindly help me understand what's the actual issue?

1 Like

So, I have figured out some part of the issue. Basically, if you try to transfer an amount > 0, the return value of the call, will assign a true value to this boolean xfer
bool xfer = IERC20(tokenAdd).transferFrom(from, address(this), amount);

And, for 0, it will assign it false, and the require check will fail. And now this has been patched in 0.8. Now i need to know, what exactly has been patched.

It's probably because on 0.8.0 your contract is using ABI coder v2. See Solidity v0.8.0 Breaking Changes> Silent Changes of the Semantics:

ABI coder v2 is activated by default.

You can choose to use the old behaviour using pragma abicoder v1; . The pragma pragma experimental ABIEncoderV2; is still valid, but it is deprecated and has no effect. If you want to be explicit, please use pragma abicoder v2; instead.

Note that ABI coder v2 supports more types than v1 and performs more sanity checks on the inputs. ABI coder v2 makes some function calls more expensive and it can also make contract calls revert that did not revert with ABI coder v1 when they contain data that does not conform to the parameter types.

You can verify by using pragma abicoder v1 to switch back to the old encoder.

1 Like