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.

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

refactor: add js-libp2p-keychain to the libp2p codebase #388 #525

Closed
wants to merge 94 commits into from

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Dec 23, 2019

This PR adds js-libp2p-switch to js-libp2p as a part of #384 and #509 .

Things to note:

  • The entire libp2p-keychain history has been added
  • Additional commits were made to move switch under ./src/keychain and its tests under ./test/keychain.
  • Some of the unneeded files were removed (like LICENSE) since libp2p already has a version of that file
  • The readme and package.json from switch were kept, to simplify the transition. Ideally this should all get cleaned up and duplicate tests should be removed in a future PR

Note on Merging:

Right now the plan (obtained from libp2p-keychain PR) is to:

  • squash the merge to minimize the commit pollution
  • update the readme to point to the original repo
  • append the Co-Author list to the squashed commit to maintain author credit
    • This co author list was generated from the libp2p-keychain package.json which is auto generated from the commit history there.
Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
Co-authored-by: Alberto Elias <hi@albertoelias.me>
Co-authored-by: Alex Potsides <alex@achingbrain.net>
Co-authored-by: David Dias <daviddias.p@gmail.com>
Co-authored-by: Hugo Dias <mail@hugodias.me>
Co-authored-by: Jacob Heun <jacobheun@gmail.com>
Co-authored-by: Maciej Krüger <mkg20001@gmail.com>
Co-authored-by: Masahiro Saito <camelmasa@gmail.com>
Co-authored-by: Richard Schneider <makaretu@gmail.com>
Co-authored-by: Vasco Santos <vasco.santos@moxy.studio>
Co-authored-by: Vasco Santos <vasco.santos@ua.pt>
Co-authored-by: Victor Bjelkholm <victorbjelkholm@gmail.com>

daviddias and others added 30 commits December 6, 2017 08:24
Add syntax highlighting to README
This commit updates all CI scripts to the latest version
Persist the key info in the store
test: key name comparision
* test: openssl interop is now the responsibility of libp2p-crypto

* feat: use libp2p-crypto, not node-forge, for key management

* fix: use libp2p-crypto.pbkdf, not node-forge

* fix: do not ship CMS

This removes all depencies on node-forge

* test: update dependencies

* test: remove dead code
@vasco-santos vasco-santos changed the base branch from master to refactor/async-await December 23, 2019 11:36
@vasco-santos vasco-santos force-pushed the refactor/keychain branch 2 times, most recently from c3a1e3c to 0896371 Compare December 23, 2019 14:23
@vasco-santos vasco-santos marked this pull request as ready for review December 23, 2019 14:31
@vasco-santos
Copy link
Member Author

@jacobheun js-libp2p users, such as js-ipfs rely on js-libp2p-keychain directly. I wonder if we should continue that path, and just modify the require, or if we should make this only usable through js-libp2p API. Let me know your thoughts

@vasco-santos vasco-santos changed the base branch from refactor/async-await to master February 3, 2020 15:50
@vasco-santos
Copy link
Member Author

@jacobheun just rebased this. We should document how to use libp2p-keychain. What do you think to be the best way?

@jacobheun
Copy link
Contributor

js-libp2p users, such as js-ipfs rely on js-libp2p-keychain directly. I wonder if we should continue that path, and just modify the require, or if we should make this only usable through js-libp2p API. Let me know your thoughts.

We should integrate this into the libp2p api, this would likely look like libp2p.keychain, and expose the necessary methods from there. Users shouldn't need to worry about importing this or using it directly it just complicates things.

I would expect users to be able to provide a datastore to libp2p and it would just handle key storage. Users should probably also be able to specify a separate datastore for libp2p-keychain. (We should likely always want these to be "separate" datastores so we're not putting dht/peer/etc data in the same place as our keys).

just rebased this. We should document how to use libp2p-keychain. What do you think to be the best way?

At the bare minimum it needs to be documented in the API and Configuration readme's, and we really need more tests around using this directly with libp2p.

  • It would be good to test the persistence. (If Libp2p.create() is given a keystore, it should retain its previous identity instead of creating a new peer id)

@vasco-santos vasco-santos mentioned this pull request Feb 5, 2020
5 tasks
@vasco-santos vasco-santos mentioned this pull request May 7, 2020
@achingbrain achingbrain deleted the refactor/keychain branch May 18, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants