search

onflow / nft-catalog

https://www.flow-nft-catalog.com/
The Unlicense
35 stars 11 forks source link

Improve performance by deprecating getCatalog, and preferring alternatives for accessing the catalog. #138

Open aishairzay opened 1 year ago

aishairzay commented 1 year ago

As https://github.com/dapperlabs/nft-catalog/pull/135 points out, we need a way to deprecate use of the getCatalog function, and have all consumers update to a new way to pull all information from the catalog because as the catalog grows, we are nearing execution limits from simply calling getCatalog() because it copies over the entire catalog map to the memory of the executing script or transaction rather than using the map reference.

I think the best way to do this will be to add support for new ways to iterate over the the catalog similar to how the following PR does https://github.com/dapperlabs/nft-catalog/pull/135

But, rather than entirely removing the getCatalog function, we can make a snapshot of the current catalog, and have the getCatalog function return the snapshotted catalog. This will make it so all existing use of getCatalog will not be affected short-term, and allows for all consumers to switch to the new functions to iterate over the catalog in a way that will not reach execution limits.

aishairzay commented 1 year ago

https://github.com/dapperlabs/nft-catalog/pull/139 has been pushed to provide an alternative to the existing getCatalog, and provide updates to all of our script usage.

https://github.com/dapperlabs/nft-catalog/pull/140 is open to provide a way to snapshot the current catalog, so we can officially replace getCatalog with a snapshotted version of the catalog.

I'm thinking next Monday, 4/3, we can run the updateSnapshot to stop continuing catalog updates to the getCatalog function.

Any consumers would ideally update their catalog usage before 4/3, and if they do not, then they will not receive future catalog updates until they do update. This is subject to change, pinging some catalog consumers to review, please comment here if you have any suggestions/alternatives!

aishairzay commented 1 year ago

Ran into some issues when testing the snapshotting of the catalog on testnet, looking into it but in the meantime, the snapshot has not happened yet as planned for today, and I'll give a new update soon.

aishairzay commented 1 year ago

New alternative approach which should not run into execution limits when snapshotting on testnet or mainnet: https://github.com/onflow/nft-catalog/pull/143

After review, will likely run the snapshot in testnet tomorrow 4/7, and on Mainnet on Monday 4/10, which afterwards will mean that getCatalog is returning a snapshotted version of the catalog.