Skip to content

Commit

Permalink
Fix gas price buffering
Browse files Browse the repository at this point in the history
Our gas price buffering logic had a bug, because bn.js has inconsistent behavior when using hex-prefixed output.  The issue has been opened with them here:
indutny/bn.js#151

We've corrected our usage in the mean time.
  • Loading branch information
danfinlay committed Nov 7, 2016
1 parent fff5a67 commit 1896928
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Current Master

- Fix gas estimation bug.
- Fix github link on info page to point at current repository.

## 2.13.6 2016-10-26
Expand Down
10 changes: 5 additions & 5 deletions app/scripts/lib/idStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,11 @@ IdentityStore.prototype.checkForDelegateCall = function (codeHex) {
}
}

IdentityStore.prototype.addGasBuffer = function (gasHex) {
var gas = new BN(gasHex, 16)
var buffer = new BN('100000', 10)
var result = gas.add(buffer)
return ethUtil.addHexPrefix(result.toString(16))
const gasBuffer = new BN('100000', 10)
IdentityStore.prototype.addGasBuffer = function (gas) {
const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16)
const correct = bnGas.add(gasBuffer)
return ethUtil.addHexPrefix(correct.toString(16))
}

// comes from metamask ui
Expand Down
14 changes: 6 additions & 8 deletions test/unit/idStore-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,20 +169,18 @@ describe('IdentityStore', function() {

const gas = '0x04ee59' // Actual estimated gas example
const tooBigOutput = '0x80674f9' // Actual bad output
const bnGas = new BN(gas, 16)
const bnGas = new BN(ethUtil.stripHexPrefix(gas), 16)
const correctBuffer = new BN('100000', 10)
const correct = bnGas.add(correctBuffer)

const tooBig = new BN(tooBigOutput, 16)
console.log(`Pure estimate is ${bnGas.toString(10)}`)
console.log(`Too big is ${tooBig.toString(10)}`)
console.log(`Buffer should be ${correctBuffer.toString(10)}`)
console.log(`correct should be ${correct.toString(10)}`)
const result = idStore.addGasBuffer(gas)
const bnResult = new BN(result, 16)
const bnResult = new BN(ethUtil.stripHexPrefix(result), 16)

console.log(`Result was ${bnResult.toString(10)}`)
assert.equal(result, correct.toString(16), 'add the right amount')
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(result, '0x' + correct.toString(16), 'Added the right amount')
assert.notEqual(result, tooBigOutput, 'not that bad estimate')
})
})
Expand Down

0 comments on commit 1896928

Please sign in to comment.