dwyl / cid

❄️cid ("content id") is a human-friendly (readable/typeable) unique ID function built for distributed/decentralised systems.
GNU General Public License v2.0
33 stars 3 forks source link

cidv1 with base32? #36

Closed SimonLab closed 11 months ago

SimonLab commented 5 years ago

image

IPFS will soon use cidv1 with base32. Should we add an option to select either base32 or base58 to keep our cid compatible with IPFS?

nelsonic commented 5 years ago

@SimonLab indeed. and we need to make it a configuration option rather than a function parameter. (please estimate how long this work will take and .then go for it!)

SimonLab commented 5 years ago

I will search what is the best way to configure the cid package (ie defining which base we want to use when the package is added as a dependency in a project). At the moment I'm thinking of defining a new config value in the mix config file of the application, then this config value will be used by the config package. I think @Danwhy done this with the current alog version.

I think elixir already provide a base32 function, so it will be quick to implement this feature (thinking a couple of hours)

SimonLab commented 5 years ago

I manage to define the new cid with Base.encode32 function and define a configuration using Application.get_env to allow the applications using excid to choose between base32 and base58, however there is an issue when the value of the configuration is changed, the dependency needs to be recompile to be able to get the new value. I'm looking at a way to not have to recompile the excid dependency.

SimonLab commented 5 years ago

The error is due to the the the fact that module attribute are created during the compilation: image see https://elixir-lang.org/getting-started/module-attributes.html#as-constants And I was defining my base to use as:

@base Application.get_env(:excid, :base)
defp create_cid()...

Instead I can assign or use directly `Application.get_env(:excid, :base) inside my function

SimonLab commented 5 years ago

using Base.encode32(binary, case: :lower, padding: false) with lowercase letters and we don't want the padding. We want something like bafkreibm6jg3ux5qumhcn2b3flc3tyu6dmlb4xa7u5bf44yegnrjhc4yeq instead of bAFKREIBM6JG3UX5QUMHCN2B3FLC3TYU6DMLB4XA7U5BF44YEGNRJHC4YEQ======

lower case and no padding (no =)

Lowercase identifiers are required in order to work with legacy URI, web browser security (sub)origins and the experimental Protocol Handler API. So we are moving to finish support for CIDv1 across the ecosystem, and use it as default

image

SimonLab commented 5 years ago

Our current test is using the ipfs command line to compare the cid created by our module to the one created by ipfs. However because cidv1 base32 can't be created directly with the ipfs command yet as there is no option to define the base (only the option to choose the version of cid) we can reuse/adap our current tests: image

There is however a way to create a cidv1 base 32 from a cidv0 with the ipfs cid command: image

So first I need to generate a cidv0, then I can get the cidv1base32 of this cidv0. Then we will be able to compare this generated cidv1 with our function Cid.cid

nelsonic commented 5 years ago

@SimonLab in that case we can leave this issue open and instead focus on ATM. We don't need cidv1base32 for any @dwyl work/projects in the next 6 months. It's not a wise use of time to focus on it unless someone in the wider IPFS community asks for it.

Note: if you are such a person in the IPFS community and really want/need this feature, comment!

SimonLab commented 5 years ago

It's not a wise use of time to focus on it unless someone in the wider IPFS community asks for it.

Agree, but I had an idea in mind for the tests that I wanted to verify. So at the end I mange to test the cid base32 using ipfs command line and I think the PR is ready for review: #37

SimonLab commented 5 years ago

Attempting to publisht the version 0.2.0 of excid with my account but: image

@RobStallion it looks like you are the owner of the package it might be why: image

iteles commented 5 years ago

Thanks @SimonLab. @RobStallion Let's transfer the ownership to the dwyl account on Monday so we can all access it 👍

RobStallion commented 5 years ago

@SimonLab good spot. @iteles sorry about this. I'll get this done first thing 👍

I have just seen this medium post about adding removing owners to/from a hex package. It looks like it should be pretty quick and painless.

Can you give me the dwyl account info for hex pm?

nelsonic commented 11 months ago

This was published. Thanks again @RobStallion and @SimonLab ✅