Where is "tokenIDForOwnerByIndex" ?

[charlesmarino]

So there is really no method to see what NFTs an owner owns? Or Am I missing something here.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol

[Amxx]

Hello @charlesmarino.

There "enumerability" functions are very expensive in term of gas, and the corresponding information is easy to rebuild offchain. Several users requested it not being available by default, so they can deploy smaller, erc721 compliant, contract.

This feature isn't gone however, it was moved to the ERC721Enumerable extension: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/extensions/ERC721Enumerable.sol

Is you want it, you can just inherit from this extensions, and everything should be good.

[charlesmarino]

It's important to remember that "Easy" is relative here and maybe a compliant contract shouldn't require an offchain service for essential methods. Because with this, Developers have to rely on a centralized service to develop anything with the token. This makes the decentralized aspect a bit pointless to me.

Being able to view the tokens an owner has is fundamental. So, is there a way to solve this for the erc721 contract or is the norm to rely on a centralized service or build your own?

[Amxx]

You have to understand that some people want to lower their gas cost as much as possible, and there is no reason for us to force them having non-standard function as part of their token when we consequence is a non-trivial gas cost increases.

You seem to value these function a lot, and be ready to pay the associated gas cost, so I invite you to inherit from ERC721Enumerable. Again, this feature is not gone, its just on another file.

[Amxx]

Also, rebuilding the historical data (of events) doesn't require much more then an ethereum node and some easy to read/find open source code.

If you care about decentralization so much, and run your very own ethereum node because you are afraid infura, alchemy and etherscan might collude, then you should know that it is very easy to run your own graphnode and connect it to your ethereum node.

[charlesmarino]

Creating a standard is not forcing anyone. It's a standard to build decentralized permissionless assets accepted by the community. And Yes I value this "non-standard" function "a lot" because the most basic functionality of these assets is being able to see which you own.

Let's say as a developer I want to build something with these assets. To even see what assets the owner has, (which should be a simple method call) I have to create my own centralized service (you suggest graphnode). which is definitely not as trivial as you are suggesting plus a lot of developers don't want to depend on a centralized service. Removing this method increases the barrier to entry to work on these assets and makes it less decentralized. This is a matter of priority. Making the standard a decentralized NFT or making these assets cheaper to use but relying on centralized services.

Do you really believe that the standard shouldn't allow people to see what assets they own without a third-party service?

[Amxx]

Do you really believe that the standard shouldn't allow people to see what assets they own without a third-party service?

Its not up to me to decide what the standard should and shouldn't include.

Also the standard doesn't restrict anyone from having these function, it doesn't require them, which is different. The standard here is a final ERC, and some users only want to have the functions that are part of this final ERC. This is why be decided to align the base implementation with the standard.

But again, if you want to expand your contract, you are free to do so, no one is telling you not to ! And this is exactly what we offer with the extension system. The counter-part is that you shouldn't force other people to have the extensions that they don't see as necessary, or that they consider are to much gas intensive to be worth it.

[frangio]

@charlesmarino Your comments are coming across as angry and pissed of at us. Let's try to keep this a productive conversation.

The change we have done here is to make ERC721 enumerability opt-in through the ERC721Enumerable extension. This was not our whim, it was based on requests from users, and is compliant with ERC721 where enumerability has always been optional. If you have an issue with the ERC721 spec this is not the place for that conversation. I'll invite you to create a thread in ethereum-magicians.org to discuss the problems you see in the ERC and possible ways to solve them.

An ERC721 token without enumerability does not require a centralized service to list the tokens owned by an account. The Transfer events emitted by the contract are enough to assemble this list of tokens using any of the JSON RPC calls that access logs (e.g. eth_getLogs), which are readily available in any high level Ethereum library like Web3.js or Ethers.js. This is no more centralized than using the same node to call an enumeration function on a contract.

Please explain the problems that you see in this solution.

[charlesmarino]

Do you really believe that the standard shouldn't allow people to see what assets they own without a third-party service?

Its not up to me to decide what the standard should and shouldn't include.

Also the standard doesn't restrict anyone from having these function, it doesn't require them, which is different. The standard here is a final ERC, and some users only want to have the functions that are part of this final ERC. This is why be decided to align the base implementation with the standard.

But again, if you want to expand your contract, you are free to do so, no one is telling you not to ! And this is exactly what we offer with the extension system. The counter-part is that you shouldn't force other people to have the extensions that they don't see as necessary, or that they consider are to much gas intensive to be worth it.

