From 92592fc905f59bc6ef776a011a6928dc918a91cb Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 30 Apr 2020 21:50:44 -0300 Subject: [PATCH] Ensure tx has value before it's added (#8486) Previously a transaction would get assigned a default value during the `addTxGasDefaults` function, after the transaction was added and sent to the UI. Instead the transaction is assigned a default value before it gets added. This flow is simpler to follow, and it avoids the race condition where the transaction is assigned a value from the UI before this default is set. In that situation, the UI-assigned value would be overridden, which is obviously not desired. --- app/scripts/controllers/transactions/index.js | 9 ++++++--- .../app/controllers/transactions/tx-controller-test.js | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/scripts/controllers/transactions/index.js b/app/scripts/controllers/transactions/index.js index e05f07c4e..fb775da12 100644 --- a/app/scripts/controllers/transactions/index.js +++ b/app/scripts/controllers/transactions/index.js @@ -233,6 +233,12 @@ export default class TransactionController extends EventEmitter { const { transactionCategory, getCodeResponse } = await this._determineTransactionCategory(txParams) txMeta.transactionCategory = transactionCategory + + // ensure value + txMeta.txParams.value = txMeta.txParams.value + ? ethUtil.addHexPrefix(txMeta.txParams.value) + : '0x0' + this.addTx(txMeta) this.emit('newUnapprovedTx', txMeta) @@ -262,9 +268,6 @@ export default class TransactionController extends EventEmitter { async addTxGasDefaults (txMeta, getCodeResponse) { const txParams = txMeta.txParams - // ensure value - - txParams.value = txParams.value ? ethUtil.addHexPrefix(txParams.value) : '0x0' txMeta.gasPriceSpecified = Boolean(txParams.gasPrice) let gasPrice = txParams.gasPrice if (!gasPrice) { diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js index 4678b8519..6de58d06e 100644 --- a/test/unit/app/controllers/transactions/tx-controller-test.js +++ b/test/unit/app/controllers/transactions/tx-controller-test.js @@ -182,6 +182,7 @@ describe('Transaction Controller', function () { assert.ok('metamaskNetworkId' in txMeta, 'should have a metamaskNetworkId') assert.ok('txParams' in txMeta, 'should have a txParams') assert.ok('history' in txMeta, 'should have a history') + assert.equal(txMeta.txParams.value, '0x0', 'should have added 0x0 as the value') const memTxMeta = txController.txStateManager.getTx(txMeta.id) assert.deepEqual(txMeta, memTxMeta) @@ -241,7 +242,6 @@ describe('Transaction Controller', function () { providerResultStub.eth_estimateGas = '5209' const txMetaWithDefaults = await txController.addTxGasDefaults(txMeta) - assert.equal(txMetaWithDefaults.txParams.value, '0x0', 'should have added 0x0 as the value') assert.ok(txMetaWithDefaults.txParams.gasPrice, 'should have added the gas price') assert.ok(txMetaWithDefaults.txParams.gas, 'should have added the gas field') })