From a441e635bd5ae47a6f5b835becf3a40aaaab899d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 18 Apr 2016 16:39:35 -0700 Subject: [PATCH 1/6] Persist transactions to config-manager Transactions are now stored, and are never deleted, they only have their status updated. We can add deleting later if we'd like. I've hacked on emitting the new unconfirmedTx key to the UI in the format it received before, I want Aaron's opinion on where I should actually do that. --- app/scripts/background.js | 4 +- app/scripts/lib/config-manager.js | 50 +++++++++++++++++++ app/scripts/lib/idStore.js | 12 ++--- test/unit/config-manager-test.js | 79 +++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 7 deletions(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index db4927083..bdf87b1a5 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -117,6 +117,7 @@ function storeSetFromObj(store, obj){ Object.keys(obj).forEach(function(key){ store.set(key, obj[key]) }) + store.set('unconfTxs', configManager.unconfirmedTxs()) } @@ -191,7 +192,8 @@ idStore.on('update', updateBadge) function updateBadge(state){ var label = '' - var count = Object.keys(state.unconfTxs).length + var unconfTxs = configManager.unconfirmedTxs() + var count = Object.keys(unconfTxs).length if (count) { label = String(count) } diff --git a/app/scripts/lib/config-manager.js b/app/scripts/lib/config-manager.js index f024729cc..356d53c22 100644 --- a/app/scripts/lib/config-manager.js +++ b/app/scripts/lib/config-manager.js @@ -134,6 +134,56 @@ ConfigManager.prototype.setData = function(data) { this.migrator.saveData(data) } +ConfigManager.prototype.getTxList = function() { + var data = this.migrator.getData() + if ('transactions' in data) { + return data.transactions + } else { + return [] + } +} + +ConfigManager.prototype._saveTxList = function(txList) { + var data = this.migrator.getData() + data.transactions = txList + this.setData(data) +} + +ConfigManager.prototype.addTx = function(tx) { + var transactions = this.getTxList() + transactions.push(tx) + this._saveTxList(transactions) +} + +ConfigManager.prototype.getTx = function(txId) { + var transactions = this.getTxList() + var matching = transactions.filter(tx => tx.id === txId) + return matching.length > 0 ? matching[0] : null +} + +ConfigManager.prototype.confirmTx = function(txId) { + this._setTxStatus(txId, 'confirmed') +} + +ConfigManager.prototype.rejectTx = function(txId) { + this._setTxStatus(txId, 'rejected') +} + +ConfigManager.prototype._setTxStatus = function(txId, status) { + var transactions = this.getTxList() + transactions.forEach((tx) => { + if (tx.id === txId) { + tx.status = status + } + }) + this._saveTxList(transactions) +} +ConfigManager.prototype.unconfirmedTxs = function() { + var transactions = this.getTxList() + return transactions.filter(tx => tx.status === 'unconfirmed') + .reduce((result, tx) => { result[tx.id] = tx; return result }, {}) +} + // observable ConfigManager.prototype.subscribe = function(fn){ diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index f44300273..14e9b5769 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -31,7 +31,6 @@ function IdentityStore(ethStore) { this._currentState = { selectedAddress: null, identities: {}, - unconfTxs: {}, } // not part of serilized metamask state - only kept in memory this._unconfTxCbs = {} @@ -140,10 +139,11 @@ IdentityStore.prototype.addUnconfirmedTransaction = function(txParams, cb){ time: time, status: 'unconfirmed', } - this._currentState.unconfTxs[txId] = txData + configManager.addTx(txData) console.log('addUnconfirmedTransaction:', txData) // keep the cb around for after approval (requires user interaction) + // This cb fires completion to the Dapp's write operation. this._unconfTxCbs[txId] = cb // signal update @@ -154,7 +154,7 @@ IdentityStore.prototype.addUnconfirmedTransaction = function(txParams, cb){ // comes from metamask ui IdentityStore.prototype.approveTransaction = function(txId, cb){ - var txData = this._currentState.unconfTxs[txId] + var txData = configManager.getTx(txId) var txParams = txData.txParams var approvalCb = this._unconfTxCbs[txId] || noop @@ -162,20 +162,20 @@ IdentityStore.prototype.approveTransaction = function(txId, cb){ cb() approvalCb(null, true) // clean up - delete this._currentState.unconfTxs[txId] + configManager.confirmTx(txId) delete this._unconfTxCbs[txId] this._didUpdate() } // comes from metamask ui IdentityStore.prototype.cancelTransaction = function(txId){ - var txData = this._currentState.unconfTxs[txId] + var txData = configManager.getTx(txId) var approvalCb = this._unconfTxCbs[txId] || noop // reject tx approvalCb(null, false) // clean up - delete this._currentState.unconfTxs[txId] + configManager.rejectTx(txId) delete this._unconfTxCbs[txId] this._didUpdate() } diff --git a/test/unit/config-manager-test.js b/test/unit/config-manager-test.js index 10b6716d6..84632b0ea 100644 --- a/test/unit/config-manager-test.js +++ b/test/unit/config-manager-test.js @@ -68,4 +68,83 @@ describe('config-manager', function() { assert.equal(secondResult, secondRpc) }) }) + + describe('transactions', function() { + beforeEach(function() { + configManager._saveTxList([]) + }) + + describe('#getTxList', function() { + it('when new should return empty array', function() { + var result = configManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 0) + }) + }) + + describe('#_saveTxList', function() { + it('saves the submitted data to the tx list', function() { + var target = [{ foo: 'bar' }] + configManager._saveTxList(target) + var result = configManager.getTxList() + assert.equal(result[0].foo, 'bar') + }) + }) + + describe('#addTx', function() { + it('adds a tx returned in getTxList', function() { + var tx = { id: 1 } + configManager.addTx(tx) + var result = configManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 1) + assert.equal(result[0].id, 1) + }) + }) + + describe('#confirmTx', function() { + it('sets the tx status to confirmed', function() { + var tx = { id: 1, status: 'unconfirmed' } + configManager.addTx(tx) + configManager.confirmTx(1) + var result = configManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 1) + assert.equal(result[0].status, 'confirmed') + }) + }) + + describe('#rejectTx', function() { + it('sets the tx status to rejected', function() { + var tx = { id: 1, status: 'unconfirmed' } + configManager.addTx(tx) + configManager.rejectTx(1) + var result = configManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 1) + assert.equal(result[0].status, 'rejected') + }) + }) + + describe('#unconfirmedTxs', function() { + it('returns unconfirmed txs in a hash', function() { + configManager.addTx({ id: '1', status: 'unconfirmed' }) + configManager.addTx({ id: '2', status: 'confirmed' }) + let result = configManager.unconfirmedTxs() + assert.equal(typeof result, 'object') + assert.equal(result['1'].status, 'unconfirmed') + assert.equal(result['0'], undefined) + assert.equal(result['2'], undefined) + }) + }) + + describe('#getTx', function() { + it('returns a tx with the requested id', function() { + configManager.addTx({ id: '1', status: 'unconfirmed' }) + configManager.addTx({ id: '2', status: 'confirmed' }) + assert.equal(configManager.getTx('1').status, 'unconfirmed') + assert.equal(configManager.getTx('2').status, 'confirmed') + }) + }) + }) }) From dc043b7f9bc717caddbc81e7f0bfcef86c44d751 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 18 Apr 2016 17:19:20 -0700 Subject: [PATCH 2/6] Fix method of emitting unconfirmedTxs to UI --- app/scripts/background.js | 1 - app/scripts/lib/idStore.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/background.js b/app/scripts/background.js index bdf87b1a5..1519f63db 100644 --- a/app/scripts/background.js +++ b/app/scripts/background.js @@ -117,7 +117,6 @@ function storeSetFromObj(store, obj){ Object.keys(obj).forEach(function(key){ store.set(key, obj[key]) }) - store.set('unconfTxs', configManager.unconfirmedTxs()) } diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 14e9b5769..363ac817c 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -82,6 +82,7 @@ IdentityStore.prototype.getState = function(){ isInitialized: !!configManager.getWallet() && !seedWords, isUnlocked: this._isUnlocked(), seedWords: seedWords, + unconfTxs: configManager.unconfirmedTxs(), })) } From cfdad0f9fec64bbc95a89b7ed4110f0b177ec17a Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 18 Apr 2016 17:19:58 -0700 Subject: [PATCH 3/6] Emit transaction list to UI --- app/scripts/lib/idStore.js | 1 + 1 file changed, 1 insertion(+) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 363ac817c..b451fd6d4 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -83,6 +83,7 @@ IdentityStore.prototype.getState = function(){ isUnlocked: this._isUnlocked(), seedWords: seedWords, unconfTxs: configManager.unconfirmedTxs(), + transactions: configManager.getTxList(), })) } From 0e39110f3c0fb78b448b375d3211195f39e265a8 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 18 Apr 2016 17:25:03 -0700 Subject: [PATCH 4/6] Bump changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e74a095dd..dd66617a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Current Master +- Pending transactions are now persisted to localStorage and resume even after browser is closed. +- Completed transactions are now persisted and can be displayed via UI. + # 1.5.1 2016-04-15 - Corrected text above account list. Selected account is visible to all sites, not just the current domain. From 1d56ab821e88863e6822c6924a5b39054756e80e Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 18 Apr 2016 17:35:42 -0700 Subject: [PATCH 5/6] Fix displayed RPC address Fixes #119 --- ui/app/config.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/app/config.js b/ui/app/config.js index 33d87bcc2..878c9955f 100644 --- a/ui/app/config.js +++ b/ui/app/config.js @@ -47,7 +47,6 @@ ConfigScreen.prototype.render = function() { currentProviderDisplay(metamaskState), - h('div', [ h('input', { placeholder: 'New RPC URL', @@ -95,7 +94,7 @@ ConfigScreen.prototype.render = function() { } function currentProviderDisplay(metamaskState) { - var rpc = metamaskState.rpcTarget + var rpc = metamaskState.provider.rpcTarget return h('div', [ h('h3', {style: { fontWeight: 'bold' }}, 'Currently using RPC'), h('p', rpc) From 28f14661fd484f215c5bfdc93e1b447ddcdb42ca Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 18 Apr 2016 17:44:06 -0700 Subject: [PATCH 6/6] Bump changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd66617a1..6a4e7fd3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Pending transactions are now persisted to localStorage and resume even after browser is closed. - Completed transactions are now persisted and can be displayed via UI. +- Fix bug on config screen where current RPC address was always displayed wrong. # 1.5.1 2016-04-15