My First ERC721 Contract

Support
1

Hi all. I finally decided to start learning solidity and with the current NFT hype, ERC721 seemed like a good place to start. I would love it someone could let me know if I have made any glaring mistakes - huge thanks in advance to anyone willing to help.

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.1;

    import 'https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.0.0/contracts/token/ERC721/ERC721.sol';
    import 'https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.0.0/contracts/token/ERC721/extensions/ERC721Enumerable.sol';
    import 'https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.0.0/contracts/access/Ownable.sol';
    import 'https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.0.0/contracts/utils/Counters.sol';

contract TestNFT is ERC721Enumerable, Ownable {

    uint256 private constant MAX_KIDS = 100;
    bool private hasSaleStarted = true;
    uint256[] private _specialIds;

    using Strings for uint256;
    using Counters for Counters.Counter;
    
    Counters.Counter private _tokenIds;
    Counters.Counter private _reserved;
    
    mapping(uint256 => string) private _generationURIs;
    mapping(uint256 => string) private _specialsURIs;
    
    event Birth(uint256 quantity);

    constructor(string memory uri) ERC721("TestNFT","KIDS")  {
        setGenerationURI(1, uri);
        _reserved._value = 5;
    }
    
    function giveSpecial(address _to, string memory _uri, uint256 _amount) public onlyOwner() {
        require(_reserved.current() > 0, "No more specials to give");
        require(_reserved.current() >= _amount, "Not enough specials to give away");
        
        for(uint256 i = 0; i < _amount; i++){
            _reserved.decrement();
            _tokenIds.increment();
            
            uint mintIndex = _tokenIds.current();
            _safeMint(_to, mintIndex);
            
            _specialIds.push(mintIndex);
            _specialsURIs[mintIndex] = _uri;
        }
    }
    
    function getSpecialIds() external view returns(uint256[] memory) {
        return _specialIds;
    }
    
    function walletOfOwner(address _owner) external view returns(uint256[] memory) {
        uint256 tokenCount = balanceOf(_owner);
        
        uint256[] memory tokensId = new uint256[](tokenCount);
        for(uint256 i = 0; i < tokenCount; i++){
          tokensId[i] = tokenOfOwnerByIndex(_owner, i);
        }
        
        return tokensId;
    }
    
    function tokenURI(uint256 tokenId) public view override returns (string memory) {
        require(_exists(tokenId), "ERC721Metadata: URI query for nonexistent token");
        
        // check specials
        if(bytes(_specialsURIs[tokenId]).length > 0){
            return _specialsURIs[tokenId];
        }
    
        // concatenate generation with tokenId.
        uint256 gen = getTokenGeneration(tokenId);
        string memory generationURI = _generationURIs[gen];
        return string(abi.encodePacked(generationURI, tokenId.toString()));
    }
    
    function totalSold() public view returns (uint256) {
        return _tokenIds.current();
    }
    
    function currentGeneration() public view returns (uint256) {
        return getTokenGeneration(_tokenIds.current());
    }
    
    function currentPrice() public view returns (uint256) {
        return calculatePrice();
    }

    function getTokenGeneration(uint256 tokenId_) public pure returns (uint256)  {
        if (tokenId_ >= 90) {
            return 10;
        } else if (tokenId_ >= 80) {
            return 9;
        } else if (tokenId_ >= 70) {
            return 8;
        } else if (tokenId_ >= 60) {
            return 7;
        } else if (tokenId_ >= 50) {
            return 6;
        } else if (tokenId_ >= 40) {
            return 5;
        } else if (tokenId_ >= 30) {
            return 4;
        } else if (tokenId_ >= 20) {
            return 3;
        } else if (tokenId_ >= 10) {
            return 2;
        }
        return 1;
    }

    function calculatePrice() private view returns (uint256) {
        // look ahead to the next token minted
        uint256 currentSupply = _tokenIds.current() + 1;
        if (currentSupply >= 90) {
            return 800000000000000000;             // 0.80 ETH: #9900-10000
        } else if (currentSupply >= 80) {
            return 400000000000000000;             // 0.40 ETH: #9000-9899
        } else if (currentSupply >= 70) {
            return 200000000000000000;             // 0.20 ETH: #8500-8999
        } else if (currentSupply >= 60) {
            return 150000000000000000;             // 0.15 ETH: #7500-8499
        } else if (currentSupply >= 50) {
            return 100000000000000000;             // 0.10 ETH: #4500-7499
        } else if (currentSupply >= 40) {
            return 80000000000000000;              // 0.08 ETH: #2600-4499
        } else if (currentSupply >= 30) {
            return 60000000000000000;              // 0.06 ETH: #1000-2599
        } else if (currentSupply >= 20) {
            return 40000000000000000;              // 0.04 ETH: #600-999
        } else if (currentSupply >= 10) {
            return 30000000000000000;              // 0.03 ETH: #300-599
        } else {
            return 20000000000000000;              // 0.02 ETH: #0-299 
        }
    }

    // We should include price/generation changes in the pricing check but we are not greedy. If someone is lucky
    // enough to bridge a generation with their multi-purchase, let them have the discount :)
    function adopt(uint256 numKids) public payable {
        require(hasSaleStarted == true, "Sale is paused");
        require(_tokenIds.current() <= sellable(), "Sale has already ended");
        require(add(_tokenIds.current(), numKids) <= sellable(), "Exceeds maximum kid supply.");
        require(numKids > 0 && numKids <= 20, "You can adopt a minimum 1, maximum 20 kids");
        require(msg.value >= mul(calculatePrice(), numKids), "Oh No. Amount of Ether sent is not correct.");

        for (uint i = 0; i < numKids; i++) {
            _tokenIds.increment();
            uint mintIndex = _tokenIds.current();
            _safeMint(msg.sender, mintIndex);
        }
        emit Birth(numKids);
    }
    
    function sellable() private view returns (uint256) {
        return sub(MAX_KIDS, _reserved.current());
    }
    
    function setGenerationURI(uint256 generation, string memory uri) public onlyOwner() {
        _generationURIs[generation] = uri;
    }
    
    function getGenerationURI(uint256 generation) public view onlyOwner returns (string memory) {
       return _generationURIs[generation];
    }
    
    function getReservedLeft() public view onlyOwner returns (uint256) {
        return _reserved.current();
    }

    function startSale() public onlyOwner {
        hasSaleStarted = true;
    }
    
    function pauseSale() public onlyOwner {
        hasSaleStarted = false;
    }

    function withdrawAll() public payable onlyOwner {
        require(payable(msg.sender).send(address(this).balance));
    }

    function sub(uint256 a, uint256 b) internal pure returns (uint256) {
        assert(b <= a);
        return a - b;
    }
  
    function mul(uint256 a, uint256 b) internal pure returns (uint256 c) {
        if (a == 0) {
          return 0;
        }
        c = a * b;
        assert(c / a == b);
        return c;
    }
  
    function add(uint256 a, uint256 b) internal pure returns (uint256 c) {
        c = a + b;
        assert(c >= a);
        return c;
    }
}
1 Like
2

Hi @Lord_Eldod , welcome to the forums.

I think your project and code is definitely on the right track and looks interesting! Making adoptable mechanisms is innovation and that’s highly valued.

Personally I think you should start with OpenZeppelin Upgrades: Step by Step Tutorial for Hardhat

And learn to create basic ERC20 tokens first. If you follow this, it can get you a dev environment up and running.

Then after proving you can deploy a token, you can follow examples of NFTs.

Something I think that you might want, is that when they adopt, they can add to the URI. So that way the new owner of the NFT can add something to the NFT. Perhaps call setGenerationURI in the adopt function.

I think there is surely a mathematical way to calculatePrice()

r_102813_Wx1Vb

You can also import SafeMath library from Open Zeppelin so you can use their math functions, so you can eliminate the add, sub, mul functions.

I think the way you are handling _tokenIds is great.

The best way to test your code is by doing it locally. Make test cases after test case. If you think of something to test, then make it into a test case. As you code your project you will think of new things to test, and when you do so - make a TODO so that you can code in a test case for it. I use chai and mocha. https://mochajs.org/ https://www.chaijs.com/

Although it’s not as fun it’s very important to do testing.

Your project shows a lot of promise, I hope it goes well.

3

Thanks for the response.

Since posting this I have setup everything in hardhat and stripped out anything that increases the gas fees. Even silly little things like a check for less than 100:
require(x <= y, 'Error')
Have become
require(x < 101, 'Error')
Saving a whole 3 gwei :slight_smile:

I would have loved to create a nice function for the calcPrice as that if block is very ugly. Sadly I really want to avoid loops and keep all the data variable fee to avoid the gas costs.

To save on gas the _tokenIds have been stripped out to and replaced for tokenSupply().

So far I have managed to get the gas cost down ~60% for the adopt function. Removing the Birth emit was a pretty big saver.

I love your idea of allowing users to change the uri and add new items and such. That could be a lot of fun :slight_smile:

I have been toying with ERC1155 to further reduce minting for the users and so far it seems very promising. My only worry is that ERC721 is hot news item right now and people would reject a ERC1155 token purely due to lack of knowledge. What do you think?

1 Like
4

Great job on the improvements!

Here is an improvement if you choose to use it. It might be more expensive on gas, so if it is definitely use yours instead.

function getTokenGeneration(uint256 tokenId_) public pure returns (uint256)  {

        uint256 divTen = tokenId_.div(10);     // divide by 10, this will exclude fractional results given solidity math
        uint256 addedOne = divTen.add(1);  // minimum will always be 1

        if(addedOne > 10){
            return 10;  // Control the maximum through this
        }

        return addedOne;
    }

I think you are right about going with ERC721, it would be more attractive. But - if you do go with ERC1155 there is nothing wrong with still using the buzzword NFT.

5

Ohhh thats a nice function, thank. Ill do some testing :slight_smile:

What are your thoughts on the gas estimation function:

web3.eth.getGasPrice

I like that I can use it to show a lower gas price than the default Metamask usage but I worry that setting the gas limit via this estimation could result in a transaction failing due to the estimation being slightly wrong.

6

My opinion is that I go with metamask default and I normally pay premium for my transactions on Uniswap because I’ve had too many failed ones costing me more than just paying the premium.

I would go with what the wallet estimates for you to use.