Zellic - Audit Reports

Category: Optimization

Unnecessary approvals of zero tokens

Informational Impact

Informational Severity

N/A Likelihood

Description

In multiple places, the contracts first approve zero amount of a token and then approve the intended amount:

IERC20(params.borrowToken).approve(address(swapRouter), 0);
IERC20(params.borrowToken).approve(address(swapRouter), borrowAmount);

This is unnecessary, as the second approval would overwrite the first one — for example, in OpenZeppelin's ERC20 contract↗:

function _approve(address owner, address spender, uint256 value, bool emitEvent) internal virtual {
    if (owner == address(0)) {
        revert ERC20InvalidApprover(address(0));
    }
    if (spender == address(0)) {
        revert ERC20InvalidSpender(address(0));
    }
!    _allowances[owner][spender] = value;
    if (emitEvent) {
        emit Approval(owner, spender, value);
    }
}

Impact

Many of the functions in the two contracts will make one or more unnecessary external calls to approve zero tokens, which will increase the gas cost of these functions.

Recommendations

Remove the first approval of zero tokens, as it is unnecessary.

Remediation

Solera noted that USDT has the following requirement↗:

function approve(address _spender, uint _value) public onlyPayloadSize(2 * 32) {

    // To change the approve amount you first have to reduce the addresses`
    //  allowance to zero by calling `approve(_spender, 0)` if it is not
    //  already 0 to mitigate the race condition described here:
    //  https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
    require(!((_value != 0) && (allowed[msg.sender][_spender] != 0)));

    allowed[msg.sender][_spender] = _value;
    Approval(msg.sender, _spender, _value);
}

However, we were informed that only the following tokens would be used: WPLUME, SPLUME, WETH, PETH, Nest assets (NRWA, NYIELD, NTBILL), USDC, and PUSD.