From 2cc95d081f97dd92454e6cbb02623db1c3a78ea9 Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 00:48:09 +0900 Subject: [PATCH 1/8] chore: Allow no blank line between class single line members --- .eslintrc.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index 3ddbe21634..598fe2621d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -52,7 +52,8 @@ module.exports = { "no-plusplus": [0], "no-console": [2, { "allow": ["warn", "error", "info"] - }] + }], + "lines-between-class-members": ["error", "always", { exceptAfterSingleLine: true }] }, "globals": { "BigInt": "readonly" From 2b0c0c4b5bd077263c026d3a24109aad29fd4c0e Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 02:34:49 +0900 Subject: [PATCH 2/8] refactor: Replace WalletStore's underlying electron-store backend with your own Store impl --- packages/neuron-wallet/src/app.ts | 6 +- .../neuron-wallet/src/services/wallets.ts | 6 +- .../neuron-wallet/src/store/walletStore.ts | 134 ++++++++---------- .../neuron-wallet/tests/walletstore.test.ts | 36 ++--- 4 files changed, 87 insertions(+), 95 deletions(-) diff --git a/packages/neuron-wallet/src/app.ts b/packages/neuron-wallet/src/app.ts index 8ee026ad15..8936465984 100644 --- a/packages/neuron-wallet/src/app.ts +++ b/packages/neuron-wallet/src/app.ts @@ -1,8 +1,10 @@ +import path from 'path' +import os from 'os' import { app as electronApp } from 'electron' const fakeApp = { - getPath(path: string): string { - return path + getPath(aPath: string): string { + return path.join(os.tmpdir(), aPath) }, } const app = electronApp || fakeApp diff --git a/packages/neuron-wallet/src/services/wallets.ts b/packages/neuron-wallet/src/services/wallets.ts index 1b6952d07e..2f4110ca46 100644 --- a/packages/neuron-wallet/src/services/wallets.ts +++ b/packages/neuron-wallet/src/services/wallets.ts @@ -36,7 +36,7 @@ export default class WalletService { public update = (walletId: string, newWallet: WalletData) => { const currentWallet = walletStore.getWallet(walletId) - walletStore.update(walletId, { ...currentWallet, ...newWallet }) + walletStore.updateWallet(walletId, { ...currentWallet, ...newWallet }) } public delete = (id: string): boolean => { @@ -49,10 +49,10 @@ export default class WalletService { } public setActive = (id: string): boolean => { - return walletStore.setActiveWallet(id) + return walletStore.setCurrentWallet(id) } public getActive = (): WalletData => { - return walletStore.getActiveWallet() + return walletStore.getCurrentWallet() } } diff --git a/packages/neuron-wallet/src/store/walletStore.ts b/packages/neuron-wallet/src/store/walletStore.ts index edff426d27..1101fbf6ec 100644 --- a/packages/neuron-wallet/src/store/walletStore.ts +++ b/packages/neuron-wallet/src/store/walletStore.ts @@ -1,8 +1,9 @@ -import Store from 'electron-store' +import fs from 'fs' import { Keystore } from '../keys/keystore' import env from '../env' import { Addresses } from '../keys/key' import app from '../app' +import Store from '../utils/store' export enum WalletStoreError { NoWallet, @@ -16,111 +17,98 @@ export interface WalletData { addresses: Addresses } -interface Options { - name?: string - cwd?: string - encryptionKey?: string | Buffer -} - -const userDataPath = app.getPath('userData') -const storePath = env.isDevMode ? `${userDataPath}/dev/wallets` : `${userDataPath}/wallets` -const WalletIDKey = 'WalletID' -const ActiveWalletID = 'ActiveID' +// TODO: Check if '/dev/wallets' path works on Windows +const defaultStorePath = env.isDevMode ? '/dev/wallets' : '/wallets' export default class WalletStore { - walletIDStore: Store - - constructor() { - const idOptions: Options = { - name: WalletIDKey, - cwd: storePath, - } - this.walletIDStore = new Store(idOptions) + private storePath: string + private listStore: Store // Save wallets (meta info except keystore, which is persisted separately) + private walletsKey = 'wallets' + private currentWalletKey = 'current' + + constructor(storePath: string = defaultStorePath) { + this.storePath = `${app.getPath('userData')}/${storePath}` + fs.mkdirSync(this.storePath, { recursive: true }) + this.listStore = new Store(this.storePath, 'wallets.json') } - private getIDList = (): string[] => { - return this.walletIDStore.get(WalletIDKey, []) as any + private getWalletStore = (id: string): Store => { + return new Store(this.storePath, `${id}.json`) } - private addWalletID = (id: string) => { - this.walletIDStore.set(WalletIDKey, this.getIDList().concat(id)) + getAllWallets = (): WalletData[] => { + return this.listStore.readSync(this.walletsKey) || [] } - private removeWalletID = (id: string) => { - const idList = this.getIDList() - idList.splice(idList.indexOf(id), 1) - this.walletIDStore.set(WalletIDKey, idList) + getWallet = (id: string): WalletData => { + const wallets = this.getAllWallets() + const wallet = wallets.find((w: WalletData) => w.id === id) + if (!wallet) { + throw WalletStoreError.NoWallet + } + return wallet as any } - private getWalletStore = (id: string): Store => { - const options: Options = { - name: id, - cwd: storePath, + saveWallet = (walletData: WalletData) => { + this.listStore.writeSync(this.walletsKey, this.getAllWallets().concat(walletData)) + // TODO: Save keystore to that store instead. + this.getWalletStore(walletData.id).writeSync(walletData.id, walletData) + if (this.getAllWallets().length === 1) { + this.setCurrentWallet(walletData.id) } - return new Store(options) } - saveWallet = (walletData: WalletData) => { - this.addWalletID(walletData.id) - this.getWalletStore(walletData.id).set(walletData.id, walletData) - if (this.getIDList().length === 1) { - this.setActiveWallet(walletData.id) + updateWallet = (id: string, newWallet: WalletData) => { + const wallets = this.getAllWallets() + const index = wallets.findIndex((w: WalletData) => w.id === id) + if (index !== -1) { + wallets[index] = newWallet + this.listStore.writeSync(this.walletsKey, wallets) + } else { + throw WalletStoreError.NoWallet } + // TODO: Save keystore to that store instead. + this.getWalletStore(id).writeSync(id, newWallet) } - getWallet = (walletId: string): WalletData => { - const wallet = this.getWalletStore(walletId).get(walletId, null) - if (!wallet) { + deleteWallet = (id: string) => { + const currentId = this.getCurrentWallet().id + const wallets = this.getAllWallets() + const index = wallets.findIndex((w: WalletData) => w.id === id) + if (index !== -1) { + wallets.splice(index, 1) + this.listStore.writeSync(this.walletsKey, wallets) + } else { throw WalletStoreError.NoWallet } - return wallet as any + this.getWalletStore(id).clear() + + if (currentId === id) { + this.setCurrentWallet(this.getAllWallets()[0].id) + } } - setActiveWallet = (walletId: string): boolean => { - const index = this.getIDList().findIndex(id => id === walletId) + setCurrentWallet = (walletId: string): boolean => { + const index = this.getAllWallets().findIndex((w: WalletData) => w.id === walletId) if (index === -1) { return false } - this.walletIDStore.set(ActiveWalletID, walletId) + this.listStore.writeSync(this.currentWalletKey, walletId) return true } - getActiveWallet = (): WalletData => { - const walletId = this.walletIDStore.get(ActiveWalletID, null) as string + getCurrentWallet = (): WalletData => { + const walletId = this.listStore.readSync(this.currentWalletKey) as string if (walletId) { return this.getWallet(walletId) } throw WalletStoreError.NoActiveWallet } - getAllWallets = (): WalletData[] => { - const walletList: WalletData[] = [] - const idList = this.getIDList() - idList.forEach(id => { - walletList.push(this.getWallet(id)) - }) - return walletList - } - - update = (walletId: string, newWallet: WalletData) => { - this.getWalletStore(walletId).set(walletId, newWallet) - } - - deleteWallet = (walletId: string) => { - const activeId = this.getActiveWallet().id - this.removeWalletID(walletId) - this.getWalletStore(walletId).clear() - const idList = this.getIDList() - if (idList.length > 0 && activeId === walletId) { - this.setActiveWallet(idList[0]) - } - } - clearAll = () => { - const idList = this.getIDList() - idList.forEach(id => { - this.getWalletStore(id).clear() + this.getAllWallets().forEach(w => { + this.getWalletStore(w.id).clear() }) - this.walletIDStore.clear() + this.listStore.clear() } } diff --git a/packages/neuron-wallet/tests/walletstore.test.ts b/packages/neuron-wallet/tests/walletstore.test.ts index 7257e48992..e19a06917c 100644 --- a/packages/neuron-wallet/tests/walletstore.test.ts +++ b/packages/neuron-wallet/tests/walletstore.test.ts @@ -1,13 +1,11 @@ -import { v4 } from 'uuid' import assert from 'assert' import WalletStore, { WalletData } from '../src/store/walletStore' -// TODO: re-enable tests after removing electron dependency -describe.skip('wallet store', () => { - const walletStore = new WalletStore() +describe('wallet store', () => { + let walletStore: WalletStore const wallet1: WalletData = { - id: v4(), + id: '1', name: 'wallet1', keystore: { version: 0, @@ -60,7 +58,7 @@ describe.skip('wallet store', () => { } const wallet2: WalletData = { - id: v4(), + id: '2', name: 'wallet2', keystore: { version: 0, @@ -112,7 +110,7 @@ describe.skip('wallet store', () => { }, } const wallet3: WalletData = { - id: v4(), + id: '3', name: 'wallet3', keystore: { version: 0, @@ -165,6 +163,10 @@ describe.skip('wallet store', () => { } beforeEach(() => { + walletStore = new WalletStore('test/wallets') + }) + + afterEach(() => { walletStore.clearAll() }) @@ -195,7 +197,7 @@ describe.skip('wallet store', () => { walletStore.saveWallet(wallet1) walletStore.saveWallet(wallet2) wallet1.name = wallet2.name - walletStore.update(wallet1.id, wallet1) + walletStore.updateWallet(wallet1.id, wallet1) const wallet = walletStore.getWallet(wallet1.id) assert.deepStrictEqual(wallet, { id: wallet1.id, @@ -238,7 +240,7 @@ describe.skip('wallet store', () => { ], } wallet1.addresses = addresses - walletStore.update(wallet1.id, wallet1) + walletStore.updateWallet(wallet1.id, wallet1) const wallet = walletStore.getWallet(wallet1.id) assert.deepStrictEqual(wallet, { id: wallet1.id, @@ -262,24 +264,24 @@ describe.skip('wallet store', () => { it('get and set active wallet', () => { walletStore.saveWallet(wallet1) walletStore.saveWallet(wallet2) - assert.strictEqual(walletStore.setActiveWallet(wallet1.id), true) - assert.deepStrictEqual(walletStore.getActiveWallet(), wallet1) - assert.strictEqual(walletStore.setActiveWallet(wallet2.id), true) - assert.deepStrictEqual(walletStore.getActiveWallet(), wallet2) - assert.strictEqual(walletStore.setActiveWallet(wallet1.id), true) + assert.strictEqual(walletStore.setCurrentWallet(wallet1.id), true) + assert.deepStrictEqual(walletStore.getCurrentWallet(), wallet1) + assert.strictEqual(walletStore.setCurrentWallet(wallet2.id), true) + assert.deepStrictEqual(walletStore.getCurrentWallet(), wallet2) + assert.strictEqual(walletStore.setCurrentWallet(wallet1.id), true) }) it('first wallet is active wallet', () => { walletStore.saveWallet(wallet1) walletStore.saveWallet(wallet2) - assert.deepStrictEqual(walletStore.getActiveWallet(), wallet1) + assert.deepStrictEqual(walletStore.getCurrentWallet(), wallet1) }) it('delete active wallet', () => { walletStore.saveWallet(wallet1) walletStore.saveWallet(wallet2) walletStore.deleteWallet(wallet1.id) - const activeWallet = walletStore.getActiveWallet() + const activeWallet = walletStore.getCurrentWallet() assert.deepStrictEqual(activeWallet, wallet2) assert.strictEqual(walletStore.getAllWallets().length, 1) }) @@ -288,7 +290,7 @@ describe.skip('wallet store', () => { walletStore.saveWallet(wallet1) walletStore.saveWallet(wallet2) walletStore.deleteWallet(wallet2.id) - const activeWallet = walletStore.getActiveWallet() + const activeWallet = walletStore.getCurrentWallet() assert.deepStrictEqual(activeWallet, wallet1) }) }) From 42dea2c868018a608c1610e34998d007ef42964e Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 02:39:51 +0900 Subject: [PATCH 3/8] chore(deps): Remove electron-store --- packages/neuron-wallet/package.json | 2 - yarn.lock | 68 +---------------------------- 2 files changed, 2 insertions(+), 68 deletions(-) diff --git a/packages/neuron-wallet/package.json b/packages/neuron-wallet/package.json index 7dc9aeb211..57c5f3fbf4 100644 --- a/packages/neuron-wallet/package.json +++ b/packages/neuron-wallet/package.json @@ -56,7 +56,6 @@ "bip39": "3.0.2", "chalk": "2.4.2", "crypto-browserify": "3.12.0", - "electron-store": "3.2.0", "electron-window-state": "5.0.3", "i18next": "15.0.5", "reflect-metadata": "0.1.13", @@ -73,7 +72,6 @@ "@nervosnetwork/neuron-ui": "0.1.0", "@types/bip39": "2.4.2", "@types/electron-devtools-installer": "2.2.0", - "@types/electron-store": "1.3.0", "@types/sqlite3": "3.1.5", "@types/uuid": "3.4.4", "devtron": "1.4.0", diff --git a/yarn.lock b/yarn.lock index bda8b56164..5274d2f0ea 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2252,13 +2252,6 @@ resolved "https://registry.yarnpkg.com/@types/electron-devtools-installer/-/electron-devtools-installer-2.2.0.tgz#32ee4ebbe99b3daf9847a6d2097dc00b5de94f10" integrity sha512-HJNxpaOXuykCK4rQ6FOMxAA0NLFYsf7FiPFGmab0iQmtVBHSAfxzy3MRFpLTTDDWbV0yD2YsHOQvdu8yCqtCfw== -"@types/electron-store@1.3.0": - version "1.3.0" - resolved "https://registry.yarnpkg.com/@types/electron-store/-/electron-store-1.3.0.tgz#340f780952bc98043e24ac8c7903e78665495091" - integrity sha512-PplQbntPl3xNcckjizjoQShPwLot72jFiaI67CZ0JOr+AuGwqXdL73sU3vfuuP+RZecOh8vX1G7yWjmxs4lvBQ== - dependencies: - "@types/node" "*" - "@types/enzyme-adapter-react-16@1.0.3": version "1.0.3" resolved "https://registry.yarnpkg.com/@types/enzyme-adapter-react-16/-/enzyme-adapter-react-16-1.0.3.tgz#0cf7025b036694ca8d596fe38f24162e7117acf1" @@ -2774,7 +2767,7 @@ ajv-keywords@^3.1.0, ajv-keywords@^3.4.0: resolved "https://registry.yarnpkg.com/ajv-keywords/-/ajv-keywords-3.4.0.tgz#4b831e7b531415a7cc518cd404e73f6193c6349d" integrity sha512-aUjdRFISbuFOl0EIZc+9e4FfZp0bDZgAdOOf30bJmw8VM9v84SHyVyxDfbWxpGYbdZD/9XoKxfHVNmxPkhwyGw== -ajv@^6.1.0, ajv@^6.10.0, ajv@^6.5.5, ajv@^6.9.1, ajv@^6.9.2: +ajv@^6.1.0, ajv@^6.5.5, ajv@^6.9.1, ajv@^6.9.2: version "6.10.0" resolved "https://registry.yarnpkg.com/ajv/-/ajv-6.10.0.tgz#90d0d54439da587cd7e843bfb7045f50bd22bdf1" integrity sha512-nffhOpkymDECQyR0mnsUtoCE8RlX38G0rYP+wgLWFyZuUyuuojSSvi/+euOiQBIn63whYwYVIIH1TvE3tu4OEg== @@ -4446,19 +4439,6 @@ concurrently@4.1.0: tree-kill "^1.1.0" yargs "^12.0.1" -conf@^4.0.1: - version "4.0.2" - resolved "https://registry.yarnpkg.com/conf/-/conf-4.0.2.tgz#cc25649295259fc77f4606840b7718cca5c1d5bd" - integrity sha512-SVEWGdAlA+BNfpJ5vF7S6SbkXA7Qk1ycWV6+UAWkC3vQvjxx7xxuYNriKnShjlbIPStUiTK0MZWdQYYtTYdwZw== - dependencies: - ajv "^6.10.0" - dot-prop "^5.0.0" - env-paths "^2.2.0" - json-schema-typed "^7.0.0" - make-dir "^3.0.0" - pkg-up "^3.0.1" - write-file-atomic "^2.4.2" - config-chain@^1.1.11: version "1.1.12" resolved "https://registry.yarnpkg.com/config-chain/-/config-chain-1.1.12.tgz#0fde8d091200eb5e808caf25fe618c02f48e4efa" @@ -5579,13 +5559,6 @@ dot-prop@^4.1.0, dot-prop@^4.1.1, dot-prop@^4.2.0: dependencies: is-obj "^1.0.0" -dot-prop@^5.0.0: - version "5.0.0" - resolved "https://registry.yarnpkg.com/dot-prop/-/dot-prop-5.0.0.tgz#64b7968af349c3a9f966aa12658dbd5829f6b953" - integrity sha512-RTmaF2jx3nOBO2GvtFqjnDLycjFUMqt+2pwRx7JVYa81lDauoj9aNkyrJI2ikR58FbBIchiIlRiGG+muLJ4oHQ== - dependencies: - is-obj "^1.0.0" - dotenv-expand@4.2.0, dotenv-expand@^4.2.0: version "4.2.0" resolved "https://registry.yarnpkg.com/dotenv-expand/-/dotenv-expand-4.2.0.tgz#def1f1ca5d6059d24a766e587942c21106ce1275" @@ -5716,14 +5689,6 @@ electron-publish@20.40.0: lazy-val "^1.0.4" mime "^2.4.1" -electron-store@3.2.0: - version "3.2.0" - resolved "https://registry.yarnpkg.com/electron-store/-/electron-store-3.2.0.tgz#50d2d6677beb46293c15814f2d230b65c6ca555e" - integrity sha512-+goKW06sPo8KyPd9ctozRQGjctJ+M4qDpZ0Dx02X08AMf6lSlHQRmYHMYbKXpVpoq4y250wRfEHNxtP72HbfVQ== - dependencies: - conf "^4.0.1" - type-fest "^0.3.1" - electron-to-chromium@^1.3.113: version "1.3.113" resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.113.tgz#b1ccf619df7295aea17bc6951dc689632629e4a9" @@ -5834,11 +5799,6 @@ env-paths@^1.0.0: resolved "https://registry.yarnpkg.com/env-paths/-/env-paths-1.0.0.tgz#4168133b42bb05c38a35b1ae4397c8298ab369e0" integrity sha1-QWgTO0K7BcOKNbGuQ5fIKYqzaeA= -env-paths@^2.2.0: - version "2.2.0" - resolved "https://registry.yarnpkg.com/env-paths/-/env-paths-2.2.0.tgz#cdca557dc009152917d6166e2febe1f039685e43" - integrity sha512-6u0VYSCo/OW6IoD5WCLLy9JUGARbamfSavcNXry/eu8aHVFei6CD3Sw+VGX5alea1i9pgPHW0mbu6Xj0uBh7gA== - env-variable@0.0.x: version "0.0.5" resolved "https://registry.yarnpkg.com/env-variable/-/env-variable-0.0.5.tgz#913dd830bef11e96a039c038d4130604eba37f88" @@ -8924,11 +8884,6 @@ json-schema-traverse@^0.4.1: resolved "https://registry.yarnpkg.com/json-schema-traverse/-/json-schema-traverse-0.4.1.tgz#69f6a87d9513ab8bb8fe63bdb0979c448e684660" integrity sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg== -json-schema-typed@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/json-schema-typed/-/json-schema-typed-7.0.0.tgz#714f3bb539637644b8cb9c99a097c4ee8f8e8c8f" - integrity sha512-ikVqF4dlAgRvAb3MDAgDQRtB/GIC8+iq+z5bczPh9bUT7bAZCdGfGCypJHBquzZNoxebql1UgPxWbImnvkSuJg== - json-schema@0.2.3: version "0.2.3" resolved "https://registry.yarnpkg.com/json-schema/-/json-schema-0.2.3.tgz#b480c892e59a2f05954ce727bd3f2a4e882f9e13" @@ -9514,13 +9469,6 @@ make-dir@^1.0.0, make-dir@^1.3.0: dependencies: pify "^3.0.0" -make-dir@^3.0.0: - version "3.0.0" - resolved "https://registry.yarnpkg.com/make-dir/-/make-dir-3.0.0.tgz#1b5f39f6b9270ed33f9f054c5c0f84304989f801" - integrity sha512-grNJDhb8b1Jm1qeqW5R/O63wUo4UXo2v2HMic6YT9i/HBlF93S8jkMgH7yugvY9ABDShH4VZMn8I+U8+fCNegw== - dependencies: - semver "^6.0.0" - make-error@1.x: version "1.3.5" resolved "https://registry.yarnpkg.com/make-error/-/make-error-1.3.5.tgz#efe4e81f6db28cadd605c70f29c831b58ef776c8" @@ -11074,13 +11022,6 @@ pkg-up@2.0.0: dependencies: find-up "^2.1.0" -pkg-up@^3.0.1: - version "3.1.0" - resolved "https://registry.yarnpkg.com/pkg-up/-/pkg-up-3.1.0.tgz#100ec235cc150e4fd42519412596a28512a0def5" - integrity sha512-nDywThFk1i4BQK4twPQ6TA4RT8bDY96yeuCVBWL3ePARCiEKDRSrNGbFIgUJpLp+XeIR65v8ra7WuJOFUBtkMA== - dependencies: - find-up "^3.0.0" - please-upgrade-node@^3.0.2, please-upgrade-node@^3.1.1: version "3.1.1" resolved "https://registry.yarnpkg.com/please-upgrade-node/-/please-upgrade-node-3.1.1.tgz#ed320051dfcc5024fae696712c8288993595e8ac" @@ -14460,11 +14401,6 @@ type-check@~0.3.2: dependencies: prelude-ls "~1.1.2" -type-fest@^0.3.1: - version "0.3.1" - resolved "https://registry.yarnpkg.com/type-fest/-/type-fest-0.3.1.tgz#63d00d204e059474fe5e1b7c011112bbd1dc29e1" - integrity sha512-cUGJnCdr4STbePCgqNFbpVNCepa+kAVohJs1sLhxzdH+gnEoOd8VhbYa7pD3zZYGiURWM2xzEII3fQcRizDkYQ== - type-is@~1.6.16: version "1.6.16" resolved "https://registry.yarnpkg.com/type-is/-/type-is-1.6.16.tgz#f89ce341541c672b25ee7ae3c73dee3b2be50194" @@ -15413,7 +15349,7 @@ write-file-atomic@2.4.1: imurmurhash "^0.1.4" signal-exit "^3.0.2" -write-file-atomic@^2.0.0, write-file-atomic@^2.3.0, write-file-atomic@^2.4.2: +write-file-atomic@^2.0.0, write-file-atomic@^2.3.0: version "2.4.2" resolved "https://registry.yarnpkg.com/write-file-atomic/-/write-file-atomic-2.4.2.tgz#a7181706dfba17855d221140a9c06e15fcdd87b9" integrity sha512-s0b6vB3xIVRLWywa6X9TOMA7k9zio0TMOsl9ZnDkliA/cfJlpHXAscj0gbHVJiTdIuAYpIyqS5GW91fqm6gG5g== From e9ab0bb26e507978d5c7aea71cdb3bc8f85e571f Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 14:17:42 +0900 Subject: [PATCH 4/8] refactor: Rename WalletData to Wallet --- .../neuron-wallet/src/controllers/wallets.ts | 14 +++++----- .../neuron-wallet/src/services/wallets.ts | 12 ++++----- .../neuron-wallet/src/store/walletStore.ts | 26 +++++++++---------- .../neuron-wallet/tests/walletstore.test.ts | 8 +++--- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/neuron-wallet/src/controllers/wallets.ts b/packages/neuron-wallet/src/controllers/wallets.ts index b5730fdc3f..d9859d8136 100644 --- a/packages/neuron-wallet/src/controllers/wallets.ts +++ b/packages/neuron-wallet/src/controllers/wallets.ts @@ -1,5 +1,5 @@ import WalletsService from '../services/wallets' -import { WalletData } from '../store/walletStore' +import { Wallet } from '../store/walletStore' import { ChannelResponse, ResponseCode } from '.' import windowManage from '../utils/windowManage' import { Channel } from '../utils/const' @@ -21,7 +21,7 @@ export enum WalletsMethod { class WalletsController { static service = new WalletsService() - public static getAll = (): ChannelResponse => { + public static getAll = (): ChannelResponse => { const wallets = WalletsController.service.getAll() if (wallets) { return { @@ -35,7 +35,7 @@ class WalletsController { } } - public static get = (id: string): ChannelResponse => { + public static get = (id: string): ChannelResponse => { const wallet = WalletsController.service.get(id) if (wallet) { return { @@ -75,7 +75,7 @@ class WalletsController { mnemonic: string receivingAddressNumber: number changeAddressNumber: number - }): Promise> => { + }): Promise> => { try { const key = await Key.fromMnemonic(mnemonic, password, receivingAddressNumber, changeAddressNumber) const wallet = WalletsController.service.create({ @@ -108,7 +108,7 @@ class WalletsController { mnemonic: string receivingAddressNumber: number changeAddressNumber: number - }): Promise> => { + }): Promise> => { const res = await WalletsController.importMnemonic({ name, password, @@ -131,7 +131,7 @@ class WalletsController { keystore: string receivingAddressNumber: number changeAddressNumber: number - }): ChannelResponse => { + }): ChannelResponse => { try { const key = Key.fromKeystore(keystore, password, receivingAddressNumber, changeAddressNumber) const wallet = WalletsController.service.create({ @@ -162,7 +162,7 @@ class WalletsController { password: string name: string newPassword?: string - }): ChannelResponse => { + }): ChannelResponse => { try { const wallet = WalletsController.service.get(id) if (wallet) { diff --git a/packages/neuron-wallet/src/services/wallets.ts b/packages/neuron-wallet/src/services/wallets.ts index 2f4110ca46..efd3bd0f3f 100644 --- a/packages/neuron-wallet/src/services/wallets.ts +++ b/packages/neuron-wallet/src/services/wallets.ts @@ -1,16 +1,16 @@ import { v4 } from 'uuid' -import WalletStore, { WalletData } from '../store/walletStore' +import WalletStore, { Wallet } from '../store/walletStore' import Key, { Addresses } from '../keys/key' import { Keystore } from '../keys/keystore' const walletStore = new WalletStore() export default class WalletService { - public getAll = (): WalletData[] => { + public getAll = (): Wallet[] => { return walletStore.getAllWallets() } - public get = (id: string): WalletData | undefined => { + public get = (id: string): Wallet | undefined => { return this.getAll().find(wallet => wallet.id === id) } @@ -22,7 +22,7 @@ export default class WalletService { name: string keystore: Keystore addresses: Addresses - }): WalletData => { + }): Wallet => { const id = v4() walletStore.saveWallet({ id, name, keystore, addresses }) return { id, name, keystore, addresses } @@ -34,7 +34,7 @@ export default class WalletService { return key.checkPassword(password) } - public update = (walletId: string, newWallet: WalletData) => { + public update = (walletId: string, newWallet: Wallet) => { const currentWallet = walletStore.getWallet(walletId) walletStore.updateWallet(walletId, { ...currentWallet, ...newWallet }) } @@ -52,7 +52,7 @@ export default class WalletService { return walletStore.setCurrentWallet(id) } - public getActive = (): WalletData => { + public getActive = (): Wallet => { return walletStore.getCurrentWallet() } } diff --git a/packages/neuron-wallet/src/store/walletStore.ts b/packages/neuron-wallet/src/store/walletStore.ts index 1101fbf6ec..5d3b6e1a9c 100644 --- a/packages/neuron-wallet/src/store/walletStore.ts +++ b/packages/neuron-wallet/src/store/walletStore.ts @@ -10,7 +10,7 @@ export enum WalletStoreError { NoActiveWallet, } -export interface WalletData { +export interface Wallet { id: string name: string keystore: Keystore @@ -36,31 +36,31 @@ export default class WalletStore { return new Store(this.storePath, `${id}.json`) } - getAllWallets = (): WalletData[] => { + getAllWallets = (): Wallet[] => { return this.listStore.readSync(this.walletsKey) || [] } - getWallet = (id: string): WalletData => { + getWallet = (id: string): Wallet => { const wallets = this.getAllWallets() - const wallet = wallets.find((w: WalletData) => w.id === id) + const wallet = wallets.find((w: Wallet) => w.id === id) if (!wallet) { throw WalletStoreError.NoWallet } return wallet as any } - saveWallet = (walletData: WalletData) => { - this.listStore.writeSync(this.walletsKey, this.getAllWallets().concat(walletData)) + saveWallet = (wallet: Wallet) => { + this.listStore.writeSync(this.walletsKey, this.getAllWallets().concat(wallet)) // TODO: Save keystore to that store instead. - this.getWalletStore(walletData.id).writeSync(walletData.id, walletData) + this.getWalletStore(wallet.id).writeSync(wallet.id, wallet) if (this.getAllWallets().length === 1) { - this.setCurrentWallet(walletData.id) + this.setCurrentWallet(wallet.id) } } - updateWallet = (id: string, newWallet: WalletData) => { + updateWallet = (id: string, newWallet: Wallet) => { const wallets = this.getAllWallets() - const index = wallets.findIndex((w: WalletData) => w.id === id) + const index = wallets.findIndex((w: Wallet) => w.id === id) if (index !== -1) { wallets[index] = newWallet this.listStore.writeSync(this.walletsKey, wallets) @@ -74,7 +74,7 @@ export default class WalletStore { deleteWallet = (id: string) => { const currentId = this.getCurrentWallet().id const wallets = this.getAllWallets() - const index = wallets.findIndex((w: WalletData) => w.id === id) + const index = wallets.findIndex((w: Wallet) => w.id === id) if (index !== -1) { wallets.splice(index, 1) this.listStore.writeSync(this.walletsKey, wallets) @@ -89,7 +89,7 @@ export default class WalletStore { } setCurrentWallet = (walletId: string): boolean => { - const index = this.getAllWallets().findIndex((w: WalletData) => w.id === walletId) + const index = this.getAllWallets().findIndex((w: Wallet) => w.id === walletId) if (index === -1) { return false } @@ -97,7 +97,7 @@ export default class WalletStore { return true } - getCurrentWallet = (): WalletData => { + getCurrentWallet = (): Wallet => { const walletId = this.listStore.readSync(this.currentWalletKey) as string if (walletId) { return this.getWallet(walletId) diff --git a/packages/neuron-wallet/tests/walletstore.test.ts b/packages/neuron-wallet/tests/walletstore.test.ts index e19a06917c..592c34604e 100644 --- a/packages/neuron-wallet/tests/walletstore.test.ts +++ b/packages/neuron-wallet/tests/walletstore.test.ts @@ -1,10 +1,10 @@ import assert from 'assert' -import WalletStore, { WalletData } from '../src/store/walletStore' +import WalletStore, { Wallet } from '../src/store/walletStore' describe('wallet store', () => { let walletStore: WalletStore - const wallet1: WalletData = { + const wallet1: Wallet = { id: '1', name: 'wallet1', keystore: { @@ -57,7 +57,7 @@ describe('wallet store', () => { }, } - const wallet2: WalletData = { + const wallet2: Wallet = { id: '2', name: 'wallet2', keystore: { @@ -109,7 +109,7 @@ describe('wallet store', () => { ], }, } - const wallet3: WalletData = { + const wallet3: Wallet = { id: '3', name: 'wallet3', keystore: { From b0396ad40e0a9c3f83fb623b69e6f6bcdad368f2 Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 14:39:11 +0900 Subject: [PATCH 5/8] refactor: Alias uuid.v4() as uuid() --- packages/neuron-wallet/src/keys/key.ts | 4 ++-- packages/neuron-wallet/src/mock.ts | 4 ++-- packages/neuron-wallet/src/services/networks.ts | 4 ++-- packages/neuron-wallet/src/services/wallets.ts | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/neuron-wallet/src/keys/key.ts b/packages/neuron-wallet/src/keys/key.ts index 70ccf08eba..020f7f3108 100644 --- a/packages/neuron-wallet/src/keys/key.ts +++ b/packages/neuron-wallet/src/keys/key.ts @@ -3,7 +3,7 @@ import * as bip39 from 'bip39' import crypto from 'crypto-browserify' import scryptsy from 'scrypt.js' import SHA3 from 'sha3' -import { v4 } from 'uuid' +import { v4 as uuid } from 'uuid' import Address, { HDAddress } from '../services/addresses' import { Keystore, KdfParams, KeysData } from './keystore' @@ -193,7 +193,7 @@ export default class Key { .replace('0x', '') return { version: 0, - id: v4(), + id: uuid(), crypto: { ciphertext: ciphertext.toString('hex'), cipherparams: { diff --git a/packages/neuron-wallet/src/mock.ts b/packages/neuron-wallet/src/mock.ts index 0e07282357..6a74224ff0 100644 --- a/packages/neuron-wallet/src/mock.ts +++ b/packages/neuron-wallet/src/mock.ts @@ -1,4 +1,4 @@ -import { v4 } from 'uuid' +import { v4 as uuid } from 'uuid' export const transactions = Array.from({ length: 200, @@ -27,7 +27,7 @@ export interface Wallet { const generateWallet = () => { const walletName = `wallet${parseInt((Math.random() * 1000).toString(), 10)}` - const walletID = v4() + const walletID = uuid() return { name: walletName, id: walletID, diff --git a/packages/neuron-wallet/src/services/networks.ts b/packages/neuron-wallet/src/services/networks.ts index b62d4c0e8d..85b924a7cd 100644 --- a/packages/neuron-wallet/src/services/networks.ts +++ b/packages/neuron-wallet/src/services/networks.ts @@ -1,4 +1,4 @@ -import { v4 } from 'uuid' +import { v4 as uuid } from 'uuid' import Store from '../utils/store' import env from '../env' @@ -78,7 +78,7 @@ export default class NetworksService extends Store { throw new Error('Network name exists') } const newOne = { - id: v4(), + id: uuid(), name, remote, type, diff --git a/packages/neuron-wallet/src/services/wallets.ts b/packages/neuron-wallet/src/services/wallets.ts index efd3bd0f3f..19a1797376 100644 --- a/packages/neuron-wallet/src/services/wallets.ts +++ b/packages/neuron-wallet/src/services/wallets.ts @@ -1,4 +1,4 @@ -import { v4 } from 'uuid' +import { v4 as uuid } from 'uuid' import WalletStore, { Wallet } from '../store/walletStore' import Key, { Addresses } from '../keys/key' import { Keystore } from '../keys/keystore' @@ -23,7 +23,7 @@ export default class WalletService { keystore: Keystore addresses: Addresses }): Wallet => { - const id = v4() + const id = uuid() walletStore.saveWallet({ id, name, keystore, addresses }) return { id, name, keystore, addresses } } From 9413146b88f432743c72c31c5c6f0dfc0afb642a Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 16:15:08 +0900 Subject: [PATCH 6/8] refactor: Combine WalletStore into WalletService Two similar objects are unnecessary. --- .../neuron-wallet/src/controllers/wallets.ts | 23 ++-- .../neuron-wallet/src/services/addresses.ts | 5 +- .../neuron-wallet/src/services/wallets.ts | 106 ++++++++++++--- .../neuron-wallet/src/store/walletStore.ts | 114 ---------------- .../tests/{ => services}/address.test.ts | 4 +- .../wallets.test.ts} | 123 ++++++++---------- 6 files changed, 157 insertions(+), 218 deletions(-) delete mode 100644 packages/neuron-wallet/src/store/walletStore.ts rename packages/neuron-wallet/tests/{ => services}/address.test.ts (94%) rename packages/neuron-wallet/tests/{walletstore.test.ts => services/wallets.test.ts} (59%) diff --git a/packages/neuron-wallet/src/controllers/wallets.ts b/packages/neuron-wallet/src/controllers/wallets.ts index d9859d8136..8559f58155 100644 --- a/packages/neuron-wallet/src/controllers/wallets.ts +++ b/packages/neuron-wallet/src/controllers/wallets.ts @@ -1,5 +1,4 @@ -import WalletsService from '../services/wallets' -import { Wallet } from '../store/walletStore' +import WalletsService, { Wallet } from '../services/wallets' import { ChannelResponse, ResponseCode } from '.' import windowManage from '../utils/windowManage' import { Channel } from '../utils/const' @@ -203,7 +202,7 @@ class WalletsController { status: ResponseCode.Success, result: { allWallets: WalletsController.service.getAll(), - activeWallet: WalletsController.service.getActive(), + activeWallet: WalletsController.service.getCurrent(), }, } } @@ -234,8 +233,8 @@ class WalletsController { } public static getActive = () => { - try { - const activeWallet = WalletsController.service.getActive() + const activeWallet = WalletsController.service.getCurrent() + if (activeWallet) { return { status: ResponseCode.Success, result: { @@ -246,21 +245,21 @@ class WalletsController { }, }, } - } catch (e) { - return { - status: ResponseCode.Fail, - msg: 'No active wallet', - } + } + + return { + status: ResponseCode.Fail, + msg: 'No active wallet', } } public static activate = (id: string) => { - const success = WalletsController.service.setActive(id) + const success = WalletsController.service.setCurrent(id) if (success) { windowManage.broadcast(Channel.Wallets, WalletsMethod.GetActive, WalletsController.getActive()) return { status: ResponseCode.Success, - result: WalletsController.service.getActive(), + result: WalletsController.service.getCurrent(), } } return { diff --git a/packages/neuron-wallet/src/services/addresses.ts b/packages/neuron-wallet/src/services/addresses.ts index 5c1ee44bab..253a417102 100644 --- a/packages/neuron-wallet/src/services/addresses.ts +++ b/packages/neuron-wallet/src/services/addresses.ts @@ -1,9 +1,8 @@ import TransactionsService from './transactions' +import WalletService from './wallets' import ckbCore from '../core' import HD from '../keys/hd' import { KeysData } from '../keys/keystore' -// TODO: Should use service -import WalletStore from '../store/walletStore' const { utils: { AddressPrefix, AddressType: Type, AddressBinIdx, pubkeyToAddress }, @@ -67,7 +66,7 @@ class Address { } public static allAddresses = () => - new WalletStore().getAllWallets().reduce((total: HDAddress[], cur) => { + new WalletService().getAll().reduce((total: HDAddress[], cur) => { return [...total, ...cur.addresses.change, ...cur.addresses.receiving] }, []) diff --git a/packages/neuron-wallet/src/services/wallets.ts b/packages/neuron-wallet/src/services/wallets.ts index 19a1797376..1f92447218 100644 --- a/packages/neuron-wallet/src/services/wallets.ts +++ b/packages/neuron-wallet/src/services/wallets.ts @@ -1,13 +1,39 @@ +import fs from 'fs' import { v4 as uuid } from 'uuid' -import WalletStore, { Wallet } from '../store/walletStore' import Key, { Addresses } from '../keys/key' import { Keystore } from '../keys/keystore' +import app from '../app' +import env from '../env' +import Store from '../utils/store' -const walletStore = new WalletStore() +export interface Wallet { + id: string + name: string + keystore: Keystore + addresses: Addresses +} + +// TODO: Check if '/dev/wallets' path works on Windows +const defaultStorePath = env.isDevMode ? '/dev/wallets' : '/wallets' export default class WalletService { + private storePath: string + private listStore: Store // Save wallets (meta info except keystore, which is persisted separately) + private walletsKey = 'wallets' + private currentWalletKey = 'current' + + constructor(storePath: string = defaultStorePath) { + this.storePath = `${app.getPath('userData')}/${storePath}` + fs.mkdirSync(this.storePath, { recursive: true }) + this.listStore = new Store(this.storePath, 'wallets.json') + } + + private getWalletStore = (id: string): Store => { + return new Store(this.storePath, `${id}.json`) + } + public getAll = (): Wallet[] => { - return walletStore.getAllWallets() + return this.listStore.readSync(this.walletsKey) || [] } public get = (id: string): Wallet | undefined => { @@ -23,36 +49,78 @@ export default class WalletService { keystore: Keystore addresses: Addresses }): Wallet => { - const id = uuid() - walletStore.saveWallet({ id, name, keystore, addresses }) - return { id, name, keystore, addresses } + const wallet = { id: uuid(), name, keystore, addresses } + this.listStore.writeSync(this.walletsKey, this.getAll().concat(wallet)) + // TODO: Save keystore to that store instead. + this.getWalletStore(wallet.id).writeSync(wallet.id, wallet) + if (this.getAll().length === 1) { + this.setCurrent(wallet.id) + } + return wallet } - public validate = ({ id, password }: { id: string; password: string }) => { - const wallet = walletStore.getWallet(id) - const key = new Key({ keystore: wallet.keystore }) - return key.checkPassword(password) + public update = (id: string, newWallet: Wallet) => { + const wallets = this.getAll() + const index = wallets.findIndex((w: Wallet) => w.id === id) + if (index !== -1) { + wallets[index] = { ...newWallet, id } + this.listStore.writeSync(this.walletsKey, wallets) + } } - public update = (walletId: string, newWallet: Wallet) => { - const currentWallet = walletStore.getWallet(walletId) - walletStore.updateWallet(walletId, { ...currentWallet, ...newWallet }) + public delete = (id: string): boolean => { + const current = this.getCurrent() + const currentId = current ? current.id : '' + const wallets = this.getAll() + const index = wallets.findIndex((w: Wallet) => w.id === id) + if (index === -1) { + return false + } + + wallets.splice(index, 1) + this.listStore.writeSync(this.walletsKey, wallets) + this.getWalletStore(id).clear() + + const newWallets = this.getAll() + if (currentId === id && newWallets.length > 0) { + this.setCurrent(newWallets[0].id) + } + + return true } - public delete = (id: string): boolean => { + public setCurrent = (id: string): boolean => { const wallet = this.get(id) if (wallet) { - walletStore.deleteWallet(id) + this.listStore.writeSync(this.currentWalletKey, id) return true } return false } - public setActive = (id: string): boolean => { - return walletStore.setCurrentWallet(id) + public getCurrent = (): Wallet | undefined => { + const walletId = this.listStore.readSync(this.currentWalletKey) as string + if (walletId) { + return this.get(walletId) + } + return undefined + } + + public validate = ({ id, password }: { id: string; password: string }) => { + const wallet = this.get(id) + if (wallet) { + const key = new Key({ keystore: wallet.keystore }) + return key.checkPassword(password) + } + + // TODO: Throw wallet not found instead. + return false } - public getActive = (): Wallet => { - return walletStore.getCurrentWallet() + public clearAll = () => { + this.getAll().forEach(w => { + this.getWalletStore(w.id).clear() + }) + this.listStore.clear() } } diff --git a/packages/neuron-wallet/src/store/walletStore.ts b/packages/neuron-wallet/src/store/walletStore.ts deleted file mode 100644 index 5d3b6e1a9c..0000000000 --- a/packages/neuron-wallet/src/store/walletStore.ts +++ /dev/null @@ -1,114 +0,0 @@ -import fs from 'fs' -import { Keystore } from '../keys/keystore' -import env from '../env' -import { Addresses } from '../keys/key' -import app from '../app' -import Store from '../utils/store' - -export enum WalletStoreError { - NoWallet, - NoActiveWallet, -} - -export interface Wallet { - id: string - name: string - keystore: Keystore - addresses: Addresses -} - -// TODO: Check if '/dev/wallets' path works on Windows -const defaultStorePath = env.isDevMode ? '/dev/wallets' : '/wallets' - -export default class WalletStore { - private storePath: string - private listStore: Store // Save wallets (meta info except keystore, which is persisted separately) - private walletsKey = 'wallets' - private currentWalletKey = 'current' - - constructor(storePath: string = defaultStorePath) { - this.storePath = `${app.getPath('userData')}/${storePath}` - fs.mkdirSync(this.storePath, { recursive: true }) - this.listStore = new Store(this.storePath, 'wallets.json') - } - - private getWalletStore = (id: string): Store => { - return new Store(this.storePath, `${id}.json`) - } - - getAllWallets = (): Wallet[] => { - return this.listStore.readSync(this.walletsKey) || [] - } - - getWallet = (id: string): Wallet => { - const wallets = this.getAllWallets() - const wallet = wallets.find((w: Wallet) => w.id === id) - if (!wallet) { - throw WalletStoreError.NoWallet - } - return wallet as any - } - - saveWallet = (wallet: Wallet) => { - this.listStore.writeSync(this.walletsKey, this.getAllWallets().concat(wallet)) - // TODO: Save keystore to that store instead. - this.getWalletStore(wallet.id).writeSync(wallet.id, wallet) - if (this.getAllWallets().length === 1) { - this.setCurrentWallet(wallet.id) - } - } - - updateWallet = (id: string, newWallet: Wallet) => { - const wallets = this.getAllWallets() - const index = wallets.findIndex((w: Wallet) => w.id === id) - if (index !== -1) { - wallets[index] = newWallet - this.listStore.writeSync(this.walletsKey, wallets) - } else { - throw WalletStoreError.NoWallet - } - // TODO: Save keystore to that store instead. - this.getWalletStore(id).writeSync(id, newWallet) - } - - deleteWallet = (id: string) => { - const currentId = this.getCurrentWallet().id - const wallets = this.getAllWallets() - const index = wallets.findIndex((w: Wallet) => w.id === id) - if (index !== -1) { - wallets.splice(index, 1) - this.listStore.writeSync(this.walletsKey, wallets) - } else { - throw WalletStoreError.NoWallet - } - this.getWalletStore(id).clear() - - if (currentId === id) { - this.setCurrentWallet(this.getAllWallets()[0].id) - } - } - - setCurrentWallet = (walletId: string): boolean => { - const index = this.getAllWallets().findIndex((w: Wallet) => w.id === walletId) - if (index === -1) { - return false - } - this.listStore.writeSync(this.currentWalletKey, walletId) - return true - } - - getCurrentWallet = (): Wallet => { - const walletId = this.listStore.readSync(this.currentWalletKey) as string - if (walletId) { - return this.getWallet(walletId) - } - throw WalletStoreError.NoActiveWallet - } - - clearAll = () => { - this.getAllWallets().forEach(w => { - this.getWalletStore(w.id).clear() - }) - this.listStore.clear() - } -} diff --git a/packages/neuron-wallet/tests/address.test.ts b/packages/neuron-wallet/tests/services/address.test.ts similarity index 94% rename from packages/neuron-wallet/tests/address.test.ts rename to packages/neuron-wallet/tests/services/address.test.ts index 2066af210a..3e465a83ac 100644 --- a/packages/neuron-wallet/tests/address.test.ts +++ b/packages/neuron-wallet/tests/services/address.test.ts @@ -1,5 +1,5 @@ -import Addresses from '../src/services/addresses' -import ckbCore from '../src/core' +import Addresses from '../../src/services/addresses' +import ckbCore from '../../src/core' describe('Key tests', () => { const { utils } = ckbCore diff --git a/packages/neuron-wallet/tests/walletstore.test.ts b/packages/neuron-wallet/tests/services/wallets.test.ts similarity index 59% rename from packages/neuron-wallet/tests/walletstore.test.ts rename to packages/neuron-wallet/tests/services/wallets.test.ts index 592c34604e..480e9fa97d 100644 --- a/packages/neuron-wallet/tests/walletstore.test.ts +++ b/packages/neuron-wallet/tests/services/wallets.test.ts @@ -1,11 +1,11 @@ import assert from 'assert' -import WalletStore, { Wallet } from '../src/store/walletStore' +import WalletService, { Wallet } from '../../src/services/wallets' -describe('wallet store', () => { - let walletStore: WalletStore +describe('wallet service', () => { + let walletService: WalletService const wallet1: Wallet = { - id: '1', + id: 'na', name: 'wallet1', keystore: { version: 0, @@ -58,7 +58,7 @@ describe('wallet store', () => { } const wallet2: Wallet = { - id: '2', + id: 'na', name: 'wallet2', keystore: { version: 0, @@ -110,7 +110,7 @@ describe('wallet store', () => { }, } const wallet3: Wallet = { - id: '3', + id: 'na', name: 'wallet3', keystore: { version: 0, @@ -163,52 +163,42 @@ describe('wallet store', () => { } beforeEach(() => { - walletStore = new WalletStore('test/wallets') + walletService = new WalletService('test/wallets') }) afterEach(() => { - walletStore.clearAll() + walletService.clearAll() }) it('save wallet', () => { - walletStore.saveWallet(wallet1) - const wallet = walletStore.getWallet(wallet1.id) - assert.deepStrictEqual(wallet, wallet1) + const { id } = walletService.create(wallet1) + const wallet = walletService.get(id) + assert.deepStrictEqual(wallet, { ...wallet1, id }) }) - it('get not exist wallet', () => { - walletStore.saveWallet(wallet1) - try { - walletStore.getWallet('1111111111') - } catch (e) { - assert.deepStrictEqual(e, 0) - } + it('wallet not exist', () => { + const wallet = walletService.get('1111111111') + expect(wallet).toBeUndefined() }) it('get all wallets', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) - walletStore.saveWallet(wallet3) - const wallets = walletStore.getAllWallets() - assert.deepStrictEqual(wallets, [wallet1, wallet2, wallet3]) + const w1 = walletService.create(wallet1) + const w2 = walletService.create(wallet2) + const w3 = walletService.create(wallet3) + assert.deepStrictEqual(walletService.getAll(), [w1, w2, w3]) }) it('rename wallet', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) + const w1 = walletService.create(wallet1) wallet1.name = wallet2.name - walletStore.updateWallet(wallet1.id, wallet1) - const wallet = walletStore.getWallet(wallet1.id) - assert.deepStrictEqual(wallet, { - id: wallet1.id, - name: wallet2.name, - keystore: wallet1.keystore, - addresses: wallet1.addresses, - }) + walletService.update(w1.id, wallet1) + const wallet = walletService.get(w1.id) + expect(wallet).toBeDefined() + expect(wallet!.name).toEqual(wallet2.name) }) it('update addresses', () => { - walletStore.saveWallet(wallet1) + const w1 = walletService.create(wallet1) const addresses = { receiving: [ { @@ -240,10 +230,10 @@ describe('wallet store', () => { ], } wallet1.addresses = addresses - walletStore.updateWallet(wallet1.id, wallet1) - const wallet = walletStore.getWallet(wallet1.id) + walletService.update(w1.id, wallet1) + const wallet = walletService.get(w1.id) assert.deepStrictEqual(wallet, { - id: wallet1.id, + id: w1.id, name: wallet1.name, keystore: wallet1.keystore, addresses, @@ -251,46 +241,43 @@ describe('wallet store', () => { }) it('delete wallet', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) - walletStore.deleteWallet(wallet1.id) - try { - assert.notDeepStrictEqual(walletStore.getWallet(wallet1.id), wallet1) - } catch (e) { - assert.strictEqual(e, 0) - } + const w1 = walletService.create(wallet1) + walletService.create(wallet2) + walletService.delete(w1.id) + const wallet = walletService.get(w1.id) + expect(wallet).toBeUndefined() }) it('get and set active wallet', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) - assert.strictEqual(walletStore.setCurrentWallet(wallet1.id), true) - assert.deepStrictEqual(walletStore.getCurrentWallet(), wallet1) - assert.strictEqual(walletStore.setCurrentWallet(wallet2.id), true) - assert.deepStrictEqual(walletStore.getCurrentWallet(), wallet2) - assert.strictEqual(walletStore.setCurrentWallet(wallet1.id), true) + const w1 = walletService.create(wallet1) + const w2 = walletService.create(wallet2) + assert.strictEqual(walletService.setCurrent(w1.id), true) + assert.deepStrictEqual(walletService.getCurrent(), w1) + assert.strictEqual(walletService.setCurrent(w2.id), true) + assert.deepStrictEqual(walletService.getCurrent(), w2) + assert.strictEqual(walletService.setCurrent(w1.id), true) }) it('first wallet is active wallet', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) - assert.deepStrictEqual(walletStore.getCurrentWallet(), wallet1) + const w1 = walletService.create(wallet1) + walletService.create(wallet2) + assert.deepStrictEqual(walletService.getCurrent(), w1) }) - it('delete active wallet', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) - walletStore.deleteWallet(wallet1.id) - const activeWallet = walletStore.getCurrentWallet() - assert.deepStrictEqual(activeWallet, wallet2) - assert.strictEqual(walletStore.getAllWallets().length, 1) + it('delete current wallet', () => { + const w1 = walletService.create(wallet1) + const w2 = walletService.create(wallet2) + walletService.delete(w1.id) + const activeWallet = walletService.getCurrent() + assert.deepStrictEqual(activeWallet, w2) + assert.strictEqual(walletService.getAll().length, 1) }) - it('delete inactive wallet', () => { - walletStore.saveWallet(wallet1) - walletStore.saveWallet(wallet2) - walletStore.deleteWallet(wallet2.id) - const activeWallet = walletStore.getCurrentWallet() - assert.deepStrictEqual(activeWallet, wallet1) + it('delete none current wallet', () => { + const w1 = walletService.create(wallet1) + const w2 = walletService.create(wallet2) + walletService.delete(w2.id) + const activeWallet = walletService.getCurrent() + assert.deepStrictEqual(activeWallet, w1) }) }) From 9d06bd732462c56ba90d3e1b0b5993ed3b3bf0fb Mon Sep 17 00:00:00 2001 From: James Chen Date: Sat, 11 May 2019 16:53:22 +0900 Subject: [PATCH 7/8] refactor: Limit the data fields that can be modified on a wallet by introducing WalletProperties interface --- .../neuron-wallet/src/services/wallets.ts | 25 ++++++++++--------- .../tests/services/wallets.test.ts | 11 +++----- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/packages/neuron-wallet/src/services/wallets.ts b/packages/neuron-wallet/src/services/wallets.ts index 1f92447218..9aa7ec512a 100644 --- a/packages/neuron-wallet/src/services/wallets.ts +++ b/packages/neuron-wallet/src/services/wallets.ts @@ -11,6 +11,15 @@ export interface Wallet { name: string keystore: Keystore addresses: Addresses + + // TODO: add explictly keystore loading func + // loadKeystore: () => Keystore +} + +export interface WalletProperties { + name: string + keystore: Keystore + addresses: Addresses } // TODO: Check if '/dev/wallets' path works on Windows @@ -40,16 +49,8 @@ export default class WalletService { return this.getAll().find(wallet => wallet.id === id) } - public create = ({ - name, - keystore, - addresses, - }: { - name: string - keystore: Keystore - addresses: Addresses - }): Wallet => { - const wallet = { id: uuid(), name, keystore, addresses } + public create = (prop: WalletProperties): Wallet => { + const wallet = { ...prop, id: uuid() } this.listStore.writeSync(this.walletsKey, this.getAll().concat(wallet)) // TODO: Save keystore to that store instead. this.getWalletStore(wallet.id).writeSync(wallet.id, wallet) @@ -59,11 +60,11 @@ export default class WalletService { return wallet } - public update = (id: string, newWallet: Wallet) => { + public update = (id: string, prop: WalletProperties) => { const wallets = this.getAll() const index = wallets.findIndex((w: Wallet) => w.id === id) if (index !== -1) { - wallets[index] = { ...newWallet, id } + wallets[index] = { ...prop, id } this.listStore.writeSync(this.walletsKey, wallets) } } diff --git a/packages/neuron-wallet/tests/services/wallets.test.ts b/packages/neuron-wallet/tests/services/wallets.test.ts index 480e9fa97d..2ae12cb0ed 100644 --- a/packages/neuron-wallet/tests/services/wallets.test.ts +++ b/packages/neuron-wallet/tests/services/wallets.test.ts @@ -1,11 +1,10 @@ import assert from 'assert' -import WalletService, { Wallet } from '../../src/services/wallets' +import WalletService from '../../src/services/wallets' describe('wallet service', () => { let walletService: WalletService - const wallet1: Wallet = { - id: 'na', + const wallet1 = { name: 'wallet1', keystore: { version: 0, @@ -57,8 +56,7 @@ describe('wallet service', () => { }, } - const wallet2: Wallet = { - id: 'na', + const wallet2 = { name: 'wallet2', keystore: { version: 0, @@ -109,8 +107,7 @@ describe('wallet service', () => { ], }, } - const wallet3: Wallet = { - id: 'na', + const wallet3 = { name: 'wallet3', keystore: { version: 0, From 2cc134846a5d002a7a35b145d73064a56d99f464 Mon Sep 17 00:00:00 2001 From: James Chen Date: Sun, 12 May 2019 00:53:53 +0900 Subject: [PATCH 8/8] refactor: Separate keystore files from wallet list store --- .../neuron-wallet/src/controllers/wallets.ts | 11 ++- .../neuron-wallet/src/services/wallets.ts | 99 +++++++++++++++---- .../tests/services/wallets.test.ts | 42 ++++---- 3 files changed, 107 insertions(+), 45 deletions(-) diff --git a/packages/neuron-wallet/src/controllers/wallets.ts b/packages/neuron-wallet/src/controllers/wallets.ts index 8559f58155..d136cb34b2 100644 --- a/packages/neuron-wallet/src/controllers/wallets.ts +++ b/packages/neuron-wallet/src/controllers/wallets.ts @@ -1,4 +1,4 @@ -import WalletsService, { Wallet } from '../services/wallets' +import WalletsService, { Wallet, WalletProperties } from '../services/wallets' import { ChannelResponse, ResponseCode } from '.' import windowManage from '../utils/windowManage' import { Channel } from '../utils/const' @@ -151,6 +151,7 @@ class WalletsController { } } + // TODO: update addresses? public static update = ({ id, name, @@ -166,12 +167,12 @@ class WalletsController { const wallet = WalletsController.service.get(id) if (wallet) { if (WalletsController.service.validate({ id, password })) { - wallet.name = name + const props: WalletProperties = { name, addresses: wallet.addresses, keystore: null } if (newPassword) { - const key = Key.fromKeystore(JSON.stringify(wallet!.keystore), password) - wallet.keystore = key.toKeystore(JSON.stringify(key.keysData!), newPassword) + const key = Key.fromKeystore(JSON.stringify(wallet!.loadKeystore()), password) + props.keystore = key.toKeystore(JSON.stringify(key.keysData!), newPassword) } - WalletsController.service.update(id, wallet) + WalletsController.service.update(id, props) windowManage.broadcast(Channel.Wallets, WalletsMethod.GetAll, WalletsController.getAll()) return { status: ResponseCode.Success, diff --git a/packages/neuron-wallet/src/services/wallets.ts b/packages/neuron-wallet/src/services/wallets.ts index 9aa7ec512a..9a124a257f 100644 --- a/packages/neuron-wallet/src/services/wallets.ts +++ b/packages/neuron-wallet/src/services/wallets.ts @@ -1,4 +1,5 @@ import fs from 'fs' +import path from 'path' import { v4 as uuid } from 'uuid' import Key, { Addresses } from '../keys/key' import { Keystore } from '../keys/keystore' @@ -9,17 +10,71 @@ import Store from '../utils/store' export interface Wallet { id: string name: string - keystore: Keystore addresses: Addresses - // TODO: add explictly keystore loading func - // loadKeystore: () => Keystore + loadKeystore: () => Keystore } export interface WalletProperties { name: string - keystore: Keystore addresses: Addresses + keystore: Keystore | null +} + +class FileKeystoreWallet implements Wallet { + id: string + name: string + addresses: Addresses + + private storePath: string + + constructor(id: string, props: WalletProperties, storePath: string) { + this.id = id + + this.name = props.name + this.addresses = props.addresses + + this.storePath = storePath + } + + static fromJSON = ( + json: { id: string; name: string; addresses: Addresses }, + storePath: string, + ): FileKeystoreWallet => { + const props = { name: json.name, addresses: json.addresses, keystore: null } + return new FileKeystoreWallet(json.id, props, storePath) + } + + update = (props: WalletProperties) => { + this.name = props.name + this.addresses = props.addresses + } + + toJSON = (): any => { + return { + id: this.id, + name: this.name, + addresses: this.addresses, + } + } + + loadKeystore = (): Keystore => { + // TODO: handle fs error + const data = fs.readFileSync(this.storeLocation(), { encoding: 'utf8' }) + return JSON.parse(data) as Keystore + } + + private storeLocation = (): string => { + return path.resolve(this.storePath, `${this.id}.json`) + } + + saveKeystore = (keystore: Keystore) => { + fs.writeFileSync(this.storeLocation(), JSON.stringify(keystore), { encoding: 'utf8' }) + } + + deleteKeystore = () => { + fs.unlinkSync(this.storeLocation()) + } } // TODO: Check if '/dev/wallets' path works on Windows @@ -37,34 +92,38 @@ export default class WalletService { this.listStore = new Store(this.storePath, 'wallets.json') } - private getWalletStore = (id: string): Store => { - return new Store(this.storePath, `${id}.json`) - } - public getAll = (): Wallet[] => { return this.listStore.readSync(this.walletsKey) || [] } public get = (id: string): Wallet | undefined => { - return this.getAll().find(wallet => wallet.id === id) + const wallet = this.getAll().find(w => w.id === id) + if (wallet) { + return FileKeystoreWallet.fromJSON(wallet, this.storePath) + } + return undefined } - public create = (prop: WalletProperties): Wallet => { - const wallet = { ...prop, id: uuid() } - this.listStore.writeSync(this.walletsKey, this.getAll().concat(wallet)) - // TODO: Save keystore to that store instead. - this.getWalletStore(wallet.id).writeSync(wallet.id, wallet) + public create = (props: WalletProperties): Wallet => { + const wallet = new FileKeystoreWallet(uuid(), props, this.storePath) + wallet.saveKeystore(props.keystore!) + this.listStore.writeSync(this.walletsKey, this.getAll().concat(wallet.toJSON())) if (this.getAll().length === 1) { this.setCurrent(wallet.id) } return wallet } - public update = (id: string, prop: WalletProperties) => { + public update = (id: string, props: WalletProperties) => { const wallets = this.getAll() const index = wallets.findIndex((w: Wallet) => w.id === id) if (index !== -1) { - wallets[index] = { ...prop, id } + const wallet = FileKeystoreWallet.fromJSON(wallets[index], this.storePath) + wallet.update(props) + if (props.keystore) { + wallet.saveKeystore(props.keystore) + } + wallets[index] = wallet.toJSON() this.listStore.writeSync(this.walletsKey, wallets) } } @@ -78,9 +137,10 @@ export default class WalletService { return false } + const wallet = FileKeystoreWallet.fromJSON(wallets[index], this.storePath) wallets.splice(index, 1) this.listStore.writeSync(this.walletsKey, wallets) - this.getWalletStore(id).clear() + wallet.deleteKeystore() const newWallets = this.getAll() if (currentId === id && newWallets.length > 0) { @@ -110,7 +170,7 @@ export default class WalletService { public validate = ({ id, password }: { id: string; password: string }) => { const wallet = this.get(id) if (wallet) { - const key = new Key({ keystore: wallet.keystore }) + const key = new Key({ keystore: wallet.loadKeystore() }) return key.checkPassword(password) } @@ -120,7 +180,8 @@ export default class WalletService { public clearAll = () => { this.getAll().forEach(w => { - this.getWalletStore(w.id).clear() + const wallet = FileKeystoreWallet.fromJSON(w, this.storePath) + wallet.deleteKeystore() }) this.listStore.clear() } diff --git a/packages/neuron-wallet/tests/services/wallets.test.ts b/packages/neuron-wallet/tests/services/wallets.test.ts index 2ae12cb0ed..8d0e8df285 100644 --- a/packages/neuron-wallet/tests/services/wallets.test.ts +++ b/packages/neuron-wallet/tests/services/wallets.test.ts @@ -1,4 +1,3 @@ -import assert from 'assert' import WalletService from '../../src/services/wallets' describe('wallet service', () => { @@ -170,7 +169,8 @@ describe('wallet service', () => { it('save wallet', () => { const { id } = walletService.create(wallet1) const wallet = walletService.get(id) - assert.deepStrictEqual(wallet, { ...wallet1, id }) + expect(wallet).toBeDefined() + expect(wallet!.name).toEqual(wallet1.name) }) it('wallet not exist', () => { @@ -179,10 +179,10 @@ describe('wallet service', () => { }) it('get all wallets', () => { - const w1 = walletService.create(wallet1) - const w2 = walletService.create(wallet2) - const w3 = walletService.create(wallet3) - assert.deepStrictEqual(walletService.getAll(), [w1, w2, w3]) + walletService.create(wallet1) + walletService.create(wallet2) + walletService.create(wallet3) + expect(walletService.getAll().length).toBe(3) }) it('rename wallet', () => { @@ -229,12 +229,8 @@ describe('wallet service', () => { wallet1.addresses = addresses walletService.update(w1.id, wallet1) const wallet = walletService.get(w1.id) - assert.deepStrictEqual(wallet, { - id: w1.id, - name: wallet1.name, - keystore: wallet1.keystore, - addresses, - }) + expect(wallet).toBeDefined() + expect(wallet!.addresses).toEqual(addresses) }) it('delete wallet', () => { @@ -248,17 +244,19 @@ describe('wallet service', () => { it('get and set active wallet', () => { const w1 = walletService.create(wallet1) const w2 = walletService.create(wallet2) - assert.strictEqual(walletService.setCurrent(w1.id), true) - assert.deepStrictEqual(walletService.getCurrent(), w1) - assert.strictEqual(walletService.setCurrent(w2.id), true) - assert.deepStrictEqual(walletService.getCurrent(), w2) - assert.strictEqual(walletService.setCurrent(w1.id), true) + expect(walletService.setCurrent(w1.id)).toBeTruthy() + expect(walletService.getCurrent()!.id).toEqual(w1.id) + expect(walletService.setCurrent(w2.id)).toBeTruthy() + expect(walletService.getCurrent()!.id).toEqual(w2.id) + expect(walletService.setCurrent(w1.id)).toBeTruthy() }) it('first wallet is active wallet', () => { const w1 = walletService.create(wallet1) walletService.create(wallet2) - assert.deepStrictEqual(walletService.getCurrent(), w1) + const activeWallet = walletService.getCurrent() + expect(activeWallet).toBeDefined() + expect(activeWallet!.id).toEqual(w1.id) }) it('delete current wallet', () => { @@ -266,8 +264,9 @@ describe('wallet service', () => { const w2 = walletService.create(wallet2) walletService.delete(w1.id) const activeWallet = walletService.getCurrent() - assert.deepStrictEqual(activeWallet, w2) - assert.strictEqual(walletService.getAll().length, 1) + expect(activeWallet).toBeDefined() + expect(activeWallet!.id).toEqual(w2.id) + expect(walletService.getAll().length).toEqual(1) }) it('delete none current wallet', () => { @@ -275,6 +274,7 @@ describe('wallet service', () => { const w2 = walletService.create(wallet2) walletService.delete(w2.id) const activeWallet = walletService.getCurrent() - assert.deepStrictEqual(activeWallet, w1) + expect(activeWallet).toBeDefined() + expect(activeWallet!.id).toEqual(w1.id) }) })