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] Deduplicate the generic StorefrontRemoveItem template #31

Closed
bartolomej opened this issue Nov 12, 2023 · 1 comment · Fixed by onflowser/flow-interaction-template-service#3 · May be fixed by #32
Closed

[FEATURE] Deduplicate the generic StorefrontRemoveItem template #31

bartolomej opened this issue Nov 12, 2023 · 1 comment · Fixed by onflowser/flow-interaction-template-service#3 · May be fixed by #32

Comments

@bartolomej
Copy link

bartolomej commented Nov 12, 2023

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
Copy link
Author

bartolomej commented Nov 12, 2023

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.

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