Wrapper Manual Review Findings

WRA-01M: Inexplicable _remnant Functionality

Description:

The current implementations permit the minting of a token outside of the underlying ERC712Shares contract by setting the _remnant boolean parameter to true. This trait would enable users to freely transact the ERC721 minted by the ERC721Wrapper whereas the shares associated with it would be a seperate entity and would not dictate actual ownership of the ERC721.

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 this functionality to be properly documented as it could lead to unfavorable scenarios such as rendering an ERC721Shares contract unredeemable since its redeem function in turn invokes the _remove function of the contract which will fail if the overlay ERC721 has been transferred to another party.

Alleviation:

The Nftfy team responded by stating that they do agree the function is not document and currently utilized and intend to expand the documentation surrounding it in a future iteration.