Migrating to ERC721A

[liarco]

This PR integrates the ERC721A contract discussed in #24.

⚠️ Disclaimer ⚠️

The implementation is purely for testing/research purposes. The main goal would be to merge this into the main branch, but this code should not be considered "stable" until then.

I encourage people testing this version and providing feedback to speed up the adoption process. Thank you.

What to expect from this PR

We should get a great improvements in gas efficiency when minting multiple tokens with a single transaction. Minting just one token is the same.

Gas report BEFORE this PR

Gas report AFTER this PR

Run transactions

The test I run is making 12 transaction:

1. 2 minted
2. 1 minted
3. 1000 minted
4. 1000 minted
5. 1000 minted
6. 1000 minted
7. 1000 minted
8. 1000 minted
9. 1000 minted
10. 1000 minted
11. 1000 minted
12. 997 minted

Comparison

Contract	Minimum	Maximum	Average
ERC721	65556	25610967	21344168
ERC721A	65575	2289361	1919228

The difference is huge (8.99% gas usage compared to the original version), but that's because of the high mint amounts (1k tokens per TX). In a more realistic situation (3 tokens per TX) the ERC721A implementation is using 69% gas compared to the original version. This is still pretty good.

[VeriumETH]

Bless you @liarco this is fantastic!

[gitrevo]

Continuation from previous thread discussion:

1. Minting via contract (write contract - owner address - mint), it will show that the transaction will likely fail and the gas is extremely high. However, mintForAddress via write contract is working perfectly.

2. I received another warning while updating whitelist.json stating that the maximum optimize list should be 5000 when I had pasted a 6000 lists. Is it advisable not to exceed this threshold?

Thanks Marco.

[liarco]

Thank you @gitrevo,
I was fixing my bad attempt at forcing the token IDs to start from 1, when I found that it's not possible with the current ERC721A version (the totalSupply() function cannot be overridden to handle the change).

I think we should wait for chiru-labs/ERC721A#66 to be merged and released so I can use the new _startTokenId() function directly.

I'm gonna get the code ready so it should work as soon as I can update the dependencies.

[joven1555]

Thank you @gitrevo, I was fixing my bad attempt at forcing the token IDs to start from 1, when I found that it's not possible with the current ERC721A version (the totalSupply() function cannot be overridden to handle the change).

I think we should wait for chiru-labs/ERC721A#66 to be merged and released so I can use the new _startTokenId() function directly.

I'm gonna get the code ready so it should work as soon as I can update the dependencies.

Can I ask a couple questions about this?
(1) Is this change really necessary? Azuki starts their tokens at #0 (OpenSea link provided) and given they are the benchmark for 721A, it's worth noting they didn't find it necessary to tinker with starting token position.
https://opensea.io/assets/0xed5af388653567af2f388e6224dc7c4b3241c544/0

(2) My limited understanding is extra work on the smart contract could lead to additional gas. Is it possible by making this change it will offset some of the gas savings we're expecting by moving to 721A?

I guess I'm wondering if this additional feature is really necessary given that starting at token #0 is really not that big of a deal.

[liarco]

@joven1555 thank you for your feedback, here are my thoughts:

1. while I really care a lot about gas efficiency and I'm aware that token IDs do not have to follow a strict standard, I'm also concerned about user experience and in my opinion having a #0 token feels weird for most "non-dev" people
2. I encourage you to take a look at the PR I linked above. That is from the ERC721A team and they are adding this feature in order to allow collection owners to choose a starting token ID (and it should have no impact on gas cost)

Also:

I guess I'm wondering if this additional feature is really necessary given that starting at token #0 is really not that big of a deal.

That's not so easy to know wether this can be a big deal or not, everything may look fine until you encounter some issues (see chiru-labs/ERC721A#45 where the staking contract is not working properly because of the token with ID = 0). Most of the collections have IDs in the range from 1 to the maxSupply, so that's what most users (and code from third-party) might expect. 😉

[joven1555]

Brilliant, you sold me! Particularly the part about most users and third-party expectations. I'm going to continue working off the ERC721 version until everything is finalized for ERC721A, hopefully I can contribute by testing for the ERC721A version.

