Skip to content
This repository has been archived by the owner on Jan 5, 2019. It is now read-only.

trace before gas charge is applied #233

Merged
merged 1 commit into from
May 10, 2018
Merged

trace before gas charge is applied #233

merged 1 commit into from
May 10, 2018

Conversation

jwasinger
Copy link
Contributor

to be compliant with the standard trace format: ethereum/tests#249

axic
axic previously approved these changes Apr 27, 2018
@axic axic dismissed their stale review April 27, 2018 10:13

Actually I'm not sure.

@@ -182,6 +182,11 @@ exports.evm2wast = function (evmCode, opts = {
const opint = evmCode[pc]
const op = opcodes(opint)

// creates a stack trace
if (opts.stackTrace) {
segment += `(call $stackTrace (i32.const ${pc}) (i32.const ${opint}) (i32.const ${gasCount}) (get_global $sp))\n`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked issue shows that gasCount here should be the actual gas usage of the instruction, but here this value is always 0 after this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean gasCount is always zero? gasCount gets updated every iteration of the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gasCount here means gas left. Not the cost of the opcode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, missed it was a for loop, sorry.

@axic
Copy link
Member

axic commented Apr 30, 2018

Please fix the test failure:

standard: Run `standard --fix` to automatically fix some problems.
  /home/builder/evm2wasm/index.js:205:1: More than 1 blank line not allowed.

@axic axic merged commit febc245 into master May 10, 2018
@axic axic deleted the trace-fix branch May 10, 2018 12:27
@axic axic removed the in progress label May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants