Add Gas Refund to Transaction Results

[mikeseese]

Fixes #283

Excerpt to #283 that this PR implements:

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.

[coveralls]

Coverage remained the same at 85.436% when pulling 6ea041d on seesemichaelj:report-gas-refund into 2eb03fd on ethereumjs:master.

[holgerd77]

You have a linting error causing the tests to fail, ; is not allowed to end a line.

See also my other comment.

[holgerd77]

I think it would be better if results.vm.gasRefund is always assigned to gasRefund than this always has a value, being it BN(0) or something else.

[mikeseese]

Sorry, I'm not following for some reason.

Do you mean something along the lines of:
var gasRefund = results.vm.gasRefund || BN(0)

or are you looking for me to make sure that results.vm.gasRefund has a value?

[holgerd77]

Along the lines of the first assignment, but you can leave the || BN(0) part, results.vm.gasRefund is already initialized with 0 I think (you might want to cross-check this).

[mikeseese]

Sad day; I'll make sure to enable linting on the project to make sure I get rid of those semicolons.

[mikeseese]

Let me know if these changes aren't sufficient; thanks!

[holgerd77]

Can you rebase this (we have deactivated the "Rebase and merge" feature for this library for security reasons, thanks for the tip with "Squash and merge" for the other PR, will use this in the future)? Then I will approve and merge.

[mikeseese]

I believe that should be properly rebased now

[mikeseese]

Just following up @holgerd77 to see if there's anything else I need to do for this PR

[holgerd77]

Looks good now.