Smart Contract Review (Lazy minting and royalties for third party brokers)

Smart Contracts Review Wanted
1

Hey guys, first time solidity developer here. We're getting ready to deploy our contract to production shortly and I thought it would be good to get a quick look-over by someone.

--What this contract implements--

  • Lazy minting with RedeemAndMint and secondary sales with Redeem
  • USD/ETH converter with chainlink price feeds
  • Various roles
  • Token Counter
  • Burn, Pause, Ownable, signature revoking
  • Rarible support with their interface, and EIP 2981 for generalized royalty info

Users may only have their signatures used for lazy minting if we have granted them the minter role. If someone tries to redeem a signature from someone who is not authorized through our website, it will fail.

One issue I've had is getting royalties to work with opensea, however I've tested the interface on our end and it does what it says it should do. Any tips at all would be greatly appreciated!

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721BurnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721PausableUpgradeable.sol";

import "@openzeppelin/contracts-upgradeable/access/AccessControlEnumerableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";

import "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/CountersUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

import "@openzeppelin/contracts-upgradeable/utils/cryptography/draft-EIP712Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/cryptography/SignatureCheckerUpgradeable.sol";
import "@chainlink/contracts/src/v0.6/interfaces/AggregatorV3Interface.sol";

//Rarible
import "@rarible/royalties/contracts/impl/RoyaltiesV2Impl.sol";
import "@rarible/royalties/contracts/LibPart.sol";
import "@rarible/royalties/contracts/LibRoyaltiesV2.sol";

