Change implementation to enable lower gas fees

[liarco]

Hi @HashLips, thank you for your hard work with this project and your YouTube channel.

I was really impressed by a recent NFT project called "Slotie NFT" that had incredibly low gas fees, so I decided to do some research about the topic.
This PR is a draft of some changes that should reduce gas fees for minting by a huge amount (40-60%), please check out this article that explains everything in detail and also has benchmarks.

I also added a package.json file that includes dependencies from OpenZeppelin so that my local VSCode could load the imported contracts, let me know if you know a better way to manage that stuff.

Important thing to notice

This implementation turns the ERC721Enumerable contract into a basic ERC721 contract with an hard-coded counter that handles IDs for new tokens. By doing this we are losing the walletOfOwner(...) function that can't be implemented with a simple integer counter. I know that it can be really useful, but I think that the gas difference is really huge and that missing function can be implemented in other ways by external code. (e.g. we may ad a single function that returns a map of token -> address and then the caller will be able to group tokens from the same address) I was wrong: I've been able to add that function back. I'm not really concerned about performance here since it's simply a view function and it shouldn't be run inside other solidity functions.

Further optimizations

I decided to open this PR (even if it still needs proper testing and review) in order to have some feedback from you, in the mean time I also added the OpenSea Whitelisting feature. This feature skips the Set Approval For All step that each user must take at least once per new collection.
It requires one more parameter in the constructor to initialize the OpenSea proxy address (I found Slotie NFT set it to 0xa5409ec958c83c3f309868babaca7c86dcb077c1). I don't know if we also need a "setter" function for this field, I couldn't find one in any implementation so far, so maybe it's something that should never change.

Benchmarks (updated: 2021-12-21)

I run some tests on a local network to verify the gas used by the new implementation VS the original one, here is a video showing the results and explaining the process: https://youtu.be/N2eNdtUrg00 (TLDR: 45-74% less gas was used)

Improvements checklist

• Explicit external dependencies
• Lower minting gas fees
• No gas fee for listing on OpenSea

I hope you find this useful, thank you for your time.

[SwatX18]

About OpenSea proxy address check this : https://github.com/ProjectOpenSea/opensea-creatures/blob/master/migrations/2_deploy_contracts.js#L26-L32
These are the default addresses you should know.

UPDATE :
Be careful to put the addresses in uppercase/lowercase checksummed version like this :
Mainnet : 0xa5409ec958C83C3f309868babACA7c86DCB077c1
Rinkeby : 0xF57B2c51dED3A29e6891aba85459d600256Cf317

[liarco]

About OpenSea proxy address check this : https://github.com/ProjectOpenSea/opensea-creatures/blob/master/migrations/2_deploy_contracts.js#L26-L32 These are the default addresses you should know.

Hi @SwatX18, thank you very much for the tip, that can also be useful for #18 in order to create a dynamic deployment script for both testing and production! 😃

[isaac-martin]

@liarco should i pull the opensea stuff in from a previous commit you had done? Or did you decide it wasn't stable enough?

[liarco]

Hi @isaac-martin, thank you for your question.

TLDR

We think that the implementation suggested by OpenSea requires some knowledge (by the collection owner) about some technical implications. We decided to make the contract safer for the wider audience possible until we find a better way to implement it.

---

Both Daniel and I agreed to keep back those changes until we have time to investigate the following concerns:

• the implementation is tight to the OpenSea marketplace only, we would rather have an implementation that allows to change the proxy address without breaking stuff
• we would like to have the possibility to enable/disable the feature on the contract (see this as an emergency solution in case someone finds a way to take over the marketplace proxy)
• we need to understand the _msgSender function better before spreading that code around

I hope this can be useful to make your own decision about it, you can also take a look at the original demo collection from OpenSea here: https://github.com/ProjectOpenSea/opensea-creatures/blob/master/contracts/ERC721Tradable.sol

Feel free to ask for any further information.