gearbox-contracts icon indicating copy to clipboard operation
gearbox-contracts copied to clipboard

_provideCreditAccountAllowance runs each time for tokens with safe96 limit

Open 0xmikko opened this issue 3 years ago • 0 comments

Problem

In current design, before any operation, creditManager checks allowance needed. To do that, it calls _provideCreditAccountAllowance:

  /// @dev Checks that credit account has enough allowance for operation by comparing existing one with x10 times more than needed
    /// @param creditAccount Credit account address
    /// @param toContract Contract to check allowance
    /// @param token Token address of contract
    function _provideCreditAccountAllowance(
        address creditAccount,
        address toContract,
        address token
    ) internal {
        // Get 10x reserve in allowance
        if (
            IERC20(token).allowance(creditAccount, toContract) <
            Constants.MAX_INT_4
        ) {
            ICreditAccount(creditAccount).approveToken(token, toContract); // T:[CM-35]
        }
    }

However, some tokens (UNI, COMP, GEAR etc.) has check which doesn't allow to set allowance more than 2^96:

/**
     * @notice Approve `spender` to transfer up to `amount` from `src`
     * @dev This will overwrite the approval amount for `spender`
     *  and is subject to issues noted [here](https://eips.ethereum.org/EIPS/eip-20#approve)
     * @param spender The address of the account which may transfer tokens
     * @param rawAmount The number of tokens that are approved (2^256-1 means infinite)
     * @return Whether or not the approval succeeded
     */
    function approve(address spender, uint256 rawAmount)
        external
        returns (bool)
    {
        uint96 amount;
        if (rawAmount == uint256(-1)) {
            amount = uint96(-1);
        } else {
            amount = safe96(rawAmount, "Gear::approve: amount exceeds 96 bits");
        }

        allowances[msg.sender][spender] = amount;

        emit Approval(msg.sender, spender, amount);
        return true;
    }

As result, approve function in such tokens set allowance equals uint96(-1) and next time this parameters asks to call it one more time, cause uint96(-1) is less than Constants.MAX_INT_4.

Solution

There are two possible solutions:

  • Reduce limit to 2^64 (for example), which is still huge number, and it would solve the task.
  • Completly remove function _provideCreditAccountAllowance and ask users to check allowance, and if it's not enough - call approve function in CreditManager.sol:

0xmikko avatar Jan 10 '22 11:01 0xmikko