Can't express how grateful I am for you and the community. It's clear ERC721A is the cutting edge, most gas efficient solution right now and our community having access to that because of you is unbelievable!

[gitrevo]

Interesting how the mint actually used lower gas on average when the starting index is 1 instead of 0.

When the index starts at 0, the very first mint will change _currentIndex from 0 to 1, which costs more gas (20000 vs 5000).

I saw Vectorized commented the above.

Continuation from previous thread discussion:

1. Minting via contract (write contract - owner address - mint), it will show that the transaction will likely fail and the gas is extremely high. However, mintForAddress via write contract is working perfectly.
2. I received another warning while updating whitelist.json stating that the maximum optimize list should be 5000 when I had pasted a 6000 lists. Is it advisable not to exceed this threshold?

Thanks Marco.

I will test this again on next test after chiru-labs merged/released.

Thanks Marco

[ghost]

@liarco I found this discussion by the actual OpenZeppelin team interesting. They talk about Azuki's ERC721A pros and cons. You might already know all of this, but thought I'd share.

https://forum.openzeppelin.com/t/erc721enumerable-gas-optimization/22795

[liarco]

https://forum.openzeppelin.com/t/erc721enumerable-gas-optimization/22795

Hi @spike-hue, thank you for your feedback. I must admit that our current implementation is based on the same exact principle as the one from the Azuki team, so what you see there as main concerns can be applied to my version as well.

Despite the fact I don't see some of those concerns as real problems for most collection types (some people may actually need the ERC721Enumerable feature because they are running specific functions on-chain, but that's not always the case), I wan't to understand their statements about some functions being a potential "denial of service attack vector" better. To me it's not clear in which scenarios this might turn into a real problem...

[liarco]

I updated the ERC721A dependency to version 3.0.0 which includes the _startTokenId() function.

The latest commit should be ready for testing, here is the new gas report:

Gas efficiency has been improved even further. I'm gonna do proper testing ASAP, any feedback is much appreciated. 😉

[gitrevo]

Gas efficiency has been improved even further. I'm gonna do proper testing ASAP, any feedback is much appreciated. 😉

Great update Marco, many thanks!

l’ll run a test soonest as I can. Is this included with the burn token feature I saw they implementing?

[liarco]

Is this included with the burn token feature I saw they implementing?

It includes everything from https://github.com/chiru-labs/ERC721A/releases/tag/v3.0.0 (but this doesn't mean that the burning feature is enabled by default, you have to add your own logic for it).

[DEXExchange]

May inquire why the double quote marks have been changed to single quote marks?

[liarco]

May inquire why the double quote marks have been changed to single quote marks?

Yes @DEXExchange, of course!

This was done for code consistency across the whole project (the rest of the codebase is using single-quotes, the Solidity source was erroneously kept with double-quotes from the old repository). The ERC721A contract is also using the same coding-style and that's what made me realize the inconsistency.

[MiyukiZ]

How can we currently test this contract out? Also kindly specify on which ( Remix or through VS Code terminal )?
Wanna test the ERC721 contract out

[liarco]

How can we currently test this contract out?

Hi @BlueGreninja,
from the branch page (always check to be on our official repo), you can download the branch using Git or as a Zip folder:

Also kindly specify on which ( Remix or through VS Code terminal )?

Everything will work, this branch now also includes the feature I showed on my latest YouTube video, you can take a look at #49 to see how to use that and avoid using private keys.

[MiyukiZ]

Thanks, just want to verify that this contract is for testing right now, right? It's not yet officially useable?

[liarco]

@BlueGreninja nothing on this repo is to be considered "ready for production" until it's released with a new version 😉

[dhararughani]

There are multiple projects on opensea which have adapted ERC721A contract. I think referring that can help speed up the testing and would be great for using it as a feedback.

[liarco]

Hi @dhararughani, we don't need to test ERC721A here, it has been tested on its team and it proved to be working fine. We need to test how our codebase reacts to the changes brought by the adoption of the new contract as a base.

A good example were the issues introduced by my first migration attempt: the different token ID management was causing the collection to be corrupted.

