WIP: Add global option to specify the multibase encoding (client side)

[kevina]

Redo and simplification of #5464.

After giving it some careful thought I think doing the conversion on the server side for now makes the most sense (see #5789). In particular this code relies on using global variables and a new type to do most of the work and avoid having to thread the cid base though to the point where they get displayed. However, using this hack means that ?enc=text http requests will also ignore the CID base.

The following command are implemented client side:

ipfs add
ipfs files ls
ipfs files stat
ipfs ls
ipfs pin add
ipfs pin ls
ipfs pin rm
ipfs pin verify
ipfs tar add
ipfs urlstore add
ipfs filestore dups
ipfs filestore ls
ipfs filestore verify

The following are implemented sever side

ipfs resolve
ipfs refs
ipfs refs local

ipfs resolve because it uses a path and ipfs refs do to the fact that a formatted string is sent via the API.

Todo (if we decide to go with this implementation):

• Rebase and update to use newer go-cidutil dep. Will also require updating WIP: Changes needed for --cid-base option in go-ipfs. go-mfs#5

Depends on ipfs/go-cidutil#8 and ipfs/go-mfs#5.

Closes #5233. Closes #5234. Closes #5349.

[alanshaw]

Are you planning on allowing people to specify CID base via the HTTP API?

It's not absolutely necessary, since the CID can be upgraded/converted client side, but it's super convenient for anyone consuming the HTTP API directly for them to be able to specify it...or to flip it round, it's a bit onerous for every API consumer to have to implement an option for doing this client side for each CID they recieve.

I have implemented it for the listed HTTP endoints here ipfs/js-ipfs#1552. I think it would be worthwile for go-ipfs to also have this option.

[daviddias]

Agreed with @alanshaw, why not add this arg to the http-api as well. This it trickles nicely from cli -> http-api -> core.

[Stebalien]

I believe the plan was to:

1. Do base-conversion client-side.
2. Switch to base32 by-default server-side for V1 CIDs by default (for performance).

Personally, I prefer keeping display logic client-side but I won't really object to a server-side option (given that we already have a text-based API).

Edit: on the other hand, I'd like to avoid complicating the server-side logic, regardless.

[alanshaw]

Understood. I don't think it needs to be complicated, for the JS side it was simply adding the additional parameter and reusing the logic that is already being used in the CLI for each route that we want to support it.

I still think there's a case for doing this. Maybe consumers might want to save bytes in their response by using a more compact encoding? I'm sure there's even more reasons (in addition to that and the others I outlined above) that we're not even considering...

[kevina]

@Stebalien @alanshaw sorry, I have been dragging my feet on this on because there is not any clean solution.

The client side solution ended up being not so nice because sometime we return formatted string with CID in them (see ipfs refs for example). Maybe when we clean these up and start passing real CIDs via the API and we can switch to client side, but for now server side makes more sense.

My current thinking is to switch to server side. Most of the time CIDs are encoded in the commands package. When there are not, for example with the CoreAPI the plan is to reencode the CIDs after we get them back before calling Emit to avoid having to thread the parameter though outside of the commands package. Eventually I think that the CoreAPI should return CIDs and not strings and we should perhaps not reuse the same datastructures for the CoreAPI and what we use to send the result over the HTTP API.

@alanshaw note, that specifying --cid-base implies an automatic upgrade to from CIDv0 to CIDv1 except for select low-level commands where this will cause a problem. I will let you know which commands after I redo this P.R. yet again.

[alanshaw]

Eventually I think things that the CoreAPI should return CIDs and not strings

Oh, agree! I opened this yesterday: ipfs-inactive/interface-js-ipfs-core#394

[alanshaw]

The client side solution ended up being not so nice because sometime we return formatted string with CID in them (see ipfs refs for example)

Ah, JS doesn't have ipfs refs yet so we'll cross that bridge when we get there ;)

My current thinking is to switch to server side

In ipfs/js-ipfs#1552, for the CLI we're able to do everything client side for the commands we currently have, aside from resolve (which we've already mentioned). Adding ?cid-base to the HTTP API obviously needed a server side conversion.

note, that specifying --cid-base implies an automatic upgrade to from CIDv0 to CIDv1

Yes, thanks 😄, that's taken into account in the PR.

select low-level commands where this will cause a problem. I will let you know which commands after I redo this P.R. yet again

Yes, interested to know which and why - I've listed the commands I've implemented it for in ipfs/js-ipfs#1552. I'm not aware of any issues...

[kevina]

@alanshaw I am confused, using the --cid-base parameter does not pass the option to the server, but you support the cid-base end points anyway? Does the js implementation not make a distinction between the client and the server like go-ipfs does?

(I am not familiar with how command line requests get translated to API requests but I am assuming --cid-base=base32 will translate to ?cid-base= query option. My apologies if I am mistaken.)

[alanshaw]

@alanshaw I am confused, using the --cid-base parameter does not pass the option to the server

We choose the options from the command line that we want to pass to the server (core).

but you support the cid-base end points anyway?

The CLI interacts with a IPFS core interface, which might be an actual instance of js-ipfs core or it might be a js-ipfs-api, depending on whether there's a daemon running or not.

So we could pass this option into the core interface to take advantage of the ?cid-base query param but that would push more logic for encoding CIDs into core, which is something I want to get away from - core should be returning CID instances.

So, the ?cid-base query param is used where we actually have to do the conversion server side (for a path) and just available on other endpoints for any other consumer of the HTTP API.

[kevina]

Closing this because the general consensus is we are going with a (mostly) server side approach.

[kevina]

(please don't delete a branch and allow it to be archived so that we have the code in case we ever need to refer back to it)