Omniscia Mean Finance Audit

TakeManyRunSwapAndTransferMany Code Style Findings

TakeManyRunSwapAndTransferMany Code Style Findings

TMR-01C: Improper Usage of Direct Statements

Description:

The swapper allow-list validation is meant to be performed via the onlyAllowlisted modifier when the swapper is only a single entity, a paradigm not followed by the contract referenced in contrast to the rest of the codebase.

Example:

solidity/contracts/extensions/TakeManyRunSwapAndTransferMany.sol
31function takeManyRunSwapAndTransferMany(TakeManyRunSwapAndTransferManyParams calldata _parameters) public payable virtual {
32  for (uint256 i; i < _parameters.takeFromCaller.length; i++) {
33    // Take from caller
34    TakeFromCaller memory _takeFromCaller = _parameters.takeFromCaller[i];
35    _takeFromMsgSender(_takeFromCaller.token, _takeFromCaller.amount);
36

37    // Approve whatever is necessary
38    _maxApproveSpenderIfNeeded(
39      _takeFromCaller.token,
40      _parameters.allowanceTarget,
41      _parameters.allowanceTarget == _parameters.swapper,
42      _takeFromCaller.amount
43    );
44  }
45

46  // Validate that the swapper is allowlisted
47  _assertSwapperIsAllowlisted(_parameters.swapper);
48

49  // Execute swap
50  _executeSwap(_parameters.swapper, _parameters.swapData, _parameters.valueInSwap);
51

52  // Transfer out whatever was left in the contract
53  for (uint256 i; i < _parameters.transferOutBalance.length; i++) {
54    TransferOutBalance memory _transferOutBalance = _parameters.transferOutBalance[i];
55    _sendBalanceOnContractToRecipient(_transferOutBalance.token, _transferOutBalance.recipient);
56  }
57}

Recommendation:

We advise the function to utilize the onlyAllowlisted modifier properly and to omit the direct validation statements.

Alleviation:

The onlyAllowlisted modifier is now properly invoked, conforming to the paradigm applied to the rest of the codebase.

TMR-02C: Loop Iterator Optimizations

Description:

The linked for loops increment / decrement their iterator "safely" due to Solidity's built - in safe arithmetics(post - 0.8.X).

Example:

solidity/contracts/extensions/TakeManyRunSwapAndTransferMany.sol
32for (uint256 i; i < _parameters.takeFromCaller.length; i++) {

Recommendation:

We advise the increment / decrement operations to be performed in an unchecked code block as the last statement within each for loop to optimize their execution cost.

Alleviation:

All for loops have been appropriately optimized via the usage of the unchecked code block for each iterator's increment.

Prev

SwapperRegistry.sol (SRY-C)

Next

TakeManyRunSwapsAndTransferMany.sol (TMS-C)

On this page