First Smart Contract deployment - looking for review

Hi, I've been preparing to deploy my first production ready smart contract. I've been spending a while in my spare time learning Solidity, studying other contracts etc so that it's not overly gas hungry, works well and doesn't have any flaws that could be exploited. I'd really appreciate anyone experienced looking over it and giving a thumbs up if it looks good.

A few things:

  • It's an ERC721 contract so the main purpose is to provide a good minting experience
  • During presale, the intention is that you can only mint 4 from your address. During the sale, you can only have 4 tokens in your wallet at one time, but technically you could transfer those 4 then mint again.
  • I've not used Enumerable, I have used Counters, and just created my own function to get all tokens that belong to an address
  • The contract isn't designed to be anything ground breaking. Really hoping to build trust and have a successful launch to continue building bigger and better things
  • Max token numbers on final contract will be 2k+ rather than 20, with 30 set aside as gifts.
  • token reveal will be done by uploading new metadata to IPFS then updating the baseURI

The contract can be viewed here on Etherscan, and I have supplied the code below:

// SPDX-License-Identifier: MIT
// v0.8.5
pragma solidity ^0.8.2;

import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/interfaces/IERC2981.sol";

import "@openzeppelin/contracts/security/Pausable.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

import "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol";
import "@openzeppelin/contracts/utils/Counters.sol";

import "./common/meta-transactions/ContentMixin.sol";
import "./common/meta-transactions/NativeMetaTransaction.sol";

contract OwnableDelegateProxy {}

/**
 * Used to delegate ownership of a contract to another address, to save on unneeded transactions to approve contract use for users
 */
contract ProxyRegistry {
    mapping(address => OwnableDelegateProxy) public proxies;
}

