Closed Vectorized closed 8 months ago
Clever approach to implement this I think. I went through all the changes and didn't see any red flags. The only obvious thing that popped into my head is whether it makes sense to package spot + sequential mint functionality together in a single contract as I don't think a lot of mints will need both together. That being said, I do like having the capability now, and it added a lot less code to the base contract than I expected. Nicely done.
@mvillere Thanks so much for your review.
One possible use case is multi-chain NFTs. L1 minting can be sequential, while NFTs bridged from L2 -> L1 are spot minted.
Was initially thinking of a subclass that wraps the ERC721A's functions.
Decided on the current approach after hard marinating over a week:
Could we consider implementing batch spot minting as a highly anticipated use case?
Nice work.
could be less fee for NFTs redeem
@Vectorized
I did not want to create new issue for this as I assume answer is clear, but I want to make sure by checking with you.
As you mentioned I use _mintSpot
for cross-chain NFT's and I also use batchTransfer
, batchBurn
etc. but I assume that using _mintSpot
disables batchTransfer
and batchBurn
by following condition: _currentIndex <= tokenId
and there is no way to make those work without sequential mint am I right?
To give you some view of my project architecture I use batchMint, batchTransfer and batchBurn (all sequential) for my SourceNFT
(blockchain A) and I use _mintSpot
for DestinationNFT
(blockchain B), so for blockchain B to transfer more tokens It is only possible to just call standard safeTransferFrom
for each token right?
There is also one more thing as your docs says _sequentialUpTo()
needs to return greater value than _startTokenId()
, so I made my sourceNFT _startTokenId = 2
and then on destination my _sequentialUpTo = 1
, so user is able to transfer even 1st token. But is it possible to somehow turn on _mintSpot
feature from index 0? So we keep minting sequentially from 0 on chain A and then we can use _mintSpot
for tokenId = 0? I believe it is not possible to do so...
Thanks in advance for any confirmations/explanation if I'm right or there is a way to solve those problems somehow
This PR gives ERC721A the ability to mint tokens in non-sequential order.
Devs will need to override a new
_sequentialUpTo()
function to return a value less than2**256 - 1
to activate this feature.About:
_startTokenId() <= tokenId <= _sequentialUpTo()
_sequentialUpTo() < tokenId
.Zero-cost abstraction:
_sequentialUpTo() == type(uint256).max
(by default), all expressions ofx > _sequentialUpTo()
,x != _sequentialUpTo()
will evaluate to false on compile time, and the dead code will be removed. If you don't override_sequentialUpTo()
, you won't pay any extra gas.Notes:
ERC721AQueryable
can still be used with spot mints. ThetokensOfOwner
function is disabled if spot mints is enabled, as it would need to scan the entire uint256 range, which is unfeasible. ThetokensOfOwnerIn
function, however, is still available for use with spot mints, as the search range can be manually restricted. Added some minor optimizations to the scanning logic.Todo: