[WIP] cmd/evm: prestate config and json trace fixes#14900
Conversation
| gasCopy = uint64(contract.Gas) | ||
| stackCopy = newstack() | ||
| for _, val := range stack.data { | ||
| stackCopy.push(val) |
There was a problem hiding this comment.
What's the intent here? The stackCopy will contain the same bigint objects as the original stack, so if any of those are modified (e.g. by being popped, added to intpool, and then reused), they will change also in the stackCopy.
There was a problem hiding this comment.
I don't know how the memory management works, but I tested it again to check. Changing it to:
if in.cfg.Debug {
pcCopy = uint64(pc)
gasCopy = uint64(contract.Gas)
stackCopy = stack
}
reproduces the bug where the mutated stack is logged (from stBoundsTest/CALL_BoundsOOG.json):
{"pc":0,"op":96,"gas":"0x1f7e8","gasCost":"0x3","memory":"0x","memSize":0,"stack":[],"depth":1,"error":null,"opName":"PUSH1"}
{"pc":2,"op":96,"gas":"0x1f7e5","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0"],"depth":1,"error":null,"opName":"PUSH1"}
{"pc":4,"op":96,"gas":"0x1f7e2","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0"],"depth":1,"error":null,"opName":"PUSH1"}
{"pc":6,"op":96,"gas":"0x1f7df","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0"],"depth":1,"error":null,"opName":"PUSH1"}
{"pc":8,"op":96,"gas":"0x1f7dc","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0"],"depth":1,"error":null,"opName":"PUSH1"}
{"pc":10,"op":115,"gas":"0x1f7d9","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0"],"depth":1,"error":null,"opName":"PUSH20"}
{"pc":31,"op":103,"gas":"0x1f7d6","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x1000000000000000000000000000000000000001"],"depth":1,"error":null,"opName":"PUSH8"}
{"pc":40,"op":241,"gas":"0x1f7d3","gasCost":"0x1efff","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x1000000000000000000000000000000000000001","0x1ed43"],"depth":1,"error":null,"opName":"CALL"}
{"pc":0,"op":96,"gas":"0x1ed43","gasCost":"0x3","memory":"0x","memSize":0,"stack":[],"depth":2,"error":null,"opName":"PUSH1"}
The 0x1ed43 gas argument to CALL is a mutated value (from the 63/64 rule).
Using stackCopy = newstack() and the for loop gives the correct (unmutated) value 0x7ffffffffffffff:
{"pc":31,"op":103,"gas":"0x1f7d6","gasCost":"0x3","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x1000000000000000000000000000000000000001"],"depth":1,"error":null,"opName":"PUSH8"}
{"pc":40,"op":241,"gas":"0x1f7d3","gasCost":"0x1efff","memory":"0x","memSize":0,"stack":["0x0","0x0","0x0","0x0","0x0","0x1000000000000000000000000000000000000001","0x7ffffffffffffff"],"depth":1,"error":null,"opName":"CALL"}
{"pc":0,"op":96,"gas":"0x1ed43","gasCost":"0x3","memory":"0x","memSize":0,"stack":[],"depth":2,"error":null,"opName":"PUSH1"}
There was a problem hiding this comment.
Ah, so apparently the 63/64-th handling does not operate on the actual bigint already on the stack, but places a new one there, so your fix works (even though it's a bit brittle).
There was a problem hiding this comment.
Happens here:
go-ethereum/core/vm/gas_table.go
Line 329 in dfd0762
| op OpCode // current opcode | ||
| mem = NewMemory() // bound memory | ||
| stack = newstack() // local stack | ||
| stackCopy = newstack() |
There was a problem hiding this comment.
Does stackCopy really need to exist in this scope? Can't it just be created on the fly when needed?
There was a problem hiding this comment.
I tried removing this line and changing L151 to stackCopy := newstack(), but I get:
core/vm/interpreter.go:129: undefined: stackCopy
core/vm/interpreter.go:197: undefined: stackCopy
2 participants
This is only half-done [WIP do not merge]. The evm command is probably broken if certain arguments aren't passed in (no default fallbacks yet).
This PR includes #14873.
Some of these changes add more support for configs given in a prestate (#14476). Others, like charging for intrinsic gas, are to reproduce the results of state tests. Those changes are mostly copy/pasted from state_test_util.go and state_transition.go.
Additionally there are changes to the json tracing. These are done to converge with the EVM traces that pyethereum produces, where it makes sense. For instance, a
stackCopyvariable was added to fix cases where the trace would print a mutated stack (the stack after the operation was applied), rather than printing the stack before the operation was applied.