Is ERC721Enumerable necessary?

[Jeiwan]

Hi,

I was experimenting with ERC721Tradable and noticed that it extends ERC721Enumerable, which is an optional extension according to EIP-721. The extension maintains three indexes to allow:

1. Finding all tokens of an owner by owner address and index.
2. Finding a token by its index (not token ID).

It also implements totalSupply which is a nice feature, however it uses an array of token IDs to calculate it. Using a counter can be a cheaper solution.

Maintaining those indexes requires updating them on each minting and transferring, which is a costly operation on Ethereum. To find out how it affects gas consumption I implemented a simple contract with 3 functions:

1. mint1 calls mintTo(_msgSender()) once.
2. mint5 calls mintTo(_msgSender()) five times in a loop.
3. mint10 calls mintTo(_msgSender()) ten times in a loop.

I then wrote a test that executes each of them 10 times. I ran this test on a contract that extends ERC721Enumerable and one that doesn't. This is what I got:

1. With ERC721Enumerable:

·························|····························|·············|······························
|  Methods                                                                                        │
··············|··········|··············|·············|·············|···············|··············
|  Contract   ·  Method  ·  Min         ·  Max        ·  Avg        ·  # calls      ·  eur (avg)  │
··············|··········|··············|·············|·············|···············|··············
|  Test       ·  mint1   ·      152193  ·     163693  ·     153343  ·           10  ·          -  │
··············|··········|··············|·············|·············|···············|··············
|  Test       ·  mint10  ·     1200639  ·    1212139  ·    1201789  ·           10  ·          -  │
··············|··········|··············|·············|·············|···············|··············
|  Test       ·  mint5   ·      619364  ·     630864  ·     620514  ·           10  ·          -  │
··············|··········|··············|·············|·············|···············|··············

2. Without ERC721Enumerable:

·························|····························|·············|······························
|  Methods                                                                                        │
··············|··········|··············|·············|·············|···············|··············
|  Contract   ·  Method  ·  Min         ·  Max        ·  Avg        ·  # calls      ·  eur (avg)  │
··············|··········|··············|·············|·············|···············|··············
|  Test       ·  mint1   ·       62983  ·     114283  ·      68113  ·           10  ·          -  │
··············|··········|··············|·············|·············|···············|··············
|  Test       ·  mint10  ·      308611  ·     359911  ·     313741  ·           10  ·          -  │
··············|··········|··············|·············|·············|···············|··············
|  Test       ·  mint5   ·      173346  ·     224646  ·     178476  ·           10  ·          -  │
··············|··········|··············|·············|·············|···············|··············

As you can see, gas consumption differences are quite significant, especially during these crazy times of high gas prices.

So, my question is: is ERC721Enumerable required for custom NFT contracts to integrate nicely with OpenSea? Are tokenOfOwnerByIndex, totalSupply, and tokenByIndex methods required for a collection to be integrated and displayed without any issues?

Maybe, if it's not required, it's worth removing it from the template so new projects using the template can launch more affordable tokens? This also significantly affects tokens claiming, which is often used to promote new tokens.

Thanks

[humanwhoexplores]

Thanks @Jeiwan Seems like folks who maintain this repo : removed importing "ERC721Enumerable" after reading your comment.

https://github.com/ProjectOpenSea/opensea-creatures/commit/6bc9477bcce2d5f755ac80273d4630d3d6fa733c