DatasetNFT Code Style Findings

DNF-01C: Duplicate Invocation of Getter Function

Type	Severity	Location
Gas Optimization		DatasetNFT.sol:L107, L108

Description:

The DatasetNFT::_contractURI functions are invoked twice redundantly in the DatasetNFT::tokenURI function.

Example:

Copy
105function tokenURI(uint256 tokenId) public view override(ERC721Upgradeable, IDatasetNFT) returns (string memory) {
106  if (!_exists(tokenId)) revert TOKEN_ID_NOT_EXISTS(tokenId);
107  string memory contractURI_ = string.concat(_contractURI(), "/");
108  return bytes(_contractURI()).length > 0 ? string.concat(contractURI_, tokenId.toString()) : "";
109}

Recommendation:

We advise the 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 the function's gas cost.

Alleviation (fb50b5c39665f7df086b2de1fdbf93ba2d836bf9):

The contractURI of the contract is now properly cached to a local string memory variable that is consequently utilized in place of the two invocations, optimizing the code's gas cost as advised.

DNF-02C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		DatasetNFT.sol:L162, L163, L166, L167, L170, L171

Description:

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

Example:

Copy
160function setManagers(uint256 id, ManagersConfig calldata config) external onlyTokenOwner(id) {
161  bool changed;
162  if (configurations[id].subscriptionManager != config.subscriptionManager) {
163    proxies[id].subscriptionManager = _cloneAndInitialize(config.subscriptionManager, id);
164    changed = true;
165  }
166  if (configurations[id].distributionManager != config.distributionManager) {
167    proxies[id].distributionManager = _cloneAndInitialize(config.distributionManager, id);
168    changed = true;
169  }
170  if (configurations[id].verifierManager != config.verifierManager) {
171    proxies[id].verifierManager = _cloneAndInitialize(config.verifierManager, id);
172    changed = true;
173  }
174  if (changed) {
175    configurations[id] = config;
176    emit ManagersConfigChange(id);
177  }
178}

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.

DNF-03C: Loop Iterator Optimization

Type	Severity	Location
Gas Optimization		DatasetNFT.sol:L192

Description:

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

Example:

Copy
192for (uint256 i; i < models.length; 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 (fb50b5c396):

The referenced iterator increment statement has been optimized, however, a continue case was newly introduced that will cause the loop to never terminate thus causing a bug to manifest.

We advise the i iterator to be utilized in the first two statements of the for loop that would assign the model and percentage so as to increment it in the unchecked code block before the remaining statements of the loop's body, optimizing the code whilst ensuring it behaves as expected.

Alleviation (938d6b02ca):

The code was re-ordered as advised, ensuring that the i iterator is incremented before the ensuing continue statement and thus no longer causing a nonterminal loop.

As such, we consider this exhibit's optimization applied in full.