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

Switch default CidBuilder to v1CidPrefix #86

Open
lidel opened this issue Apr 13, 2022 · 2 comments
Open

Switch default CidBuilder to v1CidPrefix #86

lidel opened this issue Apr 13, 2022 · 2 comments
Labels
need/triage Needs initial labeling and prioritization

Comments

@lidel
Copy link
Member

lidel commented Apr 13, 2022

Part of https://github.com/ipfs/ipfs/issues/337, initially proposed in ipfs/kubo#4143 (comment)

Switching to v1CidPrefix in CidBuilder() sounds like a good way to ensure the ecosystem moves to CIDv1 (https://github.com/ipfs/go-merkledag/network/dependents )

How to handle BREAKING CHANGE?

Switch to CIDv1 will produce different multihashes in DAGs, which is a breaking change.
I assume we want to do this, so the question is how to do it safely, and what other breaking changes we want to make.

Initial ideas:

  • release CIDv1 version as /v2 so nobody is surprised by different CIDs and DAGs
    • since we make a breaking one, also produce raw-leaves, if not enabled yet (matching ipfs add --cid-version 1 behavior)

Thoughts?

Any feedback / sanity check is appreciated.

cc @hannahhoward @hsanjuan @guseggert @whyrusleeping @willscott @Stebalien @aschmahmann

@lidel lidel added the need/triage Needs initial labeling and prioritization label Apr 13, 2022
@ipfs ipfs deleted a comment from welcome bot Apr 13, 2022
@schomatis
Copy link
Contributor

Maybe related: we already silently upgrade from v0 to v1 when using a non-protobuf codec:

func (p V0Builder) WithCodec(c uint64) Builder {
	if c == DagProtobuf {
		return p
	}
	return V1Builder{Codec: c, MhType: mh.SHA2_256}
}

@hsanjuan
Copy link
Contributor

Meh, seems low risk to me...

Assuming I have a Protonode, either:

  • I just built it, therefore I may just want the CID whatever it is.
  • I obtained it from somewhere, therefore I probably already have the working CID for it.

Given all the changelog warnings, the /v2 etc... I think it is reasonable to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

3 participants