Merge pull request #2879 from MetaMask/ImproveGasEstimation

Improve gas estimation

6 files changed, 57 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dc96203be..dd9379ba6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Current Master
 
+- Improve gas price suggestion to be closer to the lowest that will be accepted.
 - Throw an error if a application tries to submit a tx whose value is a decimal, and inform that it should be in wei.
 - Fix bug that prevented updating custom token details.
 - No longer mark long-pending transactions as failed, since we now have button to retry with higher gas.
diff --git a/app/scripts/controllers/blacklist.js b/app/scripts/controllers/blacklist.js
index dd671943f..33c31dab9 100644
--- a/app/scripts/controllers/blacklist.js
+++ b/app/scripts/controllers/blacklist.js
@@ -57,3 +57,4 @@ class BlacklistController {
 }
 
 module.exports = BlacklistController
+
diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index 7c7efb84d..469deb670 100644
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@@ -32,6 +32,7 @@ module.exports = class TransactionController extends EventEmitter {
     this.provider = opts.provider
     this.blockTracker = opts.blockTracker
     this.signEthTx = opts.signTransaction
+    this.getGasPrice = opts.getGasPrice
 
     this.memStore = new ObservableStore({})
     this.query = new EthQuery(this.provider)
@@ -179,7 +180,8 @@ module.exports = class TransactionController extends EventEmitter {
     // ensure value
     txMeta.gasPriceSpecified = Boolean(txParams.gasPrice)
     txMeta.nonceSpecified = Boolean(txParams.nonce)
-    const gasPrice = txParams.gasPrice || await this.query.gasPrice()
+    const gasPrice = txParams.gasPrice || this.getGasPrice ? this.getGasPrice()
+      : await this.query.gasPrice()
     txParams.gasPrice = ethUtil.addHexPrefix(gasPrice.toString(16))
     txParams.value = txParams.value || '0x0'
     // set gasLimit
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 23f2a1598..1b13f6567 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -35,13 +35,15 @@ const accountImporter = require('./account-import-strategies')
 const getBuyEthUrl = require('./lib/buy-eth-url')
 const Mutex = require('await-semaphore').Mutex
 const version = require('../manifest.json').version
+const BN = require('ethereumjs-util').BN
+const GWEI_BN = new BN('1000000000')
+const percentile = require('percentile')
 
 module.exports = class MetamaskController extends EventEmitter {
 
   constructor (opts) {
     super()
 
-
     this.sendUpdate = debounce(this.privateSendUpdate.bind(this), 200)
 
     this.opts = opts
@@ -139,6 +141,7 @@ module.exports = class MetamaskController extends EventEmitter {
       provider: this.provider,
       blockTracker: this.blockTracker,
       ethQuery: this.ethQuery,
+      getGasPrice: this.getGasPrice.bind(this),
     })
     this.txController.on('newUnapprovedTx', opts.showUnapprovedTx.bind(opts))
 
@@ -484,6 +487,26 @@ module.exports = class MetamaskController extends EventEmitter {
     this.emit('update', this.getState())
   }
 
+  getGasPrice () {
+    const { recentBlocksController } = this
+    const { recentBlocks } = recentBlocksController.store.getState()
+    const lowestPrices = recentBlocks.map((block) => {
+      if (!block.gasPrices) {
+        return new BN(0)
+      }
+      return block.gasPrices
+      .map(hexPrefix => hexPrefix.substr(2))
+      .map(hex => new BN(hex, 16))
+      .sort((a, b) => {
+        return a.gt(b) ? 1 : -1
+      })[0]
+    })
+    .map(number => number.div(GWEI_BN).toNumber())
+    const percentileNum = percentile(50, lowestPrices)
+    const percentileNumBn = new BN(percentileNum)
+    return '0x' + percentileNumBn.mul(GWEI_BN).toString(16)
+  }
+
   //
   // Vault Management
   //
diff --git a/package.json b/package.json
index 871ed204e..07ef76b0a 100644
--- a/package.json
+++ b/package.json
@@ -119,6 +119,7 @@
     "obj-multiplex": "^1.0.0",
     "obs-store": "^3.0.0",
     "once": "^1.3.3",
+    "percentile": "^1.2.0",
     "ping-pong-stream": "^1.0.0",
     "pojo-migrator": "^2.1.0",
     "polyfill-crypto.getrandomvalues": "^1.0.0",
diff --git a/test/unit/metamask-controller-test.js b/test/unit/metamask-controller-test.js
index fd420a70f..3deb5a1c7 100644
--- a/test/unit/metamask-controller-test.js
+++ b/test/unit/metamask-controller-test.js
@@ -3,6 +3,8 @@ const sinon = require('sinon')
 const clone = require('clone')
 const MetaMaskController = require('../../app/scripts/metamask-controller')
 const firstTimeState = require('../../app/scripts/first-time-state')
+const BN = require('ethereumjs-util').BN
+const GWEI_BN = new BN('1000000000')
 
 describe('MetaMaskController', function () {
   const noop = () => {}
@@ -45,6 +47,31 @@ describe('MetaMaskController', function () {
       metamaskController.keyringController.createNewVaultAndKeychain.restore()
     })
 
+    describe('#getGasPrice', function () {
+      it('gives the 50th percentile lowest accepted gas price from recentBlocksController', async function () {
+        const realRecentBlocksController = metamaskController.recentBlocksController
+        metamaskController.recentBlocksController = {
+          store: {
+            getState: () => {
+              return {
+                recentBlocks: [
+                  { gasPrices: [ '0x3b9aca00', '0x174876e800'] },
+                  { gasPrices: [ '0x3b9aca00', '0x174876e800'] },
+                  { gasPrices: [ '0x174876e800', '0x174876e800' ]},
+                  { gasPrices: [ '0x174876e800', '0x174876e800' ]},
+                ]
+              }
+            }
+          }
+        }
+
+        const gasPrice = metamaskController.getGasPrice()
+        assert.equal(gasPrice, '0x3b9aca00', 'accurately estimates 50th percentile accepted gas price')
+
+        metamaskController.recentBlocksController = realRecentBlocksController
+      })
+    })
+
     describe('#createNewVaultAndKeychain', function () {
       it('can only create new vault on keyringController once', async function () {