My issue isn't that I don't know how to expand my contract. It's that other developers will follow this standard and believe they are making a decentralized NFT but will then have their community face this problem. I believe it enumerability should be opt-out instead of opt-in. Otherwise, we are requiring users to create or use a centralized service in order view what NFTs a user owns. Hope I'm being clear.

[frangio]

My issue isn't that I don't know how to expand my contract.

For reference, this is the way it's done:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract MyNFT is ERC721, ERC721Enumerable {
  constructor() ERC721("Name", "SYM") {}
}

We understand that not everyone is comfortable enough to write this bit of code, so we're building an interactive tool to help with it. People will be able to choose the features they want in their contract and the tool will write this code for them. This will be released in the next couple of weeks.

[charlesmarino]

s emitted by the contract are enough to assemble this list of tokens using any of the JSON RPC calls that acces

It's more centralized because the barrier to entry to now read what NFT tokens you have is higher. For example, can you write here the lines of code that someone must now write to fetch the tokens of an Owner?

Before it was tokenOfOwnerByIndex(owner, index).

Sorry if I come off angry lol. I'm annoyed that this is the standard for the decentralized NFT and other developers cannot easily fetch what NFT tokens an owner has.

[charlesmarino]

My issue isn't that I don't know how to expand my contract.

For reference, this is the way it's done:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract MyNFT is ERC721, ERC721Enumerable {
  constructor() ERC721("Name", "SYM") {}
}

We understand that not everyone is comfortable enough to write this bit of code, so we're building an interactive tool to help with it. People will be able to choose the features they want in their contract and the tool will write this code for them. This will be released in the next couple of weeks.

I said "My issue isn't that I don't know how to expand my contract." :'(

I'm guessing you misread my sentence ? Or you felt like showing me exactly what I said isn't my issue.

I am very comfortable writing that code. I am discussing fetching what tokens an owner has from the contract that is the standard.

[frangio]

Ah, my bad, I misread.

[frangio]

For example, can you write here the lines of code that someone must now write to fetch the tokens of an Owner?

I wrote this function to list the tokens of an owner using Ethers.js and Hardhat:

async function listTokensOfOwner({ token: tokenAddress, account }) {
  const token = await ethers.getContractAt(ERC721.abi, tokenAddress, ethers.provider);

  console.error(await token.name(), 'tokens owned by', account);

  const sentLogs = await token.queryFilter(
    token.filters.Transfer(account, null),
  );
  const receivedLogs = await token.queryFilter(
    token.filters.Transfer(null, account),
  );

  const logs = sentLogs.concat(receivedLogs)
    .sort(
      (a, b) =>
        a.blockNumber - b.blockNumber ||
        a.transactionIndex - b.TransactionIndex,
    );

  const owned = new Set();

  for (const log of logs) {
    const { from, to, tokenId } = log.args;

    if (addressEqual(to, account)) {
      owned.add(tokenId.toString());
    } else if (addressEqual(from, account)) {
      owned.delete(tokenId.toString());
    }
  }

  console.log([...owned].join('\n'));
};

The code is in frangio/erc721-list.

[frangio]

Closing this as resolved. Let us know if we can help any other way.

[benjick]

@frangio thanks for the solution, but how would I do that with IERC1155?

[frangio]

@benjick You'd do something similar but using ERC1155 events. An additional complication is the presence of both "TransferSingle" and "TransferBatch" events.

At this point I think the best recommendation is to use TheGraph for querying. We have schemas for ERC1155 in OpenZeppelin Subgraphs.

[Amxx]

I would add that there is a generic subgraph for ERC1155 already deployed (for mainnet) https://thegraph.com/hosted-service/subgraph/amxx/eip1155-subgraph

[benjick]

@frangio @Amxx thanks, I will look into this!

[ghost]

Wow! This was an issue i had for a while. This discussion was informative. thanks!

[ghost]

For example, can you write here the lines of code that someone must now write to fetch the tokens of an Owner?

I wrote this function to list the tokens of an owner using Ethers.js and Hardhat:

async function listTokensOfOwner({ token: tokenAddress, account }) {
  const token = await ethers.getContractAt(ERC721.abi, tokenAddress, ethers.provider);

  console.error(await token.name(), 'tokens owned by', account);

  const sentLogs = await token.queryFilter(
    token.filters.Transfer(account, null),
  );
  const receivedLogs = await token.queryFilter(
    token.filters.Transfer(null, account),
  );

  const logs = sentLogs.concat(receivedLogs)
    .sort(
      (a, b) =>
        a.blockNumber - b.blockNumber ||
        a.transactionIndex - b.TransactionIndex,
    );

  const owned = new Set();

  for (const log of logs) {
    const { from, to, tokenId } = log.args;

    if (addressEqual(to, account)) {
      owned.add(tokenId.toString());
    } else if (addressEqual(from, account)) {
      owned.delete(tokenId.toString());
    }
  }

  console.log([...owned].join('\n'));
};

The code is in frangio/erc721-list.

How efficient is this? Is there a more efficient way?

[moscarelli]

Well as @charlesmarino mentioned, this change made impossible (or at least very hard) to follow simple examples that already exists in the web. They all use tokenIDForOwnerByIndex and if you try follow the implementation on the ethereum site (https://ethereum.org/en/developers/tutorials/how-to-write-and-deploy-an-nft/ ) for contract example and you try inherit ERC721Enumerable several issues start showup. Any sugestion on how use ERC721Enumerable without get errors like

DeclarationError: Undeclared identifier. --> contracts/nftNews.sol:25:9: | 25 | _setTokenURI(newItemId, tokenURI); | ^^^^^^^^^^^^

Error HH600: Compilation failed

or

Derived contract must override function "_beforeTokenTransfer". Two or more base classes define function with same name and parameter types. Derived contract must override function "_burn". Two or more base classes define function with same name and parameter types. Derived contract must override function "supportsInterface". Two or more base classes define function with same name and parameter types. Derived contract must override function "tokenURI". Two or more base classes define function with same name and parameter types.

if I remove any of the itens from the extension I ge errors if I add I get other type of erros.

If anyone can clarify I would apreciate.

[Amxx]

@moscarelli As we said previously, using Enumerability is generally a bad idea. Tutorial use it because its easy ... but unfortunatelly the easy solutions are often not the right ones.

Regardless, if you really want to deploy an enumerable ERC721 instance, the wizard will show you how to.

I would also argue that it would be a good idea for you to learn about function override and inheritance before pushing any code to production.

[moscarelli]

Hello and Thanks :) I understand override and inheritance, is just very frustrating to follow a tutorial to learn something, but the tutorial is no longer valid :/ . I also understand the necessity of not wasting gas on stuff is not needed, but as a tutorial and as a test for people that are trying to learn this stuff facing this kind of issue right at the beginning might be complicated :D

In the end, I came across https://wizard.openzeppelin.com/ by myself.

So, to be able to use the contract example on Ethereum tutorials and have access to the method tokenOfOwnerByIndex we also need to import ERC721URIStorage.sol.sol, otherwise, we get the error of missing _setTokenURI method.

But for the sake of the ones that get to this conversation, knowing how much time they might take for a simple answer, I will let the base implementation I'm using with the tutorials.

pragma solidity ^0.8.0;

import "@openzeppelin/contracts/utils/Counters.sol";
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721Enumerable.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@openzeppelin/contracts/access/Ownable.sol";

