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

Suggestion: Decouple minting from storage and potentially throw out concept of storage from core SDK #22

Open
etodanik opened this issue Oct 4, 2021 · 7 comments
Labels
enhancement New feature or request SDK: JS

Comments

@etodanik
Copy link
Collaborator

etodanik commented Oct 4, 2021

We were discussing it with @m-sebastiyan
The minting process is not layered enough in my opinion and depends too much on uploadFile4. I realize that it's an attempt at better UX, but it sacrifices good DX in return.

My suggestions are:

  1. Make the minting helper / process only care about pre-uploaded URL's. We don't care where (it could be IPFS, S3, Media Network CDN, Arweave and many more in the future).
  2. Either have ArweaveStorage as a completely separate helper, much simplified (it shouldn't mutate any JSON's IMHO, make it a simple 2 step process - upload images, then upload JSON). Or preferably just don't have this complexity in core and merely document the uploadFile4 (or any future version of it) REST-ful parameters and let the SDK client user decide if they wanna use it.

I think that the core SDK should be as skinny as possible and any attempt to introduce storage module awareness here will hurt that principle.

@etodanik
Copy link
Collaborator Author

etodanik commented Oct 4, 2021

@adamjeffries would love your opinion on that!

@WrRaThY
Copy link

WrRaThY commented Oct 20, 2021

couldn't agree more. very good points @israelidanny
one other thing needed - document it here:
https://docs.metaplex.com/development/clients/js-sdk/getting-started

@aheckmann
Copy link
Contributor

  1. We care about storage insofar as the default is decentralized storage.
  2. People should be able to override or skip the upload step to use the storage solution of choice.
  3. Modularizing the providers makes sense too, so there are upload modules for each common storage solution people can contribute / use. It could be a plugin approach too where we wouldn't need to store them all in this repository, they could be modules published to npm.
  4. Default option should continue to be Arweave so everyone can complete the mint flow without added effort of discovering and configuring plugins
  5. +1 on documenting all our changes at the same time we land code

@etodanik
Copy link
Collaborator Author

@aheckmann It's more of a question of layering. A low level SDK that deals with transactions , program addresses and basic things like that should provide the transactions to create a MetaData program account, for instance

The ideal layering here would be to have a separate repo for the uploader / uploaders.

This way you don't deal with extra dependencies / weight of axios, form-data in the base layer SDK. (which especially matters for browser bundle size)

I'm not arguing against shipping an Arweave uploader. I just think it's bad layering shipping it in this repo. (which is very low level otherwise)

@etodanik
Copy link
Collaborator Author

Apparently we now have better layering:
metaplex-foundation/js#66

@aheckmann aheckmann added the enhancement New feature or request label Dec 13, 2021
@aheckmann aheckmann moved this from Triage to Todo in Metaplex Dec 13, 2021
@aheckmann
Copy link
Contributor

Make a Storage interface in JS SDK. Move implementation of various storage types out into their own modules.

@genus1
Copy link

genus1 commented Dec 16, 2021

Storage is the main problem with smart contracts. Eth is outrageous and it killed most others. Agree implementations should be handled by their own module. I am looking at a method of using IPNS to point to the initial IPFS data then modify the IPFS file and change the hash in the IPNS. Data could have categories. As example the main data suchs as an image for NFT may not be changed, but other metadata and variable state could be updated when storage is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SDK: JS
Projects
Status: Todo
Development

No branches or pull requests

4 participants