Wrapper Code Style Findings

WRA-01C: Redundant Return Variable

Description:

The linked function implementations contain an explicitly named return variable yet it remains unused in the function bodies.

Example:

Copy
51function historyAt(uint256 _index) public view returns (ERC721Shares _shares)
52{
53    return ERC721Shares(history.at(_index));
54}

Recommendation:

We advise that the variable names of the return declarations are omitted from the codebase to ensure no redundant space is reserved in the EVM.

Alleviation:

The Nftfy team responded by stating that these types of variables exist to conform to their coding guidelines and to provide background for the eventual documentation of the system.

WRA-02C: Non-Standard Naming Convention

Description:

The linked function is exposed as public yet is preceded by an underscore (_) in its name declaration.

Example:

Copy
56function _insert(address _from, uint256 _tokenId, bool _remnant, ERC721Shares _shares) public onlyOwner
57{
58    require(shares[_tokenId] == ERC721Shares(0));
59    shares[_tokenId] = _shares;
60    history.add(address(_shares));
61    address _holder = _remnant ? _from : address(_shares);
62    _safeMint(_holder, _tokenId);
63    IERC721Metadata _metadata = IERC721Metadata(address(target));
64    string memory _tokenURI = _metadata.safeTokenURI(_tokenId);
65    _setTokenURI(_tokenId, _tokenURI);
66}

Recommendation:

We advise the underscore to be removed to comply with the official Solidity style guide.

Alleviation:

The Nftfy team responded by stating that these functions conform to the notion that the functions are meant to be "privileged" and thus not publicly accessible by the general userbase.

WRA-03C: Inexistent Error Messages

Description:

The linked require invocations are not accompanied by any error messages.

Example:

Copy
63IERC721Metadata _metadata = IERC721Metadata(address(target));
64    string memory _tokenURI = _metadata.safeTokenURI(_tokenId);
65    _setTokenURI(_tokenId, _tokenURI);
66}
67
68function _remove(address _from, uint256 _tokenId, bool _remnant) public
69{

Recommendation:

We advise that proper error messages are included in these invocations to ensure that the conditions imposed are properly reflected.

Alleviation:

The require statements were mostly changed to be assert statements in the new version of the codebase rendering this exhibit inapplicable.

WRA-04C: Unconditional Execution of Setter

Description:

The linked _setTokenURI invocation can execute with an empty _tokenURI as the safeTokenURI implementation returns an empty string in case the underlying NFT does not properly respond to the getter call.

Example:

Copy
56function _insert(address _from, uint256 _tokenId, bool _remnant, ERC721Shares _shares) public onlyOwner
57{
58    require(shares[_tokenId] == ERC721Shares(0));
59    shares[_tokenId] = _shares;
60    history.add(address(_shares));
61    address _holder = _remnant ? _from : address(_shares);
62    _safeMint(_holder, _tokenId);
63    IERC721Metadata _metadata = IERC721Metadata(address(target));
64    string memory _tokenURI = _metadata.safeTokenURI(_tokenId);
65    _setTokenURI(_tokenId, _tokenURI);
66}

Recommendation:

We advise that a conditional is placed before the _setTokenURI call to ensure that the _tokenURI being set contains data.

Alleviation:

The Nftfy team responded by stating that they intend to keep the codebase as is in the sake of legibility as the optimization is minimal.