plain `approve` will impact `DepositBatch` for USDT token when multi-chain deployment active

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x3b176f1644b312569f5ab491f6c1d4101a3dad77de404565732eeac32fdb36ca Severity: medium

Description: Description:

In DepositBatch, multiTokenSwapAndTransfer function, there is a plain Approve ERC20 function which need to be concerned.

When the _token being approved is USDT in Mainnet, it's a well-known issue it will be reverted. The token have limitations when it comes to modifying the allowance from a non-zero value. For instance, the approve() function of Tether (USDT) will throw an error if the current approval is not set to zero.

Based on velvet capital docs, with current depoloyment (BSC and Arbitrum) this USDT is not being an issue, but the Intent OS is aim for multi-chain deployment.

File: DepositBatch.sol
28:   function multiTokenSwapAndTransfer(
29:     FunctionParameters.BatchHandler memory data
30:   ) external payable nonReentrant{
...
50:     // Perform swaps and calculate deposit amounts for each token
51:     for (uint256 i; i < tokenLength; i++) {
52:       address _token = tokens[i];
53:       uint256 balance;
54:       if (_token == _depositToken) {
55:         //Sending encoded balance instead of swap calldata
56:         balance = abi.decode(data._callData[i], (uint256));
57:       } else {
58:         (bool success, ) = SWAP_TARGET.delegatecall(data._callData[i]);
59:         if (!success) revert ErrorLibrary.CallFailed();
60:         balance = _getTokenBalance(_token, address(this));
61:       }
62:       if (balance == 0) revert ErrorLibrary.InvalidBalanceDiff();
63: @>    IERC20(_token).approve(data._target, balance);
64:       depositAmounts[i] = balance;
65:     }

Impact:

Approval of USDT token will fail, deposit will not success for any USDT token on mainnet

Mitigation:

Recommend setting allowance to zero first before setting it to a non zero value, or use a safe increase allowance

[aj07]

For the approval of USDT,, which chain you think this will create an issue ?

We will deploy this version for EVM chain only mostly Eth, Base, BNB and arb followed by other EVM chain..

[chainNue]

For the approval of USDT,, which chain you think this will create an issue ?

We will deploy this version for EVM chain only mostly Eth, Base, BNB and arb followed by other EVM chain..

For sure, the USDT on Ethereum mainnet will be affected. Base, BSC, Arb are not affected. Other EVM, need to check USDT contract's code.

[langnavina97]

Thank you for submitting the issue. We've resolved it and pushed the changes, which can be found here: https://github.com/Velvet-Capital/velvet-core/commit/25ef3e7a2bdf56f57901dba2f84ebf5ed5df7835

@chainNue