From 7b3f1ecc41e1c57252f1d862930aebc3e9c446ea Mon Sep 17 00:00:00 2001 From: "Kevin G. Serrano" Date: Mon, 7 Nov 2016 16:02:02 -0800 Subject: [PATCH 1/8] Unify warning functions. --- ui/app/actions.js | 23 +++++++++-------------- ui/app/components/coinbase-form.js | 4 ++-- ui/app/components/shapeshift-form.js | 2 +- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/ui/app/actions.js b/ui/app/actions.js index 1f0d8fc78..3d9588083 100644 --- a/ui/app/actions.js +++ b/ui/app/actions.js @@ -43,7 +43,6 @@ var actions = { unlockInProgress: unlockInProgress, // error handling displayWarning: displayWarning, - showWarning: showWarning, // alias DISPLAY_WARNING: 'DISPLAY_WARNING', HIDE_WARNING: 'HIDE_WARNING', hideWarning: hideWarning, @@ -180,7 +179,7 @@ function createNewVault (password, entropy) { dispatch(actions.createNewVaultInProgress()) _accountManager.createNewVault(password, entropy, (err, result) => { if (err) { - return dispatch(actions.showWarning(err.message)) + return dispatch(actions.displayWarning(err.message)) } dispatch(actions.showNewVaultSeed(result)) }) @@ -355,7 +354,7 @@ function agreeToDisclaimer () { dispatch(this.showLoadingIndication()) _accountManager.agreeToDisclaimer((err) => { if (err) { - return dispatch(actions.showWarning(err.message)) + return dispatch(actions.displayWarning(err.message)) } dispatch(this.hideLoadingIndication()) @@ -420,7 +419,7 @@ function lockMetamask () { _accountManager.setLocked((err) => { dispatch(actions.hideLoadingIndication()) if (err) { - return dispatch(actions.showWarning(err.message)) + return dispatch(actions.displayWarning(err.message)) } dispatch({ @@ -436,7 +435,7 @@ function showAccountDetail (address) { _accountManager.setSelectedAddress(address, (err, address) => { dispatch(actions.hideLoadingIndication()) if (err) { - return dispatch(actions.showWarning(err.message)) + return dispatch(actions.displayWarning(err.message)) } dispatch({ @@ -466,7 +465,7 @@ function confirmSeedWords () { _accountManager.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) @@ -571,10 +570,6 @@ function hideSubLoadingIndication () { } } -function showWarning (text) { - return this.displayWarning(text) -} - function displayWarning (text) { return { type: actions.DISPLAY_WARNING, @@ -626,7 +621,7 @@ function saveAccountLabel (account, label) { _accountManager.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, @@ -721,7 +716,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: { @@ -738,7 +733,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:` _accountManager.createShapeShiftTx(response.deposit, response.depositType) @@ -760,7 +755,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:`, diff --git a/ui/app/components/coinbase-form.js b/ui/app/components/coinbase-form.js index efd05ec96..3c5708bf8 100644 --- a/ui/app/components/coinbase-form.js +++ b/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)) } } diff --git a/ui/app/components/shapeshift-form.js b/ui/app/components/shapeshift-form.js index 2bb384b94..1da549288 100644 --- a/ui/app/components/shapeshift-form.js +++ b/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)) } From 6f39924e60545ca8ac19995a799c16d0dea11b04 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 8 Nov 2016 08:32:34 -0800 Subject: [PATCH 2/8] Increase gas buffer Fixes #795 --- CHANGELOG.md | 3 ++- app/scripts/lib/idStore.js | 2 +- test/unit/idStore-test.js | 3 +-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e75502159..47a6e9b3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ ## Current Master -- Fix gas estimation bug. +- Fix bug where gas estimate would sometimes be very high. +- Increased our gas estimate safety buffer to avoid Out of Gas errors. - Fix github link on info page to point at current repository. ## 2.13.6 2016-10-26 diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 1010a5789..8c811a614 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -287,7 +287,7 @@ IdentityStore.prototype.checkForDelegateCall = function (codeHex) { } } -const gasBuffer = new BN('100000', 10) +const gasBuffer = new BN('200000', 10) IdentityStore.prototype.addGasBuffer = function (gas) { const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) const correct = bnGas.add(gasBuffer) diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index 46b3d4809..b0940d2fa 100644 --- a/test/unit/idStore-test.js +++ b/test/unit/idStore-test.js @@ -168,9 +168,8 @@ 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 correctBuffer = new BN('200000', 10) const correct = bnGas.add(correctBuffer) const tooBig = new BN(tooBigOutput, 16) From 7068d2e71c6d1ddb68095916dc16f3ff1f283590 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 8 Nov 2016 08:39:41 -0800 Subject: [PATCH 3/8] Adjust gas buffer to be 20% over estimate --- CHANGELOG.md | 2 +- app/scripts/lib/idStore.js | 3 ++- test/unit/idStore-test.js | 9 ++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47a6e9b3b..bc8cc5fa3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Current Master - Fix bug where gas estimate would sometimes be very high. -- Increased our gas estimate safety buffer to avoid Out of Gas errors. +- 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 diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 8c811a614..0ce91f471 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -287,9 +287,10 @@ IdentityStore.prototype.checkForDelegateCall = function (codeHex) { } } -const gasBuffer = new BN('200000', 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)) } diff --git a/test/unit/idStore-test.js b/test/unit/idStore-test.js index b0940d2fa..064483ba0 100644 --- a/test/unit/idStore-test.js +++ b/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: { @@ -169,18 +169,17 @@ describe('IdentityStore', function() { const gas = '0x04ee59' // Actual estimated gas example const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const correctBuffer = new BN('200000', 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') }) }) From 4a0197012ec2dfdcb8b2fe988f3d325ec0cd7116 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 8 Nov 2016 08:51:14 -0800 Subject: [PATCH 4/8] Version 2.13.7 --- CHANGELOG.md | 2 ++ app/manifest.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc8cc5fa3..a6015ccb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +## 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. diff --git a/app/manifest.json b/app/manifest.json index e35f2918d..a21809ce8 100644 --- a/app/manifest.json +++ b/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", From b57009493b49d35beb871a7776283d7f58aa7409 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 8 Nov 2016 08:54:30 -0800 Subject: [PATCH 5/8] Hard coding jazzicon at working version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2ed978cd5..4579e0166 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "iframe": "^1.0.0", "iframe-stream": "^1.0.2", "inject-css": "^0.1.1", - "jazzicon": "^1.1.3", + "jazzicon": "1.1.5", "menu-droppo": "^1.1.0", "metamask-logo": "^2.1.2", "mississippi": "^1.2.0", From b6b4be61d9f01f309b8f4ce708fa1220f84a5d8f Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 9 Nov 2016 16:29:24 -0800 Subject: [PATCH 6/8] Fix bug for increasing gas by 20 percent. --- CHANGELOG.md | 1 + app/scripts/lib/idStore.js | 2 ++ app/scripts/metamask-controller.js | 1 - 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6015ccb3..d0dbb5706 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Current Master +- Fix bug where 20% of gas estimate was not being added properly. ## 2.13.7 2016-11-8 diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 0ce91f471..1d66ee368 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -262,6 +262,8 @@ IdentityStore.prototype.addUnconfirmedTransaction = function (txParams, onTxDone query.estimateGas(txParams, function(err, result){ if (err) return cb(err) txData.estimatedGas = self.addGasBuffer(result) + txData.txParams.gas = txData.estimatedGas + txData.txParams.gasLimit = txData.estimatedGas cb() }) } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 8b593d820..da05e5f5d 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -215,7 +215,6 @@ module.exports = class MetamaskController { let err = this.enforceTxValidations(txParams) if (err) return onTxDoneCb(err) - idStore.addUnconfirmedTransaction(txParams, onTxDoneCb, (err, txData) => { if (err) return onTxDoneCb(err) this.sendUpdate() From e2b2083df03b3bd808193ed38536d07ba155ff58 Mon Sep 17 00:00:00 2001 From: Kevin Serrano Date: Wed, 9 Nov 2016 16:38:28 -0800 Subject: [PATCH 7/8] remove gas field, due to alias redundancy. --- app/scripts/lib/idStore.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/scripts/lib/idStore.js b/app/scripts/lib/idStore.js index 1d66ee368..dd895a6c0 100644 --- a/app/scripts/lib/idStore.js +++ b/app/scripts/lib/idStore.js @@ -262,7 +262,6 @@ IdentityStore.prototype.addUnconfirmedTransaction = function (txParams, onTxDone query.estimateGas(txParams, function(err, result){ if (err) return cb(err) txData.estimatedGas = self.addGasBuffer(result) - txData.txParams.gas = txData.estimatedGas txData.txParams.gasLimit = txData.estimatedGas cb() }) From 17aac2dbc54b733aeed5f16ef2e7bd8378060ca9 Mon Sep 17 00:00:00 2001 From: Frankie Date: Thu, 10 Nov 2016 19:07:12 -0800 Subject: [PATCH 8/8] fix tx data so it has the network id --- app/scripts/keyring-controller.js | 5 +++-- app/scripts/metamask-controller.js | 6 ++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 3ebf02c44..5d9dd51a4 100644 --- a/app/scripts/keyring-controller.js +++ b/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(), } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 4c01b23d8..07d365283 100644 --- a/app/scripts/metamask-controller.js +++ b/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) @@ -399,4 +400,9 @@ module.exports = class MetamaskController { cb(e) } } + + getStateNetwork () { + return this.state.network + } } +