contract MyNFT is ERC721URIStorage, Ownable, ERC721Enumerable {
    using Counters for Counters.Counter;
    Counters.Counter private _tokenIds;

    constructor() ERC721("MyNFT ", "MNFT") {}

    function mintNFT(address recipient, string memory tokenURI)
        public onlyOwner
        returns (uint256)
    {
        _tokenIds.increment();

        uint256 newItemId = _tokenIds.current();
        _mint(recipient, newItemId);
        _setTokenURI(newItemId, tokenURI);

        return newItemId;
    }   
    function _beforeTokenTransfer(address from, address to, uint256 tokenId)
        internal
        override(ERC721, ERC721Enumerable)
    {
        super._beforeTokenTransfer(from, to, tokenId);
    }

    function _burn(uint256 tokenId) internal override(ERC721, ERC721URIStorage) {
        super._burn(tokenId);
    }

    function tokenURI(uint256 tokenId)
        public
        View
        override(ERC721, ERC721URIStorage)
        returns (string memory)
    {
        return super.tokenURI(tokenId);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        View
        override(ERC721, ERC721Enumerable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}

[Amxx]

If you "just" want enumerability, you don't need to include anything but ERC721 & ERC721Enumerable.

This will compile:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

import "[@openzeppelin/contracts/token/ERC721/ERC721.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/token/ERC721/ERC721.sol)";
import "[@openzeppelin/contracts/token/ERC721/extensions/ERC721Enumerable.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/token/ERC721/extensions/ERC721Enumerable.sol)";

contract MyToken is ERC721, ERC721Enumerable {
    constructor() ERC721("MyToken", "MTK") {}

    // The following functions are overrides required by Solidity.

    function _beforeTokenTransfer(address from, address to, uint256 tokenId)
        internal
        override(ERC721, ERC721Enumerable)
    {
        super._beforeTokenTransfer(from, to, tokenId);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(ERC721, ERC721Enumerable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}

If, like in your code example, you also want URIStorage & mintable with incremental Ids, you can check that two boxes and you'll get

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;

import "[@openzeppelin/contracts/token/ERC721/ERC721.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/token/ERC721/ERC721.sol)";
import "[@openzeppelin/contracts/token/ERC721/extensions/ERC721Enumerable.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/token/ERC721/extensions/ERC721Enumerable.sol)";
import "[@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/token/ERC721/extensions/ERC721URIStorage.sol)";
import "[@openzeppelin/contracts/access/Ownable.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/access/Ownable.sol)";
import "[@openzeppelin/contracts/utils/Counters.sol](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/utils/Counters.sol)";

contract MyToken is ERC721, ERC721Enumerable, ERC721URIStorage, Ownable {
    using Counters for Counters.Counter;

    Counters.Counter private _tokenIdCounter;

    constructor() ERC721("MyToken", "MTK") {}

    function safeMint(address to, string memory uri) public onlyOwner {
        uint256 tokenId = _tokenIdCounter.current();
        _tokenIdCounter.increment();
        _safeMint(to, tokenId);
        _setTokenURI(tokenId, uri);
    }

    // The following functions are overrides required by Solidity.

    function _beforeTokenTransfer(address from, address to, uint256 tokenId)
        internal
        override(ERC721, ERC721Enumerable)
    {
        super._beforeTokenTransfer(from, to, tokenId);
    }

    function _burn(uint256 tokenId) internal override(ERC721, ERC721URIStorage) {
        super._burn(tokenId);
    }

    function tokenURI(uint256 tokenId)
        public
        view
        override(ERC721, ERC721URIStorage)
        returns (string memory)
    {
        return super.tokenURI(tokenId);
    }

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(ERC721, ERC721Enumerable)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }
}

Again, the wizard should have you covered for most "simple" needs

[Amxx]

is just very frustrating to follow a tutorial to learn something, but the tutorial is no longer valid

Is that tutorial on an OpenZeppelin website? If so, we can update it! Otherwize, there is unfortunately not much we can do. We try to do as few breaking changes as we can, but at some point they do have to happen.

[moscarelli]

Hi. No, not just the enumerability, I was following the tutorial from ethereum site (https://ethereum.org/en/developers/tutorials/how-to-write-and-deploy-an-nft/) so for that example works I need all the extensions in the original code, and it works but when I was trying to list the tokens following other tutorials code it complained about the missing method, so add enumerability to the ethereum example generated all the issues I listed, using the generation tool I was able to use the example code alongise the enumerable, the code I posted is working. :) .

[cryptoBela]

For example, can you write here the lines of code that someone must now write to fetch the tokens of an Owner?

I wrote this function to list the tokens of an owner using Ethers.js and Hardhat:

async function listTokensOfOwner({ token: tokenAddress, account }) {
  const token = await ethers.getContractAt(ERC721.abi, tokenAddress, ethers.provider);

  console.error(await token.name(), 'tokens owned by', account);

  const sentLogs = await token.queryFilter(
    token.filters.Transfer(account, null),
  );
  const receivedLogs = await token.queryFilter(
    token.filters.Transfer(null, account),
  );

  const logs = sentLogs.concat(receivedLogs)
    .sort(
      (a, b) =>
        a.blockNumber - b.blockNumber ||
        a.transactionIndex - b.TransactionIndex,
    );

  const owned = new Set();

  for (const log of logs) {
    const { from, to, tokenId } = log.args;

    if (addressEqual(to, account)) {
      owned.add(tokenId.toString());
    } else if (addressEqual(from, account)) {
      owned.delete(tokenId.toString());
    }
  }

  console.log([...owned].join('\n'));
};

The code is in frangio/erc721-list.

How efficient is this? Is there a more efficient way?

How does this works with web3 ?