From 7d3da0ae96a761461ee12bf20d8386a5ba7aade2 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 25 Jun 2018 11:45:00 -0700 Subject: [PATCH 1/7] migration 27 - remove rejected transactions from state --- app/scripts/migrations/027.js | 35 ++++++++++++++++++++++ test/unit/migrations/027-test.js | 50 ++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) create mode 100644 app/scripts/migrations/027.js create mode 100644 test/unit/migrations/027-test.js diff --git a/app/scripts/migrations/027.js b/app/scripts/migrations/027.js new file mode 100644 index 000000000..d6ebef580 --- /dev/null +++ b/app/scripts/migrations/027.js @@ -0,0 +1,35 @@ +// next version number +const version = 27 + +/* + +normalizes txParams on unconfirmed txs + +*/ +const clone = require('clone') + +module.exports = { + version, + + migrate: async function (originalVersionedData) { + const versionedData = clone(originalVersionedData) + versionedData.meta.version = version + const state = versionedData.data + const newState = transformState(state) + versionedData.data = newState + return versionedData + }, +} + +function transformState (state) { + const newState = state + + if (newState.TransactionController) { + if (newState.TransactionController.transactions) { + const transactions = newState.TransactionController.transactions + newState.TransactionController.transactions = transactions.filter((txMeta) => txMeta.status !== 'rejected') + } + } + + return newState +} diff --git a/test/unit/migrations/027-test.js b/test/unit/migrations/027-test.js new file mode 100644 index 000000000..77c0bbee6 --- /dev/null +++ b/test/unit/migrations/027-test.js @@ -0,0 +1,50 @@ +const assert = require('assert') +const migration27 = require('../../../app/scripts/migrations/027') + +const oldStorage = { + "meta": {}, + "data": { + "TransactionController": { + "transactions": [ + ] + }, + }, +} + +const transactions = [] + + +while (transactions.length < 9) { + transactions.push({status: 'rejected'}) + transactions.push({status: 'unapproved'}) + transactions.push({status: 'approved'}) +} + + +oldStorage.data.TransactionController.transactions = transactions + +describe('migration #27', () => { + it('should remove rejected transactions', (done) => { + migration27.migrate(oldStorage) + .then((newStorage) => { + const newTransactions = newStorage.data.TransactionController.transactions + assert.equal(newTransactions.length, 6, 'transactions is expected to have the length of 6') + newTransactions.forEach((txMeta) => { + if (txMeta.status === 'rejected') done(new Error('transaction was found with a status of rejected')) + }) + done() + }) + .catch(done) + }) + + it('should successfully migrate first time state', (done) => { + migration27.migrate({ + meta: {}, + data: require('../../../app/scripts/first-time-state'), + }) + .then((migratedData) => { + assert.equal(migratedData.meta.version, migration27.version) + done() + }).catch(done) + }) +}) From 9b92ba4c47d5489c5c0f822045360be708110cac Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 25 Jun 2018 15:07:54 -0700 Subject: [PATCH 2/7] trandsactions - remove rejected transactions from history --- .../controllers/transactions/tx-state-manager.js | 6 ++++++ .../transactions/tx-state-manager-test.js | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index 0aae4774b..28a18ca2e 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/app/scripts/controllers/transactions/tx-state-manager.js @@ -288,6 +288,7 @@ class TransactionStateManager extends EventEmitter { */ setTxStatusRejected (txId) { this._setTxStatus(txId, 'rejected') + this._removeTx(txId) } /** @@ -422,6 +423,11 @@ class TransactionStateManager extends EventEmitter { _saveTxList (transactions) { this.store.updateState({ transactions }) } + + _removeTx (txId) { + const transactionList = this.getFullTxList() + this._saveTxList(transactionList.filter((txMeta) => txMeta.id !== txId)) + } } module.exports = TransactionStateManager diff --git a/test/unit/app/controllers/transactions/tx-state-manager-test.js b/test/unit/app/controllers/transactions/tx-state-manager-test.js index 20bc08b94..f2d0175cb 100644 --- a/test/unit/app/controllers/transactions/tx-state-manager-test.js +++ b/test/unit/app/controllers/transactions/tx-state-manager-test.js @@ -288,4 +288,18 @@ describe('TransactionStateManager', function () { }) }) + + describe.only('#_removeTx', function () { + it('should remove the transaction from the storage', () => { + txStateManager._saveTxList([ {id: 1} ]) + txStateManager._removeTx(1) + assert(!txStateManager.getFullTxList().length, 'txList should be empty') + }) + + it('should only remove the transaction with ID 1 from the storage', () => { + txStateManager._saveTxList([ {id: 1}, {id: 2} ]) + txStateManager._removeTx(1) + assert.equal(txStateManager.getFullTxList()[0].id, 2, 'txList should have a id of 2') + }) + }) }) From 650cef103e4cbbddcd3f145903d9dff0fcabe89d Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 25 Jun 2018 15:10:13 -0700 Subject: [PATCH 3/7] add to CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d86fcd713..4ad52b795 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master + - Remove rejected transactions from transaction history + ## 4.8.0 Thur Jun 14 2018 - [#4513](https://github.com/MetaMask/metamask-extension/pull/4513): Attempting to import an empty private key will now show a clear error. From 7b1f57e05d7a186be085ffcc8807516499218cba Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 25 Jun 2018 15:16:27 -0700 Subject: [PATCH 4/7] woops - remove .only Co-authored-by: @jennypollack --- test/unit/app/controllers/transactions/tx-state-manager-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/app/controllers/transactions/tx-state-manager-test.js b/test/unit/app/controllers/transactions/tx-state-manager-test.js index f2d0175cb..8b7dc78aa 100644 --- a/test/unit/app/controllers/transactions/tx-state-manager-test.js +++ b/test/unit/app/controllers/transactions/tx-state-manager-test.js @@ -289,7 +289,7 @@ describe('TransactionStateManager', function () { }) }) - describe.only('#_removeTx', function () { + describe('#_removeTx', function () { it('should remove the transaction from the storage', () => { txStateManager._saveTxList([ {id: 1} ]) txStateManager._removeTx(1) From 94a4f99115cf549a3014d29d828e7a432bf36a62 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 25 Jun 2018 16:16:51 -0700 Subject: [PATCH 5/7] tests - fix cancle transaction test to better fit new behavior --- .../controllers/transactions/tx-controller-test.js | 13 ++++++++++--- .../transactions/tx-state-manager-test.js | 10 ---------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index 9bdfe7c1a..4328f38e5 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -357,9 +357,16 @@ describe('Transaction Controller', function () { ]) }) - it('should set the transaction to rejected from unapproved', async function () { - await txController.cancelTransaction(0) - assert.equal(txController.txStateManager.getTx(0).status, 'rejected') + it('should emit a status change to rejected', function (done) { + txController.once('tx:status-update', (txId, status) => { + try { + assert.equal(status, 'rejected', 'status should e rejected') + assert.equal(txId, 0, 'id should e 0') + done() + } catch (e) { done(e) } + }) + + txController.cancelTransaction(0) }) }) diff --git a/test/unit/app/controllers/transactions/tx-state-manager-test.js b/test/unit/app/controllers/transactions/tx-state-manager-test.js index 8b7dc78aa..39c2d6337 100644 --- a/test/unit/app/controllers/transactions/tx-state-manager-test.js +++ b/test/unit/app/controllers/transactions/tx-state-manager-test.js @@ -45,16 +45,6 @@ describe('TransactionStateManager', function () { }) describe('#setTxStatusRejected', function () { - it('sets the tx status to rejected', function () { - let tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } - txStateManager.addTx(tx) - txStateManager.setTxStatusRejected(1) - let result = txStateManager.getTxList() - assert.ok(Array.isArray(result)) - assert.equal(result.length, 1) - assert.equal(result[0].status, 'rejected') - }) - it('should emit a rejected event to signal the exciton of callback', (done) => { let tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } txStateManager.addTx(tx) From 8a678001a959b5cb3d6b1d50920882c8f9b58043 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 10 Jul 2018 11:43:00 -0700 Subject: [PATCH 6/7] test - fix for lint --- test/unit/migrations/027-test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/unit/migrations/027-test.js b/test/unit/migrations/027-test.js index 77c0bbee6..3ec9f0c0e 100644 --- a/test/unit/migrations/027-test.js +++ b/test/unit/migrations/027-test.js @@ -2,11 +2,11 @@ const assert = require('assert') const migration27 = require('../../../app/scripts/migrations/027') const oldStorage = { - "meta": {}, - "data": { - "TransactionController": { - "transactions": [ - ] + 'meta': {}, + 'data': { + 'TransactionController': { + 'transactions': [ + ], }, }, } From b30499886f5f738aeb93df2571cf07e2b9897ae1 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 10 Jul 2018 15:35:27 -0700 Subject: [PATCH 7/7] test - check that #setTxStatusRejected removes transaction from history --- .../controllers/transactions/tx-state-manager-test.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/unit/app/controllers/transactions/tx-state-manager-test.js b/test/unit/app/controllers/transactions/tx-state-manager-test.js index 089b7a8a6..88bdaa60e 100644 --- a/test/unit/app/controllers/transactions/tx-state-manager-test.js +++ b/test/unit/app/controllers/transactions/tx-state-manager-test.js @@ -43,6 +43,15 @@ describe('TransactionStateManager', function () { }) describe('#setTxStatusRejected', function () { + it('sets the tx status to rejected and removes it from history', function () { + const tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } + txStateManager.addTx(tx) + txStateManager.setTxStatusRejected(1) + const result = txStateManager.getTxList() + assert.ok(Array.isArray(result)) + assert.equal(result.length, 0) + }) + it('should emit a rejected event to signal the exciton of callback', (done) => { const tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } txStateManager.addTx(tx)