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 uuid as key #143

Merged
merged 8 commits into from Mar 13, 2019

Conversation

Projects
None yet
3 participants
@BaojunCZ
Copy link
Contributor

BaojunCZ commented Mar 11, 2019

No description provided.

@@ -58,7 +60,7 @@
"electron-devtools-installer": "^2.2.4",
"spectron": "^5.0.0",
"tslint": "^5.12.1",
"execa": "^1.0.0",
"@types/execa": "^0.9.0"
"@types/uuid": "^3.4.4",

This comment has been minimized.

Copy link
@Keith-CY

Keith-CY Mar 12, 2019

Collaborator

It seems that this dependency is added manually.

This comment has been minimized.

Copy link
@BaojunCZ

BaojunCZ Mar 12, 2019

Author Contributor

yes, I will move it under other @types

import { Keystore } from '../keys/keystore'
import env from '../env'

const keyWalletName = 'WalletName'
const encryptKey = 'Neuron'

This comment has been minimized.

Copy link
@Keith-CY

Keith-CY Mar 12, 2019

Collaborator

better to be in env

This comment has been minimized.

Copy link
@BaojunCZ

BaojunCZ Mar 12, 2019

Author Contributor

ok

const keyWalletName = 'WalletName'
const encryptKey = 'Neuron'

const WalletIDKey = 'WalletID'

This comment has been minimized.

Copy link
@Keith-CY

Keith-CY Mar 12, 2019

Collaborator

better to be in env

This comment has been minimized.

Copy link
@BaojunCZ

BaojunCZ Mar 12, 2019

Author Contributor

It is a key just used in wallet store.So I don't think it needs to go to env

This comment has been minimized.

Copy link
@ashchan

ashchan Mar 12, 2019

Member

Let's do it this way: check env, and use different root folder for keys.

For example, for production, use /keys (relative to default user data folder), for development, use /keys-dev, for test, use /keys-test. This is to avoid wiping local data when running tests or affecting production data when running in development on the same machine.

This comment has been minimized.

Copy link
@BaojunCZ

BaojunCZ Mar 12, 2019

Author Contributor

ok,test is running in different folder now.I will move dev part to new folder

@BaojunCZ BaojunCZ marked this pull request as ready for review Mar 12, 2019

@@ -28,7 +26,7 @@ export default class WalletStore {
constructor() {
const idOptions: Options = {
name: env.walletIDName,
encryptionKey: encryptKey,
encryptionKey: env.storeEncryptKey,

This comment has been minimized.

Copy link
@ashchan

ashchan Mar 12, 2019

Member

Do we really need to encrypt files?

This comment has been minimized.

Copy link
@BaojunCZ

BaojunCZ Mar 13, 2019

Author Contributor

Encryption or not has little effect.I think better than nothing

@@ -28,7 +28,7 @@

"bootstrap": "lerna bootstrap && lerna link",
"lint": "lerna run lint",
"test": "lerna run teststore && lerna run --parallel test",
"test": "lerna run --parallel test && lerna run teststore",

This comment has been minimized.

Copy link
@ashchan

ashchan Mar 12, 2019

Member

I think it's better to call teststore from neuron-wallet package instead.

This comment has been minimized.

Copy link
@BaojunCZ

BaojunCZ Mar 13, 2019

Author Contributor

ok

@Keith-CY Keith-CY merged commit 5f1b840 into nervosnetwork:develop Mar 13, 2019

@BaojunCZ BaojunCZ deleted the BaojunCZ:wallet-identifier branch Mar 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.