From 43dde3cbde4118ef5d5b40faca6af8b308a1a902 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 26 Mar 2018 15:58:36 -0700 Subject: transactions - only save up to 40 txs totall across all networks --- app/scripts/lib/tx-state-manager.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js index ad07c813f..ab344ae9b 100644 --- a/app/scripts/lib/tx-state-manager.js +++ b/app/scripts/lib/tx-state-manager.js @@ -38,11 +38,6 @@ module.exports = class TransactionStateManager extends EventEmitter { }, opts) } - // Returns the number of txs for the current network. - getTxCount () { - return this.getTxList().length - } - getTxList () { const network = this.getNetwork() const fullTxList = this.getFullTxList() @@ -88,7 +83,7 @@ module.exports = class TransactionStateManager extends EventEmitter { txMeta.history.push(snapshot) const transactions = this.getFullTxList() - const txCount = this.getTxCount() + const txCount = transactions.length const txHistoryLimit = this.txHistoryLimit // checks if the length of the tx history is -- cgit From e0d98e73e3fc2cc909b21892da3ac7f5b6d1a05e Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 27 Mar 2018 14:08:20 -0700 Subject: Revert "Revert "Ci - introduce a build job"" --- .circleci/config.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8ebf569a5..f812767ad 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -6,6 +6,9 @@ workflows: jobs: - prep-deps-npm - prep-deps-firefox + - build: + requires: + - prep-deps-npm - prep-scss: requires: - prep-deps-npm @@ -217,3 +220,17 @@ jobs: - run: name: test:integration:mascara command: npm run test:mascara + + build: + docker: + - image: circleci/node:8-browsers + steps: + - checkout + - restore_cache: + key: dependency-cache-{{ checksum "package-lock.json" }} + - run: + name: build:dist + command: npm run dist + - run: + name: build:debug + command: find dist/ -type f -exec md5sum {} \; | sort -k 2 -- cgit From 62e1cbd33b440b8dcb98446b2fdee3f66ab5c1ad Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 27 Mar 2018 14:16:58 -0700 Subject: ci - run e2e tests after build step --- .circleci/config.yml | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f812767ad..ed6dc7802 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -6,7 +6,7 @@ workflows: jobs: - prep-deps-npm - prep-deps-firefox - - build: + - prep-build: requires: - prep-deps-npm - prep-scss: @@ -17,6 +17,7 @@ workflows: - prep-deps-npm - test-e2e: requires: + - prep-build - prep-deps-npm - test-unit: requires: @@ -71,6 +72,23 @@ jobs: paths: - firefox + prep-build: + docker: + - image: circleci/node:8-browsers + steps: + - checkout + - restore_cache: + key: dependency-cache-{{ checksum "package-lock.json" }} + - run: + name: build:dist + command: npm run dist + - run: + name: build:debug + command: find dist/ -type f -exec md5sum {} \; | sort -k 2 + - save_cache: + key: build-cache-{{ .Revision }} + paths: + - dist prep-scss: docker: @@ -109,9 +127,8 @@ jobs: - checkout - restore_cache: key: dependency-cache-{{ checksum "package-lock.json" }} - - run: - name: Build - command: npm run dist + - restore_cache: + key: build-cache-{{ .Revision }} - run: name: Test command: npm run test:e2e @@ -220,17 +237,3 @@ jobs: - run: name: test:integration:mascara command: npm run test:mascara - - build: - docker: - - image: circleci/node:8-browsers - steps: - - checkout - - restore_cache: - key: dependency-cache-{{ checksum "package-lock.json" }} - - run: - name: build:dist - command: npm run dist - - run: - name: build:debug - command: find dist/ -type f -exec md5sum {} \; | sort -k 2 -- cgit From 1dea4124f5e5b5eb37c234321808e9d414865f30 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 27 Mar 2018 14:40:04 -0700 Subject: ci - end by flowing all required tests into a single job --- .circleci/config.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index ed6dc7802..dadbf4885 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -40,6 +40,15 @@ workflows: - prep-deps-npm - prep-deps-firefox - prep-scss + - all-tests-passed: + requires: + - test-lint + - test-unit + - test-e2e + - test-integration-mascara-chrome + - test-integration-mascara-firefox + - test-integration-flat-chrome + - test-integration-flat-firefox jobs: prep-deps-npm: @@ -237,3 +246,10 @@ jobs: - run: name: test:integration:mascara command: npm run test:mascara + +all-tests-pass: + docker: + - image: circleci/node:8-browsers + - run: + name: All Tests Passed + command: echo 'weew - everything passed!' -- cgit From 3ec3b09c12618c40a0769605893626e059c64fc1 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 27 Mar 2018 16:09:11 -0700 Subject: ci - end by flowing all required tests into a single job --- .circleci/config.yml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index dadbf4885..a8e18116c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -40,7 +40,7 @@ workflows: - prep-deps-npm - prep-deps-firefox - prep-scss - - all-tests-passed: + - all-tests-pass: requires: - test-lint - test-unit @@ -247,9 +247,10 @@ jobs: name: test:integration:mascara command: npm run test:mascara -all-tests-pass: - docker: - - image: circleci/node:8-browsers - - run: - name: All Tests Passed - command: echo 'weew - everything passed!' + all-tests-pass: + docker: + - image: circleci/node:8-browsers + steps: + - run: + name: All Tests Passed + command: echo 'weew - everything passed!' -- cgit From fe8472ab22fbff8d679943f0c65efd5d66c2a9c7 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 27 Mar 2018 16:10:27 -0700 Subject: ci - end by flowing all required tests into a single job --- .circleci/config.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a8e18116c..75819fc6e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -252,5 +252,5 @@ jobs: - image: circleci/node:8-browsers steps: - run: - name: All Tests Passed - command: echo 'weew - everything passed!' + name: All Tests Passed + command: echo 'weew - everything passed!' -- cgit From f0f45e6fe19fec01f7fff9df0e9e04015f82f3d2 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Mon, 26 Mar 2018 18:09:28 -0700 Subject: migration for removing unnecessary transactions from state --- app/scripts/migrations/023.js | 50 +++++++++++++++++++ app/scripts/migrations/index.js | 2 + test/unit/migrations/023-test.js | 99 ++++++++++++++++++++++++++++++++++++++ test/unit/tx-state-manager-test.js | 6 +-- 4 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 app/scripts/migrations/023.js create mode 100644 test/unit/migrations/023-test.js diff --git a/app/scripts/migrations/023.js b/app/scripts/migrations/023.js new file mode 100644 index 000000000..bce0a5bea --- /dev/null +++ b/app/scripts/migrations/023.js @@ -0,0 +1,50 @@ + +const version = 23 + +/* + +This migration removes transactions that are no longer usefull down to 40 total + +*/ + +const clone = require('clone') + +module.exports = { + version, + + migrate: function (originalVersionedData) { + const versionedData = clone(originalVersionedData) + versionedData.meta.version = version + try { + const state = versionedData.data + const newState = transformState(state) + versionedData.data = newState + } catch (err) { + console.warn(`MetaMask Migration #${version}` + err.stack) + } + return Promise.resolve(versionedData) + }, +} + +function transformState (state) { + const newState = state + const transactions = newState.TransactionController.transactions + + if (transactions.length <= 40) return newState + + let reverseTxList = transactions.reverse() + let stripping = true + while (reverseTxList.length > 40 && stripping) { + let txIndex = reverseTxList.findIndex((txMeta) => { + return (txMeta.status === 'failed' || + txMeta.status === 'rejected' || + txMeta.status === 'confirmed' || + txMeta.status === 'dropped') + }) + if (txIndex < 0) stripping = false + else reverseTxList.splice(txIndex, 1) + } + + newState.TransactionController.transactions = reverseTxList.reverse() + return newState +} diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index a0cf5f4d4..811e06b6b 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -32,4 +32,6 @@ module.exports = [ require('./019'), require('./020'), require('./021'), + require('./022'), + require('./023'), ] diff --git a/test/unit/migrations/023-test.js b/test/unit/migrations/023-test.js new file mode 100644 index 000000000..be432d9fa --- /dev/null +++ b/test/unit/migrations/023-test.js @@ -0,0 +1,99 @@ +const assert = require('assert') +const migration23 = require('../../../app/scripts/migrations/023') +const properTime = (new Date()).getTime() +const storage = { + "meta": {}, + "data": { + "TransactionController": { + "transactions": [ + ] + }, + }, +} + +const transactions = [] +const transactions40 = [] +const transactions20 = [] + +const txStates = [ + 'unapproved', + 'approved', + 'signed', + 'submitted', + 'confirmed', + 'rejected', + 'failed', + 'dropped', +] + +const deletableTxStates = [ + 'confirmed', + 'rejected', + 'failed', + 'dropped', +] + +let nonDeletableCount = 0 + +let status +while (transactions.length <= 100) { + status = txStates[Math.floor(Math.random() * Math.floor(txStates.length - 1))] + if (!deletableTxStates.find((s) => s === status)) nonDeletableCount++ + transactions.push({status}) +} + +while (transactions40.length < 40) { + status = txStates[Math.floor(Math.random() * Math.floor(txStates.length - 1))] + transactions40.push({status}) +} + +while (transactions20.length < 20) { + status = txStates[Math.floor(Math.random() * Math.floor(txStates.length - 1))] + transactions20.push({status}) +} + + + +storage.data.TransactionController.transactions = transactions + +describe('storage is migrated successfully and the proper transactions are remove from state', () => { + it('should remove transactions that are unneeded', (done) => { + migration23.migrate(storage) + .then((migratedData) => { + let leftoverNonDeletableTxCount = 0 + const migratedTransactions = migratedData.data.TransactionController.transactions + migratedTransactions.forEach((tx) => { + if (!deletableTxStates.find((s) => s === tx.status)) { + leftoverNonDeletableTxCount++ + } + }) + assert.equal(leftoverNonDeletableTxCount, nonDeletableCount, 'migration shouldnt delete transactions we want to keep') + assert((migratedTransactions.length >= 40), `should be equal or greater to 40 if they are non deletable states got ${migratedTransactions.length} transactions`) + done() + }).catch(done) + }) + + it('should not remove any transactions because 40 is the expectable limit', (done) => { + storage.meta.version = 22 + storage.data.TransactionController.transactions = transactions40 + migration23.migrate(storage) + .then((migratedData) => { + const migratedTransactions = migratedData.data.TransactionController.transactions + + assert.equal(migratedTransactions.length, 40, 'migration shouldnt delete when at limit') + done() + }).catch(done) + }) + + it('should not remove any transactions because 20 txs is under the expectable limit', (done) => { + storage.meta.version = 22 + storage.data.TransactionController.transactions = transactions20 + migration23.migrate(storage) + .then((migratedData) => { + const migratedTransactions = migratedData.data.TransactionController.transactions + assert.equal(migratedTransactions.length, 20, 'migration shouldnt delete when under limit') + done() + }).catch(done) + }) + +}) diff --git a/test/unit/tx-state-manager-test.js b/test/unit/tx-state-manager-test.js index 220bf501f..a5ac13664 100644 --- a/test/unit/tx-state-manager-test.js +++ b/test/unit/tx-state-manager-test.js @@ -240,12 +240,12 @@ describe('TransactionStateManager', function () { }) describe('#wipeTransactions', function () { - + const specificAddress = '0xaa' const otherAddress = '0xbb' it('should remove only the transactions from a specific address', function () { - + const txMetas = [ { id: 0, status: 'unapproved', txParams: { from: specificAddress, to: otherAddress }, metamaskNetworkId: currentNetworkId }, { id: 1, status: 'confirmed', txParams: { from: otherAddress, to: specificAddress }, metamaskNetworkId: currentNetworkId }, @@ -268,7 +268,7 @@ describe('TransactionStateManager', function () { { id: 1, status: 'confirmed', txParams: { from: specificAddress, to: otherAddress }, metamaskNetworkId: otherNetworkId }, { id: 2, status: 'confirmed', txParams: { from: specificAddress, to: otherAddress }, metamaskNetworkId: otherNetworkId }, ] - + txMetas.forEach((txMeta) => txStateManager.addTx(txMeta, noop)) txStateManager.wipeTransactions(specificAddress) -- cgit