FactoryV2 Code Style Findings

FV2-01C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		FactoryV2.sol:L570

Description:

The linked mathematical operation is guaranteed to be performed safely by surrounding conditionals evaluated in either require checks or if-else constructs.

Example:

Copy
568require(numCategoriesToMint[msg.sender][_tnftType] > 0, "Can't create more");
569// reducing approved tnft creations
570numCategoriesToMint[msg.sender][_tnftType]--;

Recommendation:

Given that safe arithmetics are toggled on by default in pragma versions of 0.8.X, we advise the linked statement to be wrapped in an unchecked code block thereby optimizing its execution cost.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The unchecked code block was appropriately introduced to the referenced subtraction, optimizing its gas cost.

FV2-02C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		FactoryV2.sol:L568, L570

Description:

The linked statements perform key-based lookup operations on mapping declarations from storage multiple times for the same key redundantly.

Example:

Copy
567if (msg.sender != tangibleLabs) {
568    require(numCategoriesToMint[msg.sender][_tnftType] > 0, "Can't create more");
569    // reducing approved tnft creations
570    numCategoriesToMint[msg.sender][_tnftType]--;
571}

Recommendation:

As the lookups internally perform an expensive keccak256 operation, we advise the lookups to be cached wherever possible to a single local declaration that either holds the value of the mapping in case of primitive types or holds a storage pointer to the struct contained.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

All referenced inefficient mapping lookups have been optimized to the greatest extent possible, significantly reducing the gas cost of the functions the statements were located in.

FV2-03C: Loop Iterator Optimization

Type	Severity	Location
Gas Optimization		FactoryV2.sol:L522

Description:

The linked for loop increments / decrements the iterator "safely" due to Solidity's built-in safe arithmetics (post-0.8.X).

Example:

Copy
522for (uint256 i = 0; i < tokenIdsLength; i++) {

Recommendation:

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

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The referenced loop iterator's increment statement has been relocated at the end of the for loop's body and has been unwrapped in an unchecked code block, optimizing its gas cost.

FV2-04C: Redundant Conditional Evaluation

Type	Severity	Location
Gas Optimization		FactoryV2.sol:L489

Description:

The referenced else-if clause is redundant as the preceding if clause guarantees its validity.

Example:

Copy
486if (marketplace != msg.sender) {
487    require(voucher.vendor == msg.sender, "MFSE");
488    mintCount = voucher.mintCount;
489} else if (marketplace == msg.sender) {

Recommendation:

We advise the code to replace the else-if clause with a simple else clause, optimizing its gas cost.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The redundant else-if clause has been properly omitted, optimizing the code's gas cost.

FV2-05C: Redundant Parenthesis Statements

Type	Severity	Location
Code Style		FactoryV2.sol:L198

Description:

The referenced statements are redundantly wrapped in parenthesis' (()).

Example:

Copy
198(tangibleLabs == msg.sender) || (marketplace == msg.sender),

Recommendation:

We advise them to be safely omitted, increasing the legibility of the codebase.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The redundant parenthesis statements have been safely omitted, increasing the legibility of the codebase.

FV2-06C: Sub-Optimal Fetching of Price

Type	Severity	Location
Gas Optimization		FactoryV2.sol:L426-L431

Description:

The FactoryV2::adjustStorageAndGetAmount function will fetch the tokenPrice associated with the tnft and paymentToken, however, it may not ultimately utilize it.

Example:

Copy
420function adjustStorageAndGetAmount(
421    ITangibleNFT tnft,
422    IERC20Metadata paymentToken,
423    uint256 tokenId,
424    uint256 _years
425) external onlyMarketplace returns (uint256) {
426    (uint256 tokenPrice, , ) = priceManager.oracleForCategory(tnft).usdPrice(
427        tnft,
428        paymentToken,
429        0,
430        tokenId
431    );
432
433    bool storagePriceFixed = tnft.adjustStorage(tokenId, _years);
434    //amount to pay
435    uint256 amount;
436    uint8 decimals = tnft.storageDecimals();
437
438    if (storagePriceFixed) {
439        amount =
440            toDecimals(tnft.storagePricePerYear(), decimals, paymentToken.decimals()) *
441            _years;
442    } else {
443        require(tokenPrice > 0, "Price 0");
444        amount =
445            (tokenPrice * tnft.storagePercentagePricePerYear() * _years) /
446            (100 * (10 ** decimals));
447    }
448    return amount;
449}

Recommendation:

We advise the referenced fetch operation to be relocated to the else block of the FactoryV2::adjustStorageAndGetAmount function, optimizing its gas cost when the storagePriceFixed value is true.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The tokenPrice fetching operation was relocated in the else block it is actively utilized in, optimizing the function's gas cost in the if execution path.

FV2-07C: Unreachable Code

Type	Severity	Location
Gas Optimization		FactoryV2.sol:L359-L361

Description:

The if-else-if block of the FactoryV2::_setContract function will properly handle all enum entries of the FACT_ADDRESSES enum, meaning that the referenced else clause is unreachable.

Example:

Copy
355} else if (_contractId == FACT_ADDRESSES.CURRENCY_FEED) {
356    // 8
357    old = currencyFeed;
358    currencyFeed = _contractAddress;
359} else {
360    revert("Incorrect _contractId input");
361}

Recommendation:

We advise the code to eliminate the referenced else clause and to adjust the last 8 case as a simple else clause given that Solidity does not permit a value greater than the maximum permitted by the FACT_ADDRESSES enum to be passed in as a FACT_ADDRESSES function argument.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The unreachable code segment was properly omitted from the code, reducing the bytecode size of the contract.