New evmone API: StateView & StateDiff #802

chfast · 2024-01-31T13:37:48Z

A step towards of the new API for evmone. It uses the test runners as users and modifies the State API from state.h.

the user provides read-only StateView interface with (simpler than State) representation of the State.
the transaction execution (i.e. evmone via transition()) takes the StateView and produces StateDiff.
user must apply the StateDiff to its StateView implementation.

The current state diff building is based on visiting the intra state cache of accounts loaded from initial state. This approach is relatively simple but has some false positives and may not be efficient.

codecov · 2024-02-02T10:19:05Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.23%. Comparing base (46dc5ee) to head (da4cd30).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #802      +/-   ##
==========================================
+ Coverage   94.21%   94.23%   +0.01%     
==========================================
  Files         153      155       +2     
  Lines       15933    15967      +34     
==========================================
+ Hits        15011    15046      +35     
+ Misses        922      921       -1

Flag	Coverage Δ
eof_execution_spec_tests	`17.69% <93.10%> (+0.15%)`	⬆️
ethereum_tests	`27.49% <100.00%> (+0.15%)`	⬆️
ethereum_tests_silkpre	`19.35% <93.39%> (+0.20%)`	⬆️
execution_spec_tests	`20.61% <98.27%> (+0.15%)`	⬆️
unittests	`89.02% <98.27%> (+0.01%)`	⬆️

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

Files with missing lines	Coverage Δ
test/state/account.hpp	`100.00% <100.00%> (ø)`
test/state/host.cpp	`100.00% <100.00%> (ø)`
test/state/state.cpp	`100.00% <100.00%> (ø)`
test/state/state.hpp	`100.00% <100.00%> (ø)`
test/state/state_diff.hpp	`100.00% <100.00%> (ø)`
test/state/state_view.hpp	`100.00% <100.00%> (ø)`
test/state/system_contracts.cpp	`100.00% <100.00%> (ø)`
test/state/test_state.cpp	`100.00% <100.00%> (ø)`
test/state/test_state.hpp	`88.88% <100.00%> (+8.88%)`	⬆️
test/state/transaction.hpp	`100.00% <ø> (ø)`

chfast · 2024-09-16T10:32:05Z

