onflow / flow-interaction-template-service

https://flix.flow.com
Apache License 2.0
11 stars 4 forks source link

[FEATURE] Deduplicate the generic StorefrontRemoveItem template #31

Closed bartolomej closed 6 months ago

bartolomej commented 7 months ago

Issue to be solved

Transactions for removing a listing from the storefront are generic, so they should work for any resource type.

But since the templates under templates/NFTCatalog were generated using the NFT catalog tool, each project sub-directory contains its own {ProjectName}-StorefrontRemoveItem.mainnet.template.json template file.

So if someone were to search for a "remove item" template, all these templates that essentially do the same thing would show up. Their labels would suggest that each template is built for a specific NFT collection, but that's not the case as they are generic. This makes this confusing and could present an unnecessary clutter in a template discovery tool.

I realized this when I was integrating template discovery support in Flowser: Screenshot 2023-11-12 at 11 32 23

Suggest A Solution

Deduplicate these templates, by creating a generic StorefrontRemoveItem contract and remove all the other project-specific ones.

This shouldn't be a breaking change, since these templates only differ in comments which are not present in the AST that's used for hash calculation in the scope of reverse lookup (the /templates/search?cadence_base64= endpoint).

What are you currently working on that this is blocking?

No response

bartolomej commented 7 months ago

There are however a few implications I'm not sure about:

All templates use the same pin_block_height If we replace all project templates with a single consolidated one, this means pin_block_height config will also be consolidated. I'm not entirely sure if this is a problem or not.

If this config defines the supported version of the dependency contract, maybe it won't be a problem, as this template should probably work with any version of the NFTStorefrontV2 contract?

All templates use the same template ID Consolidating these templates also means that the old template IDs won't anymore. This is a breaking change, since there is an endpoint to lookup a template by it's ID.