ERC721Enumerable gas optimization

I'm wondering whether gas optimizations made by the AGC or by the Azuki have any serious issues compared to OpenZepellin's default ERC721 and ERC721Enumerable?

Here are examples:

ERC721 override by AGC
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.11;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/utils/Context.sol";
import "@openzeppelin/contracts/utils/Strings.sol";
import "@openzeppelin/contracts/utils/introspection/ERC165.sol";

abstract contract ERC721B is Context, ERC165, IERC721, IERC721Metadata {
    using Address for address;

    // Token name
    string private _name;

    // Token symbol
    string private _symbol;

    // Mapping from token ID to owner address
    address[] internal _owners;

    // Mapping from token ID to approved address
    mapping(uint256 => address) private _tokenApprovals;

    // Mapping from owner to operator approvals
    mapping(address => mapping(address => bool)) private _operatorApprovals;

    /**
     * @dev Initializes the contract by setting a `name` and a `symbol` to the token collection.
     */
    constructor(string memory name_, string memory symbol_) {
        _name = name_;
        _symbol = symbol_;
    }

    /**
     * @dev See {IERC165-supportsInterface}.
     */
    function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) {
        return
            interfaceId == type(IERC721).interfaceId ||
            interfaceId == type(IERC721Metadata).interfaceId ||
            super.supportsInterface(interfaceId);
    }

    /**
     * @dev See {IERC721-balanceOf}.
     */
    function balanceOf(address owner) public view virtual override returns (uint256) {
        require(owner != address(0), "ERC721: balance query for the zero address");

        uint count = 0;
        uint length = _owners.length;
        for (uint i = 0; i < length; ++i) {
            if (owner == _owners[i]) {
                ++count;
            }
        }

        delete length;
        return count;
    }

    /**
     * @dev See {IERC721-ownerOf}.
     */
    function ownerOf(uint256 tokenId) public view virtual override returns (address) {
        address owner = _owners[tokenId];
        require(owner != address(0), "ERC721: owner query for nonexistent token");
        return owner;
    }

    /**
     * @dev See {IERC721Metadata-name}.
     */
    function name() public view virtual override returns (string memory) {
        return _name;
    }

    /**
     * @dev See {IERC721Metadata-symbol}.
     */
    function symbol() public view virtual override returns (string memory) {
        return _symbol;
    }

    /**
     * @dev See {IERC721-approve}.
     */
    function approve(address to, uint256 tokenId) public virtual override {
        address owner = ERC721B.ownerOf(tokenId);
        require(to != owner, "ERC721: approval to current owner");

        require(
            _msgSender() == owner || isApprovedForAll(owner, _msgSender()),
            "ERC721: approve caller is not owner nor approved for all"
        );

        _approve(to, tokenId);
    }

    /**
     * @dev See {IERC721-getApproved}.
     */
    function getApproved(uint256 tokenId) public view virtual override returns (address) {
        require(_exists(tokenId), "ERC721: approved query for nonexistent token");

        return _tokenApprovals[tokenId];
    }

    /**
     * @dev See {IERC721-setApprovalForAll}.
     */
    function setApprovalForAll(address operator, bool approved) public virtual override {
        require(operator != _msgSender(), "ERC721: approve to caller");

        _operatorApprovals[_msgSender()][operator] = approved;
        emit ApprovalForAll(_msgSender(), operator, approved);
    }

    /**
     * @dev See {IERC721-isApprovedForAll}.
     */
    function isApprovedForAll(address owner, address operator) public view virtual override returns (bool) {
        return _operatorApprovals[owner][operator];
    }


    /**
     * @dev See {IERC721-transferFrom}.
     */
    function transferFrom(
        address from,
        address to,
        uint256 tokenId
    ) public virtual override {
        //solhint-disable-next-line max-line-length
        require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: transfer caller is not owner nor approved");

        _transfer(from, to, tokenId);
    }

    /**
     * @dev See {IERC721-safeTransferFrom}.
     */
    function safeTransferFrom(
        address from,
        address to,
        uint256 tokenId
    ) public virtual override {
        safeTransferFrom(from, to, tokenId, "");
    }

    /**
     * @dev See {IERC721-safeTransferFrom}.
     */
    function safeTransferFrom(
        address from,
        address to,
        uint256 tokenId,
        bytes memory _data
    ) public virtual override {
        require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: transfer caller is not owner nor approved");
        _safeTransfer(from, to, tokenId, _data);
    }

    /**
     * @dev Safely transfers `tokenId` token from `from` to `to`, checking first that contract recipients
     * are aware of the ERC721 protocol to prevent tokens from being forever locked.
     *
     * `_data` is additional data, it has no specified format and it is sent in call to `to`.
     *
     * This internal function is equivalent to {safeTransferFrom}, and can be used to e.g.
     * implement alternative mechanisms to perform token transfer, such as signature-based.
     *
     * Requirements:
     *
     * - `from` cannot be the zero address.
     * - `to` cannot be the zero address.
     * - `tokenId` token must exist and be owned by `from`.
     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
     *
     * Emits a {Transfer} event.
     */
    function _safeTransfer(
        address from,
        address to,
        uint256 tokenId,
        bytes memory _data
    ) internal virtual {
        _transfer(from, to, tokenId);
        require(_checkOnERC721Received(from, to, tokenId, _data), "ERC721: transfer to non ERC721Receiver implementer");
    }

    /**
     * @dev Returns whether `tokenId` exists.
     *
     * Tokens can be managed by their owner or approved accounts via {approve} or {setApprovalForAll}.
     *
     * Tokens start existing when they are minted (`_mint`),
     * and stop existing when they are burned (`_burn`).
     */
    function _exists(uint256 tokenId) internal view virtual returns (bool) {
        return tokenId < _owners.length && _owners[tokenId] != address(0);
    }

    /**
     * @dev Returns whether `spender` is allowed to manage `tokenId`.
     *
     * Requirements:
     *
     * - `tokenId` must exist.
     */
    function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) {
        require(_exists(tokenId), "ERC721: operator query for nonexistent token");
        address owner = ERC721B.ownerOf(tokenId);
        return (spender == owner || getApproved(tokenId) == spender || isApprovedForAll(owner, spender));
    }

    /**
     * @dev Safely mints `tokenId` and transfers it to `to`.
     *
     * Requirements:
     *
     * - `tokenId` must not exist.
     * - If `to` refers to a smart contract, it must implement {IERC721Receiver-onERC721Received}, which is called upon a safe transfer.
     *
     * Emits a {Transfer} event.
     */
    function _safeMint(address to, uint256 tokenId) internal virtual {
        _safeMint(to, tokenId, "");
    }


    /**
     * @dev Same as {xref-ERC721-_safeMint-address-uint256-}[`_safeMint`], with an additional `data` parameter which is
     * forwarded in {IERC721Receiver-onERC721Received} to contract recipients.
     */
    function _safeMint(
        address to,
        uint256 tokenId,
        bytes memory _data
    ) internal virtual {
        _mint(to, tokenId);
        require(
            _checkOnERC721Received(address(0), to, tokenId, _data),
            "ERC721: transfer to non ERC721Receiver implementer"
        );
    }

    /**
     * @dev Mints `tokenId` and transfers it to `to`.
     *
     * WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible
     *
     * Requirements:
     *
     * - `tokenId` must not exist.
     * - `to` cannot be the zero address.
     *
     * Emits a {Transfer} event.
     */
    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);
        _owners.push(to);

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

    /**
     * @dev Destroys `tokenId`.
     * The approval is cleared when the token is burned.
     *
     * Requirements:
     *
     * - `tokenId` must exist.
     *
     * Emits a {Transfer} event.
     */
    function _burn(uint256 tokenId) internal virtual {
        address owner = ERC721B.ownerOf(tokenId);

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

        // Clear approvals
        _approve(address(0), tokenId);
        _owners[tokenId] = address(0);

        emit Transfer(owner, address(0), tokenId);
    }

    /**
     * @dev Transfers `tokenId` from `from` to `to`.
     *  As opposed to {transferFrom}, this imposes no restrictions on msg.sender.
     *
     * Requirements:
     *
     * - `to` cannot be the zero address.
     * - `tokenId` token must be owned by `from`.
     *
     * Emits a {Transfer} event.
     */
    function _transfer(
        address from,
        address to,
        uint256 tokenId
    ) internal virtual {
        require(ERC721B.ownerOf(tokenId) == from, "ERC721: transfer of token that is not own");
        require(to != address(0), "ERC721: transfer to the zero address");

        _beforeTokenTransfer(from, to, tokenId);

        // Clear approvals from the previous owner
        _approve(address(0), tokenId);
        _owners[tokenId] = to;

        emit Transfer(from, to, tokenId);
    }

    /**
     * @dev Approve `to` to operate on `tokenId`
     *
     * Emits a {Approval} event.
     */
    function _approve(address to, uint256 tokenId) internal virtual {
        _tokenApprovals[tokenId] = to;
        emit Approval(ERC721B.ownerOf(tokenId), to, tokenId);
    }


    /**
     * @dev Internal function to invoke {IERC721Receiver-onERC721Received} on a target address.
     * The call is not executed if the target address is not a contract.
     *
     * @param from address representing the previous owner of the given token ID
     * @param to target address that will receive the tokens
     * @param tokenId uint256 ID of the token to be transferred
     * @param _data bytes optional data to send along with the call
     * @return bool whether the call correctly returned the expected magic value
     */
    function _checkOnERC721Received(
        address from,
        address to,
        uint256 tokenId,
        bytes memory _data
    ) private returns (bool) {
        if (to.isContract()) {
            try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, _data) returns (bytes4 retval) {
                return retval == IERC721Receiver.onERC721Received.selector;
            } catch (bytes memory reason) {
                if (reason.length == 0) {
                    revert("ERC721: transfer to non ERC721Receiver implementer");
                } else {
                    assembly {
                        revert(add(32, reason), mload(reason))
                    }
                }
            }
        } else {
            return true;
        }
    }

    /**
     * @dev Hook that is called before any token transfer. This includes minting
     * and burning.
     *
     * Calling conditions:
     *
     * - When `from` and `to` are both non-zero, ``from``'s `tokenId` will be
     * transferred to `to`.
     * - When `from` is zero, `tokenId` will be minted for `to`.
     * - When `to` is zero, ``from``'s `tokenId` will be burned.
     * - `from` and `to` are never both zero.
     *
     * To learn more about hooks, head to xref:ROOT:extending-contracts.adoc#using-hooks[Using Hooks].
     */
    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 tokenId
    ) internal virtual {}
}
ERC721 override by Azuki
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";
import "@openzeppelin/contracts/utils/Address.sol";
import "@openzeppelin/contracts/utils/Context.sol";
import "@openzeppelin/contracts/utils/Strings.sol";
import "@openzeppelin/contracts/utils/introspection/ERC165.sol";

