Omniscia Nexera Protocol Audit

DatasetNFT Code Style Findings

DatasetNFT Code Style Findings

DNF-01C: Duplicate Invocation of Getter Function

Description:

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

Example:

contracts/DatasetNFT.sol
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

Description:

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

Example:

contracts/DatasetNFT.sol
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

Description:

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

Example:

contracts/DatasetNFT.sol
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.