From 4080ed63a4e65951ff01bce3627d3832f845eb20 Mon Sep 17 00:00:00 2001 From: Brad Decker Date: Tue, 30 Mar 2021 09:54:05 -0500 Subject: [PATCH] Refactor Tx State Manager (#10672) Co-authored-by: Mark Stacey --- app/scripts/controllers/transactions/index.js | 113 +-- .../controllers/transactions/index.test.js | 241 ++++-- .../controllers/transactions/lib/util.js | 60 +- .../transactions/tx-state-manager.js | 643 +++++++++------- .../transactions/tx-state-manager.test.js | 725 ++++++++++-------- app/scripts/metamask-controller.js | 11 +- app/scripts/metamask-controller.test.js | 4 +- app/scripts/migrations/057.js | 44 ++ app/scripts/migrations/057.test.js | 193 +++++ app/scripts/migrations/index.js | 1 + shared/constants/transaction.js | 11 +- .../send/send-footer/send-footer.component.js | 1 - 12 files changed, 1295 insertions(+), 752 deletions(-) create mode 100644 app/scripts/migrations/057.js create mode 100644 app/scripts/migrations/057.test.js diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index e61d747bd..3b0b9465d 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -150,8 +150,8 @@ export default class TransactionController extends EventEmitter { Adds a tx to the txlist @emits ${txMeta.id}:unapproved */ - addTx(txMeta) { - this.txStateManager.addTx(txMeta); + addTransaction(txMeta) { + this.txStateManager.addTransaction(txMeta); this.emit(`${txMeta.id}:unapproved`, txMeta); } @@ -273,22 +273,28 @@ export default class TransactionController extends EventEmitter { ? addHexPrefix(txMeta.txParams.value) : '0x0'; - this.addTx(txMeta); + this.addTransaction(txMeta); this.emit('newUnapprovedTx', txMeta); try { txMeta = await this.addTxGasDefaults(txMeta, getCodeResponse); } catch (error) { log.warn(error); - txMeta = this.txStateManager.getTx(txMeta.id); + txMeta = this.txStateManager.getTransaction(txMeta.id); txMeta.loadingDefaults = false; - this.txStateManager.updateTx(txMeta, 'Failed to calculate gas defaults.'); + this.txStateManager.updateTransaction( + txMeta, + 'Failed to calculate gas defaults.', + ); throw error; } txMeta.loadingDefaults = false; // save txMeta - this.txStateManager.updateTx(txMeta, 'Added new unapproved transaction.'); + this.txStateManager.updateTransaction( + txMeta, + 'Added new unapproved transaction.', + ); return txMeta; } @@ -306,7 +312,7 @@ export default class TransactionController extends EventEmitter { } = await this._getDefaultGasLimit(txMeta, getCodeResponse); // eslint-disable-next-line no-param-reassign - txMeta = this.txStateManager.getTx(txMeta.id); + txMeta = this.txStateManager.getTransaction(txMeta.id); if (simulationFails) { txMeta.simulationFails = simulationFails; } @@ -386,7 +392,7 @@ export default class TransactionController extends EventEmitter { * @returns {txMeta} */ async createCancelTransaction(originalTxId, customGasPrice, customGasLimit) { - const originalTxMeta = this.txStateManager.getTx(originalTxId); + const originalTxMeta = this.txStateManager.getTransaction(originalTxId); const { txParams } = originalTxMeta; const { gasPrice: lastGasPrice, from, nonce } = txParams; @@ -408,7 +414,7 @@ export default class TransactionController extends EventEmitter { type: TRANSACTION_TYPES.CANCEL, }); - this.addTx(newTxMeta); + this.addTransaction(newTxMeta); await this.approveTransaction(newTxMeta.id); return newTxMeta; } @@ -424,7 +430,7 @@ export default class TransactionController extends EventEmitter { * @returns {txMeta} */ async createSpeedUpTransaction(originalTxId, customGasPrice, customGasLimit) { - const originalTxMeta = this.txStateManager.getTx(originalTxId); + const originalTxMeta = this.txStateManager.getTransaction(originalTxId); const { txParams } = originalTxMeta; const { gasPrice: lastGasPrice } = txParams; @@ -447,7 +453,7 @@ export default class TransactionController extends EventEmitter { newTxMeta.txParams.gas = customGasLimit; } - this.addTx(newTxMeta); + this.addTransaction(newTxMeta); await this.approveTransaction(newTxMeta.id); return newTxMeta; } @@ -457,7 +463,10 @@ export default class TransactionController extends EventEmitter { @param {Object} txMeta - the updated txMeta */ async updateTransaction(txMeta) { - this.txStateManager.updateTx(txMeta, 'confTx: user updated transaction'); + this.txStateManager.updateTransaction( + txMeta, + 'confTx: user updated transaction', + ); } /** @@ -465,7 +474,10 @@ export default class TransactionController extends EventEmitter { @param {Object} txMeta */ async updateAndApproveTransaction(txMeta) { - this.txStateManager.updateTx(txMeta, 'confTx: user approved transaction'); + this.txStateManager.updateTransaction( + txMeta, + 'confTx: user approved transaction', + ); await this.approveTransaction(txMeta.id); } @@ -492,7 +504,7 @@ export default class TransactionController extends EventEmitter { // approve this.txStateManager.setTxStatusApproved(txId); // get next nonce - const txMeta = this.txStateManager.getTx(txId); + const txMeta = this.txStateManager.getTransaction(txId); const fromAddress = txMeta.txParams.from; // wait for a nonce let { customNonceValue } = txMeta; @@ -513,7 +525,10 @@ export default class TransactionController extends EventEmitter { if (customNonceValue) { txMeta.nonceDetails.customNonceValue = customNonceValue; } - this.txStateManager.updateTx(txMeta, 'transactions#approveTransaction'); + this.txStateManager.updateTransaction( + txMeta, + 'transactions#approveTransaction', + ); // sign transaction const rawTx = await this.signTransaction(txId); await this.publishTransaction(txId, rawTx); @@ -543,7 +558,7 @@ export default class TransactionController extends EventEmitter { @returns {string} rawTx */ async signTransaction(txId) { - const txMeta = this.txStateManager.getTx(txId); + const txMeta = this.txStateManager.getTransaction(txId); // add network/chain id const chainId = this.getChainId(); const txParams = { ...txMeta.txParams, chainId }; @@ -558,7 +573,7 @@ export default class TransactionController extends EventEmitter { txMeta.s = ethUtil.bufferToHex(ethTx.s); txMeta.v = ethUtil.bufferToHex(ethTx.v); - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions#signTransaction: add r, s, v values', ); @@ -576,13 +591,16 @@ export default class TransactionController extends EventEmitter { @returns {Promise} */ async publishTransaction(txId, rawTx) { - const txMeta = this.txStateManager.getTx(txId); + const txMeta = this.txStateManager.getTransaction(txId); txMeta.rawTx = rawTx; if (txMeta.type === TRANSACTION_TYPES.SWAP) { const preTxBalance = await this.query.getBalance(txMeta.txParams.from); txMeta.preTxBalance = preTxBalance.toString(16); } - this.txStateManager.updateTx(txMeta, 'transactions#publishTransaction'); + this.txStateManager.updateTransaction( + txMeta, + 'transactions#publishTransaction', + ); let txHash; try { txHash = await this.query.sendRawTransaction(rawTx); @@ -608,7 +626,7 @@ export default class TransactionController extends EventEmitter { async confirmTransaction(txId, txReceipt) { // get the txReceipt before marking the transaction confirmed // to ensure the receipt is gotten before the ui revives the tx - const txMeta = this.txStateManager.getTx(txId); + const txMeta = this.txStateManager.getTransaction(txId); if (!txMeta) { return; @@ -629,22 +647,22 @@ export default class TransactionController extends EventEmitter { this.txStateManager.setTxStatusConfirmed(txId); this._markNonceDuplicatesDropped(txId); - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions#confirmTransaction - add txReceipt', ); if (txMeta.type === TRANSACTION_TYPES.SWAP) { const postTxBalance = await this.query.getBalance(txMeta.txParams.from); - const latestTxMeta = this.txStateManager.getTx(txId); + const latestTxMeta = this.txStateManager.getTransaction(txId); const approvalTxMeta = latestTxMeta.approvalTxId - ? this.txStateManager.getTx(latestTxMeta.approvalTxId) + ? this.txStateManager.getTransaction(latestTxMeta.approvalTxId) : null; latestTxMeta.postTxBalance = postTxBalance.toString(16); - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( latestTxMeta, 'transactions#confirmTransaction - add postTxBalance', ); @@ -672,9 +690,9 @@ export default class TransactionController extends EventEmitter { */ setTxHash(txId, txHash) { // Add the tx hash to the persisted meta-tx object - const txMeta = this.txStateManager.getTx(txId); + const txMeta = this.txStateManager.getTransaction(txId); txMeta.hash = txHash; - this.txStateManager.updateTx(txMeta, 'transactions#setTxHash'); + this.txStateManager.updateTransaction(txMeta, 'transactions#setTxHash'); } // @@ -704,8 +722,7 @@ export default class TransactionController extends EventEmitter { this.txStateManager.getPendingTransactions(account).length; /** see txStateManager */ - this.getFilteredTxList = (opts) => - this.txStateManager.getFilteredTxList(opts); + this.getTransactions = (opts) => this.txStateManager.getTransactions(opts); } // called once on startup @@ -724,23 +741,25 @@ export default class TransactionController extends EventEmitter { _onBootCleanUp() { this.txStateManager - .getFilteredTxList({ - status: TRANSACTION_STATUSES.UNAPPROVED, - loadingDefaults: true, + .getTransactions({ + searchCriteria: { + status: TRANSACTION_STATUSES.UNAPPROVED, + loadingDefaults: true, + }, }) .forEach((tx) => { this.addTxGasDefaults(tx) .then((txMeta) => { txMeta.loadingDefaults = false; - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions: gas estimation for tx on boot', ); }) .catch((error) => { - const txMeta = this.txStateManager.getTx(tx.id); + const txMeta = this.txStateManager.getTransaction(tx.id); txMeta.loadingDefaults = false; - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'failed to estimate gas during boot cleanup.', ); @@ -749,8 +768,10 @@ export default class TransactionController extends EventEmitter { }); this.txStateManager - .getFilteredTxList({ - status: TRANSACTION_STATUSES.APPROVED, + .getTransactions({ + searchCriteria: { + status: TRANSACTION_STATUSES.APPROVED, + }, }) .forEach((txMeta) => { const txSignError = new Error( @@ -771,7 +792,7 @@ export default class TransactionController extends EventEmitter { ); this._setupBlockTrackerListener(); this.pendingTxTracker.on('tx:warning', (txMeta) => { - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions/pending-tx-tracker#event: tx:warning', ); @@ -790,7 +811,7 @@ export default class TransactionController extends EventEmitter { this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => { if (!txMeta.firstRetryBlockNumber) { txMeta.firstRetryBlockNumber = latestBlockNumber; - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions/pending-tx-tracker#event: tx:block-update', ); @@ -801,7 +822,7 @@ export default class TransactionController extends EventEmitter { txMeta.retryCount = 0; } txMeta.retryCount += 1; - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions/pending-tx-tracker#event: tx:retry', ); @@ -878,9 +899,11 @@ export default class TransactionController extends EventEmitter { */ _markNonceDuplicatesDropped(txId) { // get the confirmed transactions nonce and from address - const txMeta = this.txStateManager.getTx(txId); + const txMeta = this.txStateManager.getTransaction(txId); const { nonce, from } = txMeta.txParams; - const sameNonceTxs = this.txStateManager.getFilteredTxList({ nonce, from }); + const sameNonceTxs = this.txStateManager.getTransactions({ + searchCriteria: { nonce, from }, + }); if (!sameNonceTxs.length) { return; } @@ -890,7 +913,7 @@ export default class TransactionController extends EventEmitter { return; } otherTxMeta.replacedBy = txMeta.hash; - this.txStateManager.updateTx( + this.txStateManager.updateTransaction( txMeta, 'transactions/pending-tx-tracker#event: tx:confirmed reference to confirmed txHash with same nonce', ); @@ -936,9 +959,9 @@ export default class TransactionController extends EventEmitter { */ _updateMemstore() { const unapprovedTxs = this.txStateManager.getUnapprovedTxList(); - const currentNetworkTxList = this.txStateManager.getTxList( - MAX_MEMSTORE_TX_LIST_SIZE, - ); + const currentNetworkTxList = this.txStateManager.getTransactions({ + limit: MAX_MEMSTORE_TX_LIST_SIZE, + }); this.memStore.updateState({ unapprovedTxs, currentNetworkTxList }); } diff --git a/app/scripts/controllers/transactions/index.test.js b/app/scripts/controllers/transactions/index.test.js index 6a31e57a6..2d1709cdf 100644 --- a/app/scripts/controllers/transactions/index.test.js +++ b/app/scripts/controllers/transactions/index.test.js @@ -20,6 +20,9 @@ const noop = () => true; const currentNetworkId = '42'; const currentChainId = '0x2a'; +const VALID_ADDRESS = '0x0000000000000000000000000000000000000000'; +const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001'; + describe('Transaction Controller', function () { let txController, provider, providerResultStub, fromAccount; @@ -81,26 +84,35 @@ describe('Transaction Controller', function () { describe('#getUnapprovedTxCount', function () { it('should return the number of unapproved txs', function () { - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 2, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 3, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, ]); @@ -111,26 +123,35 @@ describe('Transaction Controller', function () { describe('#getPendingTxCount', function () { it('should return the number of pending txs', function () { - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 1, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 2, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 3, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, ]); @@ -146,7 +167,7 @@ describe('Transaction Controller', function () { from: address, to: '0xc684832530fcbddae4b4230a47e991ddcec2831d', }; - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 0, status: TRANSACTION_STATUSES.CONFIRMED, @@ -232,17 +253,19 @@ describe('Transaction Controller', function () { txParams, history: [{}], }; - txController.txStateManager._saveTxList([txMeta]); + txController.txStateManager._addTransactionsToState([txMeta]); stub = sinon .stub(txController, 'addUnapprovedTransaction') .callsFake(() => { txController.emit('newUnapprovedTx', txMeta); - return Promise.resolve(txController.txStateManager.addTx(txMeta)); + return Promise.resolve( + txController.txStateManager.addTransaction(txMeta), + ); }); }); afterEach(function () { - txController.txStateManager._saveTxList([]); + txController.txStateManager._addTransactionsToState([]); stub.restore(); }); @@ -312,7 +335,7 @@ describe('Transaction Controller', function () { 'should have added 0x0 as the value', ); - const memTxMeta = txController.txStateManager.getTx(txMeta.id); + const memTxMeta = txController.txStateManager.getTransaction(txMeta.id); assert.deepEqual(txMeta, memTxMeta); }); @@ -353,12 +376,15 @@ describe('Transaction Controller', function () { describe('#addTxGasDefaults', function () { it('should add the tx defaults if their are none', async function () { - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, ]); @@ -386,13 +412,16 @@ describe('Transaction Controller', function () { }); }); - describe('#addTx', function () { + describe('#addTransaction', function () { it('should emit updates', function (done) { const txMeta = { id: '1', status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, }; const eventNames = [ @@ -419,7 +448,7 @@ describe('Transaction Controller', function () { done(); }) .catch(done); - txController.addTx(txMeta); + txController.addTransaction(txMeta); }); }); @@ -431,6 +460,8 @@ describe('Transaction Controller', function () { status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, nonce: originalValue, gas: originalValue, gasPrice: originalValue, @@ -440,7 +471,7 @@ describe('Transaction Controller', function () { this.timeout(15000); const wrongValue = '0x05'; - txController.addTx(txMeta); + txController.addTransaction(txMeta); providerResultStub.eth_gasPrice = wrongValue; providerResultStub.eth_estimateGas = '0x5209'; @@ -456,7 +487,7 @@ describe('Transaction Controller', function () { }); await txController.approveTransaction(txMeta.id); - const result = txController.txStateManager.getTx(txMeta.id); + const result = txController.txStateManager.getTransaction(txMeta.id); const params = result.txParams; assert.equal(params.gas, originalValue, 'gas unmodified'); @@ -474,12 +505,15 @@ describe('Transaction Controller', function () { describe('#sign replay-protected tx', function () { it('prepares a tx with the chainId set', async function () { - txController.addTx( + txController.addTransaction( { id: '1', status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, }, noop, ); @@ -503,9 +537,9 @@ describe('Transaction Controller', function () { }, metamaskNetworkId: currentNetworkId, }; - txController.txStateManager.addTx(txMeta); + txController.txStateManager.addTransaction(txMeta); const approvalPromise = txController.updateAndApproveTransaction(txMeta); - const tx = txController.txStateManager.getTx(1); + const tx = txController.txStateManager.getTransaction(1); assert.equal(tx.status, TRANSACTION_STATUSES.APPROVED); await approvalPromise; }); @@ -520,53 +554,74 @@ describe('Transaction Controller', function () { describe('#cancelTransaction', function () { it('should emit a status change to rejected', function (done) { - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 0, status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, { id: 1, status: TRANSACTION_STATUSES.REJECTED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, { id: 2, status: TRANSACTION_STATUSES.APPROVED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, { id: 3, status: TRANSACTION_STATUSES.SIGNED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, { id: 4, status: TRANSACTION_STATUSES.SUBMITTED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, { id: 5, status: TRANSACTION_STATUSES.CONFIRMED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, { id: 6, status: TRANSACTION_STATUSES.FAILED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, history: [{}], }, @@ -591,13 +646,13 @@ describe('Transaction Controller', function () { }); describe('#createSpeedUpTransaction', function () { - let addTxSpy; + let addTransactionSpy; let approveTransactionSpy; let txParams; let expectedTxParams; beforeEach(function () { - addTxSpy = sinon.spy(txController, 'addTx'); + addTransactionSpy = sinon.spy(txController, 'addTransaction'); approveTransactionSpy = sinon.spy(txController, 'approveTransaction'); txParams = { @@ -607,7 +662,7 @@ describe('Transaction Controller', function () { gas: '0x5209', gasPrice: '0xa', }; - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 1, status: TRANSACTION_STATUSES.SUBMITTED, @@ -621,18 +676,18 @@ describe('Transaction Controller', function () { }); afterEach(function () { - addTxSpy.restore(); + addTransactionSpy.restore(); approveTransactionSpy.restore(); }); - it('should call this.addTx and this.approveTransaction with the expected args', async function () { + it('should call this.addTransaction and this.approveTransaction with the expected args', async function () { await txController.createSpeedUpTransaction(1); - assert.equal(addTxSpy.callCount, 1); + assert.equal(addTransactionSpy.callCount, 1); - const addTxArgs = addTxSpy.getCall(0).args[0]; - assert.deepEqual(addTxArgs.txParams, expectedTxParams); + const addTransactionArgs = addTransactionSpy.getCall(0).args[0]; + assert.deepEqual(addTransactionArgs.txParams, expectedTxParams); - const { lastGasPrice, type } = addTxArgs; + const { lastGasPrice, type } = addTransactionArgs; assert.deepEqual( { lastGasPrice, type }, { @@ -674,7 +729,10 @@ describe('Transaction Controller', function () { txMeta = { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, metamaskNetworkId: currentNetworkId, }; providerResultStub.eth_sendRawTransaction = hash; @@ -683,9 +741,9 @@ describe('Transaction Controller', function () { it('should publish a tx, updates the rawTx when provided a one', async function () { const rawTx = '0x477b2e6553c917af0db0388ae3da62965ff1a184558f61b749d1266b2e6d024c'; - txController.txStateManager.addTx(txMeta); + txController.txStateManager.addTransaction(txMeta); await txController.publishTransaction(txMeta.id, rawTx); - const publishedTx = txController.txStateManager.getTx(1); + const publishedTx = txController.txStateManager.getTransaction(1); assert.equal(publishedTx.hash, hash); assert.equal(publishedTx.status, TRANSACTION_STATUSES.SUBMITTED); }); @@ -696,9 +754,9 @@ describe('Transaction Controller', function () { }; const rawTx = '0xf86204831e848082520894f231d46dd78806e1dd93442cf33c7671f853874880802ca05f973e540f2d3c2f06d3725a626b75247593cb36477187ae07ecfe0a4db3cf57a00259b52ee8c58baaa385fb05c3f96116e58de89bcc165cb3bfdfc708672fed8a'; - txController.txStateManager.addTx(txMeta); + txController.txStateManager.addTransaction(txMeta); await txController.publishTransaction(txMeta.id, rawTx); - const publishedTx = txController.txStateManager.getTx(1); + const publishedTx = txController.txStateManager.getTransaction(1); assert.equal( publishedTx.hash, '0x2cc5a25744486f7383edebbf32003e5a66e18135799593d6b5cdd2bb43674f09', @@ -709,62 +767,92 @@ describe('Transaction Controller', function () { describe('#_markNonceDuplicatesDropped', function () { it('should mark all nonce duplicates as dropped without marking the confirmed transaction as dropped', function () { - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 1, status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, { id: 2, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, { id: 3, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, { id: 4, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, { id: 5, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, { id: 6, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, { id: 7, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, history: [{}], - txParams: { nonce: '0x01' }, + txParams: { + to: VALID_ADDRESS_TWO, + from: VALID_ADDRESS, + nonce: '0x01', + }, }, ]); txController._markNonceDuplicatesDropped(1); - const confirmedTx = txController.txStateManager.getTx(1); - const droppedTxs = txController.txStateManager.getFilteredTxList({ - nonce: '0x01', - status: TRANSACTION_STATUSES.DROPPED, + const confirmedTx = txController.txStateManager.getTransaction(1); + const droppedTxs = txController.txStateManager.getTransactions({ + searchCriteria: { + nonce: '0x01', + status: TRANSACTION_STATUSES.DROPPED, + }, }); assert.equal( confirmedTx.status, @@ -927,53 +1015,74 @@ describe('Transaction Controller', function () { describe('#getPendingTransactions', function () { it('should show only submitted and approved transactions as pending transaction', function () { - txController.txStateManager._saveTxList([ + txController.txStateManager._addTransactionsToState([ { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, }, { id: 2, status: TRANSACTION_STATUSES.REJECTED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 3, status: TRANSACTION_STATUSES.APPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 4, status: TRANSACTION_STATUSES.SIGNED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 5, status: TRANSACTION_STATUSES.SUBMITTED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 6, status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, { id: 7, status: TRANSACTION_STATUSES.FAILED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS_TWO, + }, history: [{}], }, ]); diff --git a/app/scripts/controllers/transactions/lib/util.js b/app/scripts/controllers/transactions/lib/util.js index f1cb1f661..70652a3c1 100644 --- a/app/scripts/controllers/transactions/lib/util.js +++ b/app/scripts/controllers/transactions/lib/util.js @@ -14,6 +14,12 @@ const normalizers = { gasPrice: (gasPrice) => addHexPrefix(gasPrice), }; +export function normalizeAndValidateTxParams(txParams, lowerCase = true) { + const normalizedTxParams = normalizeTxParams(txParams, lowerCase); + validateTxParams(normalizedTxParams); + return normalizedTxParams; +} + /** * Normalizes the given txParams * @param {Object} txParams - The transaction params @@ -49,22 +55,48 @@ export function validateTxParams(txParams) { ); } - validateFrom(txParams); - validateRecipient(txParams); - if ('value' in txParams) { - const value = txParams.value.toString(); - if (value.includes('-')) { - throw ethErrors.rpc.invalidParams( - `Invalid transaction value "${txParams.value}": not a positive number.`, - ); - } + Object.entries(txParams).forEach(([key, value]) => { + // validate types + switch (key) { + case 'from': + validateFrom(txParams); + break; + case 'to': + validateRecipient(txParams); + break; + case 'value': + if (typeof value !== 'string') { + throw ethErrors.rpc.invalidParams( + `Invalid transaction params: ${key} is not a string. got: (${value})`, + ); + } + if (value.toString().includes('-')) { + throw ethErrors.rpc.invalidParams( + `Invalid transaction value "${value}": not a positive number.`, + ); + } - if (value.includes('.')) { - throw ethErrors.rpc.invalidParams( - `Invalid transaction value of "${txParams.value}": number must be in wei.`, - ); + if (value.toString().includes('.')) { + throw ethErrors.rpc.invalidParams( + `Invalid transaction value of "${value}": number must be in wei.`, + ); + } + break; + case 'chainId': + if (typeof value !== 'number' && typeof value !== 'string') { + throw ethErrors.rpc.invalidParams( + `Invalid transaction params: ${key} is not a Number or hex string. got: (${value})`, + ); + } + break; + default: + if (typeof value !== 'string') { + throw ethErrors.rpc.invalidParams( + `Invalid transaction params: ${key} is not a string. got: (${value})`, + ); + } } - } + }); } /** diff --git a/app/scripts/controllers/transactions/tx-state-manager.js b/app/scripts/controllers/transactions/tx-state-manager.js index 32d10509b..9eaa260a1 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.js +++ b/app/scripts/controllers/transactions/tx-state-manager.js @@ -1,6 +1,7 @@ import EventEmitter from 'safe-event-emitter'; import { ObservableStore } from '@metamask/obs-store'; import log from 'loglevel'; +import { keyBy, mapValues, omitBy, pickBy, sortBy } from 'lodash'; import createId from '../../../../shared/modules/random-id'; import { TRANSACTION_STATUSES } from '../../../../shared/constants/transaction'; import { METAMASK_CONTROLLER_EVENTS } from '../../metamask-controller'; @@ -10,11 +11,29 @@ import { replayHistory, snapshotFromTxMeta, } from './lib/tx-state-history-helpers'; -import { getFinalStates, normalizeTxParams } from './lib/util'; +import { getFinalStates, normalizeAndValidateTxParams } from './lib/util'; /** * TransactionStatuses reimported from the shared transaction constants file - * @typedef {import('../../../../shared/constants/transaction').TransactionStatuses} TransactionStatuses + * @typedef {import( + * '../../../../shared/constants/transaction' + * ).TransactionStatusString} TransactionStatusString + */ + +/** + * @typedef {import('../../../../shared/constants/transaction').TxParams} TxParams + */ + +/** + * @typedef {import( + * '../../../../shared/constants/transaction' + * ).TransactionMeta} TransactionMeta + */ + +/** + * @typedef {Object} TransactionState + * @property {Record} transactions - TransactionMeta + * keyed by the transaction's id. */ /** @@ -22,7 +41,8 @@ import { getFinalStates, normalizeTxParams } from './lib/util'; * storing the transaction. It also has some convenience methods for finding * subsets of transactions. * @param {Object} opts - * @param {Object} [opts.initState={ transactions: [] }] - initial transactions list with the key transaction {Array} + * @param {TransactionState} [opts.initState={ transactions: {} }] - initial + * transactions list keyed by id * @param {number} [opts.txHistoryLimit] - limit for how many finished * transactions can hang around in state * @param {Function} opts.getNetwork - return network number @@ -32,15 +52,25 @@ export default class TransactionStateManager extends EventEmitter { constructor({ initState, txHistoryLimit, getNetwork, getCurrentChainId }) { super(); - this.store = new ObservableStore({ transactions: [], ...initState }); + this.store = new ObservableStore({ + transactions: {}, + ...initState, + }); this.txHistoryLimit = txHistoryLimit; this.getNetwork = getNetwork; this.getCurrentChainId = getCurrentChainId; } /** - * @param {Object} opts - the object to use when overwriting defaults - * @returns {txMeta} the default txMeta object + * Generates a TransactionMeta object consisting of the fields required for + * use throughout the extension. The argument here will override everything + * in the resulting transaction meta. + * + * TODO: Don't overwrite everything? + * + * @param {Partial} opts - the object to use when + * overwriting default keys of the TransactionMeta + * @returns {TransactionMeta} the default txMeta object */ generateTxMeta(opts) { const netId = this.getNetwork(); @@ -60,100 +90,70 @@ export default class TransactionStateManager extends EventEmitter { } /** - * Returns the full tx list for the current network + * Get an object containing all unapproved transactions for the current + * network. This is the only transaction fetching method that returns an + * object, so it doesn't use getTransactions like everything else. * - * The list is iterated backwards as new transactions are pushed onto it. - * - * @param {number} [limit] - a limit for the number of transactions to return - * @returns {Object[]} The {@code txMeta}s, filtered to the current network - */ - getTxList(limit) { - const network = this.getNetwork(); - const chainId = this.getCurrentChainId(); - const fullTxList = this.getFullTxList(); - - const nonces = new Set(); - const txs = []; - for (let i = fullTxList.length - 1; i > -1; i--) { - const txMeta = fullTxList[i]; - if (transactionMatchesNetwork(txMeta, chainId, network) === false) { - continue; - } - - if (limit !== undefined) { - const { nonce } = txMeta.txParams; - if (!nonces.has(nonce)) { - if (nonces.size < limit) { - nonces.add(nonce); - } else { - continue; - } - } - } - - txs.unshift(txMeta); - } - return txs; - } - - /** - * @returns {Array} of all the txMetas in store - */ - getFullTxList() { - return this.store.getState().transactions; - } - - /** - * @returns {Array} the tx list with unapproved status + * @returns {Record} Unapproved transactions keyed + * by id */ getUnapprovedTxList() { - const txList = this.getTxsByMetaData( - 'status', - TRANSACTION_STATUSES.UNAPPROVED, + const chainId = this.getCurrentChainId(); + const network = this.getNetwork(); + return pickBy( + this.store.getState().transactions, + (transaction) => + transaction.status === TRANSACTION_STATUSES.UNAPPROVED && + transactionMatchesNetwork(transaction, chainId, network), ); - return txList.reduce((result, tx) => { - result[tx.id] = tx; - return result; - }, {}); } /** - * @param {string} [address] - hex prefixed address to sort the txMetas for [optional] - * @returns {Array} the tx list with approved status if no address is provide - * returns all txMetas with approved statuses for the current network + * Get all approved transactions for the current network. If an address is + * provided, the list will be further refined to only those transactions + * originating from the supplied address. + * + * @param {string} [address] - hex prefixed address to find transactions for. + * @returns {TransactionMeta[]} the filtered list of transactions */ getApprovedTransactions(address) { - const opts = { status: TRANSACTION_STATUSES.APPROVED }; + const searchCriteria = { status: TRANSACTION_STATUSES.APPROVED }; if (address) { - opts.from = address; + searchCriteria.from = address; } - return this.getFilteredTxList(opts); + return this.getTransactions({ searchCriteria }); } /** - * @param {string} [address] - hex prefixed address to sort the txMetas for [optional] - * @returns {Array} the tx list submitted status if no address is provide - * returns all txMetas with submitted statuses for the current network + * Get all pending transactions for the current network. If an address is + * provided, the list will be further refined to only those transactions + * originating from the supplied address. + * + * @param {string} [address] - hex prefixed address to find transactions for. + * @returns {TransactionMeta[]} the filtered list of transactions */ getPendingTransactions(address) { - const opts = { status: TRANSACTION_STATUSES.SUBMITTED }; + const searchCriteria = { status: TRANSACTION_STATUSES.SUBMITTED }; if (address) { - opts.from = address; + searchCriteria.from = address; } - return this.getFilteredTxList(opts); + return this.getTransactions({ searchCriteria }); } /** - @param {string} [address] - hex prefixed address to sort the txMetas for [optional] - @returns {Array} the tx list whose status is confirmed if no address is provide - returns all txMetas who's status is confirmed for the current network - */ + * Get all confirmed transactions for the current network. If an address is + * provided, the list will be further refined to only those transactions + * originating from the supplied address. + * + * @param {string} [address] - hex prefixed address to find transactions for. + * @returns {TransactionMeta[]} the filtered list of transactions + */ getConfirmedTransactions(address) { - const opts = { status: TRANSACTION_STATUSES.CONFIRMED }; + const searchCriteria = { status: TRANSACTION_STATUSES.CONFIRMED }; if (address) { - opts.from = address; + searchCriteria.from = address; } - return this.getFilteredTxList(opts); + return this.getTransactions({ searchCriteria }); } /** @@ -162,13 +162,14 @@ export default class TransactionStateManager extends EventEmitter { * is in its final state. * it will also add the key `history` to the txMeta with the snap shot of * the original object - * @param {Object} txMeta - * @returns {Object} the txMeta + * @param {TransactionMeta} txMeta - The TransactionMeta object to add. + * @returns {TransactionMeta} The same TransactionMeta, but with validated + * txParams and transaction history. */ - addTx(txMeta) { + addTransaction(txMeta) { // normalize and validate txParams if present if (txMeta.txParams) { - txMeta.txParams = this.normalizeAndValidateTxParams(txMeta.txParams); + txMeta.txParams = normalizeAndValidateTxParams(txMeta.txParams, false); } this.once(`${txMeta.id}:signed`, () => { @@ -183,42 +184,43 @@ export default class TransactionStateManager extends EventEmitter { const snapshot = snapshotFromTxMeta(txMeta); txMeta.history.push(snapshot); - const transactions = this.getFullTxList(); + const transactions = this.getTransactions({ + filterToCurrentNetwork: false, + }); const txCount = transactions.length; const { txHistoryLimit } = this; - // checks if the length of the tx history is - // longer then desired persistence limit - // and then if it is removes only confirmed - // or rejected tx's. - // not tx's that are pending or unapproved + // checks if the length of the tx history is longer then desired persistence + // limit and then if it is removes the oldest confirmed or rejected tx. + // Pending or unapproved transactions will not be removed by this + // operation. + // + // TODO: we are already limiting what we send to the UI, and in the future + // we will send UI only collected groups of transactions *per page* so at + // some point in the future, this persistence limit can be adjusted. When + // we do that I think we should figure out a better storage solution for + // transaction history entries. if (txCount > txHistoryLimit - 1) { const index = transactions.findIndex((metaTx) => { return getFinalStates().includes(metaTx.status); }); if (index !== -1) { - transactions.splice(index, 1); + this._deleteTransaction(transactions[index].id); } } - const newTxIndex = transactions.findIndex( - (currentTxMeta) => currentTxMeta.time > txMeta.time, - ); - newTxIndex === -1 - ? transactions.push(txMeta) - : transactions.splice(newTxIndex, 0, txMeta); - this._saveTxList(transactions); + this._addTransactionsToState([txMeta]); return txMeta; } /** * @param {number} txId - * @returns {Object} the txMeta who matches the given id if none found + * @returns {TransactionMeta} the txMeta who matches the given id if none found * for the network returns undefined */ - getTx(txId) { - const txMeta = this.getTxsByMetaData('id', txId)[0]; - return txMeta; + getTransaction(txId) { + const { transactions } = this.store.getState(); + return transactions[txId]; } /** @@ -226,10 +228,10 @@ export default class TransactionStateManager extends EventEmitter { * @param {Object} txMeta - the txMeta to update * @param {string} [note] - a note about the update for history */ - updateTx(txMeta, note) { + updateTransaction(txMeta, note) { // normalize and validate txParams if present if (txMeta.txParams) { - txMeta.txParams = this.normalizeAndValidateTxParams(txMeta.txParams); + txMeta.txParams = normalizeAndValidateTxParams(txMeta.txParams, false); } // create txMeta snapshot for history @@ -244,232 +246,277 @@ export default class TransactionStateManager extends EventEmitter { // commit txMeta to state const txId = txMeta.id; - const txList = this.getFullTxList(); - const index = txList.findIndex((txData) => txData.id === txId); - txList[index] = txMeta; - this._saveTxList(txList); + this.store.updateState({ + transactions: { + ...this.store.getState().transactions, + [txId]: txMeta, + }, + }); } /** - * merges txParams obj onto txMeta.txParams use extend to ensure - * that all fields are filled - * @param {number} txId - the id of the txMeta - * @param {Object} txParams - the updated txParams + * SearchCriteria can search in any key in TxParams or the base + * TransactionMeta. This type represents any key on either of those two + * types. + * @typedef {TxParams[keyof TxParams] | TransactionMeta[keyof TransactionMeta]} SearchableKeys */ - updateTxParams(txId, txParams) { - const txMeta = this.getTx(txId); - txMeta.txParams = { ...txMeta.txParams, ...txParams }; - this.updateTx(txMeta, `txStateManager#updateTxParams`); - } /** - * normalize and validate txParams members - * @param {Object} txParams - txParams + * Predicates can either be strict values, which is shorthand for using + * strict equality, or a method that receives he value of the specified key + * and returns a boolean. + * @typedef {(v: unknown) => boolean | unknown} FilterPredicate */ - normalizeAndValidateTxParams(txParams) { - if (typeof txParams.data === 'undefined') { - delete txParams.data; + + /** + * Retrieve a list of transactions from state. By default this will return + * the full list of Transactions for the currently selected chain/network. + * Additional options can be provided to change what is included in the final + * list. + * + * @param opts - options to change filter behavior + * @param {Record} [opts.searchCriteria] - + * an object with keys that match keys in TransactionMeta or TxParams, and + * values that are predicates. Predicates can either be strict values, + * which is shorthand for using strict equality, or a method that receives + * the value of the specified key and returns a boolean. The transaction + * list will be filtered to only those items that the predicate returns + * truthy for. **HINT**: `err: undefined` is like looking for a tx with no + * err. so you can also search txs that don't have something as well by + * setting the value as undefined. + * @param {TransactionMeta[]} [opts.initialList] - If provided the filtering + * will occur on the provided list. By default this will be the full list + * from state sorted by time ASC. + * @param {boolean} [opts.filterToCurrentNetwork=true] - Filter transaction + * list to only those that occurred on the current chain or network. + * Defaults to true. + * @param {number} [opts.limit] - limit the number of transactions returned + * to N unique nonces. + * @returns {TransactionMeta[]} The TransactionMeta objects that all provided + * predicates return truthy for. + */ + getTransactions({ + searchCriteria = {}, + initialList, + filterToCurrentNetwork = true, + limit, + } = {}) { + const chainId = this.getCurrentChainId(); + const network = this.getNetwork(); + // searchCriteria is an object that might have values that aren't predicate + // methods. When providing any other value type (string, number, etc), we + // consider this shorthand for "check the value at key for strict equality + // with the provided value". To conform this object to be only methods, we + // mapValues (lodash) such that every value on the object is a method that + // returns a boolean. + const predicateMethods = mapValues(searchCriteria, (predicate) => { + return typeof predicate === 'function' + ? predicate + : (v) => v === predicate; + }); + + // If an initial list is provided we need to change it back into an object + // first, so that it matches the shape of our state. This is done by the + // lodash keyBy method. This is the edge case for this method, typically + // initialList will be undefined. + const transactionsToFilter = initialList + ? keyBy(initialList, 'id') + : this.store.getState().transactions; + + // Combine sortBy and pickBy to transform our state object into an array of + // matching transactions that are sorted by time. + const filteredTransactions = sortBy( + pickBy(transactionsToFilter, (transaction) => { + // default matchesCriteria to the value of transactionMatchesNetwork + // when filterToCurrentNetwork is true. + if ( + filterToCurrentNetwork && + transactionMatchesNetwork(transaction, chainId, network) === false + ) { + return false; + } + // iterate over the predicateMethods keys to check if the transaction + // matches the searchCriteria + for (const [key, predicate] of Object.entries(predicateMethods)) { + // We return false early as soon as we know that one of the specified + // search criteria do not match the transaction. This prevents + // needlessly checking all criteria when we already know the criteria + // are not fully satisfied. We check both txParams and the base + // object as predicate keys can be either. + if (key in transaction.txParams) { + if (predicate(transaction.txParams[key]) === false) { + return false; + } + } else if (predicate(transaction[key]) === false) { + return false; + } + } + + return true; + }), + 'time', + ); + if (limit !== undefined) { + // We need to have all transactions of a given nonce in order to display + // necessary details in the UI. We use the size of this set to determine + // whether we have reached the limit provided, thus ensuring that all + // transactions of nonces we include will be sent to the UI. + const nonces = new Set(); + const txs = []; + // By default, the transaction list we filter from is sorted by time ASC. + // To ensure that filtered results prefers the newest transactions we + // iterate from right to left, inserting transactions into front of a new + // array. The original order is preserved, but we ensure that newest txs + // are preferred. + for (let i = filteredTransactions.length - 1; i > -1; i--) { + const txMeta = filteredTransactions[i]; + const { nonce } = txMeta.txParams; + if (!nonces.has(nonce)) { + if (nonces.size < limit) { + nonces.add(nonce); + } else { + continue; + } + } + // Push transaction into the beginning of our array to ensure the + // original order is preserved. + txs.unshift(txMeta); + } + return txs; } - // eslint-disable-next-line no-param-reassign - txParams = normalizeTxParams(txParams, false); - this.validateTxParams(txParams); - return txParams; + return filteredTransactions; } /** - * validates txParams members by type - * @param {Object} txParams - txParams to validate - */ - validateTxParams(txParams) { - Object.keys(txParams).forEach((key) => { - const value = txParams[key]; - // validate types - switch (key) { - case 'chainId': - if (typeof value !== 'number' && typeof value !== 'string') { - throw new Error( - `${key} in txParams is not a Number or hex string. got: (${value})`, - ); - } - break; - default: - if (typeof value !== 'string') { - throw new Error( - `${key} in txParams is not a string. got: (${value})`, - ); - } - break; - } - }); - } - - /** - @param {Object} opts - an object of fields to search for eg:
- let thingsToLookFor = {
- to: '0x0..',
- from: '0x0..',
- status: 'signed', \\ (status) => status !== 'rejected' give me all txs who's status is not rejected
- err: undefined,
- }
 - optionally the values of the keys can be functions for situations like where - you want all but one status. - @param {Array} [initialList=this.getTxList()] - @returns {Array} array of txMeta with all - options matching - */ - /* - ****************HINT**************** - | `err: undefined` is like looking | - | for a tx with no err | - | so you can also search txs that | - | dont have something as well by | - | setting the value as undefined | - ************************************ - - this is for things like filtering a the tx list - for only tx's from 1 account - or for filtering for all txs from one account - and that have been 'confirmed' - */ - getFilteredTxList(opts, initialList) { - let filteredTxList = initialList; - Object.keys(opts).forEach((key) => { - filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList); - }); - return filteredTxList; - } - - /** - * @param {string} key - the key to check - * @param {any} value - the value your looking for can also be a function that returns a bool - * @param {Array} [txList=this.getTxList()] - the list to search. default is the txList - * from txStateManager#getTxList - * @returns {Array} a list of txMetas who matches the search params - */ - getTxsByMetaData(key, value, txList = this.getTxList()) { - const filter = typeof value === 'function' ? value : (v) => v === value; - - return txList.filter((txMeta) => { - if (key in txMeta.txParams) { - return filter(txMeta.txParams[key]); - } - return filter(txMeta[key]); - }); - } - - // get::set status - - /** - * @param {number} txId - the txMeta Id - * @returns {string} the status of the tx. - */ - getTxStatus(txId) { - const txMeta = this.getTx(txId); - return txMeta.status; - } - - /** - * Update the status of the tx to 'rejected'. - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'rejected'. + * After setting the status, the TransactionMeta is deleted from state. + * + * TODO: Should we show historically rejected transactions somewhere in the + * UI? Seems like it could be valuable for information purposes. Of course + * only after limit issues are reduced. + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusRejected(txId) { - this._setTxStatus(txId, 'rejected'); - this._removeTx(txId); + this._setTransactionStatus(txId, 'rejected'); + this._deleteTransaction(txId); } /** - * Update the status of the tx to 'unapproved'. - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'unapproved' + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusUnapproved(txId) { - this._setTxStatus(txId, TRANSACTION_STATUSES.UNAPPROVED); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.UNAPPROVED); } /** - * Update the status of the tx to 'approved'. - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'approved' + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusApproved(txId) { - this._setTxStatus(txId, TRANSACTION_STATUSES.APPROVED); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.APPROVED); } /** - * Update the status of the tx to 'signed'. - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'signed' + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusSigned(txId) { - this._setTxStatus(txId, TRANSACTION_STATUSES.SIGNED); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.SIGNED); } /** - * Update the status of the tx to 'submitted' and add a time stamp - * for when it was called - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'submitted' + * and sets the 'submittedTime' property with the current Unix epoch time. + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusSubmitted(txId) { - const txMeta = this.getTx(txId); + const txMeta = this.getTransaction(txId); txMeta.submittedTime = new Date().getTime(); - this.updateTx(txMeta, 'txStateManager - add submitted time stamp'); - this._setTxStatus(txId, TRANSACTION_STATUSES.SUBMITTED); + this.updateTransaction(txMeta, 'txStateManager - add submitted time stamp'); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.SUBMITTED); } /** - * Update the status of the tx to 'confirmed'. - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'confirmed' + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusConfirmed(txId) { - this._setTxStatus(txId, TRANSACTION_STATUSES.CONFIRMED); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.CONFIRMED); } /** - * Update the status of the tx to 'dropped'. - * @param {number} txId - the txMeta Id + * Update status of the TransactionMeta with provided id to 'dropped' + * + * @param {number} txId - the target TransactionMeta's Id */ setTxStatusDropped(txId) { - this._setTxStatus(txId, TRANSACTION_STATUSES.DROPPED); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.DROPPED); } /** - * Updates the status of the tx to 'failed' and put the error on the txMeta - * @param {number} txId - the txMeta Id - * @param {erroObject} err - error object + * Update status of the TransactionMeta with provided id to 'failed' and put + * the error on the TransactionMeta object. + * + * @param {number} txId - the target TransactionMeta's Id + * @param {Error} err - error object */ setTxStatusFailed(txId, err) { const error = err || new Error('Internal metamask failure'); - const txMeta = this.getTx(txId); + const txMeta = this.getTransaction(txId); txMeta.err = { message: error.toString(), rpc: error.value, stack: error.stack, }; - this.updateTx(txMeta, 'transactions:tx-state-manager#fail - add error'); - this._setTxStatus(txId, TRANSACTION_STATUSES.FAILED); + this.updateTransaction( + txMeta, + 'transactions:tx-state-manager#fail - add error', + ); + this._setTransactionStatus(txId, TRANSACTION_STATUSES.FAILED); } /** - * Removes transaction from the given address for the current network - * from the txList + * Removes all transactions for the given address on the current network, + * preferring chainId for comparison over networkId. + * * @param {string} address - hex string of the from address on the txParams * to remove */ wipeTransactions(address) { // network only tx - const txs = this.getFullTxList(); + const { transactions } = this.store.getState(); const network = this.getNetwork(); const chainId = this.getCurrentChainId(); - // Filter out the ones from the current account and network - const otherAccountTxs = txs.filter( - (txMeta) => - !( - txMeta.txParams.from === address && - transactionMatchesNetwork(txMeta, chainId, network) - ), - ); - // Update state - this._saveTxList(otherAccountTxs); + this.store.updateState({ + transactions: omitBy( + transactions, + (transaction) => + transaction.txParams.from === address && + transactionMatchesNetwork(transaction, chainId, network), + ), + }); + } + + /** + * Filters out the unapproved transactions from state + */ + clearUnapprovedTxs() { + this.store.updateState({ + transactions: omitBy( + this.store.getState().transactions, + (transaction) => transaction.status === TRANSACTION_STATUSES.UNAPPROVED, + ), + }); } // @@ -477,14 +524,37 @@ export default class TransactionStateManager extends EventEmitter { // /** - * @param {number} txId - the txMeta Id - * @param {TransactionStatuses[keyof TransactionStatuses]} status - the status to set on the txMeta - * @emits tx:status-update - passes txId and status - * @emits ${txMeta.id}:finished - if it is a finished state. Passes the txMeta - * @emits 'updateBadge' + * Updates a transaction's status in state, and then emits events that are + * subscribed to elsewhere. See below for best guesses on where and how these + * events are received. + * @param {number} txId - the TransactionMeta Id + * @param {TransactionStatusString} status - the status to set on the + * TransactionMeta + * @emits txMeta.id:txMeta.status - every time a transaction's status changes + * we emit the change passing along the id. This does not appear to be used + * outside of this file, which only listens to this to unsubscribe listeners + * of :rejected and :signed statuses when the inverse status changes. Likely + * safe to drop. + * @emits tx:status-update - every time a transaction's status changes we + * emit this event and pass txId and status. This event is subscribed to in + * the TransactionController and re-broadcast by the TransactionController. + * It is used internally within the TransactionController to try and update + * pending transactions on each new block (from blockTracker). It's also + * subscribed to in metamask-controller to display a browser notification on + * confirmed or failed transactions. + * @emits txMeta.id:finished - When a transaction moves to a finished state + * this event is emitted, which is used in the TransactionController to pass + * along details of the transaction to the dapp that suggested them. This + * pattern is replicated across all of the message managers and can likely + * be supplemented or replaced by the ApprovalController. + * @emits updateBadge - When the number of transactions changes in state, + * the badge in the browser extension bar should be updated to reflect the + * number of pending transactions. This particular emit doesn't appear to + * bubble up anywhere that is actually used. TransactionController emits + * this *anytime the state changes*, so this is probably superfluous. */ - _setTxStatus(txId, status) { - const txMeta = this.getTx(txId); + _setTransactionStatus(txId, status) { + const txMeta = this.getTransaction(txId); if (!txMeta) { return; @@ -492,7 +562,10 @@ export default class TransactionStateManager extends EventEmitter { txMeta.status = status; try { - this.updateTx(txMeta, `txStateManager: setting status to ${status}`); + this.updateTransaction( + txMeta, + `txStateManager: setting status to ${status}`, + ); this.emit(`${txMeta.id}:${status}`, txId); this.emit(`tx:status-update`, txId, status); if ( @@ -511,26 +584,32 @@ export default class TransactionStateManager extends EventEmitter { } /** - * Saves the new/updated txList. Intended only for internal use - * @param {Array} transactions - the list of transactions to save + * Adds one or more transactions into state. This is not intended for + * external use. + * + * @private + * @param {TransactionMeta[]} transactions - the list of transactions to save */ - _saveTxList(transactions) { - this.store.updateState({ transactions }); - } - - _removeTx(txId) { - const transactionList = this.getFullTxList(); - this._saveTxList(transactionList.filter((txMeta) => txMeta.id !== txId)); + _addTransactionsToState(transactions) { + this.store.updateState({ + transactions: transactions.reduce((result, newTx) => { + result[newTx.id] = newTx; + return result; + }, this.store.getState().transactions), + }); } /** - * Filters out the unapproved transactions + * removes one transaction from state. This is not intended for external use. + * + * @private + * @param {number} targetTransactionId - the transaction to delete */ - clearUnapprovedTxs() { - const transactions = this.getFullTxList(); - const nonUnapprovedTxs = transactions.filter( - (tx) => tx.status !== TRANSACTION_STATUSES.UNAPPROVED, - ); - this._saveTxList(nonUnapprovedTxs); + _deleteTransaction(targetTransactionId) { + const { transactions } = this.store.getState(); + delete transactions[targetTransactionId]; + this.store.updateState({ + transactions, + }); } } diff --git a/app/scripts/controllers/transactions/tx-state-manager.test.js b/app/scripts/controllers/transactions/tx-state-manager.test.js index f9b421eeb..79c764643 100644 --- a/app/scripts/controllers/transactions/tx-state-manager.test.js +++ b/app/scripts/controllers/transactions/tx-state-manager.test.js @@ -8,8 +8,8 @@ import { import TxStateManager from './tx-state-manager'; import { snapshotFromTxMeta } from './lib/tx-state-history-helpers'; -const noop = () => true; - +const VALID_ADDRESS = '0x0000000000000000000000000000000000000000'; +const VALID_ADDRESS_TWO = '0x0000000000000000000000000000000000000001'; describe('TransactionStateManager', function () { let txStateManager; const currentNetworkId = KOVAN_NETWORK_ID; @@ -19,7 +19,7 @@ describe('TransactionStateManager', function () { beforeEach(function () { txStateManager = new TxStateManager({ initState: { - transactions: [], + transactions: {}, }, txHistoryLimit: 10, getNetwork: () => currentNetworkId, @@ -33,11 +33,14 @@ describe('TransactionStateManager', function () { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx, noop); + txStateManager.addTransaction(tx); txStateManager.setTxStatusSigned(1); - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.ok(Array.isArray(result)); assert.equal(result.length, 1); assert.equal(result[0].status, TRANSACTION_STATUSES.SIGNED); @@ -48,12 +51,15 @@ describe('TransactionStateManager', function () { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; const clock = sinon.useFakeTimers(); const onSigned = sinon.spy(); - txStateManager.addTx(tx); + txStateManager.addTransaction(tx); txStateManager.on('1:signed', onSigned); txStateManager.setTxStatusSigned(1); clock.runAll(); @@ -69,11 +75,14 @@ describe('TransactionStateManager', function () { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx); + txStateManager.addTransaction(tx); txStateManager.setTxStatusRejected(1); - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.ok(Array.isArray(result)); assert.equal(result.length, 0); }); @@ -83,12 +92,15 @@ describe('TransactionStateManager', function () { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; const clock = sinon.useFakeTimers(); const onSigned = sinon.spy(); - txStateManager.addTx(tx); + txStateManager.addTransaction(tx); txStateManager.on('1:rejected', onSigned); txStateManager.setTxStatusRejected(1); clock.runAll(); @@ -98,17 +110,9 @@ describe('TransactionStateManager', function () { }); }); - describe('#getFullTxList', function () { + describe('#getTransactions', function () { it('when new should return empty array', function () { - const result = txStateManager.getTxList(); - assert.ok(Array.isArray(result)); - assert.equal(result.length, 0); - }); - }); - - describe('#getTxList', function () { - it('when new should return empty array', function () { - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.ok(Array.isArray(result)); assert.equal(result.length, 0); }); @@ -140,13 +144,16 @@ describe('TransactionStateManager', function () { const txm = new TxStateManager({ initState: { - transactions: [submittedTx, confirmedTx], + transactions: { + [submittedTx.id]: submittedTx, + [confirmedTx.id]: confirmedTx, + }, }, getNetwork: () => currentNetworkId, getCurrentChainId: () => currentChainId, }); - assert.deepEqual(txm.getTxList(), [submittedTx, confirmedTx]); + assert.deepEqual(txm.getTransactions(), [submittedTx, confirmedTx]); }); it('should return a list of transactions, limited by N unique nonces when there are NO duplicates', function () { @@ -200,48 +207,49 @@ describe('TransactionStateManager', function () { const txm = new TxStateManager({ initState: { - transactions: [ - submittedTx0, - unapprovedTx1, - approvedTx2, - confirmedTx3, - ], + transactions: { + [submittedTx0.id]: submittedTx0, + [unapprovedTx1.id]: unapprovedTx1, + [approvedTx2.id]: approvedTx2, + [confirmedTx3.id]: confirmedTx3, + }, }, getNetwork: () => currentNetworkId, getCurrentChainId: () => currentChainId, }); - assert.deepEqual(txm.getTxList(2), [approvedTx2, confirmedTx3]); + assert.deepEqual(txm.getTransactions({ limit: 2 }), [ + approvedTx2, + confirmedTx3, + ]); }); it('should return a list of transactions, limited by N unique nonces when there ARE duplicates', function () { - const submittedTx0s = [ - { - id: 0, - metamaskNetworkId: currentNetworkId, - time: 0, - txParams: { - from: '0xAddress', - to: '0xRecipient', - nonce: '0x0', - }, - status: TRANSACTION_STATUSES.SUBMITTED, + const submittedTx0 = { + id: 0, + metamaskNetworkId: currentNetworkId, + time: 0, + txParams: { + from: '0xAddress', + to: '0xRecipient', + nonce: '0x0', }, - { - id: 0, - metamaskNetworkId: currentNetworkId, - time: 0, - txParams: { - from: '0xAddress', - to: '0xRecipient', - nonce: '0x0', - }, - status: TRANSACTION_STATUSES.SUBMITTED, + status: TRANSACTION_STATUSES.SUBMITTED, + }; + const submittedTx0Dupe = { + id: 1, + metamaskNetworkId: currentNetworkId, + time: 0, + txParams: { + from: '0xAddress', + to: '0xRecipient', + nonce: '0x0', }, - ]; + status: TRANSACTION_STATUSES.SUBMITTED, + }; const unapprovedTx1 = { - id: 1, + id: 2, metamaskNetworkId: currentNetworkId, chainId: currentChainId, time: 1, @@ -253,85 +261,215 @@ describe('TransactionStateManager', function () { status: TRANSACTION_STATUSES.UNAPPROVED, }; - const approvedTx2s = [ - { - id: 2, - metamaskNetworkId: currentNetworkId, - time: 2, - txParams: { - from: '0xAddress', - to: '0xRecipient', - nonce: '0x2', - }, - status: TRANSACTION_STATUSES.APPROVED, + const approvedTx2 = { + id: 3, + metamaskNetworkId: currentNetworkId, + time: 2, + txParams: { + from: '0xAddress', + to: '0xRecipient', + nonce: '0x2', }, - { - id: 2, - metamaskNetworkId: currentNetworkId, - chainId: currentChainId, - time: 2, - txParams: { - from: '0xAddress', - to: '0xRecipient', - nonce: '0x2', - }, - status: TRANSACTION_STATUSES.APPROVED, + status: TRANSACTION_STATUSES.APPROVED, + }; + const approvedTx2Dupe = { + id: 4, + metamaskNetworkId: currentNetworkId, + chainId: currentChainId, + time: 2, + txParams: { + from: '0xAddress', + to: '0xRecipient', + nonce: '0x2', }, - ]; + status: TRANSACTION_STATUSES.APPROVED, + }; - const failedTx3s = [ - { - id: 3, - metamaskNetworkId: currentNetworkId, - time: 3, - txParams: { - from: '0xAddress', - to: '0xRecipient', - nonce: '0x3', - }, - status: TRANSACTION_STATUSES.FAILED, + const failedTx3 = { + id: 5, + metamaskNetworkId: currentNetworkId, + time: 3, + txParams: { + from: '0xAddress', + to: '0xRecipient', + nonce: '0x3', }, - { - id: 3, - metamaskNetworkId: currentNetworkId, - chainId: currentChainId, - time: 3, - txParams: { - from: '0xAddress', - to: '0xRecipient', - nonce: '0x3', - }, - status: TRANSACTION_STATUSES.FAILED, + status: TRANSACTION_STATUSES.FAILED, + }; + const failedTx3Dupe = { + id: 6, + metamaskNetworkId: currentNetworkId, + chainId: currentChainId, + time: 3, + txParams: { + from: '0xAddress', + to: '0xRecipient', + nonce: '0x3', }, - ]; + status: TRANSACTION_STATUSES.FAILED, + }; const txm = new TxStateManager({ initState: { - transactions: [ - ...submittedTx0s, - unapprovedTx1, - ...approvedTx2s, - ...failedTx3s, - ], + transactions: { + [submittedTx0.id]: submittedTx0, + [submittedTx0Dupe.id]: submittedTx0Dupe, + + [unapprovedTx1.id]: unapprovedTx1, + [approvedTx2.id]: approvedTx2, + [approvedTx2Dupe.id]: approvedTx2Dupe, + + [failedTx3.id]: failedTx3, + [failedTx3Dupe.id]: failedTx3Dupe, + }, }, getNetwork: () => currentNetworkId, getCurrentChainId: () => currentChainId, }); - assert.deepEqual(txm.getTxList(2), [...approvedTx2s, ...failedTx3s]); + assert.deepEqual(txm.getTransactions({ limit: 2 }), [ + approvedTx2, + approvedTx2Dupe, + failedTx3, + failedTx3Dupe, + ]); + }); + + it('returns a tx with the requested data', function () { + const txMetas = [ + { + id: 0, + status: TRANSACTION_STATUSES.UNAPPROVED, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 1, + status: TRANSACTION_STATUSES.UNAPPROVED, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 2, + status: TRANSACTION_STATUSES.UNAPPROVED, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 3, + status: TRANSACTION_STATUSES.UNAPPROVED, + txParams: { from: VALID_ADDRESS_TWO, to: VALID_ADDRESS }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 4, + status: TRANSACTION_STATUSES.UNAPPROVED, + txParams: { from: VALID_ADDRESS_TWO, to: VALID_ADDRESS }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 5, + status: TRANSACTION_STATUSES.CONFIRMED, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 6, + status: TRANSACTION_STATUSES.CONFIRMED, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 7, + status: TRANSACTION_STATUSES.CONFIRMED, + txParams: { from: VALID_ADDRESS_TWO, to: VALID_ADDRESS }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 8, + status: TRANSACTION_STATUSES.CONFIRMED, + txParams: { from: VALID_ADDRESS_TWO, to: VALID_ADDRESS }, + metamaskNetworkId: currentNetworkId, + }, + { + id: 9, + status: TRANSACTION_STATUSES.CONFIRMED, + txParams: { from: VALID_ADDRESS_TWO, to: VALID_ADDRESS }, + metamaskNetworkId: currentNetworkId, + }, + ]; + txMetas.forEach((txMeta) => txStateManager.addTransaction(txMeta)); + let searchCriteria; + + searchCriteria = { + status: TRANSACTION_STATUSES.UNAPPROVED, + from: VALID_ADDRESS, + }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 3, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); + searchCriteria = { + status: TRANSACTION_STATUSES.UNAPPROVED, + to: VALID_ADDRESS, + }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 2, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); + searchCriteria = { + status: TRANSACTION_STATUSES.CONFIRMED, + from: VALID_ADDRESS_TWO, + }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 3, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); + searchCriteria = { status: TRANSACTION_STATUSES.CONFIRMED }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 5, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); + searchCriteria = { from: VALID_ADDRESS }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 5, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); + searchCriteria = { to: VALID_ADDRESS }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 5, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); + searchCriteria = { + status: (status) => status !== TRANSACTION_STATUSES.CONFIRMED, + }; + assert.equal( + txStateManager.getTransactions({ searchCriteria }).length, + 5, + `getTransactions - ${JSON.stringify(searchCriteria)}`, + ); }); }); - describe('#addTx', function () { - it('adds a tx returned in getTxList', function () { + describe('#addTransaction', function () { + it('adds a tx returned in getTransactions', function () { const tx = { id: 1, status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx, noop); - const result = txStateManager.getTxList(); + txStateManager.addTransaction(tx); + const result = txStateManager.getTransactions(); assert.ok(Array.isArray(result)); assert.equal(result.length, 1); assert.equal(result[0].id, 1); @@ -361,10 +499,10 @@ describe('TransactionStateManager', function () { }, }; assert.throws( - txStateManager.addTx.bind(txStateManager, tx), - 'addTx should throw error', + txStateManager.addTransaction.bind(txStateManager, tx), + 'addTransaction should throw error', ); - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.ok(Array.isArray(result), 'txList should be an array'); assert.equal(result.length, 0, 'txList should be empty'); } @@ -376,18 +514,26 @@ describe('TransactionStateManager', function () { id: 1, status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; const tx2 = { id: 2, status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: otherNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx, noop); - txStateManager.addTx(tx2, noop); - const result = txStateManager.getFullTxList(); - const result2 = txStateManager.getTxList(); + txStateManager.addTransaction(tx); + txStateManager.addTransaction(tx2); + const result = txStateManager.getTransactions({ + filterToCurrentNetwork: false, + }); + const result2 = txStateManager.getTransactions(); assert.equal(result.length, 2, 'txs were deleted'); assert.equal(result2.length, 1, 'incorrect number of txs on network.'); }); @@ -400,11 +546,14 @@ describe('TransactionStateManager', function () { time: new Date(), status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx, noop); + txStateManager.addTransaction(tx); } - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.equal(result.length, limit, `limit of ${limit} txs enforced`); assert.equal(result[0].id, 1, 'early txs truncated'); }); @@ -417,11 +566,14 @@ describe('TransactionStateManager', function () { time: new Date(), status: TRANSACTION_STATUSES.REJECTED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx, noop); + txStateManager.addTransaction(tx); } - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.equal(result.length, limit, `limit of ${limit} txs enforced`); assert.equal(result[0].id, 1, 'early txs truncated'); }); @@ -432,9 +584,12 @@ describe('TransactionStateManager', function () { time: new Date(), status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(unconfirmedTx, noop); + txStateManager.addTransaction(unconfirmedTx); const limit = txStateManager.txHistoryLimit; for (let i = 1; i < limit + 1; i++) { const tx = { @@ -442,11 +597,14 @@ describe('TransactionStateManager', function () { time: new Date(), status: TRANSACTION_STATUSES.CONFIRMED, metamaskNetworkId: currentNetworkId, - txParams: {}, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, + }, }; - txStateManager.addTx(tx, noop); + txStateManager.addTransaction(tx); } - const result = txStateManager.getTxList(); + const result = txStateManager.getTransactions(); assert.equal(result.length, limit, `limit of ${limit} txs enforced`); assert.equal(result[0].id, 0, 'first tx should still be there'); assert.equal( @@ -458,30 +616,30 @@ describe('TransactionStateManager', function () { }); }); - describe('#updateTx', function () { + describe('#updateTransaction', function () { it('replaces the tx with the same id', function () { - txStateManager.addTx( - { - id: '1', - status: TRANSACTION_STATUSES.UNAPPROVED, - metamaskNetworkId: currentNetworkId, - txParams: {}, + txStateManager.addTransaction({ + id: '1', + status: TRANSACTION_STATUSES.UNAPPROVED, + metamaskNetworkId: currentNetworkId, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, }, - noop, - ); - txStateManager.addTx( - { - id: '2', - status: TRANSACTION_STATUSES.CONFIRMED, - metamaskNetworkId: currentNetworkId, - txParams: {}, + }); + txStateManager.addTransaction({ + id: '2', + status: TRANSACTION_STATUSES.CONFIRMED, + metamaskNetworkId: currentNetworkId, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, }, - noop, - ); - const txMeta = txStateManager.getTx('1'); + }); + const txMeta = txStateManager.getTransaction('1'); txMeta.hash = 'foo'; - txStateManager.updateTx(txMeta); - const result = txStateManager.getTx('1'); + txStateManager.updateTransaction(txMeta); + const result = txStateManager.getTransaction('1'); assert.equal(result.hash, 'foo'); }); @@ -497,7 +655,7 @@ describe('TransactionStateManager', function () { }; const invalidValues = [1, true, {}, Symbol('1')]; - txStateManager.addTx({ + txStateManager.addTransaction({ id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, @@ -506,7 +664,7 @@ describe('TransactionStateManager', function () { Object.keys(validTxParams).forEach((key) => { for (const value of invalidValues) { - const originalTx = txStateManager.getTx(1); + const originalTx = txStateManager.getTransaction(1); const newTx = { ...originalTx, txParams: { @@ -515,10 +673,10 @@ describe('TransactionStateManager', function () { }, }; assert.throws( - txStateManager.updateTx.bind(txStateManager, newTx), - 'updateTx should throw an error', + txStateManager.updateTransaction.bind(txStateManager, newTx), + 'updateTransaction should throw an error', ); - const result = txStateManager.getTx(1); + const result = txStateManager.getTransaction(1); assert.deepEqual(result, originalTx, 'tx should not be updated'); } }); @@ -533,12 +691,14 @@ describe('TransactionStateManager', function () { status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, txParams: { + from: VALID_ADDRESS_TWO, + to: VALID_ADDRESS, gasPrice: originalGasPrice, }, }; - txStateManager.addTx(txMeta); - const updatedTx = txStateManager.getTx('1'); + txStateManager.addTransaction(txMeta); + const updatedTx = txStateManager.getTransaction('1'); // verify tx was initialized correctly assert.equal(updatedTx.history.length, 1, 'one history item (initial)'); assert.equal( @@ -551,13 +711,13 @@ describe('TransactionStateManager', function () { snapshotFromTxMeta(updatedTx), 'first history item is initial state', ); - // modify value and updateTx + // modify value and updateTransaction updatedTx.txParams.gasPrice = desiredGasPrice; const before = new Date().getTime(); - txStateManager.updateTx(updatedTx); + txStateManager.updateTransaction(updatedTx); const after = new Date().getTime(); // check updated value - const result = txStateManager.getTx('1'); + const result = txStateManager.getTransaction('1'); assert.equal( result.txParams.gasPrice, desiredGasPrice, @@ -607,38 +767,40 @@ describe('TransactionStateManager', function () { status: TRANSACTION_STATUSES.UNAPPROVED, metamaskNetworkId: currentNetworkId, txParams: { + from: VALID_ADDRESS_TWO, + to: VALID_ADDRESS, gasPrice: '0x01', }, }; - txStateManager.addTx(txMeta); - txStateManager.updateTx(txMeta); + txStateManager.addTransaction(txMeta); + txStateManager.updateTransaction(txMeta); - const { history } = txStateManager.getTx('1'); + const { history } = txStateManager.getTransaction('1'); assert.equal(history.length, 1, 'two history items (initial + diff)'); }); }); describe('#getUnapprovedTxList', function () { it('returns unapproved txs in a hash', function () { - txStateManager.addTx( - { - id: '1', - status: TRANSACTION_STATUSES.UNAPPROVED, - metamaskNetworkId: currentNetworkId, - txParams: {}, + txStateManager.addTransaction({ + id: '1', + status: TRANSACTION_STATUSES.UNAPPROVED, + metamaskNetworkId: currentNetworkId, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, }, - noop, - ); - txStateManager.addTx( - { - id: '2', - status: TRANSACTION_STATUSES.CONFIRMED, - metamaskNetworkId: currentNetworkId, - txParams: {}, + }); + txStateManager.addTransaction({ + id: '2', + status: TRANSACTION_STATUSES.CONFIRMED, + metamaskNetworkId: currentNetworkId, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, }, - noop, - ); + }); const result = txStateManager.getUnapprovedTxList(); assert.equal(typeof result, 'object'); assert.equal(result['1'].status, TRANSACTION_STATUSES.UNAPPROVED); @@ -646,154 +808,40 @@ describe('TransactionStateManager', function () { }); }); - describe('#getTx', function () { + describe('#getTransaction', function () { it('returns a tx with the requested id', function () { - txStateManager.addTx( - { - id: '1', - status: TRANSACTION_STATUSES.UNAPPROVED, - metamaskNetworkId: currentNetworkId, - txParams: {}, + txStateManager.addTransaction({ + id: '1', + status: TRANSACTION_STATUSES.UNAPPROVED, + metamaskNetworkId: currentNetworkId, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, }, - noop, - ); - txStateManager.addTx( - { - id: '2', - status: TRANSACTION_STATUSES.CONFIRMED, - metamaskNetworkId: currentNetworkId, - txParams: {}, + }); + txStateManager.addTransaction({ + id: '2', + status: TRANSACTION_STATUSES.CONFIRMED, + metamaskNetworkId: currentNetworkId, + txParams: { + to: VALID_ADDRESS, + from: VALID_ADDRESS, }, - noop, - ); + }); assert.equal( - txStateManager.getTx('1').status, + txStateManager.getTransaction('1').status, TRANSACTION_STATUSES.UNAPPROVED, ); assert.equal( - txStateManager.getTx('2').status, + txStateManager.getTransaction('2').status, TRANSACTION_STATUSES.CONFIRMED, ); }); }); - describe('#getFilteredTxList', function () { - it('returns a tx with the requested data', function () { - const txMetas = [ - { - id: 0, - status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xaa', to: '0xbb' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 1, - status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xaa', to: '0xbb' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 2, - status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xaa', to: '0xbb' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 3, - status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xbb', to: '0xaa' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 4, - status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xbb', to: '0xaa' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 5, - status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xaa', to: '0xbb' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 6, - status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xaa', to: '0xbb' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 7, - status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xbb', to: '0xaa' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 8, - status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xbb', to: '0xaa' }, - metamaskNetworkId: currentNetworkId, - }, - { - id: 9, - status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xbb', to: '0xaa' }, - metamaskNetworkId: currentNetworkId, - }, - ]; - txMetas.forEach((txMeta) => txStateManager.addTx(txMeta, noop)); - let filterParams; - - filterParams = { status: TRANSACTION_STATUSES.UNAPPROVED, from: '0xaa' }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 3, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - filterParams = { status: TRANSACTION_STATUSES.UNAPPROVED, to: '0xaa' }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 2, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - filterParams = { status: TRANSACTION_STATUSES.CONFIRMED, from: '0xbb' }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 3, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - filterParams = { status: TRANSACTION_STATUSES.CONFIRMED }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 5, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - filterParams = { from: '0xaa' }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 5, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - filterParams = { to: '0xaa' }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 5, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - filterParams = { - status: (status) => status !== TRANSACTION_STATUSES.CONFIRMED, - }; - assert.equal( - txStateManager.getFilteredTxList(filterParams).length, - 5, - `getFilteredTxList - ${JSON.stringify(filterParams)}`, - ); - }); - }); - describe('#wipeTransactions', function () { - const specificAddress = '0xaa'; - const otherAddress = '0xbb'; + const specificAddress = VALID_ADDRESS; + const otherAddress = VALID_ADDRESS_TWO; it('should remove only the transactions from a specific address', function () { const txMetas = [ @@ -816,15 +864,15 @@ describe('TransactionStateManager', function () { metamaskNetworkId: currentNetworkId, }, ]; - txMetas.forEach((txMeta) => txStateManager.addTx(txMeta, noop)); + txMetas.forEach((txMeta) => txStateManager.addTransaction(txMeta)); txStateManager.wipeTransactions(specificAddress); const transactionsFromCurrentAddress = txStateManager - .getTxList() + .getTransactions() .filter((txMeta) => txMeta.txParams.from === specificAddress); const transactionsFromOtherAddresses = txStateManager - .getTxList() + .getTransactions() .filter((txMeta) => txMeta.txParams.from !== specificAddress); assert.equal(transactionsFromCurrentAddress.length, 0); @@ -853,15 +901,15 @@ describe('TransactionStateManager', function () { }, ]; - txMetas.forEach((txMeta) => txStateManager.addTx(txMeta, noop)); + txMetas.forEach((txMeta) => txStateManager.addTransaction(txMeta)); txStateManager.wipeTransactions(specificAddress); const txsFromCurrentNetworkAndAddress = txStateManager - .getTxList() + .getTransactions() .filter((txMeta) => txMeta.txParams.from === specificAddress); const txFromOtherNetworks = txStateManager - .getFullTxList() + .getTransactions({ filterToCurrentNetwork: false }) .filter((txMeta) => txMeta.metamaskNetworkId === otherNetworkId); assert.equal(txsFromCurrentNetworkAndAddress.length, 0); @@ -869,21 +917,26 @@ describe('TransactionStateManager', function () { }); }); - describe('#_removeTx', function () { + describe('#_deleteTransaction', function () { it('should remove the transaction from the storage', function () { - txStateManager._saveTxList([{ id: 1 }]); - txStateManager._removeTx(1); + txStateManager.addTransaction({ id: 1 }); + txStateManager._deleteTransaction(1); assert.ok( - !txStateManager.getFullTxList().length, + !txStateManager.getTransactions({ filterToCurrentNetwork: false }) + .length, 'txList should be empty', ); }); it('should only remove the transaction with ID 1 from the storage', function () { - txStateManager._saveTxList([{ id: 1 }, { id: 2 }]); - txStateManager._removeTx(1); + txStateManager.store.updateState({ + transactions: { 1: { id: 1 }, 2: { id: 2 } }, + }); + txStateManager._deleteTransaction(1); assert.equal( - txStateManager.getFullTxList()[0].id, + txStateManager.getTransactions({ + filterToCurrentNetwork: false, + })[0].id, 2, 'txList should have a id of 2', ); @@ -896,35 +949,35 @@ describe('TransactionStateManager', function () { { id: 0, status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xaa', to: '0xbb' }, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, metamaskNetworkId: currentNetworkId, }, { id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, - txParams: { from: '0xaa', to: '0xbb' }, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, metamaskNetworkId: currentNetworkId, }, { id: 2, status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xaa', to: '0xbb' }, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, metamaskNetworkId: otherNetworkId, }, { id: 3, status: TRANSACTION_STATUSES.CONFIRMED, - txParams: { from: '0xaa', to: '0xbb' }, + txParams: { from: VALID_ADDRESS, to: VALID_ADDRESS_TWO }, metamaskNetworkId: otherNetworkId, }, ]; - txMetas.forEach((txMeta) => txStateManager.addTx(txMeta, noop)); + txMetas.forEach((txMeta) => txStateManager.addTransaction(txMeta)); txStateManager.clearUnapprovedTxs(); const unapprovedTxList = txStateManager - .getFullTxList() + .getTransactions({ filterToCurrentNetwork: false }) .filter((tx) => tx.status === TRANSACTION_STATUSES.UNAPPROVED); assert.equal(unapprovedTxList.length, 0); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 4403e65a4..7e3ec936d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -322,7 +322,7 @@ export default class MetamaskController extends EventEmitter { status === TRANSACTION_STATUSES.CONFIRMED || status === TRANSACTION_STATUSES.FAILED ) { - const txMeta = this.txController.txStateManager.getTx(txId); + const txMeta = this.txController.txStateManager.getTransaction(txId); const frequentRpcListDetail = this.preferencesController.getFrequentRpcListDetail(); let rpcPrefs = {}; if (txMeta.chainId) { @@ -504,9 +504,11 @@ export default class MetamaskController extends EventEmitter { processEncryptionPublicKey: this.newRequestEncryptionPublicKey.bind(this), getPendingNonce: this.getPendingNonce.bind(this), getPendingTransactionByHash: (hash) => - this.txController.getFilteredTxList({ - hash, - status: TRANSACTION_STATUSES.SUBMITTED, + this.txController.getTransactions({ + searchCriteria: { + hash, + status: TRANSACTION_STATUSES.SUBMITTED, + }, })[0], }; const providerProxy = this.networkController.initializeProvider( @@ -763,7 +765,6 @@ export default class MetamaskController extends EventEmitter { ), createCancelTransaction: nodeify(this.createCancelTransaction, this), createSpeedUpTransaction: nodeify(this.createSpeedUpTransaction, this), - getFilteredTxList: nodeify(txController.getFilteredTxList, txController), isNonceTaken: nodeify(txController.isNonceTaken, txController), estimateGas: nodeify(this.estimateGas, this), getPendingNonce: nodeify(this.getPendingNonce, this), diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index f8b74006e..6756025ad 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -743,7 +743,7 @@ describe('MetaMaskController', function () { selectedAddressStub.returns('0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc'); getNetworkstub.returns(42); - metamaskController.txController.txStateManager._saveTxList([ + metamaskController.txController.txStateManager._addTransactionsToState([ createTxMeta({ id: 1, status: TRANSACTION_STATUSES.UNAPPROVED, @@ -771,7 +771,7 @@ describe('MetaMaskController', function () { await metamaskController.resetAccount(); assert.equal( - metamaskController.txController.txStateManager.getTx(1), + metamaskController.txController.txStateManager.getTransaction(1), undefined, ); }); diff --git a/app/scripts/migrations/057.js b/app/scripts/migrations/057.js new file mode 100644 index 000000000..3cf1ba315 --- /dev/null +++ b/app/scripts/migrations/057.js @@ -0,0 +1,44 @@ +import { cloneDeep, keyBy } from 'lodash'; +import createId from '../../../shared/modules/random-id'; + +const version = 57; + +/** + * replace 'incomingTxLastFetchedBlocksByNetwork' with 'incomingTxLastFetchedBlockByChainId' + */ +export default { + version, + async migrate(originalVersionedData) { + const versionedData = cloneDeep(originalVersionedData); + versionedData.meta.version = version; + const state = versionedData.data; + versionedData.data = transformState(state); + return versionedData; + }, +}; + +function transformState(state) { + if ( + state?.TransactionController?.transactions && + Array.isArray(state.TransactionController.transactions) && + !state.TransactionController.transactions.some( + (item) => + typeof item !== 'object' || typeof item.txParams === 'undefined', + ) + ) { + state.TransactionController.transactions = keyBy( + state.TransactionController.transactions, + // In case for some reason any of a user's transactions do not have an id + // generate a new one for the transaction. + (tx) => { + if (typeof tx.id === 'undefined' || tx.id === null) { + // This mutates the item in the array, so will result in a change to + // the state. + tx.id = createId(); + } + return tx.id; + }, + ); + } + return state; +} diff --git a/app/scripts/migrations/057.test.js b/app/scripts/migrations/057.test.js new file mode 100644 index 000000000..825a78993 --- /dev/null +++ b/app/scripts/migrations/057.test.js @@ -0,0 +1,193 @@ +import { strict as assert } from 'assert'; +import migration57 from './057'; + +describe('migration #57', function () { + it('should update the version metadata', async function () { + const oldStorage = { + meta: { + version: 56, + }, + data: {}, + }; + + const newStorage = await migration57.migrate(oldStorage); + assert.deepEqual(newStorage.meta, { + version: 57, + }); + }); + + it('should transactions array into an object keyed by id', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: [ + { + id: 0, + txParams: { foo: 'bar' }, + }, + { + id: 1, + txParams: { foo: 'bar' }, + }, + { + id: 2, + txParams: { foo: 'bar' }, + }, + { + id: 3, + txParams: { foo: 'bar' }, + }, + ], + }, + foo: 'bar', + }, + }; + + const newStorage = await migration57.migrate(oldStorage); + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: { + 0: { + id: 0, + txParams: { foo: 'bar' }, + }, + 1: { + id: 1, + txParams: { foo: 'bar' }, + }, + 2: { + id: 2, + txParams: { foo: 'bar' }, + }, + 3: { id: 3, txParams: { foo: 'bar' } }, + }, + }, + foo: 'bar', + }); + }); + + it('should handle transactions without an id, just in case', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: [ + { + id: 0, + txParams: { foo: 'bar' }, + }, + { + txParams: { foo: 'bar' }, + }, + { + txParams: { foo: 'bar' }, + }, + { + txParams: { foo: 'bar' }, + }, + ], + }, + foo: 'bar', + }, + }; + + const newStorage = await migration57.migrate(oldStorage); + const expectedTransactions = {}; + for (const transaction of Object.values( + newStorage.data.TransactionController.transactions, + )) { + // Make sure each transaction now has an id. + assert.ok( + typeof transaction.id !== 'undefined', + 'transaction id is undefined', + ); + // Build expected transaction object + expectedTransactions[transaction.id] = transaction; + } + // Ensure that we got the correct number of transactions + assert.equal( + Object.keys(expectedTransactions).length, + oldStorage.data.TransactionController.transactions.length, + ); + // Ensure that the one transaction with id is preserved, even though it is + // a falsy id. + assert.equal(newStorage.data.TransactionController.transactions[0].id, 0); + }); + + it('should not blow up if transactions are not an array', async function () { + const storageWithTransactionsAsString = { + meta: {}, + data: { + TransactionController: { + transactions: 'someone might have weird state in the future', + }, + }, + }; + const storageWithTransactionsAsArrayOfString = { + meta: {}, + data: { + TransactionController: { + transactions: 'someone might have weird state in the future'.split( + '', + ), + }, + }, + }; + const result1 = await migration57.migrate(storageWithTransactionsAsString); + + const result2 = await migration57.migrate( + storageWithTransactionsAsArrayOfString, + ); + + assert.deepEqual(storageWithTransactionsAsString.data, result1.data); + assert.deepEqual(storageWithTransactionsAsArrayOfString.data, result2.data); + }); + + it('should do nothing if transactions state does not exist', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration57.migrate(oldStorage); + assert.deepEqual(oldStorage.data, newStorage.data); + }); + + it('should convert empty array into empty object', async function () { + const oldStorage = { + meta: {}, + data: { + TransactionController: { + transactions: [], + bar: 'baz', + }, + foo: 'bar', + }, + }; + + const newStorage = await migration57.migrate(oldStorage); + assert.deepEqual(newStorage.data, { + TransactionController: { + transactions: {}, + bar: 'baz', + }, + foo: 'bar', + }); + }); + + it('should do nothing if state is empty', async function () { + const oldStorage = { + meta: {}, + data: {}, + }; + + const newStorage = await migration57.migrate(oldStorage); + assert.deepEqual(oldStorage.data, newStorage.data); + }); +}); diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index b26c3eacc..8d56dcc85 100644 --- a/app/scripts/migrations/index.js +++ b/app/scripts/migrations/index.js @@ -61,6 +61,7 @@ const migrations = [ require('./054').default, require('./055').default, require('./056').default, + require('./057').default, ]; export default migrations; diff --git a/shared/constants/transaction.js b/shared/constants/transaction.js index c50494e80..44a777a94 100644 --- a/shared/constants/transaction.js +++ b/shared/constants/transaction.js @@ -155,6 +155,14 @@ export const TRANSACTION_GROUP_CATEGORIES = { * @property {string} gas - The max amount of gwei, in hexadecimal, the user is willing to pay * @property {string} [data] - Hexadecimal encoded string representing calls to the EVM's ABI */ + +/** + * @typedef {Object} TxError + * @property {string} message - The message from the encountered error. + * @property {any} rpc - The "value" of the error. + * @property {string} [stack] - the stack trace from the error, if available. + */ + /** * An object representing a transaction, in whatever state it is in. * @typedef {Object} TransactionMeta @@ -183,6 +191,7 @@ export const TRANSACTION_GROUP_CATEGORIES = { * ready to submit to the network. * @property {string} hash - A hex string of the transaction hash, used to * identify the transaction on the network. - * @property {number} submittedTime - The time the transaction was submitted to + * @property {number} [submittedTime] - The time the transaction was submitted to * the network, in Unix epoch time (ms). + * @property {TxError} [err] - The error encountered during the transaction */ diff --git a/ui/app/pages/send/send-footer/send-footer.component.js b/ui/app/pages/send/send-footer/send-footer.component.js index 425c76086..5956d9517 100644 --- a/ui/app/pages/send/send-footer/send-footer.component.js +++ b/ui/app/pages/send/send-footer/send-footer.component.js @@ -54,7 +54,6 @@ export default class SendFooter extends Component { sign, to, unapprovedTxs, - // updateTx, update, toAccounts, history,