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

Adding a file/folder from disk should use CIDv1 by default. #2361

Open
MicahZoltu opened this issue Dec 15, 2022 · 10 comments · Fixed by #2402 · May be fixed by #2527
Open

Adding a file/folder from disk should use CIDv1 by default. #2361

MicahZoltu opened this issue Dec 15, 2022 · 10 comments · Fixed by #2402 · May be fixed by #2527
Labels
effort/hours Estimated to take one or several hours need/analysis Needs further analysis before proceeding need/author-input Needs input from the original author need/maintainers-input Needs input from the current maintainer(s) P1 High: Likely tackled by core team if no one steps up

Comments

@MicahZoltu
Copy link

Is your feature request related to a problem? Please describe.
I added a folder containing a website to IPFS via IPFS Desktop UI and it only presents me with the CIDv0 hash, not the CIDv1 hash. Since subdomain hosting is strongly recommended for any website, I now need to go convert this to CIDv1 before I can use it. When I copy CID via the UI, it just gives me CIDv0.

Describe the solution you'd like
Make it so when I add a file/folder from disk it associates it with a CIDV1 hash by default.

Describe alternatives you've considered

  • CIDv1 used everywhere (even when I provide a CIDv0 hash). Maybe there is a good reason for using CIDv0 hashes still?
  • Present both CIDv0 and CIDv1 hashes in the UI and let the user copy either. I feel like this is overly complicated?

Additional context
Short term solution may be to just add something in the UI to convert. As it is I spend 5 minutes searching the internet for an online converter tool, fail repeatedly, eventually find some instructions on StackOverflow describing how to convert from CIDv1 to CIDv0, then spin up a docker container for Kubo and run the command. This is, of course, a ridiculous workflow.

@MicahZoltu MicahZoltu added the need/triage Needs initial labeling and prioritization label Dec 15, 2022
@SgtPooki SgtPooki moved this to Needs Grooming in IPFS-GUI (PL EngRes) Dec 15, 2022
@whizzzkid
Copy link
Contributor

Thanks for submitting the issue @MicahZoltu, sounds like a great enhancement. Can you please also detail on:

I now need to go convert this to CIDv1 before I can use it.

Under what scenarios is this CIDv0 not handled?

Also, sounds like you have good context on what's happening, would you like to create a PR for this issue?

@whizzzkid whizzzkid added P1 High: Likely tackled by core team if no one steps up need/analysis Needs further analysis before proceeding need/author-input Needs input from the original author need/maintainers-input Needs input from the current maintainer(s) effort/hours Estimated to take one or several hours and removed need/triage Needs initial labeling and prioritization labels Dec 19, 2022
@MicahZoltu
Copy link
Author

MicahZoltu commented Dec 19, 2022

I now need to go convert this to CIDv1 before I can use it.

Under what scenarios is this CIDv0 not handled?

Websites that use local storage/cookies should always use CIDv1 so they get sandboxed by the browser by subdomain. See the big red box on http://docs.ipfs.tech.ipns.localhost:8080/how-to/address-ipfs-on-web/#subdomain-gateway for additional details. Also worth noting that http://docs.ipfs.tech.ipns.localhost:8080/concepts/content-addressing/#cid-versions suggests using CIDv1 if anytime you aren't sure (e.g., a reasonable default).

I have a good handle on the problem/security considerations, but I have never looked at the IPFS desktop codebase so fixing it would be quite challenging for me. 😢 I don't mind getting mentioned on a PR to review, or I'm happy to answer any questions one might have about the situation.

Presumably the API equivalent of ipfs add --cid-version 1 ... needs to be called when adding files, rather than ipfs add .... My hope is that this is something that is relatively easy to address for someone who knows the codebase.

@SgtPooki SgtPooki linked a pull request Feb 21, 2023 that will close this issue
@lidel
Copy link
Member

lidel commented Mar 21, 2023

Now that #2402 landed, I think the only remaining surface in IPFS Desktop is the root CID of MFS itself.

By default, it is CIDv0, but IPFS Desktop could upgrade it with ipfs.files.chcid:

$ ipfs files stat /
QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn
Size: 0
CumulativeSize: 4
ChildBlocks: 0
Type: directory

$ ipfs files chcid --cid-version 1 /                                                                                                                                                                       

$ ipfs files stat /
bafybeiczsscdsbs7ffqz55asqdf3smv6klcw3gofszvwlyarci47bgf354
Size: 0
CumulativeSize: 4
ChildBlocks: 0
Type: directory

Once upgraded, changing MFS will always produce CIDv1 root.

I think we could do it during startup – check if CID is v0, and if so, upgrade tov v1.

@davidd8 @SgtPooki thoughts?

@SgtPooki
Copy link
Member