[gitrevo]

1. Minting via contract (write contract - owner address - mint), it will show that the transaction will likely fail and the gas is extremely high. However, mintForAddress via write contract is working perfectly.
2. I received another warning while updating whitelist.json stating that the maximum optimize list should be 5000 when I had pasted a 6000 lists. Is it advisable not to exceed this threshold?

Thanks Marco.

Any chance that you review this Marco? Sometime owner would like to mint for the team say 100 NFTs, tested with 3 nfts and it cost 0.2 gas fee. Imagine if it’s 100 :(

[liarco]

@gitrevo the bigger the whitelist, the more it's gonna cost to verify the proof. The Merkle Tree library works fine with thousands of addresses, it might just take longer in order to calculate the proof, but you should run some tests locally to verify the gas cost of the whitelistMint() function (you can use yarn test-gas after you update the JSON list).

Any chance that you review this Marco? Sometime owner would like to mint for the team say 100 NFTs, tested with 3 nfts and it cost 0.2 gas fee. Imagine if it’s 100 :(

Please take a look at #50, it might help.

[gitrevo]

Any chance that you review this Marco? Sometime owner would like to mint for the team say 100 NFTs, tested with 3 nfts and it cost 0.2 gas fee. Imagine if it’s 100 :(

Please take a look at #50, it might help.

Do you mean, in order for an “owner” to mint without a cost; we need to use “mintForAddress” instead of “mint” right?

i.e:
mintAmount = 200 (200 pcs NFTs)
Address = Owner Wallet Address

Since “mint” will produce error “likely to fail” and huge gas, we need to use “mintForAddress” in order to achieve this correct?

[liarco]

@gitrevo please stick to the topic, you should open another issue for unrelated questions. Thank you

[liarco]

Hi everyone,
I finally completed my tests and I found a bug which I should have fixed with my latest commits.

Here you can see that the walletOfOwner() function was failing to return the last token because of bad boundaries in the loop:

There is no token with ID 0, that means that the last token ID was simply skipped by the loop.

I also made some other small improvements and I would really appreciate if you could provide some feedback on this so I can merge it and finalize everything for our next release.

Thank you.

[tab00]

How does it compare with ERC1155?

ERC1155 is considered to be a newer more efficient standard than ERC721, right? So it would be interesting to now compare ERC1155 with ERC721A.

[liarco]

@tab00 please do some research about ERC721 vs ERC1155. They are completely different standards and they should be used for different purposes. Of course you can force an ERC1155 into behaving like ERC721 (and there are projects doing that) but it can lead to bad side-effects so, at the moment, I don't recommend doing it.

[tab00]

@liarco yes I had read comparisons before, and they mostly describe how ERC1155 is superior.

e.g. from https://www.web3.university/article/comparing-erc-721-to-erc-1155:

ERC-1155 also allows batch transfers of tokens, which can reduce transaction costs and times. With ERC-721, if you want to send multiple tokens, they happen individually.

Also, there is some built-in metadata functionality in ERC1155, saving some code:

In ERC-1155, smart contracts are linked to multiple URIs and do not store additional metadata (such as file names). In comparison, ERC-721 only supports static metadata stored directly on the smart contract for each token ID,, increasing deployment costs and limiting flexibility.

You set the uri as ipfs://baf.../{id}.json and platforms like OpenSea automatically picks up the .json file for each token (as long as you had named the .json files correctly).

[liarco]

@tab00 thank you for the references, there is no doubt that there are advantages using ERC1155 when appropriate, but you are comparing apples and oranges here. As I said, there are also projects forcing ERC1155 to behave like ERC721, but that's not the same as having an actual ERC721 contract.

As you can see this project is called nft-erc721-collection, that's because it aims to provide with a good all-in-one solution for creating, deploying and managing ERC721 collections. We absolutely have plans to expand the concept to other standards, but each one will serve its purpose. ;)

[slavakurilyak]

+1 for implementation of EIP-1155: Multi Token Standard 🙋‍♂️

It would be great to see configuration files for an ERC1155 contract, similar to this project 😎