From 14e6c07d50c99335105aa8d788ced24e952a3b79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 11 Jan 2024 12:51:18 +0100 Subject: [PATCH] Use StateView from State --- test/state/account.hpp | 18 +++++++--- test/state/host.cpp | 46 +++++++++++++----------- test/state/state.cpp | 64 +++++++++++++++++++++++++-------- test/state/state.hpp | 16 ++++++--- test/state/system_contracts.cpp | 15 ++++---- test/state/system_contracts.hpp | 4 +-- test/state/test_state.cpp | 23 ++---------- test/state/test_state.hpp | 3 -- 8 files changed, 112 insertions(+), 77 deletions(-) diff --git a/test/state/account.hpp b/test/state/account.hpp index d836525ddc..fed50fe214 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,32 @@ struct Account /// The maximum allowed nonce value. static constexpr auto NonceMax = std::numeric_limits::max(); + static constexpr auto EMPTY_CODE_HASH = + 0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470_bytes32; + /// The account nonce. uint64_t nonce = 0; /// The account balance. intx::uint256 balance; + bytes32 code_hash = EMPTY_CODE_HASH; + + bool has_cold_storage = false; + /// The account storage map. - std::unordered_map storage; + std::unordered_map _storage; std::unordered_map transient_storage; /// The account code. - bytes code; + 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 +71,7 @@ struct Account [[nodiscard]] bool is_empty() const noexcept { - return code.empty() && nonce == 0 && balance == 0; + return code_hash == EMPTY_CODE_HASH && nonce == 0 && balance == 0; } }; } // namespace evmone::state diff --git a/test/state/host.cpp b/test/state/host.cpp index cefc796511..5b950858a9 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,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: + // - 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_cold_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 { // 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)); const auto* const acc = m_state.find(addr); - return (acc != nullptr && !acc->is_empty()) ? keccak256(extcode(acc->code)) : bytes32{}; + return (acc != nullptr && !acc->is_empty()) ? acc->code_hash : bytes32{}; } 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); @@ -363,7 +368,8 @@ evmc::Result Host::create(const evmc_message& msg) noexcept } } - new_acc->code = code; + new_acc->code_hash = keccak256(code); + new_acc->_code = code; return evmc::Result{result.status_code, gas_left, result.gas_refund, msg.recipient}; } @@ -405,9 +411,7 @@ 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{}; + const auto code = m_state.get_code(msg.code_address); return m_vm.execute(*this, m_rev, msg, code.data(), code.size()); } @@ -499,7 +503,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 818bd9fd8f..f0459dd17a 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 @@ -103,10 +104,10 @@ StateDiff State::build_diff(evmc_revision rev) // TODO(clang): In old Clang emplace_back without Account doesn't compile. // NOLINTNEXTLINE(modernize-use-emplace) auto& a = diff.modified_accounts.emplace_back(StateDiff::Account{addr, m.nonce, m.balance}); - if (m.just_created && !m.code.empty()) - a.code = m.code; + if (m.just_created && !m._code.empty()) + a.code = m._code; - for (const auto& [k, v] : m.storage) + for (const auto& [k, v] : m._storage) { if (v.current != v.original) a.modified_storage.emplace_back(k, v.current); @@ -124,9 +125,13 @@ Account& State::insert(const address& addr, Account account) Account* State::find(const address& addr) noexcept { - const auto it = m_accounts.find(addr); - if (it != m_accounts.end()) + if (const auto it = m_accounts.find(addr); it != m_accounts.end()) return &it->second; + if (const auto cacc = m_cold->get_account(addr); cacc) + return &insert(addr, {.nonce = cacc->nonce, + .balance = cacc->balance, + .code_hash = cacc->code_hash, + .has_cold_storage = cacc->has_storage}); return nullptr; } @@ -144,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) +{ + auto* a = find(addr); + if (a == nullptr) + return {}; + if (a->code_hash == Account::EMPTY_CODE_HASH) + return {}; + if (a->_code.empty()) + a->_code = m_cold->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 +172,21 @@ Account& State::touch(const address& addr) return acc; } +StorageValue& State::get_storage(const address& addr, const bytes32& key) +{ + auto& acc = get(addr); + const auto it = acc._storage.find(key); + if (it == acc._storage.end()) + { + const auto cold_value = m_cold->get_storage(addr, key); + return acc._storage.insert(it, {key, {cold_value, cold_value}})->second; + } + else + { + 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,7 +254,8 @@ void State::rollback(size_t checkpoint) // This account is not always "touched". TODO: Why? auto& a = get(e.addr); a.nonce = 0; - a.code.clear(); + a.code_hash = Account::EMPTY_CODE_HASH; + a._code.clear(); } else { @@ -235,7 +268,7 @@ void State::rollback(size_t checkpoint) } else if constexpr (std::is_same_v) { - auto& s = get(e.addr).storage.find(e.key)->second; + auto& s = get(e.addr)._storage.find(e.key)->second; s.current = e.prev_value; s.access_status = e.prev_access_status; } @@ -314,7 +347,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 +385,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 +412,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 +464,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 92302181f7..7b8d90fa05 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,6 +64,8 @@ class State std::variant; + const StateView* m_cold = nullptr; + std::unordered_map m_accounts; /// The state journal: the list of changes made in the state @@ -69,7 +73,7 @@ class State std::vector m_journal; public: - State() = default; + explicit State(const StateView& view) noexcept : m_cold{&view} {} State(const State&) = delete; State(State&&) = default; State& operator=(State&&) = default; @@ -87,6 +91,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); /// Returns the state journal checkpoint. It can be later used to in rollback() @@ -124,11 +132,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..072d0912da 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, const BlockInfo& block, evmc_revision rev, evmc::VM& vm) { + State ss{state}; 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.get_account_code(addr); + if (code.empty()) continue; const auto input = get_input(block); @@ -60,13 +63,11 @@ 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; + Host host{rev, vm, ss, block, empty_tx}; [[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); + return ss.build_diff(rev); } } // namespace evmone::state diff --git a/test/state/system_contracts.hpp b/test/state/system_contracts.hpp index 144a828654..f45a7dc598 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, 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 24aa4342df..0dc5144b8b 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_diff(const state::StateDiff& diff) { for (auto& m : diff.modified_accounts) @@ -77,8 +63,7 @@ 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(); - auto res = state::transition(intra_state, block, tx, rev, vm, block_gas_left, blob_gas_left); + auto res = state::transition(state, block, tx, rev, vm, block_gas_left, blob_gas_left); if (holds_alternative(res)) { const auto& r = get(res); @@ -91,15 +76,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(); - auto diff = state::finalize(intra_state, rev, coinbase, block_reward, ommers, withdrawals); + auto diff = state::finalize(state, rev, coinbase, block_reward, ommers, withdrawals); state.apply_diff(diff); } void system_call(TestState& state, const state::BlockInfo& block, evmc_revision rev, evmc::VM& vm) { - auto intra_state = state.to_intra_state(); - auto diff = state::system_call(intra_state, block, rev, vm); + auto diff = state::system_call(state, block, rev, vm); state.apply_diff(diff); } } // namespace evmone::test diff --git a/test/state/test_state.hpp b/test/state/test_state.hpp index 6680f29d8a..05fd52b9fa 100644 --- a/test/state/test_state.hpp +++ b/test/state/test_state.hpp @@ -69,9 +69,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; - void apply_diff(const state::StateDiff& diff); };