Merge pull request #1189 from MetaMask/kumavis-patch-1

tx-utils - clean and comment

2 files changed, 60 insertions, 26 deletions

diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js
index f5051eb8f..c6814c05f 100644
--- a/app/scripts/lib/tx-utils.js
+++ b/app/scripts/lib/tx-utils.js
@@ -53,29 +53,23 @@ module.exports = class txProviderUtils {
     }
     // if gasLimit not originally specified,
     // 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, blockGasLimitHex), 16)
-    // added gas buffer is too high
-    if (estimationWithBuffer.gt(blockGasLimitBn)) {
-      txParams.gas = txData.estimatedGas
-    // added gas buffer is safe
-    } else {
-      const gasWithBufferHex = ethUtil.intToHex(estimationWithBuffer)
-      txParams.gas = gasWithBufferHex
-    }
+    const recommendedGasHex = this.addGasBuffer(txData.estimatedGas, blockGasLimitHex)
+    txParams.gas = recommendedGasHex
     cb()
     return
   }
 
-  addGasBuffer (gas, blockGasLimitHex) {
-    const blockGasLimitBn = new BN(ethUtil.stripHexPrefix(blockGasLimitHex), 16)
-    const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16)
-    const bufferedGas = bnGas.muln(1.5)
-
-    if (bnGas.gt(blockGasLimitBn)) return gas
-    if (bufferedGas.lt(blockGasLimitBn)) return ethUtil.addHexPrefix(bufferedGas.toString(16))
-    return ethUtil.addHexPrefix(blockGasLimitBn.toString(16))
+  addGasBuffer (initialGasLimitHex, blockGasLimitHex) {
+    const initialGasLimitBn = hexToBn(initialGasLimitHex)
+    const blockGasLimitBn = hexToBn(blockGasLimitHex)
+    const bufferedGasLimitBn = initialGasLimitBn.muln(1.5)
+    
+    // if initialGasLimit is above blockGasLimit, dont modify it
+    if (initialGasLimitBn.gt(blockGasLimitBn)) return bnToHex(initialGasLimitBn)
+    // if bufferedGasLimit is below blockGasLimit, use bufferedGasLimit
+    if (bufferedGasLimitBn.lt(blockGasLimitBn)) return bnToHex(bufferedGasLimitBn)
+    // otherwise use blockGasLimit
+    return bnToHex(blockGasLimitBn)
   }
 
   fillInTxParams (txParams, cb) {
@@ -97,7 +91,7 @@ module.exports = class txProviderUtils {
   // builds ethTx from txParams object
   buildEthTxFromParams (txParams) {
     // apply gas multiplyer
-    let gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16)
+    let gasPrice = hexToBn(txParams.gasPrice)
     // multiply and divide by 100 so as to add percision to integer mul
     txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber())
     // normalize values
@@ -133,3 +127,11 @@ module.exports = class txProviderUtils {
 function isUndef(value) {
   return value === undefined
 }
+
+function bnToHex(inputBn) {
+  return ethUtil.addHexPrefix(inputBn.toString(16))
+}
+
+function hexToBn(inputHex) {
+  return new BN(ethUtil.stripHexPrefix(inputHex), 16)
+}
\ No newline at end of file
diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js
index 65233e1d9..e57b25e83 100644
--- a/test/unit/tx-utils-test.js
+++ b/test/unit/tx-utils-test.js
@@ -13,14 +13,46 @@ describe('txUtils', function() {
   })
 
   describe('addGasBuffer', function() {
-    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(ethUtil.stripHexPrefix(input), 'hex')
-      const outputBn = new BN(ethUtil.stripHexPrefix(output), 'hex')
+    it('multiplies by 1.5, when within block gas limit', function() {
+      // naive estimatedGas: 0x123fad (~1.2 mil)
+      const inputHex = '0x123fad'
+      // dummy gas limit: 0x3d4c52 (4 mil)
+      const blockGasLimitHex = '0x3d4c52'
+      const output = txUtils.addGasBuffer(inputHex, blockGasLimitHex)
+      const inputBn = hexToBn(inputHex)
+      const outputBn = hexToBn(output)
       const expectedBn = inputBn.muln(1.5)
       assert(outputBn.eq(expectedBn), 'returns 1.5 the input value')
     })
+    
+    it('uses original estimatedGas, when above block gas limit', function() {
+      // naive estimatedGas: 0x123fad (~1.2 mil)
+      const inputHex = '0x123fad'
+      // dummy gas limit: 0x0f4240 (1 mil)
+      const blockGasLimitHex = '0x0f4240'
+      const output = txUtils.addGasBuffer(inputHex, blockGasLimitHex)
+      const inputBn = hexToBn(inputHex)
+      const outputBn = hexToBn(output)
+      const expectedBn = hexToBn(inputHex)
+      assert(outputBn.eq(expectedBn), 'returns the original estimatedGas value')
+    })
+
+    it('buffers up to block gas limit', function() {
+      // naive estimatedGas: 0x123fad (~1.2 mil)
+      const inputHex = '0x1e8480'
+      // dummy gas limit: 0x1e8480 (2 mil)
+      const blockGasLimitHex = '0x1e8480'
+      const output = txUtils.addGasBuffer(inputHex, blockGasLimitHex)
+      const inputBn = hexToBn(inputHex)
+      const outputBn = hexToBn(output)
+      const expectedBn = hexToBn(blockGasLimitHex)
+      assert(outputBn.eq(expectedBn), 'returns the block gas limit value')
+    })
   })
 })
+
+// util
+
+function hexToBn(inputHex) {
+  return new BN(ethUtil.stripHexPrefix(inputHex), 16)
+}
\ No newline at end of file