ERC721Enumerable - Add back `_tokensOfOwner()`

[imsys]

tokensOfOwner() was first suggest on #1512 and implemented on #1522 and was removed in a bulk edit that makes me wonder if it was removed accidentally. https://github.com/OpenZeppelin/openzeppelin-contracts/commit/bd0778461da82364aebd2d763bb338795c5f9fa7

This function helps when coding web3 apps. Instead of calling the RPC multiple times to get all NFTs owned by the user, it can be done at once by calling tokensOfOwner()

[frangio]

The removal was not accidental, it's explained in https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2160.

Working with arbitrary sized arrays in Solidity is not a good idea, as it's a potential attack vector. I don't think this is something we should add back. What is your use case?

[imsys]

I can see the problem if someone tried to loop over a long array in Solidity, is there any other problem that I may be missing?

The thing is that in a webapp, if the user has 20 or more NFTs, I have to make 20 requests to the RPC to get all the ids. Maybe 20 more two get the metadata, but that I can have previously cached in the server. Isn't it too inefficient to make that many calls to a RPC server? Or am I over thinking/worrying? I know we can use 3rd party APIs like Moralis that keeps a track of all the NFTs a user owns, but I would like to not be dependent on them for this if possible.

And now thinking about it, maybe I could just create a secondary contract with tokensOfOwner() that would loop over tokenOfOwnerByIndex(), and by having a RPC calling it would be a gasless way to get the list of all NFTs owned by the user. 🤔 I will try this.

[imsys]

So, I just tested this, and it works!

I know it is an unbounded loop, but it doesn't seem to be a problem for a RPC.

pragma solidity ^0.8.0;

// SPDX-License-Identifier: MIT

import "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";

contract ListNFT {

    function listUserNFTs(address contractAddress, address owner) external view returns (uint[] memory) {

        uint balance = IERC721Enumerable(contractAddress).balanceOf(owner);

        uint[] memory tokens = new uint[](balance);

        for (uint i=0; i<balance; i++) {
            tokens[i] = (IERC721Enumerable(contractAddress).tokenOfOwnerByIndex(owner, i));
        }

        return tokens;
    }
}

[frangio]

it doesn't seem to be a problem for a RPC.

That's right, RPC eth_call allows much more gas than a transaction so it wouldn't be a problem there.

[imsys]

For me this can be closed then, unless someone else has anything else to add.