Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ERC721Enumerable - Add back _tokensOfOwner() #3106

Closed
imsys opened this issue Jan 12, 2022 · 5 comments
Closed

ERC721Enumerable - Add back _tokensOfOwner() #3106

imsys opened this issue Jan 12, 2022 · 5 comments
Assignees
Labels
feature New contracts, functions, or helpers.

Comments

@imsys
Copy link

imsys commented Jan 12, 2022

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. bd07784

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()

@Amxx Amxx added the feature New contracts, functions, or helpers. label Jan 13, 2022
@Amxx Amxx self-assigned this Jan 13, 2022
@frangio
Copy link
Contributor

frangio commented Jan 13, 2022

The removal was not accidental, it's explained in #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
Copy link
Author

imsys commented Jan 15, 2022

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
Copy link
Author

imsys commented Jan 15, 2022

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
Copy link
Contributor

frangio commented Jan 17, 2022

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
Copy link
Author

imsys commented Jan 23, 2022

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

@imsys imsys closed this as completed Jan 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New contracts, functions, or helpers.
Projects
None yet
Development

No branches or pull requests

3 participants