test/state/host.cpp

 }

 bytes32 Host::get_code_hash(const address& addr) const noexcept
 {
    // TODO: Cache code hash. It will be needed also to compute the MPT hash.
+    const auto raw_code = m_state.get_code(addr);
+    if (is_eof_container(raw_code))
+        return keccak256(raw_code.substr(0, 2));


It is probably good idea to handle EOF ext code and possible code hash caching up front.

Proposed in #1035.

test/state/state.cpp

test/t8n/t8n.cpp

test/state/state_view.hpp

pdobacz

I still need to wrap head around this, but here some early comments

pdobacz · 2024-09-16T16:44:19Z

test/state/host.cpp

@@ -92,37 +89,45 @@ bytes_view extcode(bytes_view code) noexcept
 /// as defined in the [EIP-7610](https://eips.ethereum.org/EIPS/eip-7610).
 [[nodiscard]] bool is_create_collision(const Account& acc) noexcept
 {
-    if (acc.nonce != 0 || !acc.code.empty())
+    // TODO: This requires much more testing:


better file as issue(s)?

pdobacz · 2024-09-16T16:52:58Z

test/state/state.hpp

@@ -62,14 +63,16 @@ class State
        std::variant<JournalBalanceChange, JournalTouched, JournalStorageChange, JournalNonceBump,
            JournalCreate, JournalTransientStorageChange, JournalDestruct, JournalAccessAccount>;

+    const StateView* m_cold = nullptr;


comment pls what this means, making sure to answer its relation to the notion of cold/warm storage (access lists and whatnot). I kinda know but would appreciate the comment to make sure and make it easier to digest.

I would maybe rename this and m_accounts: m_initial_state and m_modified_accounts maybe.

The "initial" and "modified" sounds nice. How about just m_initial and m_modified?

pdobacz · 2024-09-16T16:55:24Z

test/state/system_contracts.cpp

 {
+    State ss{state};


maybe to early to nitpick, but the StateView instance is usually sv

These names are because of the prototype quality. My proposal for every place:

StateView state_view; State intra_state; TestState test_state;

Actually, not so much of this is left so I'll change just this instance.

Wrap the usage of the state transition API from the `evmone::state` for tests so that the new API in `evmone::test` only exposes `TestState` and hides `state::State`. This isolates usage of the `evmone::state` to lower the disturbance caused by API modifications, e.g. in #802.

test/state/state.hpp

test/state/account.hpp

chfast · 2024-09-25T21:40:13Z

I've found Ethereum RPC "stateDiff": https://docs.infura.io/api/networks/ethereum/json-rpc-methods/trace-methods#statediff

gumb0 · 2024-09-27T15:48:59Z

test/state/state.cpp

@@ -107,6 +149,18 @@ Account& State::get_or_insert(const address& addr, Account account)
    return insert(addr, std::move(account));
 }

+bytes_view State::get_code(const address& addr)


Can be const method?

No. We potentially load an account and/or its code from initial state.

chfast · 2024-09-30T09:48:14Z

test/state/host.cpp

-    if (std::ranges::any_of(
-            acc.storage, [](auto& e) noexcept { return !is_zero(e.second.current); }))
+    if (acc.code_hash != Account::EMPTY_CODE_HASH)
+        return true;


This is not covered by any tests, so more work for #1037. Should I remove/shorten the TODO comments here?

test/t8n/t8n.cpp

Introduce the type `StateDiff` to represent a set of modifications to the State. Implement the procedure to collect the set of changes out of an intra state object.

Implement `TestState::appy(StateDiff)` procedure to apply state changes back to the state.

Return `StateDiff` out of: - `state::transition()` (as a subobject of `TransactionReceipt`) - `state::system_call()` - `state::finalize()`

Introduce `StateView` interface as a way to read the initial/cold State.

Implement `StateView` interface in `TestState`. This will be used later.

Modify the `State` to require `StateView` interface as a parameter and use it to read the initial state values. This drops `TestState` dependency on `State`.

chfast mentioned this pull request Feb 1, 2024

test: New State transition tests #806

Merged

chfast force-pushed the state/state_view branch from 0f92b8a to af88125 Compare February 2, 2024 10:15

chfast force-pushed the state/state_view branch 3 times, most recently from 1ea121a to 3f608b1 Compare February 2, 2024 14:07

chfast force-pushed the master branch from 606b735 to ffd7390 Compare February 2, 2024 14:20

chfast force-pushed the state/state_view branch 3 times, most recently from d77ecd8 to 83342e2 Compare February 6, 2024 10:34

chfast force-pushed the master branch from 7717742 to d5e55fc Compare March 26, 2024 20:50

chfast force-pushed the state/state_view branch 2 times, most recently from 0028573 to 1bab196 Compare August 30, 2024 13:33

chfast self-assigned this Sep 9, 2024

chfast force-pushed the state/state_view branch 3 times, most recently from e4e3eb4 to 838a5cf Compare September 16, 2024 10:01

chfast changed the title ~~New API prototype: StateView~~ New evmone API: StateView & StateDiff Sep 16, 2024

chfast marked this pull request as ready for review September 16, 2024 10:27

chfast requested review from gumb0 and pdobacz September 16, 2024 10:30

chfast commented Sep 16, 2024

View reviewed changes

pdobacz reviewed Sep 16, 2024

View reviewed changes

chfast mentioned this pull request Sep 18, 2024

test: Wrap usage of state transition API #1013

Merged

chfast force-pushed the state/state_view branch from 28caf35 to a7cac6e Compare September 18, 2024 07:55

chfast force-pushed the state/state_view branch from a7cac6e to 5ef5009 Compare September 18, 2024 08:41

chfast force-pushed the state/state_view branch from 5ef5009 to 65c7d8e Compare September 18, 2024 15:59

chfast force-pushed the state/state_view branch from 65c7d8e to 0cda67b Compare September 23, 2024 14:25

chfast force-pushed the state/state_view branch from 0cda67b to 53ca290 Compare September 25, 2024 15:04

gumb0 reviewed Sep 25, 2024

View reviewed changes

test/state/state.hpp Outdated Show resolved Hide resolved

gumb0 reviewed Sep 25, 2024

View reviewed changes

test/state/account.hpp Outdated Show resolved Hide resolved

chfast force-pushed the state/state_view branch 2 times, most recently from 954d0ec to 97aaffa Compare September 27, 2024 15:38

gumb0 reviewed Sep 27, 2024

View reviewed changes

chfast force-pushed the state/state_view branch from 97aaffa to 14e6c07 Compare September 28, 2024 11:26

chfast requested a review from gumb0 September 28, 2024 11:30

chfast force-pushed the state/state_view branch 2 times, most recently from 4c5c323 to d69bc4c Compare September 28, 2024 14:33

chfast requested a review from pdobacz September 28, 2024 14:33

pdobacz approved these changes Sep 30, 2024

View reviewed changes

chfast commented Sep 30, 2024

View reviewed changes

chfast added 5 commits September 30, 2024 11:51

state: Introduce StateDiff

2257785

Introduce the type `StateDiff` to represent a set of modifications to the State. Implement the procedure to collect the set of changes out of an intra state object.

test: Implement StateDiff handling in TestState

4b57dd2

Implement `TestState::appy(StateDiff)` procedure to apply state changes back to the state.

state: Produce StateDiff from state modifying functions

4398e95

Return `StateDiff` out of: - `state::transition()` (as a subobject of `TransactionReceipt`) - `state::system_call()` - `state::finalize()`

state: Introduce StateView interface

727d74d

Introduce `StateView` interface as a way to read the initial/cold State.

test: TestState implements StateView

928edb2

Implement `StateView` interface in `TestState`. This will be used later.

chfast force-pushed the state/state_view branch from d69bc4c to 8a8e576 Compare September 30, 2024 10:19

state: Use StateView from State

da4cd30

Modify the `State` to require `StateView` interface as a parameter and use it to read the initial state values. This drops `TestState` dependency on `State`.

chfast force-pushed the state/state_view branch from 8a8e576 to da4cd30 Compare September 30, 2024 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New evmone API: StateView & StateDiff #802

New evmone API: StateView & StateDiff #802

chfast commented Jan 31, 2024 •

edited

Loading

codecov bot commented Feb 2, 2024 •

edited

Loading

chfast Sep 16, 2024

chfast Sep 28, 2024

pdobacz left a comment

pdobacz Sep 16, 2024

chfast Sep 28, 2024

pdobacz Sep 16, 2024

gumb0 Sep 25, 2024

chfast Sep 28, 2024

pdobacz Sep 16, 2024

chfast Sep 28, 2024

chfast Sep 28, 2024

chfast commented Sep 25, 2024

gumb0 Sep 27, 2024

chfast Sep 28, 2024

chfast Sep 30, 2024

New evmone API: StateView & StateDiff #802

Are you sure you want to change the base?

New evmone API: StateView & StateDiff #802

Conversation

chfast commented Jan 31, 2024 • edited Loading

codecov bot commented Feb 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdobacz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chfast commented Sep 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chfast commented Jan 31, 2024 •

edited

Loading

codecov bot commented Feb 2, 2024 •

edited

Loading