Consume gasRefund From Potential ethereumjs-vm Addition #141
Conversation
Also had to tweak the `add`/`transfer` gas estimation tests as on `transfer` we'll estimate `15000` too high due to the `transfer` call setting storage back to `0` (and we don't account for this `15000` refund in our estimate). To work around the issue I increased the amount initially added to the recipient to `10` so that transfering `5` out in the transfer test doesn't set it to `0`.
…s will work I wanted to test against geth using the web3 http provider and it wouldn't work the way the tests were written.
|
@davidmurdoch FYI, sometimes you can push directly to the PR branch if they checked the box to "allow reviewers to modify it". no problem though! |
|
@seesemichaelj I used Github's "open this in GitHub Desktop" link on the PR and it automatically created a new branch instead of just opening your branch and so I just rolled with it from there. I'll be sure to do it The Right Way next time. |
lib/statemanager.js
Outdated
| @@ -594,7 +594,7 @@ StateManager.prototype.processGasEstimate = function (from, rawTx, blockNumber, | |||
| } | |||
| var result = '0x0' | |||
| if (!results.error) { | |||
| result = to.hex(results.gasUsed) | |||
| result = results.gasRefund ? to.hex(results.gasUsed.add(results.gasRefund)) : to.hex(results.gasUsed); | |||
There was a problem hiding this comment.
We expect the gasRefund field to always be there, right? If so, this squelches an error and makes things look like they're working if the gasRefund field suddenly disappears.
There was a problem hiding this comment.
fixed in latest commits
|
It might be worth adding a test to check whether our gas estimation accounts for explicit gas stipends properly (e.g. something like Relevant docs here. |
…h it anyway. Reason: if it this value gets renamed in a future version of ethereumvm-js for whatever reason we don't want to just ignore the `undefined` error.
This is the same as PR #80 with added tests. Fixes issue #26:
estimateGas returns invalid value when tx has a gasRefund.