contract NPortContract is Initializable, ContextUpgradeable, AccessControlEnumerableUpgradeable,OwnableUpgradeable, ERC721EnumerableUpgradeable,
 ERC721BurnableUpgradeable, ERC721PausableUpgradeable, EIP712Upgradeable, RoyaltiesV2Impl {
    function initialize(string memory name, string memory symbol, string memory baseTokenURI) public virtual initializer {
        __NPortContract_init(name, symbol, baseTokenURI);
    }
    using CountersUpgradeable for CountersUpgradeable.Counter;

    bytes32 public constant MOD_ROLE = keccak256("MOD_ROLE");

    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");
    mapping(uint256 => bytes) unlistedSignatures;

    CountersUpgradeable.Counter private _tokenIdTracker;

    string private _baseTokenURI;

    AggregatorV3Interface internal priceFeed;

         string[] public URIs;

    /**
     * @dev Grants `DEFAULT_ADMIN_ROLE`, `MINTER_ROLE` and `PAUSER_ROLE` to the
     * account that deploys the contract.
     *
     * Token URIs will be autogenerated based on `baseURI` and their token IDs.
     * See {ERC721-tokenURI}.
     */
    function __NPortContract_init(string memory name, string memory symbol, string memory baseTokenURI) internal initializer {
        __Context_init_unchained();
        __ERC165_init_unchained();
        __AccessControl_init_unchained();
        __AccessControlEnumerable_init_unchained();
        __Ownable_init_unchained();
        __ERC721_init_unchained(name, symbol);
        __ERC721Enumerable_init_unchained();
        __ERC721Burnable_init_unchained();
        __Pausable_init_unchained();
        __ERC721Pausable_init_unchained();
        __EIP712_init_unchained(name,  "1.0.0");
        __NPortContract_init_unchained(name, symbol, baseTokenURI);


    }

    function __NPortContract_init_unchained(string memory name, string memory symbol, string memory baseTokenURI) internal initializer {
        _baseTokenURI = baseTokenURI;

        _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
        
        _setupRole(MOD_ROLE, _msgSender());

        _setupRole(MINTER_ROLE, _msgSender());
        _setRoleAdmin(MINTER_ROLE, MOD_ROLE);

        _setupRole(PAUSER_ROLE, _msgSender());
        _setRoleAdmin(PAUSER_ROLE, MOD_ROLE);


        //This needs to be changed to mainnet before final deployment, currently rinkeby
        priceFeed = AggregatorV3Interface(0x8A753747A1Fa494EC906cE90E9f37563A8AF630e);
        //Allow contract to transfer any and all of original owners tokens from now on.
        setApprovalForAll(address(this), true);

        
    }


    function _baseURI() internal view virtual override returns (string memory) {
        return _baseTokenURI;
    }
  
    

function getPriceInUSD(uint256 val) public view returns (uint256) {
uint256 msgValueInUSD = ((val * (uint256)(getLatestPrice())/ (10 ** 18)));
return msgValueInUSD;
}

event RedeemAndMint(uint256 _tokenId, address _from, address _to, uint256 _soldPrice, uint256 _nportCut, string _tokenUri);

    function redeemAndMint(uint256 key,uint256 deadline, address signer,bytes calldata signature, string memory tokenuri)
    external payable
    {
        //Deny if art owner isn't registered with us
        require(hasRole(MINTER_ROLE, signer), "Address is not verified with NPort");
        uint256 price = getPriceInUSD(msg.value);



        //Also pass in date, if date deadline matches signed date, then we check against this date.
        require(_verify(signer, _hash(key,deadline, price, tokenuri), signature), "Invalid signature");

        require(block.timestamp < deadline,"Expired");

        //Make sure this signature wasn't revoked by lister
        require(keccak256(abi.encode(unlistedSignatures[key])) != keccak256(abi.encode(signature)), "Signature has been revoked by owner");
        //mint can only be called from within this contract
        mint(signer);

        URIs.push(tokenuri);

        IERC721Upgradeable tokenContract = IERC721Upgradeable(address(this));

        tokenContract.approve(msg.sender,_tokenIdTracker.current() - 1);
        //need to do current -1 becaues mint increments the counter
        tokenContract.safeTransferFrom(signer, msg.sender, _tokenIdTracker.current() - 1);
        
        //Set the signer as the receiver for all royalties for this token from now on.
        //10 means 105%
        uint256 cut = (msg.value * 10) / 100;
        uint256 transferAmt = msg.value - cut;

        //set royalties to original token minter to 10% (/100)
        _internalSetRoyalties(_tokenIdTracker.current() - 1, payable(signer), 1000);
        //give cut to NPort account
        owner().transfer(cut);

        //transfer the rest to the seller
        payable(signer).transfer(transferAmt);
        unlistedSignatures[key] = abi.encode(0);
        emit RedeemAndMint(_tokenIdTracker.current() - 1,signer,msg.sender, msg.value, cut, tokenuri);
       
    }

event Redeem(uint256 _tokenId, address _from, address _to, uint256 _soldPrice, uint256 _nportCut);


     //if token which is ALREADY MINTED will be set for sale again. In this case, key is tokenId
    function redeem(uint256 key,uint256 deadline,string memory tokenuri, address signer,bytes calldata signature)
    external payable
    {
        //require signer is owner of token id

        require(_exists(key), "Token does not exist");
       
        uint256 price = getPriceInUSD(msg.value);

        //Also pass in date, if date deadline matches signed date, then we check against this date.
        require(_verify(signer, _hash(key,deadline, price, tokenuri), signature), "Invalid signature");

        require(block.timestamp < deadline,"Expired");

        require(ownerOf(key)==signer, "Signer does not own token");

        //Make sure this signature wasn't revoked by lister

        require(keccak256(abi.encode(unlistedSignatures[key])) != keccak256(abi.encode(signature)), "Signature has been revoked by owner");


        IERC721Upgradeable tokenContract = IERC721Upgradeable(address(this));
        //after transfer, approval is automatically revoked, no need to implement
        tokenContract.approve(msg.sender,key);

        tokenContract.safeTransferFrom(signer, msg.sender, key);
        
        //calcualte amount original token minter should receive
        (address royaltyReceiver, uint256 royaltyAmt) = this.royaltyInfo(key, msg.value);

        //5 means 5%
        uint256 nportCut = (msg.value * 5) / 100;
        //give cut to NPort account
        owner().transfer(nportCut);
        payable(royaltyReceiver).transfer(royaltyAmt);
        uint256 transferAmt = msg.value - nportCut - royaltyAmt;

        //give cut to original minter account

        //transfer the rest to the seller
        payable(signer).transfer(transferAmt);
        unlistedSignatures[key] = abi.encode(0);

        emit Redeem(key,signer,msg.sender, msg.value, nportCut);
       
    }

event Unlist(uint256 _tokenId, address _from);

     //Invalidate authorized signature, requires fee
    function unlist(uint256 key,uint256 deadline,string memory tokenuri, address signer,bytes calldata signature, uint256 listingPrice)
    external
    {
        uint256 price = getPriceInUSD(listingPrice);
        require(_verify(signer, _hash(key,deadline, price,tokenuri), signature), "Invalid signature");

        require(signer == msg.sender, "Must own token to unlist it");
        //require signer is owner of token id
        unlistedSignatures[key] = signature;


        emit Unlist(key,msg.sender);
       
    }

    function _hash(uint256 key, uint256 deadline, uint256 price, string memory tokenuri)
    internal view returns (bytes32)
    {
        return _hashTypedDataV4(keccak256(abi.encode(
            keccak256("NFT(uint256 deadline,uint256 price,uint256 key,string tokenuri)"),
            deadline,
            price,
            key,
            keccak256(bytes(tokenuri))
        )));
    }

    function _verify(address signer, bytes32 digest, bytes memory signature)
    internal view returns (bool)
    {
        return SignatureCheckerUpgradeable.isValidSignatureNow(signer, digest, signature);
    }

    //Standard function required by minting platforms. Returns metadata uri if given token
    //is found.
    function tokenURI(uint256 tokenId) public view override returns (string memory) {
        require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
        return string(abi.encodePacked(_baseURI(), URIs[tokenId]));
    }

    

    function getLatestPrice() public view returns (int) {
    (
      uint80 roundID,
      int price,
      uint startedAt,
      uint timeStamp,
      uint80 answeredInRound
  ) = priceFeed.latestRoundData();
    return (price/ 10 ** 8); //price of 1 ether is USD
  }


    function mint(address to) private {
        // We cannot just use balanceOf to create the new tokenId because tokens
        // can be burned (destroyed), so we need a separate counter.

        _mint(to, _tokenIdTracker.current());
        _tokenIdTracker.increment();
    }


    function pause() public virtual {
        require(hasRole(PAUSER_ROLE, _msgSender()), "Must have pauser role to pause");
        _pause();
    }

    function unpause() public virtual {
        require(hasRole(PAUSER_ROLE, _msgSender()), "Must have pauser role to unpause");
        _unpause();
    }

    function _beforeTokenTransfer(address from, address to, uint256 tokenId) internal virtual override(ERC721Upgradeable, ERC721EnumerableUpgradeable, ERC721PausableUpgradeable) {
        super._beforeTokenTransfer(from, to, tokenId);
    }

      //Rarible royalties

    function setRoyalties(uint _tokenId, address payable _royaltiesRecipientAddress, uint96 _percentageBasisPoints) public onlyOwner{
        _internalSetRoyalties(_tokenId, _royaltiesRecipientAddress, _percentageBasisPoints);
    }

    function _internalSetRoyalties(uint _tokenId, address payable _royaltiesRecipientAddress, uint96 _percentageBasisPoints) internal{
        LibPart.Part[] memory _royalties = new LibPart.Part[](1);
        _royalties[0].value = _percentageBasisPoints;
        _royalties[0].account = _royaltiesRecipientAddress;
        _saveRoyalties(_tokenId,_royalties);
    }

    //EIP 2981 royalties for Opensea and mintable
      function royaltyInfo(uint256 _tokenId,uint256 _salePrice) external view returns (address receiver,uint256 royaltyAmount){
          LibPart.Part[] memory _royalties = royalties[_tokenId];
          if(_royalties.length > 0){
              return (_royalties[0].account,(_salePrice * _royalties[0].value)/10000);
          }
          return (address(0), 0);
      }
      

    /**
     * @dev See {IERC165-supportsInterface}.
     */
     bytes4 private constant _INTERFACE_ID_ERC2981 = 0x2a55205a;
    function supportsInterface(bytes4 interfaceId) public view virtual override(AccessControlEnumerableUpgradeable, ERC721Upgradeable, ERC721EnumerableUpgradeable) returns (bool) {
        if(interfaceId == LibRoyaltiesV2._INTERFACE_ID_ROYALTIES){
            return true;
        }

        if(interfaceId == _INTERFACE_ID_ERC2981){
            return true;
        }

        return super.supportsInterface(interfaceId);
    }


    uint256[48] private __gap;
}
2 Likes
2

