From e78e642eef21c0219eb025f859edea5817363b3f Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 7 Mar 2017 11:34:11 -0800 Subject: [PATCH 1/8] Add gas buffer test --- test/unit/tx-utils-test.js | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 test/unit/tx-utils-test.js diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js new file mode 100644 index 000000000..6131ed73f --- /dev/null +++ b/test/unit/tx-utils-test.js @@ -0,0 +1,38 @@ +const assert = require('assert') +const ethUtil = require('ethereumjs-util') +const BN = ethUtil.BN + +const TxUtils = require('../../app/scripts/lib/tx-utils') + + +describe('txUtils', function() { + let txUtils + + before(function() { + txUtils = new TxUtils() + }) + + describe('addGasBuffer', function() { + describe('adds a flat value', function() { + it('over an empty value', function() { + const input = '0x0' + const output = txUtils.addGasBuffer(input) + assert.notEqual(output, input, 'changed the value') + + const inputBn = new BN(input, 'hex') + const outputBn = new BN(output, 'hex') + assert(outputBn.gt(inputBn), 'returns a greater value') + }) + + it('over an value', function() { + const input = '0x123fad' + const output = txUtils.addGasBuffer(input) + assert.notEqual(output, input, 'changed the value') + + const inputBn = new BN(input, 'hex') + const outputBn = new BN(output, 'hex') + assert(outputBn.gt(inputBn), 'returns a greater value') + }) + }) + }) +}) \ No newline at end of file From 147ac416ba5184b914cdc6ed6a1a9b8b94446765 Mon Sep 17 00:00:00 2001 From: kumavis Date: Tue, 7 Mar 2017 16:16:26 -0800 Subject: [PATCH 2/8] deps - bump web3-provider-engine to 10.0.1 Fixes https://github.com/MetaMask/metamask-plugin/issues/1163 Fixes https://github.com/MetaMask/provider-engine/issues/121 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 839f7bcd2..475cdb2b3 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,7 @@ "valid-url": "^1.0.9", "vreme": "^3.0.2", "web3": "0.18.2", - "web3-provider-engine": "^10.0.0", + "web3-provider-engine": "^10.0.1", "web3-stream-provider": "^2.0.6", "xtend": "^4.0.1" }, From 4256e631a6174f04d338412f419ceb0c08d724f1 Mon Sep 17 00:00:00 2001 From: Jared Pereira Date: Tue, 7 Mar 2017 19:57:57 -0500 Subject: [PATCH 3/8] remove constant buffer and add multiplier --- app/scripts/lib/tx-utils.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 19a2d430e..15cbb5b68 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -55,7 +55,7 @@ module.exports = class txProviderUtils { // try adding an additional gas buffer to our estimation for safety const estimatedGasBn = new BN(ethUtil.stripHexPrefix(txData.estimatedGas), 16) const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) - const estimationWithBuffer = new BN(this.addGasBuffer(estimatedGasBn), 16) + const estimationWithBuffer = new BN(this.addGasBuffer(estimatedGasBn, blockGasLimitHex), 16) // added gas buffer is too high if (estimationWithBuffer.gt(blockGasLimitBn)) { txParams.gas = txData.estimatedGas @@ -68,11 +68,14 @@ module.exports = class txProviderUtils { return } - addGasBuffer (gas) { - const gasBuffer = new BN('100000', 10) + addGasBuffer (gas, blockGasLimitHex) { + const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const correct = bnGas.add(gasBuffer) - return ethUtil.addHexPrefix(correct.toString(16)) + const bufferedGas = bnGas.mul(1.5) + + if (bnGas.gt(blockGasLimitBn)) return gas + if (bufferedGas.lt(blockGasLimitBn)) return ethUtil.addHexPrefix(bufferedGas.toString(16)) + return ethUtil.addHexPrefix(blockGasLimitBn.toString(16)) } fillInTxParams (txParams, cb) { From de44cd9ba46e562471d64e6cfc56cf8518f45113 Mon Sep 17 00:00:00 2001 From: Jared Pereira Date: Tue, 7 Mar 2017 20:49:40 -0500 Subject: [PATCH 4/8] add gas buffer multiply test --- test/unit/tx-utils-test.js | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index 6131ed73f..a7f2094c4 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -13,26 +13,16 @@ describe('txUtils', function() { }) describe('addGasBuffer', function() { - describe('adds a flat value', function() { - it('over an empty value', function() { - const input = '0x0' - const output = txUtils.addGasBuffer(input) - assert.notEqual(output, input, 'changed the value') - - const inputBn = new BN(input, 'hex') - const outputBn = new BN(output, 'hex') - assert(outputBn.gt(inputBn), 'returns a greater value') - }) - - it('over an value', function() { + describe('multiplies by 1.5', function() { + it('over a value', function() { const input = '0x123fad' - const output = txUtils.addGasBuffer(input) - assert.notEqual(output, input, 'changed the value') + const output = txUtils.addGasBuffer(input, '0x3d4c52') //0x3d4c52 is 4mil for dummy gas limit const inputBn = new BN(input, 'hex') const outputBn = new BN(output, 'hex') - assert(outputBn.gt(inputBn), 'returns a greater value') + const expectedBn = inputBn.mul(1.5) + assert(outputBn.eq(expectedBn), 'returns 1.5 the input value') }) }) }) -}) \ No newline at end of file +}) From 3e8b584c9811b06526e69203a5e03f8eb82e8211 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 7 Mar 2017 17:59:03 -0800 Subject: [PATCH 5/8] fix issue where account import allows for duplicates --- app/scripts/keyring-controller.js | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index e1b1c4335..7669b9f8f 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -164,8 +164,11 @@ class KeyringController extends EventEmitter { return keyring.getAccounts() }) .then((accounts) => { + return this.checkForDuplicate(type, accounts) + }) + .then((checkedAccounts) => { this.keyrings.push(keyring) - return this.setupAccounts(accounts) + return this.setupAccounts(checkedAccounts) }) .then(() => this.persistAllKeyrings()) .then(() => this.fullUpdate()) @@ -175,6 +178,24 @@ class KeyringController extends EventEmitter { }) } + // For now just checks for simple key pairs + // but in the future + // should possibly add HD and other types + // + checkForDuplicate (type, newAccount) { + return this.getAccounts() + .then((accounts) => { + switch (type) { + case 'Simple Key Pair': + let isNotIncluded = !accounts.find((key) => key === newAccount[0] || key === ethUtil.stripHexPrefix(newAccount[0])) + return (isNotIncluded) ? Promise.resolve(newAccount) : Promise.reject(new Error('The account your are trying to import is a duplicate')) + default: + return Promise.resolve(newAccount) + } + }) + } + + // Add New Account // @number keyRingNum // From 2f7a95c25765ec040eddb8f3103bfe60d4760a26 Mon Sep 17 00:00:00 2001 From: frankiebee Date: Tue, 7 Mar 2017 18:56:38 -0800 Subject: [PATCH 6/8] Fix grammar in erro message --- app/scripts/keyring-controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/keyring-controller.js b/app/scripts/keyring-controller.js index 7669b9f8f..72f613641 100644 --- a/app/scripts/keyring-controller.js +++ b/app/scripts/keyring-controller.js @@ -188,7 +188,7 @@ class KeyringController extends EventEmitter { switch (type) { case 'Simple Key Pair': let isNotIncluded = !accounts.find((key) => key === newAccount[0] || key === ethUtil.stripHexPrefix(newAccount[0])) - return (isNotIncluded) ? Promise.resolve(newAccount) : Promise.reject(new Error('The account your are trying to import is a duplicate')) + return (isNotIncluded) ? Promise.resolve(newAccount) : Promise.reject(new Error('The account you\'re are trying to import is a duplicate')) default: return Promise.resolve(newAccount) } From 4916331c530896bbcef13a71a8f6dc0164b43c01 Mon Sep 17 00:00:00 2001 From: Jared Pereira Date: Tue, 7 Mar 2017 22:42:16 -0500 Subject: [PATCH 7/8] change BN.mul to BN.muln --- app/scripts/lib/tx-utils.js | 2 +- test/unit/tx-utils-test.js | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js index 15cbb5b68..f5051eb8f 100644 --- a/app/scripts/lib/tx-utils.js +++ b/app/scripts/lib/tx-utils.js @@ -71,7 +71,7 @@ module.exports = class txProviderUtils { addGasBuffer (gas, blockGasLimitHex) { const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16) const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16) - const bufferedGas = bnGas.mul(1.5) + const bufferedGas = bnGas.muln(1.5) if (bnGas.gt(blockGasLimitBn)) return gas if (bufferedGas.lt(blockGasLimitBn)) return ethUtil.addHexPrefix(bufferedGas.toString(16)) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index a7f2094c4..65233e1d9 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -13,16 +13,14 @@ describe('txUtils', function() { }) describe('addGasBuffer', function() { - describe('multiplies by 1.5', function() { - it('over a value', function() { - const input = '0x123fad' - const output = txUtils.addGasBuffer(input, '0x3d4c52') //0x3d4c52 is 4mil for dummy gas limit + it('multiplies by 1.5', function() { + const input = '0x123fad' + const output = txUtils.addGasBuffer(input, '0x3d4c52') //0x3d4c52 is 4mil for dummy gas limit - const inputBn = new BN(input, 'hex') - const outputBn = new BN(output, 'hex') - const expectedBn = inputBn.mul(1.5) - assert(outputBn.eq(expectedBn), 'returns 1.5 the input value') - }) + const inputBn = new BN(ethUtil.stripHexPrefix(input), 'hex') + const outputBn = new BN(ethUtil.stripHexPrefix(output), 'hex') + const expectedBn = inputBn.muln(1.5) + assert(outputBn.eq(expectedBn), 'returns 1.5 the input value') }) }) }) From 1f0a4dd0331cbcc210ab9c8fc2a323af84fe4bf5 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Tue, 7 Mar 2017 21:30:27 -0800 Subject: [PATCH 8/8] Bump changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d07154bdf..603a92df5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Add personal_sign method support. - Add ability to customize gas and gasPrice on the transaction approval screen. +- Increase default gas buffer to 1.5x estimated gas value. ## 3.3.0 2017-2-20