Skip to content

state: Replace create-revert erase with a nonexistent flag#1600

Merged
chfast merged 1 commit into
masterfrom
state/journal-nonexistent-flag
Jul 22, 2026
Merged

state: Replace create-revert erase with a nonexistent flag#1600
chfast merged 1 commit into
masterfrom
state/journal-nonexistent-flag

Conversation

@chfast

@chfast chfast commented Jul 16, 2026

Copy link
Copy Markdown
Member

Introduce nonexistent flag to Account. In case of create revert
just set the flag instead of doing full erase from the hash map.
Now we also need to recognize the flag on the query side.

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.

Pull request overview

This PR changes the test State implementation to avoid erasing m_modified entries when reverting a new-account create, preserving pointer stability by marking such entries with a revertible Account::nonexistent flag (ignored by find()/build_diff() and resurrected in-place by insert()).

Changes:

  • Add a revertible Account::nonexistent flag and journal it via JournalAccountFlags.
  • Rework create journaling: new-account creates journal a flags snapshot restoring nonexistent=true; pre-existing-account creates journal JournalCreate and roll back by resetting nonce/code.
  • Teach find()/insert()/build_diff() to treat nonexistent nodes as absent.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
test/state/state.hpp Extends JournalAccountFlags with nonexistent and simplifies JournalCreate.
test/state/state.cpp Implements nonexistent handling in build_diff(), insert(), find(), create journaling, and rollback.
test/state/account.hpp Adds and documents the new Account::nonexistent flag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/state/state.cpp
Comment on lines +281 to +285
if (!inserted)
{
assert(it->second.nonexistent);
it->second = std::move(account);
}
Comment thread test/state/state.cpp Outdated
Comment on lines 291 to 292
// TODO: Avoid double lookup (find+insert) and not cached initial state lookup for non-existent
// accounts. If we want to cache non-existent account we need a proper flag for it.
Comment thread test/state/state.cpp
Comment on lines 402 to 406
auto& a = get(e.addr);
a.access_status = e.access_status;
a.nonexistent = e.nonexistent;
a.destructed = e.destructed;
a.erase_if_empty = e.erase_if_empty;
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.40%. Comparing base (053dc9c) to head (1590364).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1600   +/-   ##
=======================================
  Coverage   97.39%   97.40%           
=======================================
  Files         164      164           
  Lines       14653    14658    +5     
  Branches     3388     3391    +3     
=======================================
+ Hits        14272    14277    +5     
  Misses        280      280           
  Partials      101      101           
Flag Coverage Δ
eest-develop 89.47% <100.00%> (+<0.01%) ⬆️
eest-develop-gmp 26.08% <100.00%> (+0.02%) ⬆️
eest-legacy 17.50% <78.94%> (+<0.01%) ⬆️
eest-libsecp256k1 27.71% <100.00%> (+0.02%) ⬆️
eest-stable 89.45% <100.00%> (+<0.01%) ⬆️
evmone-unittests 92.63% <100.00%> (+<0.01%) ⬆️

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

Components Coverage Δ
core 95.94% <100.00%> (+<0.01%) ⬆️
tooling 90.35% <ø> (ø)
tests 99.80% <ø> (ø)
Files with missing lines Coverage Δ
test/state/account.hpp 100.00% <ø> (ø)
test/state/state.cpp 99.69% <100.00%> (+<0.01%) ⬆️
test/state/state.hpp 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chfast
chfast force-pushed the state/journal-nonexistent-flag branch 4 times, most recently from 362d5ca to 4dcb30a Compare July 22, 2026 13:37
Introduce nonexistent flag to Account. In case of create revert
just set the flag instead of doing full erase from the hash map.
Now we also need to recognize the flag on the query side.
@chfast
chfast force-pushed the state/journal-nonexistent-flag branch from 4dcb30a to 5f0cc59 Compare July 22, 2026 13:50
@chfast
chfast requested a review from Copilot July 22, 2026 13:51

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

test/state/state.cpp:410

  • When rolling back a newly-created account to the "nonexistent" sentinel state, the node retains potentially large buffers (code/storage/transient_storage). This can significantly increase peak memory for transactions with many failed creates, and keeps stale data around even though find() will treat the account as absent. Consider clearing these buffers (and resetting non-revertible flags) when restoring nonexistent==true to keep the sentinel lightweight while still preserving m_modified node stability.
                    auto& a = get(e.addr);
                    a.access_status = e.access_status;
                    a.nonexistent = e.nonexistent;
                    a.destructed = e.destructed;
                    a.erase_if_empty = e.erase_if_empty;
                    // TODO: On restoring nonexistent (un-created create) the node keeps its
                    //   code/storage/transient buffers until tx end; could clear them here.

@chfast
chfast merged commit edb3c02 into master Jul 22, 2026
22 checks passed
@chfast
chfast deleted the state/journal-nonexistent-flag branch July 22, 2026 14:13
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.

2 participants