Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

standard trace fixes #4586

Merged
merged 1 commit into from
Oct 16, 2017
Merged

standard trace fixes #4586

merged 1 commit into from
Oct 16, 2017

Conversation

cdetrio
Copy link
Member

@cdetrio cdetrio commented Oct 6, 2017

Some fixes for standardized tracing ethereum/tests#249. See also #4320

@chfast chfast requested a review from gcolvin October 8, 2017 17:04
Copy link
Contributor

@gcolvin gcolvin left a comment

Choose a reason for hiding this comment

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

@cdetrio @chfast I don't see anything here that could break the interpreter, and it does appear to put all the logging hooks at the start of each operation. Why they weren't in the first place I don't know - I left them as I found them.

So I look forward to passing only a portion of the stack. And to finding a way to fix this problem:

https://github.com/cdetrio/cpp-ethereum/blob/c491cf223d011982d45b79eb854df17d3d7bd0de/libevm/VM.cpp#L199

libevm/VM.cpp Outdated
@@ -233,13 +233,17 @@ void VM::interpretCases()
CASE(CREATE2)
{
if (!m_schedule->haveCreate2)
{
ON_OP();
Copy link
Member Author

Choose a reason for hiding this comment

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

on second thought, shouldn't this go before if (!m_schedule->haveCreate2)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes!

@codecov-io
Copy link

codecov-io commented Oct 11, 2017

Codecov Report

Merging #4586 into develop will decrease coverage by 5.05%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4586      +/-   ##
===========================================
- Coverage    58.59%   53.54%   -5.06%     
===========================================
  Files         1091     1618     +527     
  Lines        55790    70543   +14753     
  Branches      3604     7224    +3620     
===========================================
+ Hits         32691    37771    +5080     
- Misses       22046    31528    +9482     
- Partials      1053     1244     +191
Impacted Files Coverage Δ
libevm/VMCalls.cpp 97.81% <ø> (+0.08%) ⬆️
libevm/VM.cpp 95.84% <83.33%> (+0.04%) ⬆️
libethereum/CommonNet.h 25% <0%> (-75%) ⬇️
libethashseal/EthashProofOfWork.h 80% <0%> (-20%) ⬇️
libethcore/LogEntry.h 43.47% <0%> (-15.35%) ⬇️
libethash/internal.h 85.71% <0%> (-14.29%) ⬇️
libethcore/ChainOperationParams.h 87.5% <0%> (-12.5%) ⬇️
libethash/io_posix.c 76.59% <0%> (-8.78%) ⬇️
test/tools/jsontests/BlockChainTests.cpp 66.25% <0%> (-8.58%) ⬇️
test/unittests/libethcore/difficulty.cpp 47.3% <0%> (-5.76%) ⬇️
... and 730 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e10157...ee64227. Read the comment docs.

@cdetrio
Copy link
Member Author

cdetrio commented Oct 11, 2017

pushed an edit, should be ready now.

Copy link
Member

@pirapira pirapira left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@pirapira pirapira merged commit c5d2b48 into ethereum:develop Oct 16, 2017
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.

None yet

4 participants