Skip to content

tools: Add option to dump state-diff in state tests - #1722

Merged
rodiazet merged 1 commit into
masterfrom
post-state-log-on-master
Sep 23, 2026
Merged

rodiazet merged 1 commit into
masterfrom
post-state-log-on-master

Conversation

@rodiazet

@rodiazet rodiazet commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Add new flag --state-diff to evmone test. If used, the execution summary is extended with JSON encoded transaction state diff.

@rodiazet
rodiazet requested a review from chfast September 18, 2026 08:38
@codecov

codecov Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.46835% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.98%. Comparing base (c60dd55) to head (fdd0069).

Files with missing lines Patch % Lines
test/utils/statetest_runner.cpp 88.88% 0 Missing and 1 partial ⚠️
tools/evmone/main.cpp 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1722      +/-   ##
==========================================
- Coverage   97.98%   97.98%   -0.01%     
==========================================
  Files         182      183       +1     
  Lines       16778    16850      +72     
  Branches     3832     3856      +24     
==========================================
+ Hits        16440    16510      +70     
  Misses        250      250              
- Partials       88       90       +2     
Flag Coverage Δ
eest-develop 81.72% <26.47%> (-0.26%) ⬇️
eest-develop-gmp 25.85% <11.39%> (-0.08%) ⬇️
eest-legacy 17.06% <6.32%> (-0.06%) ⬇️
eest-libsecp256k1 28.04% <11.39%> (-0.09%) ⬇️
eest-stable 81.72% <26.47%> (-0.26%) ⬇️
evmone-unittests 94.41% <97.46%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 95.94% <ø> (ø)
tooling 94.28% <94.11%> (-0.03%) ⬇️
tests 99.81% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
test/unittests/statediff_export_test.cpp 100.00% <100.00%> (ø)
test/unittests/statetest_runner_test.cpp 100.00% <100.00%> (ø)
test/utils/statetest.hpp 84.61% <ø> (ø)
test/utils/statetest_export.cpp 99.05% <100.00%> (+0.15%) ⬆️
test/utils/test_driver.cpp 98.07% <100.00%> (+0.03%) ⬆️
test/utils/test_driver.hpp 100.00% <ø> (ø)
test/utils/statetest_runner.cpp 98.50% <88.88%> (-1.50%) ⬇️
tools/evmone/main.cpp 96.42% <83.33%> (-0.55%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed

codspeed Bot commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 129 untouched benchmarks


Comparing post-state-log-on-master (fdd0069) with master (c60dd55)

Open in CodSpeed

@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 3 times, most recently from 9945ee2 to 400b836 Compare September 18, 2026 11:13

@chfast chfast left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd split the two features. the log hash check and output is no-brainer. for the state diff I have some questions.

@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 2 times, most recently from 6721480 to 08e3150 Compare September 18, 2026 11:34
@rodiazet rodiazet changed the title test: Add dumping post state and reporting log hash for state test running. test: Add dumping post state for state test running. Sep 18, 2026
@rodiazet
rodiazet changed the base branch from master to trace-summary-log-hash September 18, 2026 11:36
@rodiazet
rodiazet added this pull request to stack #1724 September 18, 2026 11:36
@rodiazet

Copy link
Copy Markdown
Member Author

Splited. I make them dependent only because I need to have two changes in one branch. They are independent.

@rodiazet
rodiazet force-pushed the post-state-log-on-master branch from 08e3150 to 85534b5 Compare September 18, 2026 13:41

@chfast chfast left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some general comments:

  1. Do you want to use the "standardized" state diff JSON format?
  2. I think this should rather go to the summary JSON as dedicated field next to state root hash and logs hash. The --statediff param would force --summary.
  3. We are reworking this code to be testable via unit tests: this applies here, but is not required: it's better to pass std::ostream to run_state_test() to be able to capture summary output and test these in unit tests.

@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 2 times, most recently from b596463 to 01332c9 Compare September 18, 2026 14:45
@rodiazet

rodiazet commented Sep 18, 2026 •

Copy link
Copy Markdown
Member Author
  1. I wanna stay with this evmone format. That one looks kinda overdone.
  2. Will merge it with --trace-summary output.
  3. Will rewrite the tests if it will be possible.

Base automatically changed from trace-summary-log-hash to master September 19, 2026 09:01
@chfast
chfast force-pushed the post-state-log-on-master branch from 01332c9 to 5ddc30c Compare September 19, 2026 09:01
@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 3 times, most recently from 0930802 to 38bc015 Compare September 21, 2026 12:21
@rodiazet

Copy link
Copy Markdown
Member Author

Redone

@rodiazet
rodiazet requested review from chfast and a balanced review from Copilot September 21, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

User-facing documentation misidentifies the output stream and overstates the precision of StateDiff.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
What changed in this PR

Adds JSON state-diff reporting to state-test trace summaries.

Changes:

  • Serializes transaction StateDiff objects to JSON.
  • Emits state diffs after trace summaries.
  • Adds export and runner tests.
File Description
tools/​evmone/​main.cpp Updates CLI help.
test/​utils/​test_driver.hpp Documents trace-summary behavior.
test/​utils/​statetest.hpp Declares StateDiff serialization.
test/​utils/​statetest_runner.cpp Emits transaction state diffs.
test/​utils/​statetest_export.cpp Implements JSON serialization.
test/​unittests/​statetest_runner_test.cpp Updates expected summary output.
test/​unittests/​statediff_export_test.cpp Tests StateDiff serialization.
test/​unittests/​CMakeLists.txt Registers the new tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/utils/statetest.hpp Outdated
Comment thread tools/evmone/main.cpp Outdated
@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 2 times, most recently from fdf872a to 5f11306 Compare September 21, 2026 14:46
@rodiazet
rodiazet requested a balanced review from Copilot September 21, 2026 14:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The implementation and coverage are complete, with only a minor capitalization correction noted.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (2)

Comment thread test/utils/test_driver.hpp Outdated
@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 2 times, most recently from dbda981 to c085973 Compare September 23, 2026 07:19
@rodiazet
rodiazet requested a balanced review from Copilot September 23, 2026 07:22

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The remaining feedback is a non-blocking test-coverage nit for cleared code serialization.

Review effort: Balanced
Findings: None

Resolved since last review (1)

@rodiazet
rodiazet force-pushed the post-state-log-on-master branch 3 times, most recently from e5dd294 to 841c525 Compare September 23, 2026 10:47
@chfast chfast changed the title test: Add dumping post state for state test running. tools: Add option to dump state-diff in state tests Sep 23, 2026
Comment thread tools/evmone/main.cpp Outdated
Comment thread test/utils/statetest_runner.cpp Outdated
Comment thread tools/evmone/main.cpp Outdated
Comment thread tools/evmone/main.cpp Outdated
@rodiazet
rodiazet force-pushed the post-state-log-on-master branch from 841c525 to 7c60cb6 Compare September 23, 2026 12:12
@chfast
chfast force-pushed the post-state-log-on-master branch from 7c60cb6 to fdd0069 Compare September 23, 2026 13:06
@rodiazet
rodiazet merged commit 0f5a9fc into master Sep 23, 2026
24 of 25 checks passed
@rodiazet
rodiazet deleted the post-state-log-on-master branch September 23, 2026 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants