solady icon indicating copy to clipboard operation
solady copied to clipboard

Feature request: generic / reference ERC721 implementation

Open RogerPodacter opened this issue 3 years ago • 2 comments

Okay don't kill me, but you shouldn't have to mint sequentially to have a payable transferFrom!

More seriously, mainstream ERC721 implementations are missing core "free money" optimizations like data hitchhiking. I wrote a proof of concept Better OpenZeppelin ERC721 but I don't have the clout to get it off the ground!

And this isn't even to mention all the assembly stuff which I cannot even do without at least learning more.

Also there are some "mistakes" (STRICTLY IMO, SORRY!) in ERC721A like startTimestamp which could be fixed in a new base contract that didn't have ERC721A's backwards compatibility requirements.

Finally, I think a move like this would raise Solady's profile which could attract more contributors and improve the other parts of the library, thus potentially offsetting some of the effort involved in developing it.

Someday Solmate's README is going to say that IT is a laboratory for SOLADY!

RogerPodacter avatar Oct 14 '22 21:10 RogerPodacter

Actually, we are in a no-complete agreement with Solmate/ERC721A/OpenZeppelin for tokens. Hehe.

Ok, jokes aside... for now, I'd prefer to keep Solady focused on utility contracts that can work with any token implementation. Maybe I'll consider if someone funds me or I have a few consecutive weeks of free time.

I agree that 721A can be refactored better if we don't need to care about backwards compatibility.

The startTimestamp is not used by most collections, and I doubt most collections using it can even last billions of years to make 64 bits worthwhile.

Also, checkout:

  • https://github.com/z0r0z/solbase
  • https://github.com/0xInuarashi/ERC721G

Vectorized avatar Oct 15 '22 01:10 Vectorized

That makes sense! FWIW what I had in mind was more "OZ + numberMinted" versus "ERC721A - startTimestamp."

Like is this not an improvement over what we have today in the reference implementations?

function _mint(address to, uint256 tokenId) internal virtual {
    require(to != address(0), "ERC721: mint to the zero address");
    require(!_exists(tokenId), "ERC721: token already minted");

    _beforeTokenTransfer(address(0), to, tokenId);

    AddressData storage addressData = _addressData[to];
    addressData.balance += 1;
    addressData.numberMinted += 1;
    
    TokenData storage tokenData = _tokenData[tokenId];
    tokenData.owner = to;

    emit Transfer(address(0), to, tokenId);

    _afterTokenTransfer(address(0), to, tokenId);
}

At the same time I guess sometimes people will want full 256 bit balances so you'd still have to put "ERC721S is not a one-size-fits-all solution" at the end?

Perhaps there is more wisdom in the non-compete than I originally thought...

RogerPodacter avatar Oct 15 '22 14:10 RogerPodacter

Moved to #352

Vectorized avatar Apr 03 '23 00:04 Vectorized