Wrapper Code Style Findings

WRE-01C: Ineffectual Usage of Safe Arithmetics

Type	Severity	Location
Language Specific		Wrapper.sol: • I-1: L126 • I-2: L305 • I-3: L586

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
126uint256 nftId = ++_nftCounter;

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

An unchecked code block has been introduced around each relevant arithmetic operation optimizing their gas cost safely.

WRE-02C: Inefficient mapping Lookups

Type	Severity	Location
Gas Optimization		Wrapper.sol: • I-1: L76, L78 • I-2: L85, L87 • I-3: L278, L296

Description:

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

Example:

Copy
75function approveSpenderOfApprovedAssets(address spender) external {
76    if (_isApprovedForSpendingApprovedAssets[_msgSender()][spender]) revert SpenderAlreadyApprovedOrUnapproved(spender);
77
78    _isApprovedForSpendingApprovedAssets[_msgSender()][spender] = true;
79
80    emit SpenderApprovedOrUnapproved(spender, _msgSender(), true);
81}

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.

As the compiler's optimizations may take care of these caching operations automatically at-times, we advise the optimization to be selectively applied, tested, and then fully adopted to ensure that the proposed caching model indeed leads to a reduction in gas costs.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

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.

WRE-03C: Suboptimal Conditional

Type	Severity	Location
Gas Optimization		Wrapper.sol:L578-L582

Description:

The referenced if-else structure will evaluate a conditional and effectively yield its value.

Example:

Copy
577function _checkIfSupportedPercentageType(WrappedObjectType wrappedObjectType) internal pure returns (bool) {
578    if (wrappedObjectType == WrappedObjectType.ERC20) {
579        return true;
580    } else {
581        return false;
582    }
583}

Recommendation:

We advise the code to return the if clause's conditional itself, optimizing the code's syntax and potentially its gas cost.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The referenced logical structure was properly replaced by a direct return statement, optimizing its gas cost.

WRE-04C: Unreachable Code

Type	Severity	Location
Gas Optimization		Wrapper.sol: • I-1: L166, L184-L186 • I-2: L218, L223, L228, L234-L236 • I-3: L240, L244, L248, L255-L257 • I-4: L361, L374, L387, L407-L410 • I-5: L436, L441, L452, L464-L467

Description:

By default, Solidity does not permit an enum declaration to exceed its explicitly declared values.

As such, manual handling of an else case which would signal "no type" is incorrect as the code would revert earlier.

Example:

Copy
166} else if (wtype == WrappedObjectType.ERC1155) {
167    _wrapERC1155(nftId, IERC1155(token), id, value, provider);
168
169    (bool contained, uint256 wrappedObjectExistingIndex) = _nftTokens[nftId].tryGet(token);
170
171    if (!contained) {
172        (WrappedObject storage wrappedObject, uint256 wrappedObjectIndex) = _createWrappedObject();
173        _nftTokens[nftId].set(token, wrappedObjectIndex);
174        wrappedObject.wtype = WrappedObjectType.ERC1155;
175        wrappedObject.ids.add(id);
176        wrappedObject.values[id] = value;
177    } else {
178        WrappedObject storage wrappedObject = _wrappedObjects[wrappedObjectExistingIndex];
179        if (wrappedObject.values[id] != 0) revert CannotDoubleWrapSameERC1155TokenId(token, id);
180
181        wrappedObject.ids.add(id);
182        wrappedObject.values[id] = value;
183    }
184} else {
185    revert WrongWrappedType();
186}

Recommendation:

We advise the else branch to be omitted and the else if branch of the ERC1155 object type to be set as an else branch, optimizing the code's gas cost.

The optimization can be similarly applied in all instances of the exhibit.

Alleviation (c6b23c23d8bcd8cce85049ad959cbd711a37126b):

The referenced if-else structures have been properly optimized as advised, addressing this exhibit.