From c2046be0d8606ded9a328f392baf62cad802b98f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Wed, 30 Mar 2016 19:15:49 -0700 Subject: [PATCH] Made configuration migrateable Abstract all configuration data into a singleton called `configManager`, who is responsible for reading and writing to the persisted storage (localStorage, in our case). Uses my new module [pojo-migrator](https://www.npmjs.com/package/pojo-migrator), and wraps it with the `ConfigManager` class, which we can hang any state setting or getting methods we need. By keeping all the persisted state in one place, we can stabilize its outward-facing API, making the interactions increasingly atomic, which will allow us to add features that require restructuring the persisted data in the long term without having to rewrite UI or even `background.js` code. All the restructuring and data-type management is kept in one neat little place. This should make it very easy to add new configuration options like user-configured providers, per-domain vaults, and more! I know this doesn't seem like a big user-facing feature, but we have a big laundry list of features that I think this will really help streamline. --- app/scripts/background.js | 22 +-- app/scripts/lib/config-manager-singleton.js | 3 + app/scripts/lib/config-manager.js | 161 ++++++++++++++++++++ app/scripts/lib/etherscan-provider.js | 62 -------- app/scripts/lib/idStore.js | 38 ++--- package.json | 6 +- test/helper.js | 6 +- test/unit/config-manager-test.js | 71 +++++++++ 8 files changed, 266 insertions(+), 103 deletions(-) create mode 100644 app/scripts/lib/config-manager-singleton.js create mode 100644 app/scripts/lib/config-manager.js delete mode 100644 app/scripts/lib/etherscan-provider.js create mode 100644 test/unit/config-manager-test.js diff --git a/app/scripts/background.js b/app/scripts/background.js index 1a0d36ef8..772c1de89 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -9,8 +9,7 @@ const PortStream = require('./lib/port-stream.js') const MetaMaskProvider = require('web3-provider-engine/zero.js') const IdentityStore = require('./lib/idStore') const createTxNotification = require('./lib/tx-notification.js') - -console.log('ready to roll') +const configManager = require('./lib/config-manager-singleton') // // connect to other contexts @@ -37,10 +36,9 @@ function handleEthRpcRequestStream(stream){ // state and network // -var config = getConfig() var idStore = new IdentityStore() var zeroClient = MetaMaskProvider({ - rpcUrl: config.rpcTarget, + rpcUrl: configManager.getCurrentRpcAddress(), getAccounts: function(cb){ var selectedAddress = idStore.getSelectedAddress() var result = selectedAddress ? [selectedAddress] : [] @@ -62,7 +60,7 @@ function getState(){ var state = extend( ethStore.getState(), idStore.getState(), - getConfig() + configManager.getConfig() ) return state } @@ -177,22 +175,10 @@ function addUnconfirmedTx(txParams, cb){ // called from popup function setRpcTarget(rpcTarget){ - var config = getConfig() - config.rpcTarget = rpcTarget - setConfig(config) + configManager.setRpcTarget(rpcTarget) chrome.runtime.reload() } -function getConfig(){ - return extend({ - rpcTarget: 'https://rawtestrpc.metamask.io/', - }, JSON.parse(localStorage['config'] || '{}')) -} - -function setConfig(state){ - localStorage['config'] = JSON.stringify(state) -} - // util function jsonParseStream(){ diff --git a/app/scripts/lib/config-manager-singleton.js b/app/scripts/lib/config-manager-singleton.js new file mode 100644 index 000000000..5915c401b --- /dev/null +++ b/app/scripts/lib/config-manager-singleton.js @@ -0,0 +1,3 @@ +var ConfigManager = require('./config-manager') + +module.exports = new ConfigManager() diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js new file mode 100644 index 000000000..038774a30 --- /dev/null +++ b/app/scripts/lib/config-manager.js @@ -0,0 +1,161 @@ +const Migrator = require('pojo-migrator') +const extend = require('xtend') + +const STORAGE_KEY = 'metamask-config' +var DEFAULT_RPC = 'https://rawtestrpc.metamask.io/' + +/* The config-manager is a convenience object + * wrapping a pojo-migrator. + * + * It exists mostly to allow the creation of + * convenience methods to access and persist + * particular portions of the state. + */ +module.exports = ConfigManager +function ConfigManager() { + + /* The migrator exported on the config-manager + * has two methods the user should be concerned with: + * + * getData(), which returns the app-consumable data object + * saveData(), which persists the app-consumable data object. + */ + this.migrator = new Migrator({ + + // Migrations must start at version 1 or later. + // They are objects with a `version` number + // and a `migrate` function. + // + // The `migrate` function receives the previous + // config data format, and returns the new one. + migrations: [], + + // How to load initial config. + // Includes step on migrating pre-pojo-migrator data. + loadData: loadData, + + // How to persist migrated config. + setData: function(data) { + window.localStorage[STORAGE_KEY] = JSON.stringify(data) + }, + }) +} + +ConfigManager.prototype.setConfig = function(config) { + var data = this.migrator.getData() + data.config = config + this.setData(data) +} + +ConfigManager.prototype.setRpcTarget = function(rpcUrl) { + var config = this.getConfig() + config.provider = { + type: 'rpc', + rpcTarget: rpcUrl, + } + this.setConfig(config) +} + +ConfigManager.prototype.getConfig = function() { + var data = this.migrator.getData() + if ('config' in data) { + return data.config + } else { + return { + provider: { + type: 'rpc', + rpcTarget: DEFAULT_RPC, + } + } + } +} + +ConfigManager.prototype.setData = function(data) { + this.migrator.saveData(data) +} + +ConfigManager.prototype.getData = function() { + return this.migrator.getData() +} + +ConfigManager.prototype.setWallet = function(wallet) { + var data = this.migrator.getData() + data.wallet = wallet + this.setData(data) +} + +ConfigManager.prototype.getWallet = function() { + return this.migrator.getData().wallet +} + +ConfigManager.prototype.getSeedWords = function() { + return this.migrator.getData().seedWords +} + +ConfigManager.prototype.setSeedWords = function(seedWords) { + var data = this.migrator.getData() + data.seedWords = seedWords + this.setData(data) +} + +ConfigManager.prototype.clearSeedWords = function() { + var data = this.migrator.getData() + delete data.seedWords + this.setData(data) +} + +ConfigManager.prototype.getCurrentRpcAddress = function() { + var config = this.getConfig() + if (!config) return null + return config.provider && config.provider.rpcTarget ? config.provider.rpcTarget : DEFAULT_RPC +} + +ConfigManager.prototype.clearWallet = function() { + var data = this.getConfig() + delete data.wallet + this.setData(data) +} + +function loadData() { + + var oldData = getOldStyleData() + var newData + try { + newData = JSON.parse(window.localStorage[STORAGE_KEY]) + } catch (e) {} + + var data = extend({ + version: 0, + data: { + config: { + rpcTarget: DEFAULT_RPC, + } + } + }, oldData ? oldData : null, newData ? newData : null) + return data +} + +function getOldStyleData() { + var config, wallet, seedWords + + var result = { + meta: { version: 0 }, + data: {}, + } + + try { + config = JSON.parse(window.localStorage['config']) + result.data.config = config + } catch (e) {} + try { + wallet = JSON.parse(window.localStorage['lightwallet']) + result.data.wallet = wallet + } catch (e) {} + try { + seedWords = window.localStorage['seedWords'] + result.data.seedWords = seedWords + } catch (e) {} + + return result +} + diff --git a/app/scripts/lib/etherscan-provider.js b/app/scripts/lib/etherscan-provider.js deleted file mode 100644 index a2334c367..000000000 --- a/app/scripts/lib/etherscan-provider.js +++ /dev/null @@ -1,62 +0,0 @@ -const xhr = process.browser ? require('xhr') : require('request') -const inherits = require('util').inherits -const createPayload = require('../util/create-payload.js') - -module.exports = EtherScanProvider - -function EtherScanProvider(opts) { - - this.url = opts.url || 'http://testnet.etherscan.io/api?module=proxy&' - -} - -EtherScanProvider.prototype.setEngine = function(engine) { - const self = this - self.engine = engine - engine.on('block', function(block) { - self.currentBlock = block - }) -} - -EtherScanProvider.prototype.handleRequest = function(payload, next, end) { - - const self = this - var method = payload.method - var targetUrl = self.rpcUrl + 'action=' + method - var params = payload.params - - var newPayload = createPayload(payload) - - xhr({ - uri: targetUrl, - method: 'POST', - headers: { - 'Accept': 'application/json', - 'Content-Type': 'application/json', - }, - body: JSON.stringify(newPayload), - rejectUnauthorized: false, - }, function(err, res, body) { - if (err) return end(err) - - // parse response into raw account - var data - try { - data = JSON.parse(body) - if (data.error) return end(data.error) - } catch (err) { - console.error(err.stack) - return end(err) - } - - // console.log('network:', payload.method, payload.params, '->', data.result) - - end(null, data.result) - }) - -} - -EtherScanProvider.prototype.emitPayload = function(payload, cb){ - const self = this - self.engine.sendAsync(createPayload(payload), cb) -} diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index ea873e627..425a4348c 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -8,6 +8,7 @@ const clone = require('clone') const extend = require('xtend') const createId = require('web3-provider-engine/util/random-id') const autoFaucet = require('./auto-faucet') +const configManager = require('./config-manager-singleton') module.exports = IdentityStore @@ -41,11 +42,11 @@ function IdentityStore(ethStore) { IdentityStore.prototype.createNewVault = function(password, entropy, cb){ delete this._keyStore - delete window.localStorage['lightwallet'] + configManager.clearWallet() this._createIdmgmt(password, null, entropy, (err) => { if (err) return cb(err) var seedWords = this._idmgmt.getSeed() - this._cacheSeedWordsUntilConfirmed(seedWords) + configManager.setSeedWords(seedWords) this._loadIdentities() this._didUpdate() this._autoFaucet() @@ -68,14 +69,17 @@ IdentityStore.prototype.setStore = function(store){ } IdentityStore.prototype.clearSeedWordCache = function(cb) { - delete window.localStorage['seedWords'] + configManager.clearSeedWords() cb() } IdentityStore.prototype.getState = function(){ - const cachedSeeds = window.localStorage['seedWords'] + const cachedSeeds = configManager.getSeedWords() + var wallet = configManager.getWallet() + console.log('wallet:') + console.dir(wallet) return clone(extend(this._currentState, { - isInitialized: !!window.localStorage['lightwallet'] && !cachedSeeds, + isInitialized: !!configManager.getWallet() && !cachedSeeds, isUnlocked: this._isUnlocked(), seedWords: cachedSeeds, })) @@ -181,10 +185,6 @@ IdentityStore.prototype._isUnlocked = function(){ return result } -IdentityStore.prototype._cacheSeedWordsUntilConfirmed = function(seedWords) { - window.localStorage['seedWords'] = seedWords -} - // load identities from keyStoreet IdentityStore.prototype._loadIdentities = function(){ if (!this._isUnlocked()) throw new Error('not unlocked') @@ -216,14 +216,18 @@ IdentityStore.prototype._createIdmgmt = function(password, seed, entropy, cb){ var keyStore = null LightwalletKeyStore.deriveKeyFromPassword(password, (err, derivedKey) => { if (err) return cb(err) - var serializedKeystore = window.localStorage['lightwallet'] + var serializedKeystore = configManager.getWallet() + console.log("DESERIALIZED WALLET AND IT LOOKS LIKE THIS") + console.dir(serializedKeystore) if (seed) { keyStore = this._restoreFromSeed(password, seed, derivedKey) - // returning user, recovering from localStorage + // returning user, recovering from storage } else if (serializedKeystore) { - keyStore = this._loadFromLocalStorage(serializedKeystore, derivedKey, cb) + keyStore = this.deserializeKeystore(serializedKeystore) + console.log("DESERIALIZED KEYSTORE AND IT LOOKS LIKE THIS") + console.dir(keyStore) var isCorrect = keyStore.isDerivedKeyCorrect(derivedKey) if (!isCorrect) return cb(new Error('Lightwallet - password incorrect')) @@ -249,12 +253,12 @@ IdentityStore.prototype._restoreFromSeed = function(password, seed, derivedKey) keyStore.setDefaultHdDerivationPath(this.hdPathString) keyStore.generateNewAddress(derivedKey, 3) - window.localStorage['lightwallet'] = keyStore.serialize() - console.log('restored from seed. saved to keystore localStorage') + configManager.setWallet(keyStore.serialize()) + console.log('restored from seed. saved to keystore') return keyStore } -IdentityStore.prototype._loadFromLocalStorage = function(serializedKeystore, derivedKey) { +IdentityStore.prototype.deserializeKeystore = function(serializedKeystore) { return LightwalletKeyStore.deserialize(serializedKeystore) } @@ -265,8 +269,8 @@ IdentityStore.prototype._createFirstWallet = function(entropy, derivedKey) { keyStore.setDefaultHdDerivationPath(this.hdPathString) keyStore.generateNewAddress(derivedKey, 3) - window.localStorage['lightwallet'] = keyStore.serialize() - console.log('saved to keystore localStorage') + configManager.setWallet(keyStore.serialize()) + console.log('saved to keystore') return keyStore } diff --git a/package.json b/package.json index f482fd4f6..be61f1d14 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,8 @@ "private": true, "scripts": { "start": "gulp dev", - "test": "mocha --compilers js:babel-register --recursive" + "test": "mocha --require test/helper.js --compilers js:babel-register --recursive", + "watch": "mocha watch --compilers js:babel-register --recursive" }, "dependencies": { "async": "^1.5.2", @@ -21,6 +22,7 @@ "inject-css": "^0.1.1", "metamask-ui": "^1.5.0", "multiplex": "^6.7.0", + "pojo-migrator": "^2.1.0", "pumpify": "^1.3.4", "readable-stream": "^2.0.5", "through2": "^2.0.1", @@ -39,10 +41,10 @@ "gulp-util": "^3.0.7", "gulp-watch": "^4.3.5", "jsdom": "^8.1.0", + "jsdom-global": "^1.7.0", "jshint-stylish": "~0.1.5", "lodash.assign": "^4.0.6", "mocha": "^2.4.5", - "mocha-jsdom": "^1.1.0", "mocha-sinon": "^1.1.5", "sinon": "^1.17.3", "vinyl-buffer": "^1.0.0", diff --git a/test/helper.js b/test/helper.js index 7746b90e0..4c7f8b4c6 100644 --- a/test/helper.js +++ b/test/helper.js @@ -1,4 +1,2 @@ -require('mocha-sinon')() -var jsdom = require('mocha-jsdom') -jsdom() - +require('jsdom-global')() +window.localStorage = {} diff --git a/test/unit/config-manager-test.js b/test/unit/config-manager-test.js new file mode 100644 index 000000000..10b6716d6 --- /dev/null +++ b/test/unit/config-manager-test.js @@ -0,0 +1,71 @@ +var assert = require('assert') +var ConfigManager = require('../../app/scripts/lib/config-manager') +var configManager + +describe('config-manager', function() { + + before(function() { + window.localStorage = {} // Hacking localStorage support into JSDom + configManager = new ConfigManager() + }) + + describe('#setConfig', function() { + window.localStorage = {} // Hacking localStorage support into JSDom + + it('should set the config key', function () { + var testConfig = { + provider: { + type: 'rpc', + rpcTarget: 'foobar' + } + } + configManager.setConfig(testConfig) + var result = configManager.getData() + + assert.equal(result.config.provider.type, testConfig.provider.type) + assert.equal(result.config.provider.rpcTarget, testConfig.provider.rpcTarget) + }) + + it('setting wallet should not overwrite config', function() { + var testConfig = { + provider: { + type: 'rpc', + rpcTarget: 'foobar' + } + } + configManager.setConfig(testConfig) + + var testWallet = { + name: 'this is my fake wallet' + } + configManager.setWallet(testWallet) + + var result = configManager.getData() + assert.equal(result.wallet.name, testWallet.name, 'wallet name is set') + assert.equal(result.config.provider.rpcTarget, testConfig.provider.rpcTarget) + + testConfig.provider.type = 'something else!' + configManager.setConfig(testConfig) + + result = configManager.getData() + assert.equal(result.wallet.name, testWallet.name, 'wallet name is set') + assert.equal(result.config.provider.rpcTarget, testConfig.provider.rpcTarget) + assert.equal(result.config.provider.type, testConfig.provider.type) + }) + }) + + describe('rpc manipulations', function() { + it('changing rpc should return a different rpc', function() { + var firstRpc = 'first' + var secondRpc = 'second' + + configManager.setRpcTarget(firstRpc) + var firstResult = configManager.getCurrentRpcAddress() + assert.equal(firstResult, firstRpc) + + configManager.setRpcTarget(secondRpc) + var secondResult = configManager.getCurrentRpcAddress() + assert.equal(secondResult, secondRpc) + }) + }) +})