standardized vmtrace logs

[cdetrio]

This fixes the VM trace logs to be aligned with the other clients, following the work-in-progress standard format ethereum/tests#249.

Some edge cases still remain, which might cause the cpp trace to differ under some circumstances, but this is a good start (traces are equivalent for all EIP158 state tests).

[gcolvin]

One thing worries me - the ON_OP() macro ultimately calls the _onOp() funtion object passed to VM::exec(), which is the hook for more than logging. Moving the place it is called will cause different values to be passed in, which apparently is what we want for logging, but might break other tools. @yann300 ?

[codecov-io]

Codecov Report

Merging #4320 into develop will increase coverage by 0.05%.
The diff coverage is 89.47%.

@@             Coverage Diff             @@
##           develop    #4320      +/-   ##
===========================================
+ Coverage    67.27%   67.33%   +0.05%     
===========================================
  Files          307      307              
  Lines        23576    23567       -9     
===========================================
+ Hits         15861    15869       +8     
+ Misses        7715     7698      -17

Impacted Files	Coverage Δ
libethereum/Executive.cpp	60.15% <0%> (-0.24%)	⬇️
test/tools/libtesteth/ImportTest.cpp	50.36% <0%> (+0.24%)	⬆️
libevm/VM.cpp	95.59% <100%> (ø)	⬆️
libevm/VMCalls.cpp	97.72% <100%> (-0.13%)	⬇️
test/tools/libtesteth/BlockChainHelper.cpp	77.07% <0%> (-0.58%)	⬇️
libdevcore/RLP.cpp	86.57% <0%> (-0.47%)	⬇️
libethereum/BlockChain.cpp	69.78% <0%> (-0.14%)	⬇️
libp2p/Common.cpp	52.8% <0%> (+0.79%)	⬆️
libp2p/Host.cpp	73.41% <0%> (+1.05%)	⬆️
test/tools/fuzzTesting/fuzzHelper.cpp	51.95% <0%> (+5.46%)	⬆️

[gumb0]

@gcolvin I haven't seen it used for anything besides tracing

[gumb0]

I would remove all mentions on m_onFail and use ON_OP() in VM::throwBadStack

[yann300]

That might break some stuff in remix if we change where on_op is called (is on_op called after or before the actual execution of the step).
Just ping us when all the implementations agree on smt and we'll adapt.

[gcolvin]

@yann300 @cdetrio @winsvega I'm wondering if this is premature, given that the standard format isn't done? It seems that as soon we merge this remix will break. All implementations need to change at once.
@gumb0 If m_onFail isn't used elsewhere I agree it should go. I'm on the road right now and can't check the code.

[chfast]

Looks ok to me. What do you think @gcolvin?

Also there is an issue with VM optimizer that injects PUSHC virtual opcodes. Can they be reverted in the trace?

[gcolvin]

@chfast I have some worries I've already stated.
Right now injected opcodes are just passed to m_onOp like any other opcode, which doesn't seem to be causing trouble. I think geth does a similar transformation, but I'm not sure how it handles tracing it, @obscuren.
I think the current C++ VM could reconstruct the original opcodes, but in principle, a VM could transform the codes into something completely different, even machine code, so there is a bigger problem of how and whether to trace and debug optimized code.

[pirapira]

Trace is an artificial layer and when we go down the optimization we will need to fake it.

@cdetrio says clients produce the same trace with this, I think it should be added.

If we try to keep remix working, one way is to emit the new format with a special command line option, and keep the old behavior without that option. Is that preferable?

[yann300]

I'm totally fine to update remix to the new trace as long as all the clients have the same format.

[pirapira]

I'm buying this

traces are equivalent for all EIP158 state tests