contract AETestSC is
    ERC721,
    IERC2981,
    Pausable,
    Ownable,
    ReentrancyGuard,
    ContextMixin,
    NativeMetaTransaction
{
    using Counters for Counters.Counter;
    Counters.Counter private _tokenIdCounter;

    bool public preLiveToggle;
    bool public saleLiveToggle;
    bool public freezeURI;
    bool public contractRoyalties;

    // need to apend 0x to front to indicate it is hexadecimal
    bytes32 private wlisteRoot;
    bytes32 private xileMkRoot;

    uint32 public constant MAX_NFT = 20;
    uint32 private constant MAX_MINT = 4;
    uint32 private constant MAX_GIFT = 0;
    uint32 public GIFT_COUNT = 0;
    uint256 public PRICE = 0.0007 ether;
    uint256 public XILE_PRICE = 0.0005 ether;

    address private _creators;
    address public proxyRegistryAddress;

    string private _contractURI;
    string private _metadataBaseURI;

    mapping(address => uint256) private presalePurchased;

    // ** MODIFIERS ** //
    // *************** //

    modifier saleLive() {
        require(saleLiveToggle == true, "Sale is closed");
        _;
    }

    modifier preSaleLive() {
        require(preLiveToggle == true, "Presale is closed");
        _;
    }

    modifier allocTokens(uint32 numToMint) {
        require(
            _tokenIdCounter.current() + numToMint <=
                (MAX_NFT - (MAX_GIFT - GIFT_COUNT)),
            "Sorry, there are not enough artworks remaining."
        );
        _;
    }

    modifier maxOwned(uint32 numToMint) {
        require(
            presalePurchased[_msgSender()] + numToMint <= MAX_MINT,
            "Max 4 mints for presale"
        );
        _;
    }

    modifier correctPayment(uint256 mintPrice, uint32 numToMint) {
        require(
            msg.value == mintPrice * numToMint,
            "Payment failed, please ensure you are paying the correct amount."
        );
        _;
    }

    constructor(
        string memory _cURI,
        string memory _mURI,
        address _creatorAdd,
        address _proxyRegistryAddress
    ) ERC721("Test Alpha", "ATS") {
        _contractURI = _cURI;
        _metadataBaseURI = _mURI;
        _creators = _creatorAdd;
        proxyRegistryAddress = _proxyRegistryAddress;
        // start at 1 to save expensive 0 mint
        _tokenIdCounter.increment();
    }

    // ** MINTING FUNCS ** //
    // ******************* //

    function aeMint(uint32 mintNum)
        external
        payable
        nonReentrant
        saleLive
        allocTokens(mintNum - 1)
        correctPayment(PRICE, mintNum)
    {
        require(
            balanceOf(_msgSender()) + mintNum <= MAX_MINT,
            "Limit of 4 per wallet"
        );
        for (uint32 i = 0; i < mintNum; i++) {
            uint256 tokenId = _tokenIdCounter.current();
            _tokenIdCounter.increment();
            _safeMint(_msgSender(), tokenId);
        }
    }

    function preMint(uint32 mintNum, bytes32[] calldata merkleProof)
        external
        payable
        nonReentrant
        preSaleLive
        maxOwned(mintNum)
        correctPayment(PRICE, mintNum)
    {
        require(
            MerkleProof.verify(
                merkleProof,
                wlisteRoot,
                keccak256(abi.encodePacked(_msgSender()))
            ),
            "Not on whitelist"
        );

        presalePurchased[_msgSender()] += mintNum;

        for (uint32 i = 0; i < mintNum; i++) {
            uint256 tokenId = _tokenIdCounter.current();
            _tokenIdCounter.increment();
            _safeMint(_msgSender(), tokenId);
        }
    }

    function xileMint(uint32 mintNum, bytes32[] calldata merkleProof)
        external
        payable
        nonReentrant
        preSaleLive
        maxOwned(mintNum)
        correctPayment(XILE_PRICE, mintNum)
    {
        require(
            MerkleProof.verify(
                merkleProof,
                xileMkRoot,
                keccak256(abi.encodePacked(_msgSender()))
            ),
            "Not on Xile whitelist"
        );

        presalePurchased[_msgSender()] += mintNum;

        for (uint32 i = 0; i < mintNum; i++) {
            uint256 tokenId = _tokenIdCounter.current();
            _tokenIdCounter.increment();
            _safeMint(_msgSender(), tokenId);
        }
    }

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 tokenId
    ) internal override whenNotPaused {
        super._beforeTokenTransfer(from, to, tokenId);
    }

    // ** ADMIN ** //
    // *********** //

    function giftMint(address[] calldata receivers) public onlyOwner {
        require(
            _tokenIdCounter.current() + receivers.length < MAX_NFT,
            "Sorry, there are not enough artworks remaining."
        );
        require(
            GIFT_COUNT + receivers.length <= MAX_GIFT,
            "no gifts remaining"
        );

        for (uint32 i = 0; i < receivers.length; i++) {
            uint256 tokenId = _tokenIdCounter.current();
            _tokenIdCounter.increment();
            GIFT_COUNT++;
            _safeMint(receivers[i], tokenId);
        }
    }

    function getOwnersTokens(address _owner)
        public
        view
        returns (uint256[] memory)
    {
        require(balanceOf(_owner) > 0, "You don't currently hold any tokens");
        uint256 tokenCount = balanceOf(_owner);
        uint256 foundTokens = 0;
        uint256[] memory tokenIds = new uint256[](tokenCount);

        for (uint256 i = 1; i < _tokenIdCounter.current(); i++) {
            if (ownerOf(i) == _owner) {
                tokenIds[foundTokens] = i;
                foundTokens++;
            }
        }

        return tokenIds;
    }

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

    function withdrawFunds(uint256 _amt) public onlyOwner {
        uint256 pay_amt;
        if (_amt == 0) {
            pay_amt = address(this).balance;
        } else {
            pay_amt = _amt;
        }

        (bool success, ) = payable(_creators).call{value: pay_amt}("");
        require(success, "Failed to send payment, let the artists starve!");
    }

    // ** SETTINGS ** //
    // ************** //

    function metaURI(string calldata _URI) external onlyOwner {
        require(freezeURI == false, "Metadata has been frozen");
        _metadataBaseURI = _URI;
    }

    function cntURI(string calldata _URI) external onlyOwner {
        _contractURI = _URI;
    }

    function tglLive() external onlyOwner {
        saleLiveToggle = !saleLiveToggle;
    }

    function tglPresale() external onlyOwner {
        preLiveToggle = !preLiveToggle;
    }

    function freezeAll() external onlyOwner {
        require(freezeURI == false, "Metadata is already frozen");
        freezeURI = true;
    }

    /**
     * @dev Reserve ability to make use of {IERC165-royaltyInfo} standard to implement royalties.
     */
    function tglRoyalties() external onlyOwner {
        contractRoyalties = !contractRoyalties;
    }

    function pause() public onlyOwner {
        _pause();
    }

    function unpause() public onlyOwner {
        _unpause();
    }

    function updatePrice(uint256 _price, uint32 _list) external onlyOwner {
        if (_list == 0) {
            // sale price
            PRICE = _price;
        } else {
            // xile price
            XILE_PRICE = _price;
        }
    }

    function setMerkleRoot(bytes32 _root, uint32 _list) external onlyOwner {
        if (_list == 0) {
            // update main whitelist
            wlisteRoot = _root;
        } else {
            // update xile whitelist
            xileMkRoot = _root;
        }
    }

    function contractURI() external view returns (string memory) {
        return _contractURI;
    }

    function totalSupply() public view returns (uint256) {
        return _tokenIdCounter.current() - 1;
    }

    function setCreator(address to) external onlyOwner returns (address) {
        _creators = to;
        return _creators;
    }

    function setProxyRegistryAddress(address _proxyRegistryAddress)
        external
        onlyOwner
    {
        proxyRegistryAddress = _proxyRegistryAddress;
    }

    /**
     * @dev See {IERC165-royaltyInfo}.
     */
    function royaltyInfo(uint256 tokenId, uint256 salePrice)
        external
        view
        override
        returns (address receiver, uint256 royaltyAmount)
    {
        require(_exists(tokenId), "Nonexistent token");
        require(contractRoyalties == true, "Royalties dissabled");

        return (address(this), (salePrice * 7) / 100);
    }

    function isApprovedForAll(address owner, address operator)
        public
        view
        override
        returns (bool)
    {
        // Whitelist OpenSea proxy contract for easy trading.
        ProxyRegistry proxyRegistry = ProxyRegistry(proxyRegistryAddress);
        if (address(proxyRegistry.proxies(owner)) == operator) {
            return true;
        }

        return super.isApprovedForAll(owner, operator);
    }

    function _msgSender() internal view override returns (address sender) {
        return ContextMixin.msgSender();
    }
}

Thanks in advance to any feedback.

1 Like

any considerations why do you include these 2?
_beforeTokenTransfer
_msgSender()