Merge branch 'uat' of https://github.com/MetaMask/metamask-extension into cb-254

4 files changed, 55 insertions, 20 deletions

diff --git a/app/scripts/lib/account-tracker.js b/app/scripts/lib/account-tracker.js
index ce6642150..8c3dd8c71 100644
--- a/app/scripts/lib/account-tracker.js
+++ b/app/scripts/lib/account-tracker.js
@@ -117,8 +117,6 @@ class AccountTracker extends EventEmitter {
     const query = this._query
     async.parallel({
       balance: query.getBalance.bind(query, address),
-      nonce: query.getTransactionCount.bind(query, address),
-      code: query.getCode.bind(query, address),
     }, cb)
   }
 
diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js
index dc6e526fd..e8869e6b8 100644
--- a/app/scripts/lib/pending-tx-tracker.js
+++ b/app/scripts/lib/pending-tx-tracker.js
@@ -23,7 +23,6 @@ module.exports = class PendingTransactionTracker extends EventEmitter {
     this.query = new EthQuery(config.provider)
     this.nonceTracker = config.nonceTracker
     // default is one day
-    this.retryTimePeriod = config.retryTimePeriod || 86400000
     this.getPendingTransactions = config.getPendingTransactions
     this.getCompletedTransactions = config.getCompletedTransactions
     this.publishTransaction = config.publishTransaction
@@ -106,12 +105,6 @@ module.exports = class PendingTransactionTracker extends EventEmitter {
       this.emit('tx:block-update', txMeta, latestBlockNumber)
     }
 
-    if (Date.now() > txMeta.time + this.retryTimePeriod) {
-      const hours = (this.retryTimePeriod / 3.6e+6).toFixed(1)
-      const err = new Error(`Gave up submitting after ${hours} hours.`)
-      return this.emit('tx:failed', txMeta.id, err)
-    }
-
     const firstRetryBlockNumber = txMeta.firstRetryBlockNumber || latestBlockNumber
     const txBlockDistance = Number.parseInt(latestBlockNumber, 16) - Number.parseInt(firstRetryBlockNumber, 16)
 
@@ -185,7 +178,8 @@ module.exports = class PendingTransactionTracker extends EventEmitter {
   }
 
   async _checkIfNonceIsTaken (txMeta) {
-    const completed = this.getCompletedTransactions()
+    const address = txMeta.txParams.from
+    const completed = this.getCompletedTransactions(address)
     const sameNonce = completed.filter((otherMeta) => {
       return otherMeta.txParams.nonce === txMeta.txParams.nonce
     })
diff --git a/app/scripts/lib/tx-gas-utils.js b/app/scripts/lib/tx-gas-utils.js
index 7e72ea71d..e80e0467e 100644
--- a/app/scripts/lib/tx-gas-utils.js
+++ b/app/scripts/lib/tx-gas-utils.js
@@ -4,6 +4,7 @@ const {
   BnMultiplyByFraction,
   bnToHex,
 } = require('./util')
+const SIMPLE_GAS_COST = '0x5208' // Hex for 21000, cost of a simple send.
 
 /*
 tx-utils are utility methods for Transaction manager
@@ -22,7 +23,11 @@ module.exports = class txProvideUtil {
     try {
       estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit)
     } catch (err) {
-      if (err.message.includes('Transaction execution error.')) {
+      const simulationFailed = (
+        err.message.includes('Transaction execution error.') ||
+        err.message.includes('gas required exceeds allowance or always failing transaction')
+      )
+      if (simulationFailed) {
         txMeta.simulationFails = true
         return txMeta
       }
@@ -33,14 +38,30 @@ module.exports = class txProvideUtil {
 
   async estimateTxGas (txMeta, blockGasLimitHex) {
     const txParams = txMeta.txParams
+
     // check if gasLimit is already specified
     txMeta.gasLimitSpecified = Boolean(txParams.gas)
-    // if not, fallback to block gasLimit
-    if (!txMeta.gasLimitSpecified) {
-      const blockGasLimitBN = hexToBn(blockGasLimitHex)
-      const saferGasLimitBN = BnMultiplyByFraction(blockGasLimitBN, 19, 20)
-      txParams.gas = bnToHex(saferGasLimitBN)
+
+    // if it is, use that value
+    if (txMeta.gasLimitSpecified) {
+      return txParams.gas
     }
+
+    // if recipient has no code, gas is 21k max:
+    const recipient = txParams.to
+    const hasRecipient = Boolean(recipient)
+    const code = await this.query.getCode(recipient)
+    if (hasRecipient && (!code || code === '0x')) {
+      txParams.gas = SIMPLE_GAS_COST
+      txMeta.simpleSend = true // Prevents buffer addition
+      return SIMPLE_GAS_COST
+    }
+
+    // if not, fall back to block gasLimit
+    const blockGasLimitBN = hexToBn(blockGasLimitHex)
+    const saferGasLimitBN = BnMultiplyByFraction(blockGasLimitBN, 19, 20)
+    txParams.gas = bnToHex(saferGasLimitBN)
+
     // run tx
     return await this.query.estimateGas(txParams)
   }
@@ -51,7 +72,7 @@ module.exports = class txProvideUtil {
 
     // if gasLimit was specified and doesnt OOG,
     // use original specified amount
-    if (txMeta.gasLimitSpecified) {
+    if (txMeta.gasLimitSpecified || txMeta.simpleSend) {
       txMeta.estimatedGas = txParams.gas
       return
     }
@@ -77,8 +98,26 @@ module.exports = class txProvideUtil {
   }
 
   async validateTxParams (txParams) {
-    if (('value' in txParams) && txParams.value.indexOf('-') === 0) {
-      throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)
+    this.validateRecipient(txParams)
+    if ('value' in txParams) {
+      const value = txParams.value.toString()
+      if (value.includes('-')) {
+        throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)
+      }
+
+      if (value.includes('.')) {
+        throw new Error(`Invalid transaction value of ${txParams.value} number must be in wei`)
+      }
+    }
+  }
+  validateRecipient (txParams) {
+    if (txParams.to === '0x') {
+      if (txParams.data) {
+        delete txParams.to
+      } else {
+        throw new Error('Invalid recipient address')
+      }
     }
+    return txParams
   }
 }
diff --git a/app/scripts/lib/tx-state-manager.js b/app/scripts/lib/tx-state-manager.js
index 0fd6bed4b..a8ef39891 100644
--- a/app/scripts/lib/tx-state-manager.js
+++ b/app/scripts/lib/tx-state-manager.js
@@ -187,6 +187,10 @@ module.exports = class TransactionStateManger extends EventEmitter {
     this._setTxStatus(txId, 'rejected')
   }
 
+  // should update the status of the tx to 'unapproved'.
+  setTxStatusUnapproved (txId) {
+    this._setTxStatus(txId, 'unapproved')
+  }
   // should update the status of the tx to 'approved'.
   setTxStatusApproved (txId) {
     this._setTxStatus(txId, 'approved')
@@ -236,7 +240,7 @@ module.exports = class TransactionStateManger extends EventEmitter {
     txMeta.status = status
     this.emit(`${txMeta.id}:${status}`, txId)
     this.emit(`tx:status-update`, txId, status)
-    if (status === 'submitted' || status === 'rejected') {
+    if (['submitted', 'rejected', 'failed'].includes(status)) {
       this.emit(`${txMeta.id}:finished`, txMeta)
     }
     this.updateTx(txMeta, `txStateManager: setting status to ${status}`)