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 Peer ID represented as CID #105

Merged
merged 4 commits into from
Nov 4, 2019
Merged

feat: support Peer ID represented as CID #105

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d7d78de
feat: support Peer ID represented as CID
lidel Oct 24, 2019
60218d0
refactor: rename toCIDString to toString
lidel Oct 28, 2019
d4fee10
docs: comment tests
lidel Oct 28, 2019
2b11192
feat: validate CID multicodec
lidel Oct 28, 2019
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
23 changes: 21 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,16 @@
- [Import](#import)
- [`createFromHexString(str)`](#createfromhexstringstr)
- [`createFromBytes(buf)`](#createfrombytesbuf)
- [`createFromCID(cid)`](#createfromcidcid)
- [`createFromB58String(str)`](#createfromb58stringstr)
- [`createFromPubKey(pubKey)`](#createfrompubkeypubkey)
- [`createFromPrivKey(privKey)`](#createfromprivkeyprivkey)
- [`createFromJSON(obj)`](#createfromjsonobj)
- [Export](#export)
- [`toHexString()`](#tohexstring)
- [`toBytes()`](#tobytes)
- [`toString()`](#tostring)
- [`toB58String()`](#tob58string)
- [`toHexString()`](#tohexstring)
- [`toJSON()`](#tojson)
- [`toPrint()`](#toprint)
- [License](#license)
Expand Down Expand Up @@ -145,6 +147,14 @@ Creates a Peer ID from a buffer representing the key's multihash.

Returns `PeerId`.

### `createFromCID(cid)`

- `cid: CID|String|Buffer` - The multihash encoded as [CID](https://github.com/ipld/js-cid) (object, `String` or `Buffer`)

Creates a Peer ID from a CID representation of the key's multihash ([RFC 0001](https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md)).

Returns `PeerId`.

### `createFromB58String(str)`

Creates a Peer ID from a Base58 string representing the key's multihash.
Expand Down Expand Up @@ -197,9 +207,18 @@ Returns the Peer ID's `id` as a buffer.
<Buffer 12 20 d6 24 39 98 f2 fc 56 34 3a d7 ed 03 42 ab 78 86 a4 eb 18 d7 36 f1 b6 7d 44 b3 7f cc 81 e0 f3 9f>
```


### `toString()`
Copy link
Member

Choose a reason for hiding this comment

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

Personally I would add toCID and have toString as () => this.toCID().toString().

This creates a symmetric API createFromCID/toCID (although "create" is superfluous IMHO, but meh, maybe a refactor for another day) and a nice sensible representation as a string that also enables ${peerId} or peerID + '' without having to explicitly call toString.

Copy link
Member Author

@lidel lidel Oct 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

My reasoning: there is no toCID because there was not toMultihash :)
We want to keep API surface small, liberal in inputs (accept CID object), but strict in outputs (just basic types: PeerID, String and Buffer)

Copy link
Member

Choose a reason for hiding this comment

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

I agree in keeping this in the context of this PR. We can iterate on this in a new PR if it justifies


Returns the Peer ID's `id` as a self-describing CIDv1 in Base32 ([RFC 0001](https://github.com/libp2p/specs/blob/master/RFC/0001-text-peerid-cid.md))

```
bafzbeigweq4zr4x4ky2dvv7nanbkw6egutvrrvzw6g3h2rftp7gidyhtt4
```

### `toB58String()`

Returns the Peer ID's `id` as a base58 string.
Returns the Peer ID's `id` as a base58 string (multihash/CIDv0).

```
QmckZzdVd72h9QUFuJJpQqhsZqGLwjhh81qSvZ9BhB2FQi
Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"dirty-chai": "^2.0.1"
},
"dependencies": {
"cids": "~0.7.1",
"class-is": "^1.1.0",
"libp2p-crypto": "~0.17.0",
"multihashes": "~0.4.15",
Expand Down
25 changes: 24 additions & 1 deletion src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
'use strict'

const mh = require('multihashes')
const CID = require('cids')
const cryptoKeys = require('libp2p-crypto/src/keys')
const assert = require('assert')
const withIs = require('class-is')
Expand Down Expand Up @@ -122,6 +123,16 @@ class PeerId {
return this._idB58String
Copy link
Member

Choose a reason for hiding this comment

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

This will now become rarely used. Should we generate on demand instead of in the constructor?

Copy link
Member Author

@lidel lidel Oct 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

It is used in isPeerId and changing this triggers a bigger refactor.
I'd like to keep it as-is in this PR (to keep it small) and change it in Stage 2 of libp2p/specs#216
(when we "Format peer IDs as CIDs by default.")

}

// return self-describing String representation
// in default format from RFC 0001: https://github.com/libp2p/specs/pull/209
toString () {
if (!this._idCIDString) {
const cid = new CID(1, 'libp2p-key', this.id, 'base32')
this._idCIDString = cid.toBaseEncodedString('base32')
Copy link
Member

Choose a reason for hiding this comment

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

If you cache the CID instance you could use it in a toCID also. Note that CID's cache their string representation so calling toString on it won't re-encode.

Copy link
Member Author

Choose a reason for hiding this comment

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

As noted above, we don't want to expose CID types in outputs, just Strings.

}
return this._idCIDString
}

isEqual (id) {
if (Buffer.isBuffer(id)) {
return this.id.equals(id)
Expand Down Expand Up @@ -184,7 +195,19 @@ exports.createFromBytes = (buf) => {
}

exports.createFromB58String = (str) => {
return new PeerIdWithIs(mh.fromB58String(str))
return exports.createFromCID(str) // B58String is CIDv0
}

exports.createFromCID = (cid) => {
if (typeof cid === 'string' || Buffer.isBuffer(cid)) {
lidel marked this conversation as resolved.
Show resolved Hide resolved
cid = new CID(cid)
} else if (CID.isCID(cid)) {
CID.validateCID(cid) // throws on error
lidel marked this conversation as resolved.
Show resolved Hide resolved
} else {
// provide more meaningful error than the one in CID.validateCID
throw new Error('Supplied cid value is neither String|CID|Buffer')
}
Copy link
Member

Choose a reason for hiding this comment

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

TODO: validate the codec is libp2p-key or that it's a v0?

Copy link
Member Author

@lidel lidel Oct 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This is tricky.
We want people to be able to convert CIDv0→CIDv1 and be able to use it.

See scenario below:

  • Execute ipfs id
  • How to convert PeerID to CIDv1?
  • Caveat: conversion produces a valid CIDv1 with (now explicit) dag-pb multicodec
    • the multihash inside of CIDv1 still represents a valid PeerID, and this library does not care about the rest od CID anyway

I think the only way is to handle it gracefully is to check for both libp2p-key and dag-pb. Updated this PR to do just that + tests.

return new PeerIdWithIs(cid.multihash)
}
lidel marked this conversation as resolved.
Show resolved Hide resolved

// Public Key input will be a buffer
Expand Down
58 changes: 51 additions & 7 deletions test/peer-id.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ chai.use(dirtyChai)
const expect = chai.expect
const crypto = require('libp2p-crypto')
const mh = require('multihashes')
const CID = require('cids')

const PeerId = require('../src')

Expand All @@ -17,6 +18,8 @@ const testId = require('./fixtures/sample-id')
const testIdHex = testId.id
const testIdBytes = mh.fromHexString(testId.id)
const testIdB58String = mh.toB58String(testIdBytes)
const testIdCID = new CID(1, 'libp2p-key', testIdBytes)
const testIdCIDString = testIdCID.toBaseEncodedString('base32')

const goId = require('./fixtures/go-private-key')

Expand Down Expand Up @@ -63,27 +66,68 @@ describe('PeerId', () => {
}).to.throw(/immutable/)
})

it('recreate an Id from Hex string', () => {
it('recreate from Hex string', () => {
const id = PeerId.createFromHexString(testIdHex)
expect(testIdBytes).to.deep.equal(id.id)
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('Recreate an Id from a Buffer', () => {
it('recreate from a Buffer', () => {
const id = PeerId.createFromBytes(testIdBytes)
expect(testId.id).to.equal(id.toHexString())
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('Recreate a B58 String', () => {
it('recreate from a B58 String', () => {
const id = PeerId.createFromB58String(testIdB58String)
expect(testIdB58String).to.equal(id.toB58String())
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('Recreate from a Public Key', async () => {
it('recreate from CID object', () => {
const id = PeerId.createFromCID(testIdCID)
expect(testIdCIDString).to.equal(id.toString())
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('recreate from Base58 String (CIDv0))', () => {
const id = PeerId.createFromCID(testIdB58String)
expect(testIdCIDString).to.equal(id.toString())
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('recreate from CIDv1 Base32', () => {
const id = PeerId.createFromCID(testIdCIDString)
expect(testIdCIDString).to.equal(id.toString())
expect(testId.id).to.equal(id.toHexString())
})

it('recreate from CID Buffer', () => {
const id = PeerId.createFromCID(testIdCID.buffer)
expect(testIdCIDString).to.equal(id.toString())
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('throws on invalid CID value', () => {
const invalidCID = 'QmaozNR7DZHQK1ZcU9p7QdrshMvXqWK6gpu5rmrkPdT3L'
lidel marked this conversation as resolved.
Show resolved Hide resolved
expect(() => {
PeerId.createFromCID(invalidCID)
}).to.throw(/multihash unknown function code: 0x50/)
})

it('throws on invalid CID object', () => {
const invalidCID = {}
expect(() => {
PeerId.createFromCID(invalidCID)
}).to.throw(/Supplied cid value is neither String|CID|Buffer/)
})

it('recreate from a Public Key', async () => {
const id = await PeerId.createFromPubKey(testId.pubKey)
expect(testIdB58String).to.equal(id.toB58String())
expect(testIdBytes).to.deep.equal(id.toBytes())
})

it('Recreate from a Private Key', async () => {
it('recreate from a Private Key', async () => {
const id = await PeerId.createFromPrivKey(testId.privKey)
expect(testIdB58String).to.equal(id.toB58String())
const encoded = Buffer.from(testId.privKey, 'base64')
Expand All @@ -92,7 +136,7 @@ describe('PeerId', () => {
expect(id.marshalPubKey()).to.deep.equal(id2.marshalPubKey())
})

it('Recreate from Protobuf', async () => {
it('recreate from Protobuf', async () => {
const id = await PeerId.createFromProtobuf(testId.marshaled)
expect(testIdB58String).to.equal(id.toB58String())
const encoded = Buffer.from(testId.privKey, 'base64')
Expand Down