orbitdb-archive / orbit-db-keystore Goto Github PK

A few questions about keystore's API:

• Are all the methods available being used? Some of them seem redundant when we have getKey? e.g: importPublicKey and importPrivateKey (Maybe I didn't get how they're being used or where. I saw that the former is used by ipfs-log it seems)

• Should it be responsible for generating / creating keys or just loading it? Currently, we are creating a new key every time a key isn't found at the keystore storage maybe that should be explicit so we don't end up doing that accidentally. Also, for cases where we don't wanna store the key on local storage or leveldb, we could keep generateKey and make it return a key-pair.

• Suggestion: Can we give the keystore both a key and a signer object via constructor?
  Why: For a dApp you most probably already have a key and a wallet you can use to sign messages.

error Command failed.
Exit code: 128
Command: git
Arguments: ls-remote --tags --heads ssh://[email protected]/hugomrdias/pull-ws.git
Directory: /Users/ryo/.ghq/github.com/orbitdb/orbit-db-keystore
Output:
ERROR: Repository not found.
fatal: Could not read from remote repository.

One of the dependent packages has been renamed and cannot be resolved as a dependency.

orbit-db-test-utils -> ipfs -> libp2p-websockets -> pull-ws (this package repository is renamed.)

So we need to update orbit-db-test-utils version.

Left intentionally blank.

It would fix https://github.com/orbitdb/orbit-db/issues/382

background
src/keystore.js requires 'secp256k1' without adding it to dependencies and ends up using node_modules/secp256k1. recently this has introduced some issues as version 4 comes with an api change. this change makes it so, in browsers, Uint8Array is used instead of Buffer.

the issue
when node_modules/secp256k1 is version 4
https://github.com/orbitdb/orbit-db-keystore/blob/2e32c00727929019694474dbdfab236e9a2771e4/src/keystore.js#L84
https://github.com/orbitdb/orbit-db-keystore/blob/2e32c00727929019694474dbdfab236e9a2771e4/src/keystore.js#L176
these two lines use secp256k1 directly and expect the output to be a buffer so they can use buffer.toString(encoding). in the browser, the secp256k1 package will return a Uint8 TypeArray. the toString method on this differs from buffer and does not accept any arguments like encoding. Uint8Array.toString('hex') will not return the hex encoded string but the buffer values joined by ',' (eg [1,2,3] -> '1,2,3').
This results in a malformed publicKey in the identity and identityProvider.verifyIdentity(entry.identity), used in access controllers, to always return false.

fixes
both lines above could be fixed by explicitly converting the output of secp256k1.publicKeyConvert to Buffers or by adding the secp256k1 package to dependencies.

This is a problem that happened to me on test environment (ava). It fails the test because of a unhandled promise rejection. Maybe there's a better way to, well, handle it? 😄

paging @shamb0t

There doesn't seem to be any platform checking on the 'fs' import.

I am having the following error when running with Next.js: Module not found: Can't resolve 'fs'

The problem seems to have arisen with this commit, affecting the latest release of orbit-db in browser.
042ff1d

@haydenyoung

Hi,

Could it be possible to provide the instance of the cache to use as an argument of the constructor as below?

const keystore1 = new Keystore()
const keystore2 = new Keystore(undefined, new LRU(10))
const keystore3 = new Keystore(level('./somewhere'), new LRU(10))
const keystore4 = new Keystore(level('./somewhere'))

Or using an options object?

const keystore1 = new Keystore()
const keystore2 = new Keystore({
  cache: new LRU(10)
})
const keystore3 = new Keystore({
  store: level('./somewhere'),
  cache: new LRU(10)
})
const keystore4 = new Keystore({
  store: level('./somewhere')
})
// for compatibilities
const keystore4bis = new Keystore(level('./somewhere'))

If you are agree I can do the pull request with the additional tests.

Best regards

Keystore currently opens/closes level store with each interaction with storage to avoid level error LOCK:already held by process. This could be avoided by either changing the path used for the keystore or returning the same keystore if the path is already opened

There are currently no stand-alone tests

We should use js-libp2p-crypto module for the signing keys.