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

Issues with NFT.Storage? #50

Open
dchoi27 opened this issue Oct 18, 2021 · 5 comments
Open

Issues with NFT.Storage? #50

dchoi27 opened this issue Oct 18, 2021 · 5 comments

Comments

@dchoi27
Copy link

dchoi27 commented Oct 18, 2021

Hey there! I'm a PM with the NFT.Storage team. I heard that your project might have had trouble with using NFT.Storage at first (also saw a snippet here around getting different CIDs in different environments, which is probably solvable in case you were looking to continue working on your project - not sure if this was the core problem you all faced though).

Would love to chat more about what your experience was so we can make the product better! Could be a reply to this issue or if you were able to pop into #nft-storage in the Filecoin community chat https://docs.filecoin.io/community/chat-and-discussion-forums/

Thank you!

@elmariachi111
Copy link
Member

Hey, basically NFT.storage works like a breeze, it's pretty cool. Our main struggle when adapting it, is its usage of CID v1 in base32. We're actually storing the CIDs on chain (in contrast to preminted collection NFTs that can simply use an IPFS folder of metadata json files) and I wanted to make this storage efficient. For CIDv0s the commonly accepted way of doing so is, stripping the first 2 bytes and store the remaining 32. For CIDv1 I can't actually remember that in detail, but I think you need to strip the first 4 bytes and store them separately. Anyway, the "hardest" part of it actually was to add the CIDv0 base58 on Solidity (got library code for that) -> I haven't found a decent base32 encoder, though. So our current "solution" is to naively store the base32 CIDv1 as string on Solidity (~64bytes). That's highly inefficient, but does the job for our demo. For the real thing we're going to implement the process I mentioned.

What I actually noticed are the latencies involved - of course IPFS will need time to look up data and stuff that's pinned on nft.storage usually is available with lookup times of ~5-30 seconds but that still somewhat kills the user experience (it particularly frequently breaks the NFT metadata & payload lookups on our verifier side, running into general timeouts). I'm wondering if we could speedup these lookups by connecting to an NFT.storage gateway node or put it in the bootstrap config of our own IPFS node (we currently don't use it, but rely on ipfs.io as a public gateway). Do you know, whether there is a dedicated nft.storage gateway / IPFS node to use that'll be able to resolve "known" content much faster, @dchoi27 ?

@dchoi27
Copy link
Author

dchoi27 commented Oct 20, 2021

Thanks! Re: CIDv0 vs. CIDv1, that's interesting. Not sure about your exact implementation, but FYI there are some pitfalls using CIDv0: nftstorage/nft.storage#571 When you say "For CIDv0s the commonly accepted way of doing so" - is that a practice you see often in the community?

Re: latencies, yes the latencies are a known issue, especially from the ipfs.io gateway, but since our nodes are currently peered to the gateway, it's probably the best solution at the moment (since it's not relying on the DHT). We're working on some intermediate term solutions that we're excited about to improve this, but it will at least be a few months unfortunately. I've asked the team what we can potentially do in the meantime!

@elmariachi111
Copy link
Member

After all it boils down to two things to save chain storage for CIDs while being useful in the NFT context: decode and store the CID as binary (bytes32 +x) in Solidity and be able to encode it into a format that IPFS clients ("browsers") can understand in plain Solidity, i.e. creating a CIDv0/1 by encoding it as base32/58 on a tokenURI call. For b58 there's a good implementation available (https://github.com/cod1ng-earth/splicenft/blob/main/packages/contracts/contracts/Base58.sol, source linked in the contract) but I haven't found a b32 impl as of now (and during a hackathon you definitely don't want to spend time on writing that yourself ;) ). I just noticed that CIDv1s are also encodeable in b58 - even if that's obvious I must check whether gateways and clients support that and I'm pretty sure I can fix this during the day.

I've seen lots of contracts storing the rightmost 32 bytes of a CIDv0, and just assuming the first two bytes to be Qm, yes. With v1 that's slightly different, but conceptually the same idea. Just haven't seen it yet because I wasn't looking for it ;)

For the latency part, it's mostly noticeable on our validator. I actually have a solution for that in mind: we can simply pin that content on some shared IPFS node that our Dapp and the (still quite centralized) validator backend both agree on. Once the frontend confirms that it's pinned there, our validator will find it instantly. Must do that for the blob & metadata, but that's also just some technical UX detail that just didn't fit into the hackathon.

Besides that, I would expect that an "ipfs.io" gateway (or an ipfs.nft.storage one?) should respond to CIDs (metadata & blob) that I just requested to add very swiftly (under 1s).

@dchoi27
Copy link
Author

dchoi27 commented Oct 20, 2021

Thanks - I'll reply more later but just got the word that it would be great to give you our peer IDs to peer with our nodes! Will follow up.

@dchoi27
Copy link
Author

dchoi27 commented Oct 21, 2021

Hey - might be worth tracking this issue - we'll aim to get a list of our peer IDs so you can peer your node with them! Should help with latency nftstorage/nft.storage#644 And as mentioned we have improvements coming in the pipeline. The ipfs.io gateway is under heavy load these days - even though it's peered to our nodes, there's just new issues at this scale.

Don't think your pinning solution is a bad idea and wouldn't necessarily call it centralized. Even though you're relying on a shared node, users can always validate content by hashing it themselves and comparing it to the CID, so it's still trustless. As long as you're storing it in other places like nft.storage (to ensure persistence in a bunch of different places) seems good to me!

What you're saying makes sense on the hash length. Just flagging it's not recommended best practice by the IPFS and IPLD folks - that we've seen some issues using case sensitive CIDs in the past: nftstorage/nft.storage#571 (comment)

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

No branches or pull requests

2 participants