Skip to content

Commit

Permalink
Ensure tx has value before it's added (#8486)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gudahtt authored May 1, 2020
1 parent c2b5889 commit 92592fc
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
9 changes: 6 additions & 3 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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')
})
Expand Down

0 comments on commit 92592fc

Please sign in to comment.