diff --git a/test/state/account.hpp b/test/state/account.hpp index d836525ddc..d08c009559 100644 --- a/test/state/account.hpp +++ b/test/state/account.hpp @@ -12,6 +12,7 @@ namespace evmone::state using evmc::address; using evmc::bytes; using evmc::bytes32; +using namespace evmc::literals; /// The representation of the account storage value. struct StorageValue @@ -31,25 +32,38 @@ struct Account /// The maximum allowed nonce value. static constexpr auto NonceMax = std::numeric_limits::max(); + /// The keccak256 hash of the empty input. Used to identify empty account's code. + static constexpr auto EMPTY_CODE_HASH = + 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470_bytes32; + /// The account nonce. uint64_t nonce = 0; /// The account balance. intx::uint256 balance; - /// The account storage map. + bytes32 code_hash = EMPTY_CODE_HASH; + + /// If the account has non-empty initial storage (when accessing the cold account). + bool has_initial_storage = false; + + /// The cached and modified account storage entries. std::unordered_map storage; + /// The EIP-1153 transient (transaction-level lifetime) storage. std::unordered_map transient_storage; - /// The account code. + /// The cache of the account code. + /// + /// Check code_hash to know if an account code is empty. + /// Empty here only means it has not been loaded from the initial storage. bytes code; - /// The account has been destructed and should be erased at the end of of a transaction. + /// The account has been destructed and should be erased at the end of a transaction. bool destructed = false; /// The account should be erased if it is empty at the end of a transaction. - /// This flag means the account has been "touched" as defined in EIP-161 + /// This flag means the account has been "touched" as defined in EIP-161, /// or it is a newly created temporary account. /// /// Yellow Paper uses term "delete" but it is a keyword in C++ while @@ -63,7 +77,7 @@ struct Account [[nodiscard]] bool is_empty() const noexcept { - return code.empty() && nonce == 0 && balance == 0; + return nonce == 0 && balance == 0 && code_hash == EMPTY_CODE_HASH; } }; } // namespace evmone::state diff --git a/test/state/host.cpp b/test/state/host.cpp index 258aba2eba..a227d09142 100644 --- a/test/state/host.cpp +++ b/test/state/host.cpp @@ -18,10 +18,7 @@ bool Host::account_exists(const address& addr) const noexcept bytes32 Host::get_storage(const address& addr, const bytes32& key) const noexcept { - const auto& acc = m_state.get(addr); - if (const auto it = acc.storage.find(key); it != acc.storage.end()) - return it->second.current; - return {}; + return m_state.get_storage(addr, key).current; } evmc_storage_status Host::set_storage( @@ -30,7 +27,7 @@ evmc_storage_status Host::set_storage( // Follow EVMC documentation https://evmc.ethereum.org/storagestatus.html#autotoc_md3 // and EIP-2200 specification https://eips.ethereum.org/EIPS/eip-2200. - auto& storage_slot = m_state.get(addr).storage[key]; + auto& storage_slot = m_state.get_storage(addr, key); const auto& [current, original, _] = storage_slot; const auto dirty = original != current; @@ -92,23 +89,28 @@ 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: + // - what if an account had storage but is destructed? + // - what if an account had cold storage but it was emptied? + // - what if an account without cold storage gain one? + if (acc.nonce != 0) return true; - - // acc.storage may have entries from access list, even if account storage is empty. - // Check for non-zero current values. - 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; + if (acc.has_initial_storage) return true; + // The hot storage is ignored because it can contain elements from access list. + // TODO: Is this correct for destructed accounts? + assert(!acc.destructed && "untested"); return false; } } // namespace size_t Host::get_code_size(const address& addr) const noexcept { - const auto* const acc = m_state.find(addr); - return (acc != nullptr) ? extcode(acc->code).size() : 0; + const auto raw_code = m_state.get_code(addr); + return extcode(raw_code).size(); } bytes32 Host::get_code_hash(const address& addr) const noexcept @@ -116,17 +118,20 @@ bytes32 Host::get_code_hash(const address& addr) const noexcept const auto* const acc = m_state.find(addr); if (acc == nullptr || acc->is_empty()) return {}; - if (is_eof_container(acc->code)) + + // Load code and check if not EOF. + // TODO: Optimize the second account lookup here. + if (is_eof_container(m_state.get_code(addr))) return EOF_CODE_HASH_SENTINEL; - // TODO: Cache code hash. It will be needed also to compute the MPT hash. - return keccak256(acc->code); + + return acc->code_hash; } size_t Host::copy_code(const address& addr, size_t code_offset, uint8_t* buffer_data, size_t buffer_size) const noexcept { - const auto* const acc = m_state.find(addr); - const auto code = (acc != nullptr) ? extcode(acc->code) : bytes_view{}; + const auto raw_code = m_state.get_code(addr); + const auto code = extcode(raw_code); const auto code_slice = code.substr(std::min(code_offset, code.size())); const auto num_bytes = std::min(buffer_size, code_slice.size()); std::copy_n(code_slice.begin(), num_bytes, buffer_data); @@ -367,6 +372,7 @@ evmc::Result Host::create(const evmc_message& msg) noexcept } } + new_acc->code_hash = keccak256(code); new_acc->code = code; return evmc::Result{result.status_code, gas_left, result.gas_refund, msg.recipient}; @@ -409,9 +415,8 @@ evmc::Result Host::execute_message(const evmc_message& msg) noexcept if (is_precompile(m_rev, msg.code_address)) return call_precompile(m_rev, msg); - // In case msg.recipient == msg.code_address, this is the second lookup of the same address. - const auto* const code_acc = m_state.find(msg.code_address); - const auto code = code_acc != nullptr ? bytes_view{code_acc->code} : bytes_view{}; + // TODO: get_code() performs the account lookup. Add a way to get an account with code? + const auto code = m_state.get_code(msg.code_address); return m_vm.execute(*this, m_rev, msg, code.data(), code.size()); } @@ -503,7 +508,7 @@ evmc_access_status Host::access_account(const address& addr) noexcept evmc_access_status Host::access_storage(const address& addr, const bytes32& key) noexcept { - auto& storage_slot = m_state.get(addr).storage[key]; + auto& storage_slot = m_state.get_storage(addr, key); m_state.journal_storage_change(addr, key, storage_slot); return std::exchange(storage_slot.access_status, EVMC_ACCESS_WARM); } diff --git a/test/state/state.cpp b/test/state/state.cpp index 47375ddff1..a756d29e93 100644 --- a/test/state/state.cpp +++ b/test/state/state.cpp @@ -5,6 +5,7 @@ #include "state.hpp" #include "../utils/stdx/utility.hpp" #include "host.hpp" +#include "state_view.hpp" #include #include #include @@ -81,7 +82,7 @@ evmc_message build_message( StateDiff State::build_diff(evmc_revision rev) const { StateDiff diff; - for (const auto& [addr, m] : m_accounts) + for (const auto& [addr, m] : m_modified) { if (m.destructed) { @@ -103,6 +104,9 @@ StateDiff State::build_diff(evmc_revision rev) const // TODO(clang): In old Clang emplace_back without Account doesn't compile. // NOLINTNEXTLINE(modernize-use-emplace) auto& a = diff.modified_accounts.emplace_back(StateDiff::Entry{addr, m.nonce, m.balance}); + + // Output only the new code. + // TODO: Output also the code hash. It will be needed for DB update and MPT hash. if (m.just_created && !m.code.empty()) a.code = m.code; @@ -117,16 +121,22 @@ StateDiff State::build_diff(evmc_revision rev) const Account& State::insert(const address& addr, Account account) { - const auto r = m_accounts.insert({addr, std::move(account)}); + const auto r = m_modified.insert({addr, std::move(account)}); assert(r.second); return r.first->second; } Account* State::find(const address& addr) noexcept { - const auto it = m_accounts.find(addr); - if (it != m_accounts.end()) + // 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. + if (const auto it = m_modified.find(addr); it != m_modified.end()) return &it->second; + if (const auto cacc = m_initial.get_account(addr); cacc) + return &insert(addr, {.nonce = cacc->nonce, + .balance = cacc->balance, + .code_hash = cacc->code_hash, + .has_initial_storage = cacc->has_storage}); return nullptr; } @@ -144,6 +154,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) +{ + auto* a = find(addr); + if (a == nullptr) + return {}; + if (a->code_hash == Account::EMPTY_CODE_HASH) + return {}; + if (a->code.empty()) + a->code = m_initial.get_account_code(addr); + return a->code; +} + Account& State::touch(const address& addr) { auto& acc = get_or_insert(addr, {.erase_if_empty = true}); @@ -155,6 +177,19 @@ Account& State::touch(const address& addr) return acc; } +StorageValue& State::get_storage(const address& addr, const bytes32& key) +{ + // TODO: Avoid account lookup by giving the reference to the account's storage to Host. + auto& acc = get(addr); + const auto [it, missing] = acc.storage.try_emplace(key); + if (missing) + { + const auto initial_value = m_initial.get_storage(addr, key); + it->second = {initial_value, initial_value}; + } + return it->second; +} + void State::journal_balance_change(const address& addr, const intx::uint256& prev_balance) { m_journal.emplace_back(JournalBalanceChange{{addr}, prev_balance}); @@ -222,6 +257,7 @@ void State::rollback(size_t checkpoint) // This account is not always "touched". TODO: Why? auto& a = get(e.addr); a.nonce = 0; + a.code_hash = Account::EMPTY_CODE_HASH; a.code.clear(); } else @@ -230,7 +266,7 @@ void State::rollback(size_t checkpoint) // so we need to delete them here explicitly. // This should be changed by tuning "erasable" flag // and clear in all revisions. - m_accounts.erase(e.addr); + m_modified.erase(e.addr); } } else if constexpr (std::is_same_v) @@ -314,7 +350,7 @@ std::variant validate_transaction(const Account& sende if (tx.max_gas_price < block.base_fee) return make_error_code(FEE_CAP_LESS_THEN_BLOCKS); - if (!sender_acc.code.empty()) + if (sender_acc.code_hash != Account::EMPTY_CODE_HASH) return make_error_code(SENDER_NOT_EOA); // Origin must not be a contract (EIP-3607). if (sender_acc.nonce == Account::NonceMax) // Nonce value limit (EIP-2681). @@ -352,10 +388,11 @@ std::variant validate_transaction(const Account& sende return execution_gas_limit; } -StateDiff finalize(State& state, evmc_revision rev, const address& coinbase, +StateDiff finalize(const StateView& state_view, evmc_revision rev, const address& coinbase, std::optional block_reward, std::span ommers, std::span withdrawals) { + State state{state_view}; // TODO: The block reward can be represented as a withdrawal. if (block_reward.has_value()) { @@ -378,10 +415,11 @@ StateDiff finalize(State& state, evmc_revision rev, const address& coinbase, return state.build_diff(rev); } -std::variant transition(State& state, const BlockInfo& block, - const Transaction& tx, evmc_revision rev, evmc::VM& vm, int64_t block_gas_left, - int64_t blob_gas_left) +std::variant transition(const StateView& state_view, + const BlockInfo& block, const Transaction& tx, evmc_revision rev, evmc::VM& vm, + int64_t block_gas_left, int64_t blob_gas_left) { + State state{state_view}; auto* sender_ptr = state.find(tx.sender); // Validate transaction. The validation needs the sender account, so in case @@ -429,10 +467,9 @@ std::variant transition(State& state, const host.access_account(*tx.to); for (const auto& [a, storage_keys] : tx.access_list) { - host.access_account(a); // TODO: Return account ref. - auto& storage = state.get(a).storage; + host.access_account(a); for (const auto& key : storage_keys) - storage[key].access_status = EVMC_ACCESS_WARM; + state.get_storage(a, key).access_status = EVMC_ACCESS_WARM; } // EIP-3651: Warm COINBASE. // This may create an empty coinbase account. The account cannot be created unconditionally diff --git a/test/state/state.hpp b/test/state/state.hpp index 04db13c3e5..edeef64583 100644 --- a/test/state/state.hpp +++ b/test/state/state.hpp @@ -15,6 +15,8 @@ namespace evmone::state { +class StateView; + /// The Ethereum State: the collection of accounts mapped by their addresses. class State { @@ -62,17 +64,21 @@ class State std::variant; - std::unordered_map m_accounts; + /// The read-only view of the initial (cold) state. + const StateView& m_initial; + + /// The accounts loaded from the initial state and potentially modified. + std::unordered_map m_modified; - /// The state journal: the list of changes made in the state + /// The state journal: the list of changes made to the state /// with information how to revert them. std::vector m_journal; public: - State() = default; + explicit State(const StateView& state_view) noexcept : m_initial{state_view} {} State(const State&) = delete; - State(State&&) = default; - State& operator=(State&&) = default; + State(State&&) = delete; + State& operator=(State&&) = delete; /// Inserts the new account at the address. /// There must not exist any account under this address before. @@ -87,6 +93,10 @@ class State /// Gets an existing account or inserts new account. Account& get_or_insert(const address& addr, Account account = {}); + bytes_view get_code(const address& addr); + + StorageValue& get_storage(const address& addr, const bytes32& key); + StateDiff build_diff(evmc_revision rev) const; /// Returns the state journal checkpoint. It can be later used to in rollback() @@ -124,11 +134,11 @@ class State /// /// Applies block reward to coinbase, withdrawals (post Shanghai) and deletes empty touched accounts /// (post Spurious Dragon). -[[nodiscard]] StateDiff finalize(State& state, evmc_revision rev, const address& coinbase, - std::optional block_reward, std::span ommers, +[[nodiscard]] StateDiff finalize(const StateView& state_view, evmc_revision rev, + const address& coinbase, std::optional block_reward, std::span ommers, std::span withdrawals); -[[nodiscard]] std::variant transition(State& state, +[[nodiscard]] std::variant transition(const StateView& state, const BlockInfo& block, const Transaction& tx, evmc_revision rev, evmc::VM& vm, int64_t block_gas_left, int64_t blob_gas_left); diff --git a/test/state/system_contracts.cpp b/test/state/system_contracts.cpp index 4258fb4e23..5c19675202 100644 --- a/test/state/system_contracts.cpp +++ b/test/state/system_contracts.cpp @@ -4,6 +4,7 @@ #include "system_contracts.hpp" #include "host.hpp" +#include "state_view.hpp" namespace evmone::state { @@ -35,8 +36,10 @@ static_assert(std::ranges::is_sorted(SYSTEM_CONTRACTS, } // namespace -StateDiff system_call(State& state, const BlockInfo& block, evmc_revision rev, evmc::VM& vm) +StateDiff system_call( + const StateView& state_view, const BlockInfo& block, evmc_revision rev, evmc::VM& vm) { + State state{state_view}; for (const auto& [since, addr, get_input] : SYSTEM_CONTRACTS) { if (rev < since) @@ -44,8 +47,8 @@ StateDiff system_call(State& state, const BlockInfo& block, evmc_revision rev, e // Skip the call if the target account doesn't exist. This is by EIP-4788 spec. // > if no code exists at [address], the call must fail silently. - const auto acc = state.find(addr); - if (acc == nullptr) + const auto code = state_view.get_account_code(addr); + if (code.empty()) continue; const auto input = get_input(block); @@ -61,10 +64,8 @@ StateDiff system_call(State& state, const BlockInfo& block, evmc_revision rev, e const Transaction empty_tx{}; Host host{rev, vm, state, block, empty_tx}; - const auto& code = acc->code; [[maybe_unused]] const auto res = vm.execute(host, rev, msg, code.data(), code.size()); assert(res.status_code == EVMC_SUCCESS); - assert(acc->access_status == EVMC_ACCESS_COLD); } // TODO: Should we return empty diff if no system contracts? return state.build_diff(rev); diff --git a/test/state/system_contracts.hpp b/test/state/system_contracts.hpp index 144a828654..6b4def136c 100644 --- a/test/state/system_contracts.hpp +++ b/test/state/system_contracts.hpp @@ -20,12 +20,12 @@ constexpr auto HISTORY_STORAGE_ADDRESS = 0x0aae40965e6800cd9b1f4b05ff21581047e3f struct BlockInfo; struct StateDiff; -class State; +class StateView; /// Performs the system call: invokes system contracts. /// /// Executes code of pre-defined accounts via pseudo-transaction from the system sender (0xff...fe). /// The sender's nonce is not increased. [[nodiscard]] StateDiff system_call( - State& state, const BlockInfo& block, evmc_revision rev, evmc::VM& vm); + const StateView& state_view, const BlockInfo& block, evmc_revision rev, evmc::VM& vm); } // namespace evmone::state diff --git a/test/state/test_state.cpp b/test/state/test_state.cpp index f7610bc939..8d84ee2277 100644 --- a/test/state/test_state.cpp +++ b/test/state/test_state.cpp @@ -27,20 +27,6 @@ bytes TestState::get_account_code(const address& addr) const noexcept return it->second.code; } -state::State TestState::to_intra_state() const -{ - state::State intra_state; - for (const auto& [addr, acc] : *this) - { - auto& intra_acc = intra_state.insert( - addr, {.nonce = acc.nonce, .balance = acc.balance, .code = get_account_code(addr)}); - auto& storage = intra_acc.storage; - for (const auto& [key, value] : acc.storage) - storage[key] = {.current = value, .original = value}; - } - return intra_state; -} - void TestState::apply(const state::StateDiff& diff) { for (const auto& m : diff.modified_accounts) @@ -77,9 +63,8 @@ bytes32 TestState::get_storage(const address& addr, const bytes32& key) const no const state::BlockInfo& block, const state::Transaction& tx, evmc_revision rev, evmc::VM& vm, int64_t block_gas_left, int64_t blob_gas_left) { - auto intra_state = state.to_intra_state(); const auto result_or_error = - state::transition(intra_state, block, tx, rev, vm, block_gas_left, blob_gas_left); + state::transition(state, block, tx, rev, vm, block_gas_left, blob_gas_left); if (const auto result = get_if(&result_or_error)) state.apply(result->state_diff); return result_or_error; @@ -89,16 +74,13 @@ void finalize(TestState& state, evmc_revision rev, const address& coinbase, std::optional block_reward, std::span ommers, std::span withdrawals) { - auto intra_state = state.to_intra_state(); - const auto diff = - state::finalize(intra_state, rev, coinbase, block_reward, ommers, withdrawals); + const auto diff = state::finalize(state, rev, coinbase, block_reward, ommers, withdrawals); state.apply(diff); } void system_call(TestState& state, const state::BlockInfo& block, evmc_revision rev, evmc::VM& vm) { - auto intra_state = state.to_intra_state(); - const auto diff = state::system_call(intra_state, block, rev, vm); + const auto diff = state::system_call(state, block, rev, vm); state.apply(diff); } } // namespace evmone::test diff --git a/test/state/test_state.hpp b/test/state/test_state.hpp index 6150d54dba..0fbb17f6bb 100644 --- a/test/state/test_state.hpp +++ b/test/state/test_state.hpp @@ -20,7 +20,6 @@ struct StateDiff; struct Transaction; struct TransactionReceipt; struct Withdrawal; -class State; } // namespace state namespace test @@ -69,9 +68,6 @@ class TestState : public state::StateView, public std::map /// TODO: deprecate this method. TestAccount& get(const address& addr) { return (*this)[addr]; } - /// Converts the TestState to intra state for transaction execution. - [[nodiscard]] state::State to_intra_state() const; - /// Apply the state changes. void apply(const state::StateDiff& diff); };