Im down with updating it to cidv1, but need to look further into gotchas and edge-cases.

Is there a tracking issue for kubo to default to cidv1?

@lidel
Copy link
Member

lidel commented Apr 25, 2023

Switching MFS root to CIDv1 should not have any gotchas. Newly created dirs will be CIDv1, that is all.

@SgtPooki Kubo issue for switching to CIDv1 is ipfs/kubo#4143, but tldr

  • we not only change CIDv0 to CIDv1, but also switch default from raw-leaves=false to raw-leaves=true
  • most of the commands (block put, dag put, refs, name publish) are migrated, only remaining one is ipfs add and MFS root
    • afaik the only reason we did not switch everything to CIDv1 in Kubo yet is priorities/time/resourcing: someone needs to update all the tests where we used hard-coded CIDv0 and raw-leaves=false (I never had time to finish test: ipfs add with CIDv1 as default kubo#8185)
  • 👉 the historical UX gotcha around this was that if IPFS Desktop and ipfs add from Kubo use different defaults, users may be confused why they got different CID. I think the times changed, and people in ecosystem are used to different apps having different defaults (e.g. web3.storage defaulting to 1MiB chunk instead of 256KiB like Kubo), and we should not try to keep these things in sync across ecosystem, but instead migrate each system whenever we have time.
    • With this in mind, I think it is fine for IPFS Desktop/WebUI to switch to CIDv1 and even adjust default block size to 1MiB, without waiting for Kubo.

@MicahZoltu
Copy link
Author

Now that #2402 landed, I think the only remaining surface in IPFS Desktop is the root CID of MFS itself.

By default, it is CIDv0, but IPFS Desktop could upgrade it with ipfs.files.chcid:

$ ipfs files stat /
QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn
Size: 0
CumulativeSize: 4
ChildBlocks: 0
Type: directory

$ ipfs files chcid --cid-version 1 /                                                                                                                                                                       

$ ipfs files stat /
bafybeiczsscdsbs7ffqz55asqdf3smv6klcw3gofszvwlyarci47bgf354
Size: 0
CumulativeSize: 4
ChildBlocks: 0
Type: directory

Once upgraded, changing MFS will always produce CIDv1 root.

Is this a button that could be added to the UI on a shorter timeline than changing the default? As an IPFS Desktop user, I don't even know how to get to an IPFS command line on a host with only IPFS Desktop installed, and I would like to be able to recommend this solution to people who don't know how to use a command prompt.

@whizzzkid whizzzkid linked a pull request Jun 22, 2023 that will close this issue
@whizzzkid
Copy link
Contributor

whizzzkid commented Jun 22, 2023

@lidel if this were to be prioritized, does this task list look good?

  • Implement chcid in the client libs (both ipfs-http-client and kubo-rpc-client don't do it.
  • Migrate ipfs-desktop to kubo-rpc-client
  • Implement the upgrade like: feat: default MFS to CIDv1 #2527

Is there something I'm missing?

@lidel
Copy link
Member

lidel commented Jun 23, 2023

@whizzzkid sounds about right for ipfs-desktop itself.
I think we still have to update ipfs-webui to import files via ipfs.add with cidVersion: 1 here(?) to have CIDv1 produced when user adds file via GUI (#2402 only applies to import via context menu on Windows).

Changing this for everyone in webui is tricky, as will impact Kubo, which is still CIDv0, and we don't want those users to have different CIDs on CLI and webui.

I think we could make this smart and not hardcode cid version used during import via webui, but base it on the CID version of the MFS root (can be checked via ipfs files stat /, if returned value is not Qm, you know it is CIDv1).

If user's MFS is CIDv1, it is safe to apply the same to files that will be imported there.

@whizzzkid whizzzkid moved this from Needs Grooming to Needs Prep Work / Blocked in IPFS-GUI (PL EngRes) Aug 3, 2023
@agmap
Copy link

agmap commented Jul 21, 2024

I want to change the standard to CIDv1 in ipfs-desktop windows.
What needs to be changed in the config file?
I think:

"Import": {
"CidVersion": 1,
"HashFunction": "sha2-256",
"UnixFSChunker": "size-1048576",
"UnixFSRawLeaves": true
}

Can someone confirm please

@SgtPooki
Copy link
Member

@agmap in order to make ipfs-desktop default to CID v1 we would need to make some code changes in ipfs-desktop and ipfs-webui

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours need/analysis Needs further analysis before proceeding need/author-input Needs input from the original author need/maintainers-input Needs input from the current maintainer(s) P1 High: Likely tackled by core team if no one steps up
Projects
No open projects
Status: Needs Prep Work / Blocked
Development

Successfully merging a pull request may close this issue.

5 participants