FragmentNFT Code Style Findings

FNF-01C: Duplicate Invocation of Getter Functions

Type	Severity	Location
Gas Optimization		FragmentNFT.sol:L122, L123, L142, L144

Description:

The FragmentNFT::_contractURI and FragmentNFT::_baseURI functions are invoked twice redundantly in the FragmentNFT::tokenURI and FragmentNFT::_contractURI functions respectively.

Example:

Copy
120function tokenURI(uint256 tokenId) public view override returns (string memory) {
121  if (!_exists(tokenId)) revert TOKEN_ID_NOT_EXISTS(tokenId);
122  string memory contractURI_ = string.concat(_contractURI(), "/");
123  return bytes(_contractURI()).length > 0 ? string.concat(contractURI_, tokenId.toString()) : "";
124}

Recommendation:

We advise each function to be invoked once and its result to be stored to a string memory variable that is consequently re-used in place of its second invocation, optimizing each function's gas cost.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The contractURI and baseURI members of the contract are now properly cached to a local string memory variable that is consequently utilized in place of the paired invocations in each case, optimizing the code's gas cost as advised.

FNF-02C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		FragmentNFT.sol:L480, L495, L563

Description:

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

Example:

Copy
562if (arr.length == 0) arr.push();
563return arr[arr.length - 1];

Recommendation:

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

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

All referenced arithmetic statements have been safely wrapped in an unchecked code block, optimizing each code block's gas cost.

FNF-03C: Inefficient Loop Limit Evaluation

Type	Severity	Location
Gas Optimization		FragmentNFT.sol:L208

Description:

The linked for loop evaluates its limit inefficiently on each iteration.

Example:

Copy
208for (uint256 i; i < tagCount.length(); i++) {

Recommendation:

We advise the statements within the for loop limit to be relocated outside to a local variable declaration that is consequently utilized for the evaluation to significantly reduce the codebase's gas cost. We should note the same optimization is applicable for storage reads present in such limits as they are newly read on each iteration (i.e. length members of arrays in storage).

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The referenced loop limit evaluation (tagCount.length()) has been properly cached outside the for loop to a dedicated local variable totalTagCount that is consequently utilized, optimizing the loop's gas cost significantly.

FNF-04C: Inefficient Maintenance of Account Snapshots

Type	Severity	Location
Gas Optimization		FragmentNFT.sol:L427, L452, L561-L564

Description:

In the current structure of the FragmentNFT, the _accountSnapshotIds will always possess an empty 0 entry as its first element which would indicate that the first snapshot ID is the "last" snapshot of all accounts incorrectly.

Example:

Copy
422function _updateAccountSnapshot(address account, uint256 firstTokenId, uint256 batchSize, bool add) private {
423  uint256 currentSnapshot = _currentSnapshotId();
424  EnumerableMap.Bytes32ToUintMap storage currentAccountTagCount = _snapshots[currentSnapshot].accountTagCount[
425    account
426  ];
427  uint256 lastAccountSnapshot = _lastUint256ArrayElement(_accountSnapshotIds[account]);
428  if (lastAccountSnapshot < currentSnapshot) {
429    _copy(_snapshots[lastAccountSnapshot].accountTagCount[account], currentAccountTagCount);
430    _accountSnapshotIds[account].push(currentSnapshot);
431  }
432  for (uint256 i; i < batchSize; i++) {
433    uint256 id = firstTokenId + i;
434    bytes32 tag = tags[id];
435    (, uint256 currentCount) = currentAccountTagCount.tryGet(tag);
436    currentAccountTagCount.set(tag, add ? (currentCount + 1) : (currentCount - 1));
437  }
438}

Recommendation:

We advise the code to simply initialize the _snapshots entry with two empty snapshots rather than one in FragmentNFT::initialize.

This will ensure that the valid FragmentNFT::_currentSnapshotId values start from 1 rather than 0, allowing the FragmentNFT::_lastUint256ArrayElement function to immediately yield 0 instead of pushing an empty element to its arr, significantly optimizing the account snapshot system.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

Our recommendation was applied to the codebase, introducing a second _snapshots.push() statement to the contract's FragmentNFT::initialize function and thus optimizing the account snapshot ID system.

FNF-05C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		FragmentNFT.sol:L427, L430, L452, L455

Description:

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

Example:

Copy
422function _updateAccountSnapshot(address account, uint256 firstTokenId, uint256 batchSize, bool add) private {
423  uint256 currentSnapshot = _currentSnapshotId();
424  EnumerableMap.Bytes32ToUintMap storage currentAccountTagCount = _snapshots[currentSnapshot].accountTagCount[
425    account
426  ];
427  uint256 lastAccountSnapshot = _lastUint256ArrayElement(_accountSnapshotIds[account]);
428  if (lastAccountSnapshot < currentSnapshot) {
429    _copy(_snapshots[lastAccountSnapshot].accountTagCount[account], currentAccountTagCount);
430    _accountSnapshotIds[account].push(currentSnapshot);
431  }
432  for (uint256 i; i < batchSize; i++) {
433    uint256 id = firstTokenId + i;
434    bytes32 tag = tags[id];
435    (, uint256 currentCount) = currentAccountTagCount.tryGet(tag);
436    currentAccountTagCount.set(tag, add ? (currentCount + 1) : (currentCount - 1));
437  }
438}

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 (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

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.

FNF-06C: Loop Iterator Optimizations

Type	Severity	Location
Gas Optimization		FragmentNFT.sol:L184, L208, L234, L286, L365, L457, L549

Description:

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

Example:

Copy
184for (uint256 i; i < 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 (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The referenced loop iterator increment statements have been relocated at the end of each respective for loop's body and have been unwrapped in an unchecked code block, optimizing their gas cost.