Omniscia vfat Audit

FlashloanStrategy Code Style Findings

FlashloanStrategy Code Style Findings

FSG-01C: Illegible Selector Extraction

Description:

The FlashloanStrategy::extractSelector function represents an assembly block that extracts the function selector of a bytes payload.

Example:

contracts/strategies/FlashloanStrategy.sol
709function extractSelector(bytes memory params)
710    public
711    pure
712    returns (bytes4 selector)
713{
714    assembly {
715        // 1. Load 4 bytes from `params`
716        // 2. Shift the bytes left by 224 bits/28 bytes so that they're at
717        //    the beginning of the 32-byte memory slot as required by
718        //    Solidity ABI spec
719        // 3. Store the result in `selector`
720        selector := shl(224, mload(add(params, 4)))
721    }
722}

Recommendation:

We advise this to be achieved via Solidity's built-in syntax and namely slices by assigning the value of params[:4] to the selector variable after casting to the bytes4 data type.

Alleviation (6ab7af3bb4):

The vfat team evaluated this exhibit and stated that the optimization is inapplicable due to the input argument’s memory data location.

In this case, we advise the bytes4 cast to be performed directly on the params argument thus increasing the legibility of the codebase whilst minimizing usage of assembly blocks and achieving the same end result.

Alleviation (986c6b0a71):

The code was updated per our recommendation, optimizing its syntax whilst retaining the same computational result.

FSG-02C: Inexecutable Code

Description:

The FlashloanStrategy::initiateFlashloan function contains an else branch that will never be executed as the decoding process of an enum will prevent non-explicit values from being stored in it.

Example:

contracts/strategies/FlashloanStrategy.sol
235if (providerType == FlashloanProvider.AAVEV2) {
236    uint256[] memory modes = new uint256[](assets.length);
237    // mode 0 = no debt incurred, everything repaid at end of flashloan
238    bytes memory aaveV2params = abi.encode(sickleAddress, params);
239    // storing the hash of the callback data for safety checks
240    flashloanDataHash = keccak256(aaveV2params);
241    ILendingPoolV2(aaveV2LendingPool).flashLoan(
242        address(this), // flashloan receiver
243        assets,
244        amounts,
245        modes,
246        address(0), // onBehalfOf variable
247        aaveV2params,
248        0 // referral code
249    );
250} else if (providerType == FlashloanProvider.AAVEV3) {
251    bytes memory aaveV3params = abi.encode(sickleAddress, params);
252    // storing the hash of the callback data for safety checks
253    flashloanDataHash = keccak256(aaveV3params);
254    if (assets.length == 1) {
255        IPoolV3(aaveV3LendingPool).flashLoanSimple(
256            address(this), // flashloan receiver
257            assets[0],
258            amounts[0],
259            aaveV3params,
260            0 // referral code
261        );
262    } else {
263        uint256[] memory modes = new uint256[](assets.length);
264        // mode 0 = no debt incurred, everything repaid at end of
265        // flashloan
266        IPoolV3(aaveV3LendingPool).flashLoan(
267            address(this), // flashloan receiver
268            assets,
269            amounts,
270            modes,
271            address(0),
272            aaveV3params,
273            0 // referral code
274        );
275    }
276} else if (providerType == FlashloanProvider.BALANCER) {
277    bytes memory balancerParams = abi.encode(sickleAddress, params);
278    // storing the hash of the callback data for safety checks
279    flashloanDataHash = keccak256(balancerParams);
280    IBalancerVault(balancerVault).flashLoan(
281        this, assets, amounts, balancerParams
282    );
283} else if (providerType == FlashloanProvider.UNIV2) {
284    if (assets.length != 2) revert NotAnAssetPair();
285    address poolAddress = IUniswapV2Factory(quickswapFactoryAddr)
286        .getPair(assets[0], assets[1]);
287    (,, uint256[] memory premiums,) =
288        abi.decode(params[4:], (address[], uint256[], uint256[], bytes));
289    bytes memory uniswapFlashParams = abi.encode(
290        sickleAddress, poolAddress, assets, amounts, premiums, params
291    );
292    // storing the hash of the callback data for safety checks
293    flashloanDataHash = keccak256(uniswapFlashParams);
294    IUniswapV2Pair(poolAddress).swap(
295        amounts[0], amounts[1], address(this), uniswapFlashParams
296    );
297} else if (providerType == FlashloanProvider.UNIV3) {
298    if (assets.length != 2) revert NotAnAssetPair();
299    address poolAddress = IUniswapV3Factory(uniswapV3FactoryAddr)
300        .getPool(assets[0], assets[1], providerFee);
301    (,, uint256[] memory premiums,) =
302        abi.decode(params[4:], (address[], uint256[], uint256[], bytes));
303    bytes memory uniswapFlashParams = abi.encode(
304        sickleAddress, poolAddress, assets, amounts, premiums, params
305    );
306    // storing the hash of the callback data for safety checks
307    flashloanDataHash = keccak256(uniswapFlashParams);
308    IUniswapV3Pool(poolAddress).flash(
309        address(this), amounts[0], amounts[1], uniswapFlashParams
310    );
311} else if (providerType == FlashloanProvider.MORPHO) {
312    if (assets.length != 1) revert NotSingleAsset();
313    bytes memory morphoParams =
314        abi.encode(sickleAddress, assets[0], params);
315    // storing the hash of the callback data for safety checks
316    flashloanDataHash = keccak256(morphoParams);
317    IMorpho(morpho).flashLoan(assets[0], amounts[0], morphoParams);
318} else {
319    revert UnauthorizedOperation();
320}

Recommendation:

We advise the else branch to be omitted, and the last else-if branch to be converted to an else branch optimizing the code's gas cost.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The inexecutable else branch has been omitted from the codebase and the else if branch of the MORPHO provider type has been optimized per our recommendations.

FSG-03C: Redundant Zero-Value Assignments

TypeSeverityLocation
Gas OptimizationFlashloanStrategy.sol:
• I-1: L624
• I-2: L642
• I-3: L658
• I-4: L675
• I-5: L690

Description:

All values of the premiums array on initialization are zero rendering assignments of zero to be inefficient.

Example:

contracts/strategies/FlashloanStrategy.sol
684for (uint256 i; i < length;) {
685    if (amounts[i] > 0) {
686        premiums[i] =
687            ((amounts[i] * providerFee) / 10_000 / 100) + 1; // hundredths
688            // of basis points
689    } else {
690        premiums[i] = 0;
691    }
692

693    unchecked {
694        ++i;
695    }
696}

Recommendation:

We advise all referenced assignments to be omitted, optimizing the code's gas cost and legibility.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

All redundant zero-value assignments have been omitted as advised.

FSG-04C: Repetitive Value Literal

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

contracts/strategies/FlashloanStrategy.sol
621/ 10_000

Recommendation:

We advise it to be set to a constant variable instead, optimizing the legibility of the codebase.

In case the constant has already been declared, we advise it to be properly re-used across the code.

Alleviation (6ab7af3bb495b817ffec469255ea679b1813eecb):

The referenced value literal 10_000 has been properly relocated to a contract-level constant declaration labelled BPS_BASIS, optimizing the code's legibility.

Prev

FarmStrategy.sol (FSY-C)

Next

LendingMigrator.sol (LMR-C)

On this page