/**
 * @dev Implementation of https://eips.ethereum.org/EIPS/eip-721[ERC721] Non-Fungible Token Standard, including
 * the Metadata and Enumerable extension. Built to optimize for lower gas during batch mints.
 *
 * Assumes serials are sequentially minted starting at 0 (e.g. 0, 1, 2, 3..).
 *
 * Assumes the number of issuable tokens (collection size) is capped and fits in a uint128.
 *
 * Does not support burning tokens to address(0).
 */
contract ERC721A is
  Context,
  ERC165,
  IERC721,
  IERC721Metadata,
  IERC721Enumerable
{
  using Address for address;
  using Strings for uint256;

  struct TokenOwnership {
    address addr;
    uint64 startTimestamp;
  }

  struct AddressData {
    uint128 balance;
    uint128 numberMinted;
  }

  uint256 private currentIndex = 0;

  uint256 internal immutable collectionSize;
  uint256 internal immutable maxBatchSize;

  // Token name
  string private _name;

  // Token symbol
  string private _symbol;

  // Mapping from token ID to ownership details
  // An empty struct value does not necessarily mean the token is unowned. See ownershipOf implementation for details.
  mapping(uint256 => TokenOwnership) private _ownerships;

  // Mapping owner address to address data
  mapping(address => AddressData) private _addressData;

  // Mapping from token ID to approved address
  mapping(uint256 => address) private _tokenApprovals;

  // Mapping from owner to operator approvals
  mapping(address => mapping(address => bool)) private _operatorApprovals;

  /**
   * @dev
   * `maxBatchSize` refers to how much a minter can mint at a time.
   * `collectionSize_` refers to how many tokens are in the collection.
   */
  constructor(
    string memory name_,
    string memory symbol_,
    uint256 maxBatchSize_,
    uint256 collectionSize_
  ) {
    require(
      collectionSize_ > 0,
      "ERC721A: collection must have a nonzero supply"
    );
    require(maxBatchSize_ > 0, "ERC721A: max batch size must be nonzero");
    _name = name_;
    _symbol = symbol_;
    maxBatchSize = maxBatchSize_;
    collectionSize = collectionSize_;
  }

  /**
   * @dev See {IERC721Enumerable-totalSupply}.
   */
  function totalSupply() public view override returns (uint256) {
    return currentIndex;
  }

  /**
   * @dev See {IERC721Enumerable-tokenByIndex}.
   */
  function tokenByIndex(uint256 index) public view override returns (uint256) {
    require(index < totalSupply(), "ERC721A: global index out of bounds");
    return index;
  }

  /**
   * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}.
   * This read function is O(collectionSize). If calling from a separate contract, be sure to test gas first.
   * It may also degrade with extremely large collection sizes (e.g >> 10000), test for your use case.
   */
  function tokenOfOwnerByIndex(address owner, uint256 index)
    public
    view
    override
    returns (uint256)
  {
    require(index < balanceOf(owner), "ERC721A: owner index out of bounds");
    uint256 numMintedSoFar = totalSupply();
    uint256 tokenIdsIdx = 0;
    address currOwnershipAddr = address(0);
    for (uint256 i = 0; i < numMintedSoFar; i++) {
      TokenOwnership memory ownership = _ownerships[i];
      if (ownership.addr != address(0)) {
        currOwnershipAddr = ownership.addr;
      }
      if (currOwnershipAddr == owner) {
        if (tokenIdsIdx == index) {
          return i;
        }
        tokenIdsIdx++;
      }
    }
    revert("ERC721A: unable to get token of owner by index");
  }

  /**
   * @dev See {IERC165-supportsInterface}.
   */
  function supportsInterface(bytes4 interfaceId)
    public
    view
    virtual
    override(ERC165, IERC165)
    returns (bool)
  {
    return
      interfaceId == type(IERC721).interfaceId ||
      interfaceId == type(IERC721Metadata).interfaceId ||
      interfaceId == type(IERC721Enumerable).interfaceId ||
      super.supportsInterface(interfaceId);
  }

  /**
   * @dev See {IERC721-balanceOf}.
   */
  function balanceOf(address owner) public view override returns (uint256) {
    require(owner != address(0), "ERC721A: balance query for the zero address");
    return uint256(_addressData[owner].balance);
  }

  function _numberMinted(address owner) internal view returns (uint256) {
    require(
      owner != address(0),
      "ERC721A: number minted query for the zero address"
    );
    return uint256(_addressData[owner].numberMinted);
  }

  function ownershipOf(uint256 tokenId)
    internal
    view
    returns (TokenOwnership memory)
  {
    require(_exists(tokenId), "ERC721A: owner query for nonexistent token");

    uint256 lowestTokenToCheck;
    if (tokenId >= maxBatchSize) {
      lowestTokenToCheck = tokenId - maxBatchSize + 1;
    }

    for (uint256 curr = tokenId; curr >= lowestTokenToCheck; curr--) {
      TokenOwnership memory ownership = _ownerships[curr];
      if (ownership.addr != address(0)) {
        return ownership;
      }
    }

    revert("ERC721A: unable to determine the owner of token");
  }

  /**
   * @dev See {IERC721-ownerOf}.
   */
  function ownerOf(uint256 tokenId) public view override returns (address) {
    return ownershipOf(tokenId).addr;
  }

  /**
   * @dev See {IERC721Metadata-name}.
   */
  function name() public view virtual override returns (string memory) {
    return _name;
  }

  /**
   * @dev See {IERC721Metadata-symbol}.
   */
  function symbol() public view virtual override returns (string memory) {
    return _symbol;
  }

  /**
   * @dev See {IERC721Metadata-tokenURI}.
   */
  function tokenURI(uint256 tokenId)
    public
    view
    virtual
    override
    returns (string memory)
  {
    require(
      _exists(tokenId),
      "ERC721Metadata: URI query for nonexistent token"
    );

    string memory baseURI = _baseURI();
    return
      bytes(baseURI).length > 0
        ? string(abi.encodePacked(baseURI, tokenId.toString()))
        : "";
  }

  /**
   * @dev Base URI for computing {tokenURI}. If set, the resulting URI for each
   * token will be the concatenation of the `baseURI` and the `tokenId`. Empty
   * by default, can be overriden in child contracts.
   */
  function _baseURI() internal view virtual returns (string memory) {
    return "";
  }

  /**
   * @dev See {IERC721-approve}.
   */
  function approve(address to, uint256 tokenId) public override {
    address owner = ERC721A.ownerOf(tokenId);
    require(to != owner, "ERC721A: approval to current owner");

    require(
      _msgSender() == owner || isApprovedForAll(owner, _msgSender()),
      "ERC721A: approve caller is not owner nor approved for all"
    );

    _approve(to, tokenId, owner);
  }

  /**
   * @dev See {IERC721-getApproved}.
   */
  function getApproved(uint256 tokenId) public view override returns (address) {
    require(_exists(tokenId), "ERC721A: approved query for nonexistent token");

    return _tokenApprovals[tokenId];
  }

  /**
   * @dev See {IERC721-setApprovalForAll}.
   */
  function setApprovalForAll(address operator, bool approved) public override {
    require(operator != _msgSender(), "ERC721A: approve to caller");

    _operatorApprovals[_msgSender()][operator] = approved;
    emit ApprovalForAll(_msgSender(), operator, approved);
  }

  /**
   * @dev See {IERC721-isApprovedForAll}.
   */
  function isApprovedForAll(address owner, address operator)
    public
    view
    virtual
    override
    returns (bool)
  {
    return _operatorApprovals[owner][operator];
  }

  /**
   * @dev See {IERC721-transferFrom}.
   */
  function transferFrom(
    address from,
    address to,
    uint256 tokenId
  ) public override {
    _transfer(from, to, tokenId);
  }

  /**
   * @dev See {IERC721-safeTransferFrom}.
   */
  function safeTransferFrom(
    address from,
    address to,
    uint256 tokenId
  ) public override {
    safeTransferFrom(from, to, tokenId, "");
  }

  /**
   * @dev See {IERC721-safeTransferFrom}.
   */
  function safeTransferFrom(
    address from,
    address to,
    uint256 tokenId,
    bytes memory _data
  ) public override {
    _transfer(from, to, tokenId);
    require(
      _checkOnERC721Received(from, to, tokenId, _data),
      "ERC721A: transfer to non ERC721Receiver implementer"
    );
  }

  /**
   * @dev Returns whether `tokenId` exists.
   *
   * Tokens can be managed by their owner or approved accounts via {approve} or {setApprovalForAll}.
   *
   * Tokens start existing when they are minted (`_mint`),
   */
  function _exists(uint256 tokenId) internal view returns (bool) {
    return tokenId < currentIndex;
  }

  function _safeMint(address to, uint256 quantity) internal {
    _safeMint(to, quantity, "");
  }

  /**
   * @dev Mints `quantity` tokens and transfers them to `to`.
   *
   * Requirements:
   *
   * - there must be `quantity` tokens remaining unminted in the total collection.
   * - `to` cannot be the zero address.
   * - `quantity` cannot be larger than the max batch size.
   *
   * Emits a {Transfer} event.
   */
  function _safeMint(
    address to,
    uint256 quantity,
    bytes memory _data
  ) internal {
    uint256 startTokenId = currentIndex;
    require(to != address(0), "ERC721A: mint to the zero address");
    // We know if the first token in the batch doesn't exist, the other ones don't as well, because of serial ordering.
    require(!_exists(startTokenId), "ERC721A: token already minted");
    require(quantity <= maxBatchSize, "ERC721A: quantity to mint too high");

    _beforeTokenTransfers(address(0), to, startTokenId, quantity);

    AddressData memory addressData = _addressData[to];
    _addressData[to] = AddressData(
      addressData.balance + uint128(quantity),
      addressData.numberMinted + uint128(quantity)
    );
    _ownerships[startTokenId] = TokenOwnership(to, uint64(block.timestamp));

    uint256 updatedIndex = startTokenId;

    for (uint256 i = 0; i < quantity; i++) {
      emit Transfer(address(0), to, updatedIndex);
      require(
        _checkOnERC721Received(address(0), to, updatedIndex, _data),
        "ERC721A: transfer to non ERC721Receiver implementer"
      );
      updatedIndex++;
    }

    currentIndex = updatedIndex;
    _afterTokenTransfers(address(0), to, startTokenId, quantity);
  }

  /**
   * @dev Transfers `tokenId` from `from` to `to`.
   *
   * Requirements:
   *
   * - `to` cannot be the zero address.
   * - `tokenId` token must be owned by `from`.
   *
   * Emits a {Transfer} event.
   */
  function _transfer(
    address from,
    address to,
    uint256 tokenId
  ) private {
    TokenOwnership memory prevOwnership = ownershipOf(tokenId);

    bool isApprovedOrOwner = (_msgSender() == prevOwnership.addr ||
      getApproved(tokenId) == _msgSender() ||
      isApprovedForAll(prevOwnership.addr, _msgSender()));

    require(
      isApprovedOrOwner,
      "ERC721A: transfer caller is not owner nor approved"
    );

    require(
      prevOwnership.addr == from,
      "ERC721A: transfer from incorrect owner"
    );
    require(to != address(0), "ERC721A: transfer to the zero address");

    _beforeTokenTransfers(from, to, tokenId, 1);

    // Clear approvals from the previous owner
    _approve(address(0), tokenId, prevOwnership.addr);

    _addressData[from].balance -= 1;
    _addressData[to].balance += 1;
    _ownerships[tokenId] = TokenOwnership(to, uint64(block.timestamp));

    // If the ownership slot of tokenId+1 is not explicitly set, that means the transfer initiator owns it.
    // Set the slot of tokenId+1 explicitly in storage to maintain correctness for ownerOf(tokenId+1) calls.
    uint256 nextTokenId = tokenId + 1;
    if (_ownerships[nextTokenId].addr == address(0)) {
      if (_exists(nextTokenId)) {
        _ownerships[nextTokenId] = TokenOwnership(
          prevOwnership.addr,
          prevOwnership.startTimestamp
        );
      }
    }

    emit Transfer(from, to, tokenId);
    _afterTokenTransfers(from, to, tokenId, 1);
  }

  /**
   * @dev Approve `to` to operate on `tokenId`
   *
   * Emits a {Approval} event.
   */
  function _approve(
    address to,
    uint256 tokenId,
    address owner
  ) private {
    _tokenApprovals[tokenId] = to;
    emit Approval(owner, to, tokenId);
  }

  uint256 public nextOwnerToExplicitlySet = 0;

  /**
   * @dev Explicitly set `owners` to eliminate loops in future calls of ownerOf().
   */
  function _setOwnersExplicit(uint256 quantity) internal {
    uint256 oldNextOwnerToSet = nextOwnerToExplicitlySet;
    require(quantity > 0, "quantity must be nonzero");
    uint256 endIndex = oldNextOwnerToSet + quantity - 1;
    if (endIndex > collectionSize - 1) {
      endIndex = collectionSize - 1;
    }
    // We know if the last one in the group exists, all in the group exist, due to serial ordering.
    require(_exists(endIndex), "not enough minted yet for this cleanup");
    for (uint256 i = oldNextOwnerToSet; i <= endIndex; i++) {
      if (_ownerships[i].addr == address(0)) {
        TokenOwnership memory ownership = ownershipOf(i);
        _ownerships[i] = TokenOwnership(
          ownership.addr,
          ownership.startTimestamp
        );
      }
    }
    nextOwnerToExplicitlySet = endIndex + 1;
  }

  /**
   * @dev Internal function to invoke {IERC721Receiver-onERC721Received} on a target address.
   * The call is not executed if the target address is not a contract.
   *
   * @param from address representing the previous owner of the given token ID
   * @param to target address that will receive the tokens
   * @param tokenId uint256 ID of the token to be transferred
   * @param _data bytes optional data to send along with the call
   * @return bool whether the call correctly returned the expected magic value
   */
  function _checkOnERC721Received(
    address from,
    address to,
    uint256 tokenId,
    bytes memory _data
  ) private returns (bool) {
    if (to.isContract()) {
      try
        IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, _data)
      returns (bytes4 retval) {
        return retval == IERC721Receiver(to).onERC721Received.selector;
      } catch (bytes memory reason) {
        if (reason.length == 0) {
          revert("ERC721A: transfer to non ERC721Receiver implementer");
        } else {
          assembly {
            revert(add(32, reason), mload(reason))
          }
        }
      }
    } else {
      return true;
    }
  }

  /**
   * @dev Hook that is called before a set of serially-ordered token ids are about to be transferred. This includes minting.
   *
   * startTokenId - the first token id to be transferred
   * quantity - the amount to be transferred
   *
   * Calling conditions:
   *
   * - When `from` and `to` are both non-zero, ``from``'s `tokenId` will be
   * transferred to `to`.
   * - When `from` is zero, `tokenId` will be minted for `to`.
   */
  function _beforeTokenTransfers(
    address from,
    address to,
    uint256 startTokenId,
    uint256 quantity
  ) internal virtual {}

  /**
   * @dev Hook that is called after a set of serially-ordered token ids have been transferred. This includes
   * minting.
   *
   * startTokenId - the first token id to be transferred
   * quantity - the amount to be transferred
   *
   * Calling conditions:
   *
   * - when `from` and `to` are both non-zero.
   * - `from` and `to` are never both zero.
   */
  function _afterTokenTransfers(
    address from,
    address to,
    uint256 startTokenId,
    uint256 quantity
  ) internal virtual {}
}

What is AGC? That contract has a very expensive balanceOf function and it could potentially run out of gas, this can in some cases be used as an attack vector. The issue is it does a storage read for every token id.

I haven't seen Azuki's code in detail but on quick inspection the for loops seem bounded, which is what you want, except for tokenOfOwnerByIndex which is arguably less of a problem since it will be invoked off chain. It will not perform better though and may result in a very slow UI if it uses this method.

Again I haven't seen these in detail yet so take this with a grain of salt.

Thank you for your answer.

Both examples are contracts for different NFT projects where they sell a collection of 10k tokens. Both are trying to optimize mint gas price by removing/improving the ERC721Enumerable.
The AGC is the Alpha Girls Club project.
The Azuki is the Azuki project.

I don't think I got what's the problem with the balanceOf function in the first example given that we have a limited number of tokens (only 10k). Would that still be a problem if the total amount for iteration can't exceed 10k items? My local tests haven't shown any problems.

For the second one, the biggest limitation is that it doesn't support token burning yet. Another thing I don't like is that they've changed tokens ownership. If you mint multiple tokens their contract sets an ownership record only for the first one. The rest is supposed to be "yours" before the next "ownership record". Here is an example, you mint 1, 2, 3. They set ownership on for the token with id 1. They assume that everything that goes after 1 and before the next ownership record belongs to your address. I might be unclear but you can get details here. This trick makes it possible to significantly save on gas prices for bulk mints.

