multiformats / js-cid

CID implementation in JavaScript
MIT License
98 stars 39 forks source link

Converting CID from V0 to V1 generate V1 and base58 CID #145

Closed ocknamo closed 3 years ago

ocknamo commented 3 years ago

Type: Bug

In version: v1.1.7

When I change CID V0 to V1, the base become base58btc.

import CID from 'cids';

const cid = new CID('Qmd286K6pohQcTKYqnS1YhWrCiS4gz7Xi34sdwMe9USZ7u')
console.log('LOG→ V0', cid.toV0().toString());
// LOG→ V0 Qmd286K6pohQcTKYqnS1YhWrCiS4gz7Xi34sdwMe9USZ7u
const cidV1 = cid.toV1();
console.log('LOG→ V1', cidV1.toString());
// When v1.1.7
// LOG→ V1 zdj7Wk7NTkAgHB1VAdUaXPdR5MQpvdWQXXfaMUQJjkJxv1THq
// When v1.1.6
// LOG→ V1 bafybeig2ea5p2xw2d5c552x3ocxj2xavsb6nglwczv2hyza7yhu2wvny5a

When v1.1.7. the converted CID object is as follow.

Screenshot from 2021-06-18 23-24-15

This is different from the documentation. https://docs.ipfs.io/concepts/content-addressing/#v0-to-v1

Perhaps this is the cause. https://github.com/multiformats/js-cid/commit/1ee726dd4446785cbd751a7d5728707c75ec33ad#diff-bfe9874d239014961b1ae4e89875a6155667db834a410aaaa2ebe3cf89820556R259

I think V1 CID of base58btc is not useful.

When CID is converted from V0 to V1, should the default base be base32?

mkaranta commented 3 years ago

When CID is converted from V0 to V1, should the default base be base32?

Yes. I just upgraded the package version v1.1.6 -> v1.1.7 and am experiencing this bug as well.

ocknamo commented 3 years ago

The bug can be avoided by specifying the base when using toString.

new CID('QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR').toV1().toString('base32')
// → bafybeigdyrzt5sfp7udm7hu76uh7y26nf3efuylqabf3oclgtqy55fbzdi

But this is breaking change so the Document or API should be fixed.

lidel commented 3 years ago

This is expected behavior – https://github.com/multiformats/js-cid/pull/146#issuecomment-868550683, conversions like this belong to useland (new CID('QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR').toV1().toString('base32'))