Skip to content
This repository has been archived by the owner on Feb 26, 2021. It is now read-only.

Async Crypto Endeavour #22

Merged
merged 11 commits into from
Nov 3, 2016
Merged

Async Crypto Endeavour #22

merged 11 commits into from
Nov 3, 2016

Conversation

dignifiedquire
Copy link
Member

Ref libp2p/js-libp2p-crypto#10

BREAKING CHANGE

New method PeerInfo.create(cb) replaces the previous new PeerInfo() to create a fresh id

})

it('add multiaddr', () => {
const pi = new PeerInfo()
expect(pi).to.exist
Copy link
Member

Choose a reason for hiding this comment

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

checking something to exist every time doesn't seem useful

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed

expect(pi).to.exist
const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
pi.multiaddr.add(mh)
expect(pi.multiaddrs.length).to.equal(1)
})

it('add multiaddr that are buffers', () => {
const pi = new PeerInfo()
expect(pi).to.exist
Copy link
Member

Choose a reason for hiding this comment

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

same

expect(pi).to.exist
const mh = Multiaddr('/ip4/127.0.0.1/tcp/5001')
pi.multiaddr.add(mh.buffer)
expect(pi.multiaddrs[0] instanceof Multiaddr).to.equal(true)
})

it('add repeated multiaddr', () => {
const pi = new PeerInfo()
expect(pi).to.exist
Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Member

Choose a reason for hiding this comment

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

and it goes on

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@daviddias
Copy link
Member

How did you solve the PCKS#1 vs PKCS#6. Can we confirm that we are manipulating and storing the keys in the same way as in go? This will be crucial for config file share, identify and secio crypto handshake.

@dignifiedquire
Copy link
Member Author

How did you solve the PCKS#1 vs PKCS#6. Can we confirm that we are manipulating and storing the keys in the same way as in go? This will be crucial for config file share, identify and secio crypto handshake.

We already had tests for this (the go interop part) here and in libp2p-crypto. You can see the details of the conversion here: libp2p/js-libp2p-crypto@eb5de10

@@ -91,3 +91,13 @@ function Peer (peerId) {
// TODO: add features to fetch multiaddr using filters
// look at https://github.com/whyrusleeping/js-mafmt/blob/master/src/index.js
}

Peer.create = (cb) => {
Copy link
Member

@daviddias daviddias Oct 5, 2016

Choose a reason for hiding this comment

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

Ok, to simplify, let's default to use PeerInfo.create

  • s/Peer/PeerInfo
  • signature becomes (peerId, callback) =>
  • Use always PeerInfo.create everywhere

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean with peerId being optional?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

return cb(err)
}

cb(null, new Peer(key))
Copy link
Member

Choose a reason for hiding this comment

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

key? or Id?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@daviddias
Copy link
Member

daviddias commented Oct 5, 2016

  • This PR is missing the documentation update

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

LGTM

@daviddias daviddias changed the title refactor: PeerId.create is now async Async Crypto Endeavour Oct 30, 2016
env: CXX=g++-4.8
- node_js: stable
env:
- SAUCE=true
Copy link
Member

Choose a reason for hiding this comment

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

Please do Sauce on LTS

@dignifiedquire
Copy link
Member Author

@diasdavid should be ready now

@daviddias daviddias merged commit 5dc1162 into master Nov 3, 2016
@daviddias daviddias deleted the fast-keys branch November 3, 2016 07:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants