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

WIP: Store multibase prefix with CID #66

Closed
wants to merge 4 commits into from
Closed

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Aug 6, 2018

The CID spec defines the multibase as part of the CID, yet we do not store it anywhere.

This fixes that, It also creates a variable to change the default base for CIDv1.

This should make preserving the base of a CID a lot less painful for command like ipfs resolve and ipfs pin add or any command that takes a CID from the user.

Simplifier version of #64.

@ghost ghost assigned kevina Aug 6, 2018
@ghost ghost added the status/in-progress In progress label Aug 6, 2018
@Stebalien
Copy link
Member

So, CIDs always use a multibase in text. The spec has always been a bit unclear on this subject but I'd be strongly against any attempt to carry the multibase along with a CID. That mixes data with representation a bit too much for my tastes.

Ideally, on the command line, we'd:

  1. Accept any multibase.
  2. Return the multibase requested by the user.

@kevina
Copy link
Contributor Author

kevina commented Aug 6, 2018

@Stebalien this doesn't need to go in right away. Can we hold off on making a quick judgement. I will write a full proposal (with several alternatives) on how to handle multibases later today.

@kevina kevina changed the title Store multibase prefix with CID WIP: Store multibase prefix with CID Aug 6, 2018
@kevina
Copy link
Contributor Author

kevina commented Aug 7, 2018

See ipfs/kubo#5349 solution 2 for the motivation for this change.

@kevina kevina closed this Aug 30, 2018
@ghost ghost removed the status/in-progress In progress label Aug 30, 2018
@mvdan mvdan deleted the kevina/withbase branch July 1, 2021 16:13
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

Successfully merging this pull request may close these issues.

2 participants