Out of Gas Issues Due to Interpreting "gasUsed" as "gasConsumed"

[mikeseese]

This issue comes about from generic revert errors reported in issue trufflesuite/ganache#412.

The first revert error was mitigated from trufflesuite/ganache@2421f4a and is not related to this issue.

However the second revert was due to running out of gas when using the estimated gas for a transaction. Even increasing the gas limit a little to account for minor fluctuations didn't solve the issue. It turns out the transaction was receiving a gas refund, and ethereumjs-vm implements this by subtracting the refund from gasUsed (see https://github.com/ethereumjs/ethereumjs-vm/blob/2a63a921f4f3b8aa85ffcc3c05dc9cc3922699a9/lib/runTx.js#L140-L148) which is a little counter-intuitive because the gas is used, and later refunded. ethereumjs-vm correctly implements the consumption of this gas at first, and then reports less gas was used because of a refund but does not report a refund was used.

Therefore, if you estimated a transaction that was eligible for a SSTORE refund of 15,000 gas, the gasUsed reported by ethereumjs-vm would be up to 15,000 gas more than what was actually consumed before a refund. If the user then tried to run the transaction with the same amount of gas used of the estimate, the EVM could run out of gas in the middle of the transaction, and report an OOG error (or revert in some cases it seems).

I can see why we shouldn't change the reported gasUsed, because it's the amount of net gas that was removed from the sending account. However, ethereumjs-vm should at a minimum report the gasRefund to let callers know that additional gas may be needed to actually execute the transaction. I believe this proposal makes sense, but I'd love to talk alternatives if it's not quite right. I'm going to go ahead open a PR that implements this proposal.

[mikeseese]

A potential option which would have prevented the OOG errors is to add a binary search method to estimating gas (see the discussion in trufflesuite/ganache#26). Regardless, I feel that reporting the gasRefund may be a good addition to the transaction results for posterity's sake.

[holgerd77]

Hi @seesemichaelj, thanks for your wonderful and extensive error descriptions, will have a first look into this.

[holgerd77]

Think there should be no reason to not also report the gasRefund. Just followed the code path, looks to me that this would be added to result in the afterBlock event here: https://github.com/ethereumjs/ethereumjs-vm/blob/master/lib/runBlock.js#L54

Did you come to the same conclusion?

[mikeseese]

@holgerd77 no problem! I maintained a repo for a few months and realized great issue descriptions => happy maintainers. And I'm trying to get you to like me before the more controversial PRs start coming in ;)

Yes, that is the conclusion I came up with as well!

[holgerd77]

Hehe. 😀