Merge pull request #802 from MetaMask/confTxFix

Conf tx fix

CHANGELOG.md

@@ -1,8 +1,12 @@
 # Changelog
 
 ## Current Master
+- Fix bug where 20% of gas estimate was not being added properly.
 
-- Fix gas estimation bug.
+## 2.13.7 2016-11-8
+
+- Fix bug where gas estimate would sometimes be very high.
+- Increased our gas estimate from 100k gas to 20% of estimate.
 - Fix github link on info page to point at current repository.
 
 ## 2.13.6 2016-10-26

app/manifest.json

@@ -1,7 +1,7 @@
 {
   "name": "MetaMask",
   "short_name": "Metamask",
-  "version": "2.13.6",
+  "version": "2.13.7",
   "manifest_version": 2,
   "author": "https://metamask.io",
   "description": "Ethereum Browser Extension",

app/scripts/keyring-controller.js

@@ -38,7 +38,7 @@ module.exports = class KeyringController extends EventEmitter {
     this._unconfTxCbs = {}
     this._unconfMsgCbs = {}
 
-    this.network = opts.network
+    this.getNetwork = opts.getNetwork
 
     // TEMPORARY UNTIL FULL DEPRECATION:
     this.idStoreMigrator = new IdStoreMigrator({
@@ -344,13 +344,14 @@ module.exports = class KeyringController extends EventEmitter {
     var time = (new Date()).getTime()
     var txId = createId()
     txParams.metamaskId = txId
-    txParams.metamaskNetworkId = this.network
+    txParams.metamaskNetworkId = this.getNetwork()
     var txData = {
       id: txId,
       txParams: txParams,
       time: time,
       status: 'unconfirmed',
       gasMultiplier: configManager.getGasMultiplier() || 1,
+      metamaskNetworkId: this.getNetwork(),
     }
 
 

app/scripts/lib/idStore.js

@@ -260,6 +260,7 @@ IdentityStore.prototype.addUnconfirmedTransaction = function (txParams, onTxDone
     query.estimateGas(txParams, function(err, result){
       if (err) return cb(err)
       txData.estimatedGas = self.addGasBuffer(result)
+      txData.txParams.gasLimit = txData.estimatedGas
       cb()
     })
   }
@@ -285,9 +286,10 @@ IdentityStore.prototype.checkForDelegateCall = function (codeHex) {
   }
 }
 
-const gasBuffer = new BN('100000', 10)
 IdentityStore.prototype.addGasBuffer = function (gas) {
   const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16)
+  const five = new BN('5', 10)
+  const gasBuffer = bnGas.div(five)
   const correct = bnGas.add(gasBuffer)
   return ethUtil.addHexPrefix(correct.toString(16))
 }

app/scripts/metamask-controller.js

@@ -17,6 +17,7 @@ module.exports = class MetamaskController {
     this.configManager = new ConfigManager(opts)
     this.keyringController = new KeyringController({
       configManager: this.configManager,
+      getNetwork: this.getStateNetwork.bind(this),
     })
     this.provider = this.initializeProvider(opts)
     this.ethStore = new EthStore(this.provider)
@@ -218,7 +219,6 @@ module.exports = class MetamaskController {
 
     let err = this.enforceTxValidations(txParams)
     if (err) return onTxDoneCb(err)
-
     keyringController.addUnconfirmedTransaction(txParams, onTxDoneCb, (err, txData) => {
       if (err) return onTxDoneCb(err)
       this.sendUpdate()
@@ -400,4 +400,9 @@ module.exports = class MetamaskController {
       cb(e)
     }
   }
+
+  getStateNetwork () {
+    return this.state.network
+  }
 }
+

test/unit/idStore-test.js

@@ -159,7 +159,7 @@ describe('IdentityStore', function() {
       assert.equal(result.indexOf('0x'), 0, 'include hex prefix')
     })
 
-    it('buffers reasonably', function() {
+    it('buffers 20%', function() {
       const idStore = new IdentityStore({
         configManager: configManagerGen(),
         ethStore: {
@@ -168,20 +168,18 @@ describe('IdentityStore', function() {
       })
 
       const gas = '0x04ee59' // Actual estimated gas example
-      const tooBigOutput = '0x80674f9' // Actual bad output
       const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16)
-      const correctBuffer = new BN('100000', 10)
+      const five = new BN('5', 10)
+      const correctBuffer = bnGas.div(five)
       const correct = bnGas.add(correctBuffer)
 
-      const tooBig = new BN(tooBigOutput, 16)
       const result = idStore.addGasBuffer(gas)
       const bnResult = new BN(ethUtil.stripHexPrefix(result), 16)
 
       assert.equal(result.indexOf('0x'), 0, 'included hex prefix')
       assert(bnResult.gt(bnGas), 'Estimate increased in value.')
-      assert.equal(bnResult.sub(bnGas).toString(10), '100000', 'added 100k gas')
+      assert.equal(bnResult.sub(bnGas).toString(10), correctBuffer.toString(10), 'added 20% gas')
       assert.equal(result, '0x' + correct.toString(16), 'Added the right amount')
-      assert.notEqual(result, tooBigOutput, 'not that bad estimate')
     })
   })
 

ui/app/actions.js

@@ -44,7 +44,6 @@ var actions = {
   unlockInProgress: unlockInProgress,
   // error handling
   displayWarning: displayWarning,
-  showWarning: showWarning, // alias
   DISPLAY_WARNING: 'DISPLAY_WARNING',
   HIDE_WARNING: 'HIDE_WARNING',
   hideWarning: hideWarning,
@@ -184,7 +183,7 @@ function confirmSeedWords () {
     background.clearSeedWordCache((err, account) => {
       dispatch(actions.hideLoadingIndication())
       if (err) {
-        return dispatch(actions.showWarning(err.message))
+        return dispatch(actions.displayWarning(err.message))
       }
 
       console.log('Seed word cache cleared. ' + account)
@@ -384,7 +383,7 @@ function agreeToDisclaimer () {
     dispatch(this.showLoadingIndication())
     background.agreeToDisclaimer((err) => {
       if (err) {
-        return dispatch(actions.showWarning(err.message))
+        return dispatch(actions.displayWarning(err.message))
       }
 
       dispatch(this.hideLoadingIndication())
@@ -456,7 +455,7 @@ function lockMetamask () {
     background.setLocked((err) => {
       dispatch(actions.hideLoadingIndication())
       if (err) {
-        return dispatch(actions.showWarning(err.message))
+        return dispatch(actions.displayWarning(err.message))
       }
 
       dispatch({
@@ -472,7 +471,7 @@ function showAccountDetail (address) {
     background.setSelectedAddress(address, (err, address) => {
       dispatch(actions.hideLoadingIndication())
       if (err) {
-        return dispatch(actions.showWarning(err.message))
+        return dispatch(actions.displayWarning(err.message))
       }
 
       dispatch({
@@ -586,10 +585,6 @@ function hideSubLoadingIndication () {
   }
 }
 
-function showWarning (text) {
-  return this.displayWarning(text)
-}
-
 function displayWarning (text) {
   return {
     type: actions.DISPLAY_WARNING,
@@ -641,7 +636,7 @@ function saveAccountLabel (account, label) {
     background.saveAccountLabel(account, label, (err) => {
       dispatch(actions.hideLoadingIndication())
       if (err) {
-        return dispatch(actions.showWarning(err.message))
+        return dispatch(actions.displayWarning(err.message))
       }
       dispatch({
         type: actions.SAVE_ACCOUNT_LABEL,
@@ -717,7 +712,7 @@ function shapeShiftSubview (network) {
     shapeShiftRequest('marketinfo', {pair}, (mktResponse) => {
       shapeShiftRequest('getcoins', {}, (response) => {
         dispatch(actions.hideSubLoadingIndication())
-        if (mktResponse.error) return dispatch(actions.showWarning(mktResponse.error))
+        if (mktResponse.error) return dispatch(actions.displayWarning(mktResponse.error))
         dispatch({
           type: actions.SHAPESHIFT_SUBVIEW,
           value: {
@@ -734,7 +729,7 @@ function coinShiftRquest (data, marketData) {
   return (dispatch) => {
     dispatch(actions.showLoadingIndication())
     shapeShiftRequest('shift', { method: 'POST', data}, (response) => {
-      if (response.error) return dispatch(actions.showWarning(response.error))
+      if (response.error) return dispatch(actions.displayWarning(response.error))
       var message = `
         Deposit your ${response.depositType} to the address bellow:`
       background.createShapeShiftTx(response.deposit, response.depositType)
@@ -756,7 +751,7 @@ function reshowQrCode (data, coin) {
   return (dispatch) => {
     dispatch(actions.showLoadingIndication())
     shapeShiftRequest('marketinfo', {pair: `${coin.toLowerCase()}_eth`}, (mktResponse) => {
-      if (mktResponse.error) return dispatch(actions.showWarning(mktResponse.error))
+      if (mktResponse.error) return dispatch(actions.displayWarning(mktResponse.error))
 
       var message = [
         `Deposit your ${coin} to the address bellow:`,

ui/app/components/coinbase-form.js

@@ -116,10 +116,10 @@ CoinbaseForm.prototype.toCoinbase = function () {
     props.dispatch(actions.buyEth(address, props.buyView.amount))
   } else if (!isValidAmountforCoinBase(amount).valid) {
     message = isValidAmountforCoinBase(amount).message
-    return props.dispatch(actions.showWarning(message))
+    return props.dispatch(actions.displayWarning(message))
   } else {
     message = 'Receiving address is invalid.'
-    return props.dispatch(actions.showWarning(message))
+    return props.dispatch(actions.displayWarning(message))
   }
 }
 

ui/app/components/shapeshift-form.js

@@ -244,7 +244,7 @@ ShapeshiftForm.prototype.updateCoin = function (event) {
 
   if (!coinOptions[coin.toUpperCase()] || coin.toUpperCase() === 'ETH') {
     var message = 'Not a valid coin'
-    return props.dispatch(actions.showWarning(message))
+    return props.dispatch(actions.displayWarning(message))
   } else {
     return props.dispatch(actions.pairUpdate(coin))
   }