In your redeemAndMint and redeem functions you have:

But, once a user performs a lazy mint via your redeemAndMint function, don't you want to deactivate the signature so it can't be used by the recipient to replay the minting function? i.e. shouldn't this line be:

ulinstedSignatures[key] = signature;

like you are doing in the unlist function of your contract.

I could be misunderstanding your intention with the function though.

1 Like
3

Great catch, yes you're absolutely right. Thank you! Saved my bacon

1 Like
4

Hi,

Thanks for posting this, it is helpful to see someone come up with something with both lazy minting and royalties!!

Could you tell me how your experience with this contract has been? Has it succeeded in everything you hoped it would have?

5

Everything seems to be working correctly after TheBC01's fix, yep. The main thing is that the functions are extremely expensive to call on mainnet, so we're trying to optimize them at the moment

1 Like
6

Specifically, revoking signatures by adding them to a dictionary is exceptionally expensive. Looking to find a way to prevent a token from being minted twice using a duplicate signature, while writing less data to the contract.

1 Like
8

Are the royalties working as they are suppose to in OpenSea? What if I want to distribute royalties between two wallets? Is this even possible?

9

I have one question: Current NPM package for Rarible Royalties is currently written for versions of Solidity below 0.8.0. But your contract requires a solidity version higher than 0.8.0. Did you fix this issue? If so, how?

10

The same issue also exists for the chainlink version. You are importing v0.6 however your contract pragma statement says solidity ^0.8.0

11

The owner() (in RedeemAndMint, owner().transfer(cut);) is also just an address. In order to get some cut from NFT sales, you need to make it address payable.