diff --git a/CHANGELOG.md b/CHANGELOG.md index f9ed01fef..5adf16cb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Current Master - Estimating gas limit for simple ether sends now faster & cheaper, by avoiding VM usage on recipients with no code. +- 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. diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js index ded9739a8..bb9253175 100644 --- a/app/scripts/controllers/transactions.js +++ b/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 } diff --git a/test/stub/provider.js b/test/stub/provider.js index 8a306f6d9..85e1da707 100644 --- a/test/stub/provider.js +++ b/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)) diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js index aeefd5ec6..36f3e1c68 100644 --- a/test/unit/tx-controller-test.js +++ b/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,23 +112,16 @@ 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) { txController.once('newUnapprovedTx', (txMetaFromEmit) => { @@ -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 () { diff --git a/ui/app/app.js b/ui/app/app.js index bc198b482..f0dfef34f 100644 --- a/ui/app/app.js +++ b/ui/app/app.js @@ -395,7 +395,7 @@ App.prototype.renderDropdown = function () { h(DropdownMenuItem, { closeMenu: () => this.setState({ isMainMenuOpen: !isOpen }), onClick: () => { this.props.dispatch(actions.lockMetamask()) }, - }, 'Lock'), + }, 'Log Out'), h(DropdownMenuItem, { closeMenu: () => this.setState({ isMainMenuOpen: !isOpen }), diff --git a/ui/app/conf-tx.js b/ui/app/conf-tx.js index cb1afedfe..ce4d153b5 100644 --- a/ui/app/conf-tx.js +++ b/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', {