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 return exceed gas limit error if starting index is high enough #800

Open
ymc182 opened this issue Apr 23, 2022 · 2 comments
Open

Comments

@ymc182
Copy link

ymc182 commented Apr 23, 2022

Hi I have a collection with over 2k nft and when I try to get tokens start from around 600 , it return exceed gas limit gas, everything under that certain number is fine

@austinabell
Copy link
Contributor

So the issue is that this collection is a TreeMap which can't be optimized more because skip(n) requires iterating over all keys before n in order since the collection is ordered.

Ideally, the standards are re-written to be more efficient and low-level so this won't be a problem, but unclear what the best path for developers to optimize this. There doesn't seem to be a concise solution that isn't state breaking

@austinabell
Copy link
Contributor

Update on this for more context:

We don't currently have the bandwidth for a re-write to solve this and other issues in the near future. We will likely start a fork of the standards to fix and optimize, but also this can be done by anyone in the community, there requires no change in the NEP standard for NFTs.

The reasons this can't just be changed like in #810:

  • TreeMap and UnorderedMap have a different storage format and will silently break existing contracts
  • Some frontends might expect the ordered nature of TreeMap, so this could cause bugs in dapps
  • TreeMap can't just be optimized for nth without a state breaking change, there would need to be additional data stored to indicate how many nodes to left/right

Medium-term plan is to re-write, optimize, and make this library more configurable, but if anyone needs changes in short term, best to fork or create new.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: NEW❗
Development

No branches or pull requests

2 participants