Potentially Misleading Use of totalSupply (ERC721)

Hello!

Was going through some final checks on a 721 I'm writing and saw a potential pitfall due to my (mis)use of totalSupply and have seen others falling into the same trap.

totalSupply() is better thought of as how many tokens are currently in circulation and very much not how many tokens have been minted, which, for me, was unexpected. This is due to its implementation - it is tracking a dynamic array that can both grow and shrink - the latter by burning a token.

In practice, that means if you have a contract like:

// External
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721Enumerable.sol";

contract MyERC721 is ERC721Enumerable {
    uint256 constant MAX_SUPPLY = 10;

    constructor() ERC721Enumerable("MyERC721", "M721") {}

    // ...

    function mint() external {
        require(totalSupply() < MAX_SUPPLY);
        _mint(msg.sender, getTokenId());
    }
}

The require statement may not be doing what you want - and could potentially be gamed.

Of course, if you are keeping a tokenId tracker or what have you, that provides a simple solution - simply change that boolean condition. In my use case, however, I don't have that readily available - hence, I used the above construct.

Practically, I'm not sure what exactly to do about this. I suppose I really just don't want anyone to fall into that trap, cause I definitely have seen that in a good amount of contracts. Maybe adding either a comment in the code or documentation to warn about the situation could help, because the current documentation reads:

Returns the total amount of tokens stored by the contract.

Which to me is a bit vague.

Either way, safe coding!

1 Like