About tokensOfOwner function

[Vectorized]

tokensOfOwner function is definitely quite useful for small capped collections
(probably > 90% of the collections you see these days).

/// @dev Returns the tokenIds of the address. O(totalSupply) in complexity.
function tokensOfOwner(address owner) external view returns (uint256[] memory) {
    unchecked {
        uint256[] memory a = new uint256[](balanceOf(owner)); 
        uint256 end = _currentIndex;
        uint256 tokenIdsIdx;
        address currOwnershipAddr;
        for (uint256 i; i < end; i++) {
            TokenOwnership memory ownership = _ownerships[i];
            if (ownership.burned) {
                continue;
            }
            if (ownership.addr != address(0)) {
                currOwnershipAddr = ownership.addr;
            }
            if (currOwnershipAddr == owner) {
                a[tokenIdsIdx++] = i;
            }
        }
        return a;    
    }
}

However, we have to consider the implications of including a O(totalSupply) function in the main library. Even if we know if it supports collections of 10,000 unique items, it is still probable that someone will use this function for a collection of say 100,000 unique items, and result in off-chain queries from Infura (Metamask) / Alchemy failing.

Even if we include a cautionary comment, someone might just not read it and use the function without understanding the caveats.

One possible way to provide users convenience, but still keep the main library general is to include this in an examples folder, instead of an extension.

Let me know what you think.

@fulldecent @cygaar

Related:

• Remove tokenOfOwnerByIndex #107
• Comments on ERC721A #18
• Listing all kitties owned by a user is O(n^2) dapperlabs/cryptokitties-bounty#4
• https://eips.ethereum.org/EIPS/eip-721

[Austinhs]

It seems like I have seen this debate between small cap/large cap a few times now. Thoughts on creating a "small cap" extension that could include multiple functions that might only be possible for these small cap collections?

[ahbanavi]

I have a question related to tokensOfOwner usage, Is checking for owners tokens on-chain really necessary?
Doesn't Off-chain APIs like Moralis do the job?

[Austinhs]

I have a question related to tokensOfOwner usage, Is checking for owners tokens on-chain really necessary? Doesn't Off-chain APIs like Moralis do the job?

Nothing against Moralis, but if it could be done without adding a 3rd party why would you make it a requirement to get this information? Also tokensOfOwner may have use cases within the contract. Say you have some sort of functionality like nft_id % 4 == 0 and you need to find how many of these tokens the user owns. You will need a list of their current holdings within the contract. You can't do that if you rely on Moralis to what I assume keep track of the balances based on transfer events.

[syffs]

@Austinhs you want to avoid O(totalSupply) methods which introduces DOS risks.

IMO it's acceptable to add this to a contract situationnally if:

• your totalSupply is low enough to mitigate the DOS risk
• you have a functional need to do this on-chain

Otherwise it's quite straightforward to do this off-chain through contract logs, or (assuming a wallet has less than 10k transfer logs) even etherscan api https://api.etherscan.io/api?module=account&action=tokennfttx&contractaddress=${contract}&address=${address}&apikey=${apikey}

[Austinhs]

@Austinhs you want to avoid O(totalSupply) methods which introduces DOS risks.

IMO it's acceptable to add this to a contract situationnally if:

• your totalSupply is low enough to mitigate the DOS risk
• you have a functional need to do this on-chain

Otherwise it's quite straightforward to do this off-chain through contract logs, or (assuming a wallet has less than 10k transfer logs) even etherscan api https://api.etherscan.io/api?module=account&action=tokennfttx&contractaddress=${contract}&address=${address}&apikey=${apikey}

Yeah I tend to agree with this like @Vectorized originally said this works well with majority of the low cap supplies <= 10,000 which 90% are. But a collection for example like crypto kiddies that have breeding and can easily hit 100k+ are the problem here. I think I will move this into a ERC721ALowCap extension that will allow helper functions like this to exist.

Also there is the option to just not add this -- but personally I've been doing some community service in twitter spaces to help newer dev's and ERC721A is a common topic. It would be hard for a newer dev to understand the architecture of ERC721A. Especially how to read the data, so I've actually been pointing them to this function to just explain how reading from ERC721A is a bit more complicated, I think adding a helper extension like ERC721ALowCap would be very beneficial in that regard

[Austinhs]

Updated #114 to add this as a extension

[kyokosdream]

What we did is just remove ERC721Enumerable from our version of the ERC721A contract. In my opinion most projects just don't require this functionality and it's just not necessary to include it. The only function we keep from ERC721Enumerable is totalSupply() which is an obvious necessity.

I'm having a hard time understanding why a project would need the extra ERC721Enumerable functions. Is it just so that they can easily enumerate an address' owned tokens so they can display them on their project website or some other frontend environment?

Would love some clarification, appreciate your help!

[wiasliaw]

@kyokosdream
I'm having a hard time understanding why a project would need the extra ERC721Enumerable functions. Is it just so that they can easily enumerate an address' owned tokens so they can display them on their project website or some other frontend environment?

Yes, some of the market places might require NFT should use ERC721Enumerable. It is more friendly to list all tokens in frontend. If someone ask for this feature in future, we might implement enumerable in extensions for developers to use.

[sherbakovdev]

Enumerable is useful when implementing staking UI and getting all the tokens of a specific wallet. It is always good to have the contract as a source of truth and not 3rd party API.

[zamzaaam]

I agree with what is said above. On a ERC721 test project, I loop on balanceOf along with tokenOfOwnerByIndex to display on the frontend the nft minted by the connected address without using a third party service. In terms of user experience it seems to me to be a real plus.
In terms of performance however I don't know if it's a good practice, I didn't know anything about smart contract until 2 months ago, and I'm working on marketing (no one is perfect...).

I'm interested if you have alternative solutions for that usecase.

[emirhansirkeci]

Hi, i have 10.000 items in my collection can i use this function? Is there any possiblity that this function crash my contract or my dapp?

[Vectorized]

@justChargin 10k is safe.

[Austinhs]

We added this, could we close this issue now?
@Vectorized

[sayhicoelho]

How can I call this function inside my contract? Why is it external and not public?

[fulldecent]

How can I call this function inside my contract? Why is it external and not public?

Our general advice is that you shouldn't be doing that.

But if we will be more specific, please explain exactly what you're doing, and the sizes of all things involved.

[Vectorized]

@sayhicoelho It is marked external on purpose, because it is intended only for off-chain queries.

Calling it on-chain is highly not recommended, as it can cost millions of gas for typically sized collections.

If you need to call it in your function (hopefully intended for off-chain queries), you can prefix it with a this..