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

nft_tokens exceed gas limits #449

Open
codingshot opened this issue Jan 19, 2023 · 4 comments
Open

nft_tokens exceed gas limits #449

codingshot opened this issue Jan 19, 2023 · 4 comments

Comments

@codingshot
Copy link
Contributor

"the implementation of nft_tokens in the near_contract_standards Rust library has a reputation of being unreliable (it easily exceeds gas limits above a certain from_index, even with a small limit) and indexers etc have to find other ways around getting the data that this method should provide. And it makes it difficult for marketplaces like FnF to support contracts that use this library. So it would be good if someone from the standards team could take a look at that."
@lachlanglen

from NFT Builder group

also from indexer group

"Hey guys, I was hoping perhaps someone can help me out with a problem I am having.

The problem is I’m trying to get all the token ids by running view call nft_tokens and using offsets and limit of 10. However once I cross a certain offset number I’m getting GasLimitExceeded error. Any solution for this?"

@frol
Copy link
Collaborator

frol commented Jan 23, 2023

So it would be good if someone from the standards team could take a look at that

I don't see anything the standards team can offer here unless there is a proposal to add some interface that will solve the indexing problem differently. In fact, when it comes to indexers, I rely on JSON Events and incrementally index the NFTs instead of fetching the whole list over and over again.

I believe near-sdk-rs/near-contract-standards team or anyone from the community should benchmark the current implementation to pinpoint the exact source of inefficiency. My wild guess is that the root cause is the fact that nft_tokens uses owner_by_id (which is a TreeMap) to iterate over the whole list, so I would try using UnorderedMap instead and measure the gas and storage consumption.

@lachlanglen
Copy link
Contributor

lachlanglen commented Jan 24, 2023

Hey @frol - thanks for your comment.

I don't think changing the TreeMap to an UnorderedMap will solve this issue (I just did a quick check on that and it didn't appear to make a meaningful difference).

We have solved this on our Few and Far specification by utilizing a TokenId standard (a variation on the usual stringified incrementing integer) to determine token IDs without needing to iterate through owner_by_id. I guess this won't work for NEP-171 since there is unfortunately no standard for token IDs.

@frol
Copy link
Collaborator

frol commented Jan 27, 2023

@lachlanglen Do you see an opportunity for improvement in NEP-171 (NFT standard) or maybe introducing a new NEP regarding TokenId naming strategy allowing more efficient implementations down the road?

@lachlanglen
Copy link
Contributor

lachlanglen commented Feb 13, 2023

@frol I think a new standard should be proposed for a multi-collection NFT contract that implements a TokenId standard, which would solve this and many other issues. Many projects are implementing this kind of NFT contract but there is no consistent interface. I'm working on this and will introduce a proposal soon.

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

No branches or pull requests

3 participants