Skip to content

Commit

Permalink
Async Crypto Endeavour (#33)
Browse files Browse the repository at this point in the history
* refactor: make import and creation async - This allows the use of native key generation in the browser

BREAKING CHANGE:

This changes the interface of .create, .createFromPrivKey,
.createFromPubKey, .createFromJSON
  • Loading branch information
dignifiedquire authored and daviddias committed Nov 3, 2016
1 parent e08907f commit 31701e2
Show file tree
Hide file tree
Showing 8 changed files with 325 additions and 167 deletions.
16 changes: 0 additions & 16 deletions .aegir.js

This file was deleted.

3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ build/Release
# https://www.npmjs.org/doc/misc/npm-faq.html#should-i-check-my-node_modules-folder-into-git
node_modules

lib
dist
dist
26 changes: 19 additions & 7 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
sudo: false
language: node_js
node_js:
- 4
- 5
- stable

matrix:
include:
- node_js: 4
env: CXX=g++-4.8
- node_js: 6
env:
- SAUCE=true
- CXX=g++-4.8
- node_js: stable
env: CXX=g++-4.8

# Make sure we have new NPM.
before_install:
Expand All @@ -14,12 +21,17 @@ script:
- npm test
- npm run coverage

addons:
firefox: 'latest'

before_script:
- export DISPLAY=:99.0
- sh -e /etc/init.d/xvfb start

after_success:
- npm run coverage-publish

addons:
firefox: latest
apt:
sources:
- ubuntu-toolchain-r-test
packages:
- g++-4.8
57 changes: 47 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,40 @@
[![Coverage Status](https://coveralls.io/repos/github/libp2p/js-peer-id/badge.svg?branch=master)](https://coveralls.io/github/libp2p/js-peer-id?branch=master)
[![Dependency Status](https://david-dm.org/libp2p/js-peer-id.svg?style=flat-square)](https://david-dm.org/libp2p/js-peer-id)
[![js-standard-style](https://img.shields.io/badge/code%20style-standard-brightgreen.svg?style=flat-square)](https://github.com/feross/standard)
![](https://img.shields.io/badge/npm-%3E%3D3.0.0-orange.svg?style=flat-square)
![](https://img.shields.io/badge/Node.js-%3E%3D4.0.0-orange.svg?style=flat-square)

[![Sauce Test Status](https://saucelabs.com/browser-matrix/ipfs-js-peer-id.svg)](https://saucelabs.com/u/ipfs-js-peer-id)

> [IPFS](https://github.com/ipfs/ipfs) Peer ID implementation in JavaScript.
- [Description](#description)
- [Example](#example)
- [Installation](#installation)
- [npm](#npm)
- [Setup](#setup)
- [Node.js](#nodejs)
- [Browser: Browserify, Webpack, other bundlers](#browser-browserify-webpack-other-bundlers)
- [Browser: `<script>` Tag](#browser-script-tag)
- [API](#api)
- [Create](#create)
- [`new PeerId(id[, privKey, pubKey])`](#new-peeridid-privkey-pubkey)
- [`create([opts], callback)`](#createopts-callback)
- [Import](#import)
- [`createFromHexString(str)`](#createfromhexstringstr)
- [`createFromBytes(buf)`](#createfrombytesbuf)
- [`createFromB58String(str)`](#createfromb58stringstr)
- [`createFromPubKey(pubKey)`](#createfrompubkeypubkey)
- [`createFromPrivKey(privKey)`](#createfromprivkeyprivkey)
- [`createFromJSON(obj)`](#createfromjsonobj)
- [Export](#export)
- [`toHexString()`](#tohexstring)
- [`toBytes()`](#tobytes)
- [`toB58String()`](#tob58string)
- [`toJSON()`](#tojson)
- [`toPrint()`](#toprint)
- [License](#license)

# Description

Generate, import, and export PeerIDs, for use with [IPFS](https://github.com/ipfs/ipfs).
Expand All @@ -26,17 +57,17 @@ to the multihash for ID generation.*
var PeerId = require('peer-id')
var bs58 = require('bs58')

var id = PeerId.create({ bits: 32 })

console.log('id ', id.toB58String())
console.log('priv key ', bs58.encode(id.privKey.bytes))
console.log('pub key ', bs58.encode(id.pubKey.bytes))
PeerId.create({ bits: 1024 }, (err, id) => {
console.log(JSON.stringify(id.toJSON(), null, 2)
})
```
```
id QmeeLFb92nkZJGj3gXLqXrEMzCMYs6uBgQLVNbrcXEvYXk
priv key 6ibrcPAbevzvPpkq6EA6XmLyuhmUrJrEvUfgQDtEiSEPzGnGU8Ejwf6b11DVm6opnFGo
pub key 2BeBZVKJ9RQs4i4LbGv4ReEeuBA5dck2Gje3wt67e44XuyyPq5jE
{
"id": "Qma9T5YraSnpRDZqRR4krcSJabThc8nwZuJV3LercPHufi",
"privKey": "CAAS4AQwggJcAgEAAoGBAMBgbIqyOL26oV3nGPBYrdpbv..",
"pubKey": "CAASogEwgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMBgbIqyOL26oV3nGPBYrdpbvzCY..."
}
```
# Installation
Expand Down Expand Up @@ -93,11 +124,14 @@ const PeerId = require('peer-id')
The key format is detailed in [libp2p-crypto](https://github.com/libp2p/js-libp2p-crypto).
### `create([opts])`
### `create([opts], callback)`
Generates a new Peer ID, complete with public/private keypair.
- `opts: Object`: Default: `{bits: 2048}`
- `callback: Function`
Calls back `callback` with `err, id`.
## Import
Expand All @@ -114,10 +148,14 @@ Creates a Peer ID from a Base58 string representing the key's multihash.
### `createFromPubKey(pubKey)`
- `publicKey: Buffer`
Creates a Peer ID from a buffer containing a public key.
### `createFromPrivKey(privKey)`
- `privKey: Buffer`
Creates a Peer ID from a buffer containing a private key.
### `createFromJSON(obj)`
Expand All @@ -126,7 +164,6 @@ Creates a Peer ID from a buffer containing a private key.
- `obj.pubKey: String` - The public key in protobuf format, encoded in 'base64'
- `obj.privKey: String` - The private key in protobuf format, encoded in 'base 64'

## Export
### `toHexString()`
Expand Down
18 changes: 9 additions & 9 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
"name": "peer-id",
"version": "0.7.0",
"description": "IPFS Peer Id implementation in Node.js",
"main": "lib/index.js",
"main": "src/index.js",
"bin": "src/bin.js",
"jsnext:main": "src/index.js",
"scripts": {
"lint": "aegir-lint",
"build": "aegir-build",
Expand All @@ -27,24 +26,25 @@
"test"
],
"engines": {
"node": "^4.3.0"
"node": ">=4.0.0"
},
"bugs": {
"url": "https://github.com/diasdavid/js-peer-id/issues"
"url": "https://github.com/libp2p/js-peer-id/issues"
},
"homepage": "https://github.com/diasdavid/js-peer-id",
"homepage": "https://github.com/libp2p/js-peer-id",
"devDependencies": {
"aegir": "^8.0.0",
"aegir": "^9.0.1",
"chai": "^3.5.0",
"pre-commit": "^1.1.3"
},
"dependencies": {
"libp2p-crypto": "^0.6.1",
"async": "^2.0.1",
"libp2p-crypto": "^0.7.0",
"multihashes": "^0.2.2"
},
"repository": {
"type": "git",
"url": "https://github.com/diasdavid/js-peer-id.git"
"url": "https://github.com/libp2p/js-peer-id.git"
},
"contributors": [
"David Dias <daviddias.p@gmail.com>",
Expand All @@ -53,4 +53,4 @@
"dignifiedquire <dignifiedquire@gmail.com>",
"nginnever <ginneversource@gmail.com>"
]
}
}
8 changes: 7 additions & 1 deletion src/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,10 @@

const PeerId = require('./index.js')

console.log(JSON.stringify(PeerId.create().toJSON(), null, ' '))
PeerId.create((err, id) => {
if (err) {
throw err
}

console.log(JSON.stringify(id.toJSON(), null, 2))
})
Loading

7 comments on commit 31701e2

@parkan
Copy link

@parkan parkan commented on 31701e2 Nov 10, 2016

Choose a reason for hiding this comment

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

What informed the decision to implement this with callbacks instead of modern promise style? Trying to merge in a greenkeeper update against this release and the amount of refactoring with callback style is more than seems necessary.

@parkan
Copy link

@parkan parkan commented on 31701e2 Nov 10, 2016

Choose a reason for hiding this comment

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

Ok, looking around this seems consistent with other usage in the codebase. My vote is still for promises 😄

@dignifiedquire
Copy link
Member Author

Choose a reason for hiding this comment

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

@parkan yes this was not a new decision, just keeping in line with the rest of the code base. For more fun discussions around this please visit ipfs/js-ipfs#557 and weigh in

@parkan
Copy link

@parkan parkan commented on 31701e2 Nov 11, 2016

Choose a reason for hiding this comment

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

@dignifiedquire thanks for the explanation, I'll chime in there.

I tried to wrap these in promises, and I'm actually running into some trouble. Tried using thenable and es6-promisify and in both cases I end up with promises that never complete (stuck in pending state and neither handler fires). The code is pretty simple and I've done this many times before, so not sure what could be the cause -- to further confuse things if I use thenable's withCallback and pass a callback to the transformed function it still works.

Any idea why this might happen, or maybe an example of promise-wrapped use somewhere?

@dignifiedquire
Copy link
Member Author

Choose a reason for hiding this comment

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

@parkan could you file an issue with some repro code please? Then it's easier for me to fix

@parkan
Copy link

@parkan parkan commented on 31701e2 Nov 11, 2016

Choose a reason for hiding this comment

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

Hrm can't reproduce this behavior anymore after shuffling around branches and npm updating, it's possible my modules were in a weird state. However, I noticed something else: PeerId.create does not check for the presence of a callback like createFromPubKey etc do: https://github.com/libp2p/js-peer-id/blob/master/src/index.js#L88 so you can crash like so:

> const PeerId = require('peer-id')
undefined
> PeerId.create()
undefined
> /Users/arkadiy/mine/aleph/node_modules/peer-id/src/index.js:106
    callback(null, new PeerId(digest, privKey))
    ^

TypeError: callback is not a function
    at waterfall (/Users/arkadiy/mine/aleph/node_modules/peer-id/src/index.js:106:5)
    at /Users/arkadiy/mine/aleph/node_modules/peer-id/node_modules/async/internal/once.js:12:16
    at nextTask (/Users/arkadiy/mine/aleph/node_modules/peer-id/node_modules/async/waterfall.js:15:29)
    at /Users/arkadiy/mine/aleph/node_modules/peer-id/node_modules/async/waterfall.js:22:13
    at apply (/Users/arkadiy/mine/aleph/node_modules/lodash/_apply.js:15:25)
    at /Users/arkadiy/mine/aleph/node_modules/lodash/_overRest.js:32:12
    at /Users/arkadiy/mine/aleph/node_modules/peer-id/node_modules/async/internal/onlyOnce.js:12:16
    at privKey.public.hash (/Users/arkadiy/mine/aleph/node_modules/peer-id/src/index.js:99:7)
    at Multihashing.Multihashing.digest (/Users/arkadiy/mine/aleph/node_modules/multihashing-async/src/index.js:23:5)
    at sha2256 (/Users/arkadiy/mine/aleph/node_modules/multihashing-async/src/crypto.js:13:3)

@parkan
Copy link

@parkan parkan commented on 31701e2 Nov 11, 2016

Choose a reason for hiding this comment

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

I think a related issue (from a code style viewpoint) is the mixed callback/direct return style API with no way to tell which functions have which behaviors without examining the signature or getting the "callback required" warning.

Please sign in to comment.