Skip to content
This repository has been archived by the owner on Apr 8, 2020. It is now read-only.

Adds key-composer to support import and export to pem #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AlbertoElias
Copy link
Contributor

Working on bringing https://github.com/ipfs-shipyard/js-crypto-key-composer into libp2p-crypto to support import and export in non-RSA keys like ed25519 and secp256k1.

This is the latter, and necessary for the work happening here: https://github.com/AlbertoElias/js-libp2p-crypto/tree/use-key-composer

This is part of the work done for this issue: libp2p/js-libp2p-crypto#145

cc @satazor @dignifiedquire @hugomrdias

src/index.js Outdated
@@ -2,6 +2,7 @@

const bs58 = require('bs58')
const multihashing = require('multihashing-async')
const keyComposer = require('crypto-key-composer')
Copy link

Choose a reason for hiding this comment

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

Perhaps it would be better to do const { composePrivateKey, decomposePrivateKey }. The benefit is that it will be easier to perform tree-shaking by bundlers, as we are importing only what we need at the require level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I'll do that when fixing the linting errors which I'm not sure what the maintainers prefer

@AlbertoElias
Copy link
Contributor Author

Hey, there's a linting error due to too many callbacks in the tests. Should I structure tests differently to avoid this? That would mean that the tests would be less clean

@hugomrdias
Copy link
Member

You can do a block disable for now in the tests because we will change to async await shortly

@AlbertoElias
Copy link
Contributor Author

@satazor @hugomrdias done!

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.

3 participants