I'm wondering what's the best way to optimize gas cost given that a project needs the same functionality offered by ERC721Enumerable (on-chain enumeration of all tokens or those owned by an account)?

I was curious, so I spent the time of running a benchmark of Azuki vs OZ.

One should keep in mind that there are important differences. OZ includes a burn mechanism that Azuki doesn't have. OZ also allow minting arbitrary tokenIds, which is necessary to some use cases. On the other hand, Azuki allows minting multiple tokens at one, which OZ doesn't support.

Having said that, lets have a look at gas usage report, produced by our behavior test that was slightly modified to support Azuki sequential minting. Note that this runs on a optimized and stripped (revert strings removed) version of te contract.

ยท-----------------------------------|---------------------------|-------------|-----------------------------ยท
|       Solc version: 0.8.11        ยท  Optimizer enabled: true  ยท  Runs: 200  ยท  Block limit: 30000000 gas  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  Methods                          ยท               68 gwei/gas               ยท       2880.02 usd/eth       โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  Contract   ยท  Method             ยท  Min        ยท  Max        ยท  Avg        ยท  # calls      ยท  usd (avg)  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  AzukiMock  ยท  approve            ยท      28795  ยท      53236  ยท      47280  ยท          226  ยท       9.26  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  AzukiMock  ยท  safeTransferFrom   ยท      40755  ยท      77139  ยท      67111  ยท           62  ยท      13.14  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  AzukiMock  ยท  safeTransferFrom   ยท      41432  ยท      78261  ยท      67989  ยท           62  ยท      13.32  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  AzukiMock  ยท  setApprovalForAll  ยท      26210  ยท      46122  ยท      45493  ยท          190  ยท       8.91  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  AzukiMock  ยท  transferFrom       ยท      40399  ยท      69803  ยท      62453  ยท           29  ยท      12.23  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  OZMock     ยท  approve            ยท      26649  ยท      51090  ยท      45134  ยท          226  ยท       8.84  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  OZMock     ยท  safeTransferFrom   ยท      33267  ยท      72488  ยท      62004  ยท           62  ยท      12.14  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  OZMock     ยท  safeTransferFrom   ยท      33991  ยท      73654  ยท      62928  ยท           62  ยท      12.32  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  OZMock     ยท  setApprovalForAll  ยท      26290  ยท      46202  ยท      45573  ยท          190  ยท       8.93  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท
|  OZMock     ยท  transferFrom       ยท      32887  ยท      65127  ยท      57289  ยท           29  ยท      11.22  โ”‚
ยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยทยท|ยทยทยทยทยทยทยทยทยทยทยทยทยทยท

IMO Azuki is possibly a good solution in usecases that need batch sequential minting, and that do not require burning. But it is not gas optimized compared to OZ for most "standard" user operations.

Edit: this is OZ without enumerability

4 Likes

Not that ERC721Enumerable include

  • totalSupply
  • tokenOfOwnerByIndex
  • tokenByIndex

When you are minting sequentially and never burn, totalSupply and tokenByIndex are trivial function (the first one is your "counter", the second one is just the identity function.

tokenOfOwnerByIndex requires a loop over the totalSupply. Each iteration of the loop is > 2100gas (that is just the cost of the load to get the i-th token owner) ...

  • so this is going to work fine for off-chain accesses (which could be achieved using other solutions, including but not limited to subgraphs)
  • this is going to break most contract that need to query that onchain.

If the minting is not sequential, and if it includes burn ... then good luck with that :stuck_out_tongue:

IMO, If you don't need that onchain, you should not include it at all. Having it available and then causing out of gas reverts when someone tries to use it onchain does more harm then good IMO. If you really need it onchain, then pay some price on each transfer so that it can actually be queried.

The bad part is that it may generally work, but it's a denial of service attack vector and will be abused when the incentives align.

I always felt like ERC721Enumerable.tokenOfOwnerByIndex made no sense in most cases. Too many implications on-chain, while it is quite straightforward to do it off-chain looping through logs.
IMO, usecases that need batch sequential minting and no burning could use Azuki without tokenOfOwnerByIndex

Azuki is well written and has optimised gas in many areas, batch minting etc, but agreed with @Amxx that exposing functions which is bound to run out of gas is a concern. Problem is I assume most people did not consider querying for this information on chain vs off chain.

This thread sums up our position pretty well.

2 Likes