Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Difference between gasUsed and execResult.gasUsed #1446

Closed
Tracked by #1024
yann300 opened this issue Sep 1, 2021 · 13 comments
Closed
Tracked by #1024

Difference between gasUsed and execResult.gasUsed #1446

yann300 opened this issue Sep 1, 2021 · 13 comments

Comments

@yann300
Copy link
Contributor

yann300 commented Sep 1, 2021

the afterTx event (link below) used to contain 2 different values for the gas consumption (tx cost and exec cost): { gasUsed } and { execResult: { gasUsed } } . The latter seems now to also return the tx cost.

await this._emit('afterTx', event)

@ryanio
Copy link
Contributor

ryanio commented Sep 1, 2021

Hm, when did this change for you? I am looking around the code in evm.ts and not seeing much modified in the past few releases, but I might be missing something. Could you share a code sample of the expected vs actual results?

I also noticed that the debug output is outputting the same variable twice (results.gasUsed) instead of results.execResult.gasUsed the second time:

`Received tx results gasUsed=${results.gasUsed} execResult: [ gasUsed=${
results.gasUsed

and also here
`Received tx results gasUsed=${results.gasUsed} execResult: [ gasUsed=${
results.gasUsed

@holgerd77
Copy link
Member

holgerd77 commented Sep 2, 2021

Just for the record: the mentioned PR #1375 has been merged on July 25, so there are the two VM releases v5.5.1 and v5.5.2 affected by this.

@holgerd77
Copy link
Member

Update: oh, just stricing the above comment through again, I was writing based on the un-edited text I received via mail.

@ryanio so, just for the (updated) record 😄: you came to the conclusion after further analysis that #1375 didn't introduce this as a new bug?

@ryanio
Copy link
Contributor

ryanio commented Sep 2, 2021

@ryanio so, just for the (updated) record 😄: you came to the conclusion after further analysis that #1375 didn't introduce this as a new bug?

I thought it might have, but the line I was considering (here) was actually gasRefund not gasUsed.

It could still be introduced by that PR, but it's not immediately obvious to me yet.

For the OP issue, one common cause is that It could be a mutated BN we need to clone that's getting accidentally modified. I am wondering between which versions this behavior changed for @yann300.

@holgerd77
Copy link
Member

Additional note: this should be easily be verifiable by looking at some execution with VM.runTx() I guess, e.g. by looking at the results of one of the tests in runTx.spec.ts.

@ryanio
Copy link
Contributor

ryanio commented Jan 13, 2022

@yann300 any further thoughts or follow-up here? do you remember what you ended up doing?

"gasUsed" is a little confusing because we have two different meanings:

  • cumulativeGasUsed (as used in the tx receipt, gas used in the block including the tx)
  • txGasUsed (gas used only for one tx)

I would like to at least resolve my comment above about the second log output not outputting results.execResult.gasUsed, and meanwhile would be good to note the differences between the two values in the typedoc (if there is)

@holgerd77
Copy link
Member

@ryanio maybe we want to rename on the breaking releases? If so, could you directly add a note with suggested names to #1024 ?

@ryanio
Copy link
Contributor

ryanio commented Jan 14, 2022

that's a good idea, will do, let me dig into it a bit more and I'll report my findings here and add next steps to that issue

@holgerd77
Copy link
Member

that's a good idea, will do, let me dig into it a bit more and I'll report my findings here and add next steps to that issue

👍 🙂

Ok, have already added the link to the comment here as a placeholder so that we don't forget.

@ryanio
Copy link
Contributor

ryanio commented Feb 1, 2022

Ok I've investigated further, and based on evm.ts:L457 below they do seem to be the same value:

return {
gasUsed: result.gasUsed,
createdAddress: message.to,
execResult: result,
}

Which I think is fine, it was probably a decision to surface that important number in EVMResult from ExecResult:

/**
* Result of executing a message via the {@link EVM}.
*/
export interface EVMResult {
/**
* Amount of gas used by the transaction
*/
gasUsed: BN

/**
* Result of executing a call via the {@link EVM}.
*/
export interface ExecResult {
runState?: RunState
/**
* Description of the exception, if any occured
*/
exceptionError?: VmError
/**
* Amount of gas left
*/
gas?: BN
/**
* Amount of gas the code used to run
*/
gasUsed: BN

(edit: I've looked into this further and the reason we have a gasUsed on the EVMResult is that we modify it in runTx to add the baseFee and subtract the refund, so I think we can move this to RunTxResult)

@holgerd77
Copy link
Member

@jochem-brouwer can you please write a qualified comment on the state of this and eventually close here? 😋

@jochem-brouwer
Copy link
Member

@holgerd77 Yup!

This can be closed, as in V6 we have now renamed the vars:

execResult.gasUsed is now execResult.executionGasUsed

gasUsed is now totalGasSpent

The difference in the name now explains what the semantical difference is. In runTx, the totalGasSpent includes the executionGasUsed (thus execResult.gasUsed) and the upfront cost (to pay for the tx cost (21000 for normal txs, some more for txs which create a contract)) and the data cost (this includes the access list cost). The executionGasUsed is "simply" the gas used by the execution engine, i.e. the cost to actually execute the code.

Will close.

@holgerd77
Copy link
Member

Thanks a lot, that's a really great explanation! 🙏🙂👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants