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

[Feature Request] nft.tokenOwner(collectionId,serialNumber) function #452

Open
felixshu opened this issue Feb 16, 2023 · 3 comments
Open

Comments

@felixshu
Copy link
Collaborator

felixshu commented Feb 16, 2023

Hi guys, we noticed that we removed the nft.tokenOwner(collectionId,serialNumber) function. Frontend cannot check the owner before sending extrinsic.

Senario:
I’m a user and I’m buying the NFT on marketplace, meanwhile the token owner accepted the offer before I buy the token, the transaction will failed (gas fee paid). Indexer cannot guarantee the realtime feedback, so we need this function. Can you add them back?

This is the standard function of ERC721 ownerOf(tokenId)

@justinfrevert
Copy link
Contributor

justinfrevert commented Feb 21, 2023

Hi Felix, I think this wasn't a part of Root since its inception. Ultimately, this list can be unbounded, and result in a query that is unreasonably large. Thus, we've opted to avoid functionality similar to this because it is hazardous to the node. We will perhaps index certain items and make them available, but there is no concrete plan at this moment.

For the time being, the recommendation is to index data like this in order to retrieve the requested owned tokens data.

@justinfrevert
Copy link
Contributor

Slight correct to my previous comment - there isn't an unbounded response length to this one, I was thinking of another case which is super similar and being looked at at the same time. However, our stance on offloading query work to indexers still stands. It just as important to the health of the blockchain that these sort of queries generally move to indexers.

@felixshu
Copy link
Collaborator Author

Slight correct to my previous comment - there isn't an unbounded response length to this one, I was thinking of another case which is super similar and being looked at at the same time. However, our stance on offloading query work to indexers still stands. It just as important to the health of the blockchain that these sort of queries generally move to indexers.

Thanks Justin, we have our own NFT indexers to tracking these changes, however, from my understanding:

  1. root network is the only truth of source, not indexer
  2. indexer is not real-time, especially when you have more than 200 txs in one block
  3. we can skip the ownership check before we send extrinsic

I have already read the code, and totally understand this is caused by data structure design of the pallet. Specifically, you need to get the collection information first, and then dig into the token owner. if you have 10K tokens in a collection, the query will be nightmare.

I will try to handle these from my side first.

cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants