From 199f36585a90c59c5a90296a20c001198a90e7c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 8 Nov 2022 21:48:56 +0100 Subject: [PATCH 1/2] test: Improve precision of SSTORE cost in EVMHost Add "original" field to storage_value to precise track "dirty" state of a storage slot as defined in EIP-2200. In case a current storage value is restored to original (after multiple modifications in a single transaction), the storage slot is not considered "dirty" any more. Previously, we only had a bool dirty flag to model this and a storage slot was always considered "dirty" after first modification. --- test/EVMHost.cpp | 4 ++-- test/ExecutionFramework.cpp | 2 +- test/evmc/mocked_host.hpp | 33 +++++++++++---------------------- 3 files changed, 14 insertions(+), 25 deletions(-) diff --git a/test/EVMHost.cpp b/test/EVMHost.cpp index 3e62a39de..e997c394a 100644 --- a/test/EVMHost.cpp +++ b/test/EVMHost.cpp @@ -179,7 +179,7 @@ void EVMHost::newTransactionFrame() for (auto& [slot, value]: account.storage) { value.access_status = EVMC_ACCESS_COLD; // Clear EIP-2929 storage access indicator - value.dirty = false; // Clear EIP-2200 dirty slot flag + value.original = value.current; // Clear EIP-2200 dirty slot } // Process selfdestruct list for (auto& [address, _]: recorded_selfdestructs) @@ -1195,7 +1195,7 @@ void EVMHostPrinter::storage() m_stateStream << " " << m_host.convertFromEVMC(slot) << ": " - << m_host.convertFromEVMC(value.value) + << m_host.convertFromEVMC(value.current) << endl; } diff --git a/test/ExecutionFramework.cpp b/test/ExecutionFramework.cpp index be2a283f7..ff0838c09 100644 --- a/test/ExecutionFramework.cpp +++ b/test/ExecutionFramework.cpp @@ -282,7 +282,7 @@ bool ExecutionFramework::storageEmpty(h160 const& _addr) const if (it != m_evmcHost->accounts.end()) { for (auto const& entry: it->second.storage) - if (!(entry.second.value == evmc::bytes32{})) + if (entry.second.current != evmc::bytes32{}) return false; } return true; diff --git a/test/evmc/mocked_host.hpp b/test/evmc/mocked_host.hpp index c7d99e6ad..5aee8ab05 100644 --- a/test/evmc/mocked_host.hpp +++ b/test/evmc/mocked_host.hpp @@ -15,30 +15,20 @@ namespace evmc /// The string of bytes. using bytes = std::basic_string; -/// Extended value (by dirty flag) for account storage. +/// Extended value (with original value and access flag) for account storage. struct storage_value { - /// The storage value. - bytes32 value; + /// The current storage value. + bytes32 current; - /// True means this value has been modified already by the current transaction. - bool dirty{false}; + /// The original storage value. + bytes32 original; /// Is the storage key cold or warm. - evmc_access_status access_status{EVMC_ACCESS_COLD}; + evmc_access_status access_status = EVMC_ACCESS_COLD; /// Default constructor. storage_value() noexcept = default; - - /// Constructor. - storage_value(const bytes32& _value, bool _dirty = false) noexcept // NOLINT - : value{_value}, dirty{_dirty} - {} - - /// Constructor with initial access status. - storage_value(const bytes32& _value, evmc_access_status _access_status) noexcept - : value{_value}, access_status{_access_status} - {} }; /// Mocked account. @@ -176,7 +166,7 @@ public: const auto storage_iter = account_iter->second.storage.find(key); if (storage_iter != account_iter->second.storage.end()) - return storage_iter->second.value; + return storage_iter->second.current; return {}; } @@ -195,14 +185,13 @@ public: // Follow https://eips.ethereum.org/EIPS/eip-1283 specification. // WARNING! This is not complete implementation as refund is not handled here. - if (old.value == value) + if (old.current == value) return EVMC_STORAGE_UNCHANGED; evmc_storage_status status{}; - if (!old.dirty) + if (old.original == old.current) // Storage slot not dirty { - old.dirty = true; - if (!old.value) + if (!old.current) status = EVMC_STORAGE_ADDED; else if (value) status = EVMC_STORAGE_MODIFIED; @@ -212,7 +201,7 @@ public: else status = EVMC_STORAGE_MODIFIED_AGAIN; - old.value = value; + old.current = value; return status; } From 4830194383d46b7f302965536efa4853493a78ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Tue, 8 Nov 2022 22:17:23 +0100 Subject: [PATCH 2/2] test: Update test expectations --- .../array/byte_array_storage_layout.sol | 4 ++-- ..._storage_storage_different_base_nested.sol | 4 ++-- .../copying/copy_byte_array_to_storage.sol | 6 ++--- .../delete/delete_storage_array_packed.sol | 2 ++ .../struct_delete_storage_with_array.sol | 12 +++++----- .../array_storage_index_zeroed_test.sol | 24 +++++++++---------- 6 files changed, 27 insertions(+), 25 deletions(-) diff --git a/test/libsolidity/semanticTests/array/byte_array_storage_layout.sol b/test/libsolidity/semanticTests/array/byte_array_storage_layout.sol index bb1e904f0..8c899d218 100644 --- a/test/libsolidity/semanticTests/array/byte_array_storage_layout.sol +++ b/test/libsolidity/semanticTests/array/byte_array_storage_layout.sol @@ -46,8 +46,8 @@ contract c { // storageEmpty -> 0 // test_long() -> 67 // gas irOptimized: 89148 -// gas legacy: 105839 -// gas legacyOptimized: 103293 +// gas legacy: 108639 +// gas legacyOptimized: 106093 // storageEmpty -> 0 // test_pop() -> 1780731860627700044960722568376592200742329637303199754547598369979433020 // gas legacy: 61930 diff --git a/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base_nested.sol b/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base_nested.sol index 3e59244bd..85a2f1905 100644 --- a/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base_nested.sol +++ b/test/libsolidity/semanticTests/array/copying/array_copy_storage_storage_different_base_nested.sol @@ -24,5 +24,5 @@ contract c { // ---- // test() -> 3, 4 // gas irOptimized: 189690 -// gas legacy: 195353 -// gas legacyOptimized: 192441 +// gas legacy: 215253 +// gas legacyOptimized: 212341 diff --git a/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol b/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol index 92f2a1f2e..05eff8ac4 100644 --- a/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol @@ -46,6 +46,6 @@ contract C { } // ---- // f() -> 0xff -// gas irOptimized: 119584 -// gas legacy: 132274 -// gas legacyOptimized: 123756 +// gas irOptimized: 179284 +// gas legacy: 191974 +// gas legacyOptimized: 183456 diff --git a/test/libsolidity/semanticTests/array/delete/delete_storage_array_packed.sol b/test/libsolidity/semanticTests/array/delete/delete_storage_array_packed.sol index 22f31b1ef..9665201c9 100644 --- a/test/libsolidity/semanticTests/array/delete/delete_storage_array_packed.sol +++ b/test/libsolidity/semanticTests/array/delete/delete_storage_array_packed.sol @@ -15,3 +15,5 @@ contract C { // ---- // f() -> 0, 0, 0 // gas irOptimized: 90992 +// gas legacy: 111037 +// gas legacyOptimized: 109633 diff --git a/test/libsolidity/semanticTests/structs/struct_delete_storage_with_array.sol b/test/libsolidity/semanticTests/structs/struct_delete_storage_with_array.sol index 1bcd9685a..eb563b947 100644 --- a/test/libsolidity/semanticTests/structs/struct_delete_storage_with_array.sol +++ b/test/libsolidity/semanticTests/structs/struct_delete_storage_with_array.sol @@ -42,10 +42,10 @@ contract C { } // ---- // f() -> -// gas irOptimized: 121657 -// gas legacy: 122132 -// gas legacyOptimized: 121500 +// gas irOptimized: 141557 +// gas legacy: 142032 +// gas legacyOptimized: 141400 // g() -> -// gas irOptimized: 145472 -// gas legacy: 145707 -// gas legacyOptimized: 144940 +// gas irOptimized: 148272 +// gas legacy: 148507 +// gas legacyOptimized: 147740 diff --git a/test/libsolidity/semanticTests/viaYul/array_storage_index_zeroed_test.sol b/test/libsolidity/semanticTests/viaYul/array_storage_index_zeroed_test.sol index f0a582277..3eb553fe9 100644 --- a/test/libsolidity/semanticTests/viaYul/array_storage_index_zeroed_test.sol +++ b/test/libsolidity/semanticTests/viaYul/array_storage_index_zeroed_test.sol @@ -52,18 +52,18 @@ contract C { // ---- // test_zeroed_indicies(uint256): 1 -> // test_zeroed_indicies(uint256): 5 -> -// gas irOptimized: 153874 -// gas legacy: 155001 -// gas legacyOptimized: 152239 +// gas irOptimized: 165074 +// gas legacy: 166201 +// gas legacyOptimized: 163439 // test_zeroed_indicies(uint256): 10 -> -// gas irOptimized: 277077 -// gas legacy: 279488 -// gas legacyOptimized: 274412 +// gas irOptimized: 282677 +// gas legacy: 285088 +// gas legacyOptimized: 280012 // test_zeroed_indicies(uint256): 15 -> -// gas irOptimized: 399822 -// gas legacy: 403538 -// gas legacyOptimized: 396227 +// gas irOptimized: 405422 +// gas legacy: 409138 +// gas legacyOptimized: 401827 // test_zeroed_indicies(uint256): 0xFF -> -// gas irOptimized: 6399212 -// gas legacy: 6460633 -// gas legacyOptimized: 6327477 +// gas irOptimized: 6404812 +// gas legacy: 6466233 +// gas legacyOptimized: 6333077