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.

Sign up for GitHub

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

Jump to bottom

feat: support v0 identity cids #91

Closed
wants to merge 1 commit into from
Closed

feat: support v0 identity cids #91

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/cid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ export class CID {

let version = next()
let codec = DAG_PB_CODE
if (version === 18) { // CIDv0

if (version === 18 || version === 0) { // CIDv0 sha2-256 or identity
Copy link

@aschmahmann aschmahmann Jun 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PeerIDs are not CIDs by spec https://github.com/libp2p/specs/blob/master/peer-ids/peer-ids.md, https://github.com/multiformats/cid.

CIDv0 is always DAG-PB and sha2-256 (and encoded as base58btc).

Conflating these specs may lead to user confusion here. In go-libp2p peerIDs are defined as a separate type (i.e. https://github.com/libp2p/go-libp2p-core/blob/525a0b13017263bde889a3295fa2e4212d7af8c5/peer/peer.go#L172 vs https://github.com/ipfs/go-cid/blob/979bf3fb8572224c2b2fbfaf153f94b98734807c/cid.go#L243)

What's wrong with using https://github.com/libp2p/js-peer-id/blob/master/src/index.js?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PeerIDs are not CIDs by spec

Hmm: https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md

The peer-id module carries a lot of crypto baggage with it that makes it unsuitable for use in the http client as it's too large, to the point that we don't use it in the http client, only in in-process or daemon js-ipfs/js-libp2p.

For whatever reason we decided instead to pass peer IDs around as CIDs as a 'lighter weight' version of peer-id so it has methods to convert to and from a CID (v0 identity or v1 libp2p-key).

Maybe it shouldn't but that's a much bigger scope change than upgrading multiformats to get the new @ipld/dag-cbor @ipld/dag-pb module goodness.

All this PR really does is add a feature that was in the cids module but is missing from this one.

Copy link

@aschmahmann aschmahmann Jun 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm: https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md

This is a spec detailing how to encode peerIDs as CIDs in text, which is a subset of the peerID spec (e.g. peerIDs are represented as multihashes, not CIDs, in byte encoding and peerIDs are also representable as base58btc multihashes in text).

All this PR really does is add a feature that was in the cids module but is missing from this one.

🤷‍♀️ if the previous implementation (js-cid) wasn't spec compliant and the idea is you want to make this one match the other one then I guess that's up to you + others familiar with the js codebase. You should probably file an issue then to make a peer-id type that you can use so that you can make the CID type spec compliant again.

version = 0
offset = 0
} else if (version === 1) {
Expand Down Expand Up @@ -382,7 +383,8 @@ export class CID {
const parseCIDtoBytes = (source, base) => {
switch (source[0]) {
// CIDv0 is parsed differently
case 'Q': {
case 'Q': // sha2-256
case '1': { // identity
const decoder = base || base58btc
return [base58btc.prefix, decoder.decode(`${base58btc.prefix}${source}`)]
}
Expand Down
10 changes: 10 additions & 0 deletions test/test-cid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,16 @@ describe('CID', () => {
const parsed = CID.parse(cid.toString(base64), base64)
digestsame(cid, parsed)
})

test('parses base58btc implicit encoded identity CIDv0', async () => {
const prefix = '1'
const hash = '6Uiu2HAmGKisxKeBRLyGyKD8MuQFAiUUnPABGhvWna28CSd7WzFE'
const cid = CID.parse(`${prefix}${hash}`)

assert.strictEqual(cid.multihash.code, 0x00)

same(cid.multihash.digest, base58btc.decode(`z${hash}`).slice(1))
})
})

test('inspect bytes', () => {
Expand Down