Merge pull request #2962 from MetaMask/open-popup

Open popup before estimateGas finishes

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Current Master
 
+- Open metamask popup for transaction confirmation before gas estimation finishes and add a loading screen over transaction confirmation.
 - Fix bug that prevented eth_signTypedData from signing bytes.
 - Further improve gas price estimation.
 

app/scripts/controllers/transactions.js

@@ -139,7 +139,6 @@ module.exports = class TransactionController extends EventEmitter {
   async newUnapprovedTransaction (txParams) {
     log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`)
     const initialTxMeta = await this.addUnapprovedTransaction(txParams)
-    this.emit('newUnapprovedTx', initialTxMeta)
     // listen for tx completion (success, fail)
     return new Promise((resolve, reject) => {
       this.txStateManager.once(`${initialTxMeta.id}:finished`, (finishedTxMeta) => {
@@ -167,11 +166,16 @@ module.exports = class TransactionController extends EventEmitter {
       status: 'unapproved',
       metamaskNetworkId: this.getNetwork(),
       txParams: txParams,
+      loadingDefaults: true,
     }
+    this.addTx(txMeta)
+    this.emit('newUnapprovedTx', txMeta)
     // add default tx params
     await this.addTxDefaults(txMeta)
+
+    txMeta.loadingDefaults = false
     // save txMeta
-    this.addTx(txMeta)
+    this.txStateManager.updateTx(txMeta)
     return txMeta
   }
 

test/stub/provider.js

@@ -5,7 +5,8 @@ module.exports = {
   createEngineForTestData,
   providerFromEngine,
   scaffoldMiddleware,
-  createStubedProvider
+  createEthJsQueryStub,
+  createStubedProvider,
 }
 
 
@@ -18,6 +19,18 @@ function providerFromEngine (engine) {
   return provider
 }
 
+function createEthJsQueryStub (stubProvider) {
+  return new Proxy({}, {
+    get: (obj, method) => {
+      return (...params) => {
+        return new Promise((resolve, reject) => {
+          stubProvider.sendAsync({ method: `eth_${method}`, params }, (err, ress) => resolve(ress.result))
+        })
+      }
+    },
+  })
+}
+
 function createStubedProvider (resultStub) {
   const engine = createEngineForTestData()
   engine.push(scaffoldMiddleware(resultStub))

test/unit/tx-controller-test.js

@@ -5,7 +5,7 @@ const ObservableStore = require('obs-store')
 const sinon = require('sinon')
 const TransactionController = require('../../app/scripts/controllers/transactions')
 const TxGasUtils = require('../../app/scripts/lib/tx-gas-utils')
-const { createStubedProvider } = require('../stub/provider')
+const { createStubedProvider, createEthJsQueryStub } = require('../stub/provider')
 
 const noop = () => true
 const currentNetworkId = 42
@@ -30,6 +30,8 @@ describe('Transaction Controller', function () {
         resolve()
       }),
     })
+    txController.query = createEthJsQueryStub(provider)
+    txController.txGasUtil.query = createEthJsQueryStub(provider)
     txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop })
     txController.txProviderUtils = new TxGasUtils(txController.provider)
   })
@@ -110,22 +112,15 @@ describe('Transaction Controller', function () {
         history: [],
       }
       txController.txStateManager._saveTxList([txMeta])
-      stub = sinon.stub(txController, 'addUnapprovedTransaction').returns(Promise.resolve(txController.txStateManager.addTx(txMeta)))
+      stub = sinon.stub(txController, 'addUnapprovedTransaction').callsFake(() => {
+        txController.emit('newUnapprovedTx', txMeta)
+        return Promise.resolve(txController.txStateManager.addTx(txMeta))
     })
 
     afterEach(function () {
       txController.txStateManager._saveTxList([])
       stub.restore()
     })
-
-    it('should emit newUnapprovedTx event and pass txMeta as the first argument', function (done) {
-      txController.once('newUnapprovedTx', (txMetaFromEmit) => {
-        assert(txMetaFromEmit, 'txMeta is falsey')
-        assert.equal(txMetaFromEmit.id, 1, 'the right txMeta was passed')
-        done()
-      })
-      txController.newUnapprovedTransaction(txParams)
-      .catch(done)
   })
 
     it('should resolve when finished and status is submitted and resolve with the hash', function (done) {
@@ -160,8 +155,17 @@ describe('Transaction Controller', function () {
   })
 
   describe('#addUnapprovedTransaction', function () {
+    let addTxDefaults
+    beforeEach(() => {
+      addTxDefaults = txController.addTxDefaults
+      txController.addTxDefaults = function addTxDefaultsStub () { return Promise.resolve() }
+
+    })
+    afterEach(() => {
+      txController.addTxDefaults = addTxDefaults
+    })
+
     it('should add an unapproved transaction and return a valid txMeta', function (done) {
-      const addTxDefaultsStub = sinon.stub(txController, 'addTxDefaults').callsFake(() => Promise.resolve())
       txController.addUnapprovedTransaction({})
       .then((txMeta) => {
         assert(('id' in txMeta), 'should have a id')
@@ -172,10 +176,20 @@ describe('Transaction Controller', function () {
 
         const memTxMeta = txController.txStateManager.getTx(txMeta.id)
         assert.deepEqual(txMeta, memTxMeta, `txMeta should be stored in txController after adding it\n  expected: ${txMeta} \n  got: ${memTxMeta}`)
-        addTxDefaultsStub.restore()
         done()
       }).catch(done)
     })
+
+    it('should emit newUnapprovedTx event and pass txMeta as the first argument', function (done) {
+      providerResultStub.eth_gasPrice = '4a817c800'
+      txController.once('newUnapprovedTx', (txMetaFromEmit) => {
+        assert(txMetaFromEmit, 'txMeta is falsey')
+        done()
+      })
+      txController.addUnapprovedTransaction({})
+      .catch(done)
+    })
+
   })
 
   describe('#addTxDefaults', function () {

ui/app/conf-tx.js

@@ -4,6 +4,7 @@ const h = require('react-hyperscript')
 const connect = require('react-redux').connect
 const actions = require('./actions')
 const NetworkIndicator = require('./components/network')
+const LoadingIndicator = require('./components/loading')
 const txHelper = require('../lib/tx-helper')
 const isPopupOrNotification = require('../../app/scripts/lib/is-popup-or-notification')
 
@@ -60,6 +61,11 @@ ConfirmTxScreen.prototype.render = function () {
 
     h('.flex-column.flex-grow', [
 
+      h(LoadingIndicator, {
+        isLoading: txData.loadingDefaults,
+        loadingMessage: 'Estimating transaction cost…',
+      }),
+
       // subtitle and nav
       h('.section-title.flex-row.flex-center', [
         !isNotification ? h('i.fa.fa-arrow-left.fa-lg.cursor-pointer', {