TNFTMetadata Code Style Findings

TNF-01C: Inefficient Array Shifts

Type	Severity	Location
Gas Optimization		TNFTMetadata.sol:L196-L198, L211, L326

Description:

The referenced array shift operations will assign the last element of the array to the index of the element to-be-removed, however, these operations are inefficiently performed when the index of the element-to-be-removed is the actual last element of the array.

Example:

Copy
320function _removeFromType(uint256 _tnftType, uint256 _indexInType) internal {
321    require(tnftTypes[_tnftType].added, "non-existing tnftType");
322
323    uint256 last = typeFeatures[_tnftType].length - 1; // get last index
324    uint256 lastItem = typeFeatures[_tnftType][last]; // grab last item
325
326    typeFeatures[_tnftType][_indexInType] = lastItem; // set last item to removed item's index
327    typeFeatures[_tnftType].pop(); // remove last item
328}

Recommendation:

We advise all operations to avoid shifting elements if the element-to-be-removed is the last element of the array, performing a direct pop operation instead.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The Tangible team evaluated this exhibit but opted to acknowledge it in the current iteration of the codebase

TNF-02C: Inefficient Index Sequence

Type	Severity	Location
Gas Optimization		TNFTMetadata.sol:L143

Description:

The TNFTMetadata::addFeatures function will inefficiently read the dynamic length member of the featureList array on each iteration.

Example:

Copy
129function addFeatures(
130    uint256[] calldata _featureList,
131    string[] calldata _featureDescriptions
132) external onlyFactoryOwner {
133    uint256 length = _featureList.length;
134    require(length == _featureDescriptions.length, "not the same size");
135
136    for (uint256 i; i < length; ) {
137        uint256 item = _featureList[i];
138        require(!featureInfo[item].added, "already added");
139
140        featureInfo[item].added = true; // added
141        featureInfo[item].description = _featureDescriptions[i]; // set description
142        featureList.push(item); // add to featureList
143        featureIndexInList[item] = featureList.length - 1; // update mapping for removing
144
145        emit FeatureAdded(item, _featureDescriptions[i]);
146
147        unchecked {
148            ++i;
149        }
150    }
151}

Recommendation:

Given that each iteration will sequentially increase the featureList.length value by 1, we advise it to be stored before the loop to a local variable and incremented locally thus optimizing the gas cost of the function significantly.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

The optimization was applied per our recommendation, greatly reducing the gas cost of the function on each iteration beyond the first.

TNF-03C: Inefficient Loop Limit Evaluation

Type	Severity	Location
Gas Optimization		TNFTMetadata.sol:L228

Description:

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

Example:

Copy
228for (uint256 i; i < typeFeatures[_type].length; ) {

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 (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

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

TNF-04C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		TNFTMetadata.sol:L138, L140, L141, L167, L169, L186, L189, L191, L192, L196-L198, L199, L228, L229, L248, L250-L252, L272, L274, L275, L276, L323-L324, L326-L327

Description:

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

Example:

Copy
137uint256 item = _featureList[i];
138require(!featureInfo[item].added, "already added");
139
140featureInfo[item].added = true; // added
141featureInfo[item].description = _featureDescriptions[i]; // set description

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 (1f394a00cc2ed1dc2020a9c07f982cff9029077d):

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.

TNF-05C: Inexistent Error Message

Type	Severity	Location
Code Style		TNFTMetadata.sol:L195

Description:

The linked require check has no error message explicitly defined.

Example:

Copy
195require(_index != type(uint256).max);

Recommendation:

We advise one to be set so to increase the legibility of the codebase and aid in validating the require check's condition.

Alleviation (2ad448279d9e8e4b6edd94bcd2eb22129b6f7357):

A proper error message has been introduced to the referenced require check addressing this exhibit.