From e35beb2b16ff8e755a8f0804ca5578b74af94b6c Mon Sep 17 00:00:00 2001 From: Bartosz Zawistowski Date: Mon, 16 Sep 2024 08:56:47 +0200 Subject: [PATCH 1/6] WiP --- silkworm/core/execution/evm.cpp | 58 +++++++++++++++++++++++++++++++++ silkworm/core/execution/evm.hpp | 2 ++ 2 files changed, 60 insertions(+) diff --git a/silkworm/core/execution/evm.cpp b/silkworm/core/execution/evm.cpp index 073f71bbb7..9beef2a96f 100644 --- a/silkworm/core/execution/evm.cpp +++ b/silkworm/core/execution/evm.cpp @@ -89,6 +89,10 @@ CallResult EVM::execute(const Transaction& txn, uint64_t gas) noexcept { .code_address = destination, }; + if (!validate_gas_and_funds(message)) { + return CallResult { .status = EVMC_INSUFFICIENT_BALANCE}; + } + evmc::Result res{contract_creation ? create(message) : call(message)}; const auto gas_left = static_cast(res.gas_left); @@ -270,6 +274,60 @@ evmc::Result EVM::call(const evmc_message& message) noexcept { return res; } +bool EVM::validate_gas_and_funds(const Transaction& txn, const evmc_message& message) const { + + // EIP-1559 normal gas cost + intx::uint256 want; + if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) { + // This method should be called after check (max_fee and base_fee) present in pre_check() method + const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas) + : txn.max_priority_fee_per_gas}; + want = txn.gas_limit * effective_gas_price; + } else { + want = 0; + } + + // EIP-4844 blob gas cost (calc_data_fee) + if (evm.block().header.blob_gas_used && rev >= EVMC_CANCUN) { + // compute blob fee for eip-4844 data blobs if any + const intx::uint256 blob_gas_price{evm.block().header.blob_gas_price().value_or(0)}; + want += txn.total_blob_gas() * blob_gas_price; + } + + intx::uint512 max_want = want; + if (txn.type != silkworm::TransactionType::kLegacy && txn.type != silkworm::TransactionType::kAccessList) { + max_want = txn.maximum_gas_cost(); + } + + const auto have = ibs_state_.get_balance(*txn.sender()); + if (have < max_want + txn.value) { + Bytes data{}; + std::string from = address_to_hex(*txn.sender()); + std::string msg = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(have) + " want " + intx::to_string(max_want + txn.value); + return {std::nullopt, txn.gas_limit, data, msg, PreCheckErrorCode::kInsufficientFunds}; + } + ibs_state_.subtract_from_balance(*txn.sender(), want); + + if(!gas_bailout_) { + // Call + if (!gas_bailout_ && message.kind != EVMC_DELEGATECALL && have < value) { + res.status_code = EVMC_INSUFFICIENT_BALANCE; + return res; + } + // Create + if (!gas_bailout_ && have < value) { + res.status_code = EVMC_INSUFFICIENT_BALANCE; + + for (auto tracer : tracers_) { + tracer.get().on_pre_check_failed(res.raw(), message); + } + + return res; + } + } + return true; +} + evmc_result EVM::execute(const evmc_message& message, ByteView code, const evmc::bytes32* code_hash) noexcept { const evmc_revision rev{revision()}; if (exo_evm) { diff --git a/silkworm/core/execution/evm.hpp b/silkworm/core/execution/evm.hpp index 86db1c950c..20d803e062 100644 --- a/silkworm/core/execution/evm.hpp +++ b/silkworm/core/execution/evm.hpp @@ -135,6 +135,8 @@ class EVM { gsl::owner acquire_state() const noexcept; void release_state(gsl::owner state) const noexcept; + bool validate_gas_and_funds(const evmc_message& message) const; + const Block& block_; IntraBlockState& state_; const ChainConfig& config_; From c757cdfdeb81c60d59da68a53c826ffb6e731465 Mon Sep 17 00:00:00 2001 From: Bartosz Zawistowski Date: Mon, 16 Sep 2024 10:38:40 +0200 Subject: [PATCH 2/6] Progress --- silkworm/core/execution/evm.cpp | 72 +++++++++++------------------- silkworm/core/execution/evm.hpp | 12 ++--- silkworm/rpc/core/evm_executor.cpp | 40 ++--------------- 3 files changed, 36 insertions(+), 88 deletions(-) diff --git a/silkworm/core/execution/evm.cpp b/silkworm/core/execution/evm.cpp index 9beef2a96f..99702369ac 100644 --- a/silkworm/core/execution/evm.cpp +++ b/silkworm/core/execution/evm.cpp @@ -59,12 +59,12 @@ class DelegatingTracer : public evmone::Tracer { IntraBlockState& intra_block_state_; }; -EVM::EVM(const Block& block, IntraBlockState& state, const ChainConfig& config, bool gas_bailout) noexcept +EVM::EVM(const Block& block, IntraBlockState& state, const ChainConfig& config, bool bailout) noexcept : beneficiary{block.header.beneficiary}, block_{block}, state_{state}, config_{config}, - gas_bailout_{gas_bailout}, + bailout_{bailout}, evm1_{static_cast(evmc_create_evmone())} // NOLINT(cppcoreguidelines-pro-type-static-cast-downcast) {} @@ -89,10 +89,6 @@ CallResult EVM::execute(const Transaction& txn, uint64_t gas) noexcept { .code_address = destination, }; - if (!validate_gas_and_funds(message)) { - return CallResult { .status = EVMC_INSUFFICIENT_BALANCE}; - } - evmc::Result res{contract_creation ? create(message) : call(message)}; const auto gas_left = static_cast(res.gas_left); @@ -104,8 +100,8 @@ evmc::Result EVM::create(const evmc_message& message) noexcept { evmc::Result res{EVMC_SUCCESS, message.gas, 0}; auto value{intx::be::load(message.value)}; - const auto have = state_.get_balance(message.sender); - if (!gas_bailout_ && have < value) { + const auto owned_funds = state_.get_balance(message.sender); + if (!bailout_ && owned_funds < value) { res.status_code = EVMC_INSUFFICIENT_BALANCE; for (auto tracer : tracers_) { @@ -154,7 +150,7 @@ evmc::Result EVM::create(const evmc_message& message) noexcept { state_.set_nonce(contract_addr, 1); } - transfer(state_, message.sender, contract_addr, value, gas_bailout_); + transfer(state_, message.sender, contract_addr, value, bailout_); const evmc_message deploy_message{ .kind = message.depth > 0 ? message.kind : EVMC_CALL, @@ -209,8 +205,8 @@ evmc::Result EVM::call(const evmc_message& message) noexcept { evmc::Result res{EVMC_SUCCESS, message.gas}; const auto value{intx::be::load(message.value)}; - const auto have = state_.get_balance(message.sender); - if (!gas_bailout_ && message.kind != EVMC_DELEGATECALL && have < value) { + const auto owned_funds = state_.get_balance(message.sender); + if (!bailout_ && message.kind != EVMC_DELEGATECALL && owned_funds < value) { res.status_code = EVMC_INSUFFICIENT_BALANCE; return res; } @@ -223,7 +219,7 @@ evmc::Result EVM::call(const evmc_message& message) noexcept { // https://github.com/ethereum/go-ethereum/blob/v1.9.25/core/vm/evm.go#L391 state_.touch(message.recipient); } else { - transfer(state_, message.sender, message.recipient, value, gas_bailout_); + transfer(state_, message.sender, message.recipient, value, bailout_); } } @@ -274,58 +270,42 @@ evmc::Result EVM::call(const evmc_message& message) noexcept { return res; } -bool EVM::validate_gas_and_funds(const Transaction& txn, const evmc_message& message) const { +CallResult EVM::deduct_entry_fees(const Transaction& txn) const { + const evmc_revision rev{revision()}; + const intx::uint256 base_fee_per_gas{block().header.base_fee_per_gas.value_or(0)}; // EIP-1559 normal gas cost - intx::uint256 want; + intx::uint256 required_funds; if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) { // This method should be called after check (max_fee and base_fee) present in pre_check() method const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas) : txn.max_priority_fee_per_gas}; - want = txn.gas_limit * effective_gas_price; + required_funds = txn.gas_limit * effective_gas_price; } else { - want = 0; + required_funds = 0; } // EIP-4844 blob gas cost (calc_data_fee) - if (evm.block().header.blob_gas_used && rev >= EVMC_CANCUN) { + if (block().header.blob_gas_used && rev >= EVMC_CANCUN) { // compute blob fee for eip-4844 data blobs if any - const intx::uint256 blob_gas_price{evm.block().header.blob_gas_price().value_or(0)}; - want += txn.total_blob_gas() * blob_gas_price; + const intx::uint256 blob_gas_price{block().header.blob_gas_price().value_or(0)}; + required_funds += txn.total_blob_gas() * blob_gas_price; } - intx::uint512 max_want = want; - if (txn.type != silkworm::TransactionType::kLegacy && txn.type != silkworm::TransactionType::kAccessList) { - max_want = txn.maximum_gas_cost(); + intx::uint512 maximum_cost = required_funds; + if (txn.type != TransactionType::kLegacy && txn.type != TransactionType::kAccessList) { + maximum_cost = txn.maximum_gas_cost(); } - const auto have = ibs_state_.get_balance(*txn.sender()); - if (have < max_want + txn.value) { + const auto owned_funds = state_.get_balance(*txn.sender()); + if (owned_funds < maximum_cost + txn.value) { Bytes data{}; std::string from = address_to_hex(*txn.sender()); - std::string msg = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(have) + " want " + intx::to_string(max_want + txn.value); - return {std::nullopt, txn.gas_limit, data, msg, PreCheckErrorCode::kInsufficientFunds}; - } - ibs_state_.subtract_from_balance(*txn.sender(), want); - - if(!gas_bailout_) { - // Call - if (!gas_bailout_ && message.kind != EVMC_DELEGATECALL && have < value) { - res.status_code = EVMC_INSUFFICIENT_BALANCE; - return res; - } - // Create - if (!gas_bailout_ && have < value) { - res.status_code = EVMC_INSUFFICIENT_BALANCE; - - for (auto tracer : tracers_) { - tracer.get().on_pre_check_failed(res.raw(), message); - } - - return res; - } + std::string msg = "insufficient funds for gas * price + value: address " + from + " has " + intx::to_string(owned_funds) + " requires " + intx::to_string(maximum_cost + txn.value); + return {.status = EVMC_INSUFFICIENT_BALANCE, .error_message = msg}; } - return true; + state_.subtract_from_balance(*txn.sender(), required_funds); + return {.status = EVMC_SUCCESS}; } evmc_result EVM::execute(const evmc_message& message, ByteView code, const evmc::bytes32* code_hash) noexcept { diff --git a/silkworm/core/execution/evm.hpp b/silkworm/core/execution/evm.hpp index 20d803e062..cab5147816 100644 --- a/silkworm/core/execution/evm.hpp +++ b/silkworm/core/execution/evm.hpp @@ -40,6 +40,7 @@ struct CallResult { uint64_t gas_left{0}; uint64_t gas_refund{0}; Bytes data; + std::string error_message; }; class EvmTracer { @@ -79,8 +80,7 @@ using TransferFunc = void(IntraBlockState& state, const evmc::address& sender, c // See consensus.Transfer in Erigon inline void standard_transfer(IntraBlockState& state, const evmc::address& sender, const evmc::address& recipient, const intx::uint256& amount, bool bailout) { - // TODO(yperbasis) why is the bailout condition different from Erigon? - if (!bailout || state.get_balance(sender) >= amount) { + if (!bailout) { state.subtract_from_balance(sender, amount); } state.add_to_balance(recipient, amount); @@ -92,7 +92,7 @@ class EVM { EVM(const EVM&) = delete; EVM& operator=(const EVM&) = delete; - EVM(const Block& block, IntraBlockState& state, const ChainConfig& config, bool gas_bailout = false) noexcept; + EVM(const Block& block, IntraBlockState& state, const ChainConfig& config, bool bailout = false) noexcept; ~EVM(); @@ -120,6 +120,8 @@ class EVM { gsl::not_null transfer{standard_transfer}; + CallResult deduct_entry_fees(const Transaction& txn) const; + private: friend class EvmHost; @@ -135,12 +137,10 @@ class EVM { gsl::owner acquire_state() const noexcept; void release_state(gsl::owner state) const noexcept; - bool validate_gas_and_funds(const evmc_message& message) const; - const Block& block_; IntraBlockState& state_; const ChainConfig& config_; - bool gas_bailout_; + bool bailout_; const Transaction* txn_{nullptr}; std::vector block_hashes_{}; EvmTracers tracers_; diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index 2b9c9d5c5d..b60052fdc9 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -260,46 +260,14 @@ ExecutionResult EVMExecutor::call( const intx::uint128 g0{protocol::intrinsic_gas(txn, rev)}; SILKWORM_ASSERT(g0 <= UINT64_MAX); // true due to the precondition (transaction must be valid) - const auto pre_check_result = pre_check(evm, txn, base_fee_per_gas, g0); - if (pre_check_result) { + if (const auto pre_check_result = pre_check(evm, txn, base_fee_per_gas, g0)) { Bytes data{}; return {std::nullopt, txn.gas_limit, data, pre_check_result->pre_check_error, pre_check_result->pre_check_error_code}; } - if (!gas_bailout) { - // EIP-1559 normal gas cost - intx::uint256 want; - if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) { - // This method should be called after check (max_fee and base_fee) present in pre_check() method - const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas) - : txn.max_priority_fee_per_gas}; - want = txn.gas_limit * effective_gas_price; - } else { - want = 0; - } - - // EIP-4844 blob gas cost (calc_data_fee) - if (evm.block().header.blob_gas_used && rev >= EVMC_CANCUN) { - // compute blob fee for eip-4844 data blobs if any - const intx::uint256 blob_gas_price{evm.block().header.blob_gas_price().value_or(0)}; - want += txn.total_blob_gas() * blob_gas_price; - } - - intx::uint512 max_want = want; - if (txn.type != silkworm::TransactionType::kLegacy && txn.type != silkworm::TransactionType::kAccessList) { - max_want = txn.maximum_gas_cost(); - } - - const auto have = ibs_state_.get_balance(*txn.sender()); - if (have < max_want + txn.value) { - Bytes data{}; - std::string from = address_to_hex(*txn.sender()); - std::string msg = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(have) + " want " + intx::to_string(max_want + txn.value); - return {std::nullopt, txn.gas_limit, data, msg, PreCheckErrorCode::kInsufficientFunds}; - } - ibs_state_.subtract_from_balance(*txn.sender(), want); + if (const auto result = evm.deduct_entry_fees(txn); result.status != EVMC_SUCCESS) { + return {std::nullopt, txn.gas_limit, {}, result.error_message, PreCheckErrorCode::kInsufficientFunds}; } - if (txn.to.has_value()) { ibs_state_.access_account(*txn.to); // EVM itself increments the nonce for contract creation @@ -312,7 +280,7 @@ ExecutionResult EVMExecutor::call( } } - silkworm::CallResult result; + CallResult result; try { SILK_DEBUG << "EVMExecutor::call execute on EVM txn: " << &txn << " g0: " << static_cast(g0) << " start"; result = evm.execute(txn, txn.gas_limit - static_cast(g0)); From 7d16127a928031931761a6afa284e6933d866f81 Mon Sep 17 00:00:00 2001 From: Bartosz Zawistowski Date: Mon, 16 Sep 2024 10:49:36 +0200 Subject: [PATCH 3/6] Unused var --- silkworm/core/execution/evm.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/silkworm/core/execution/evm.cpp b/silkworm/core/execution/evm.cpp index 99702369ac..1d5c575b0f 100644 --- a/silkworm/core/execution/evm.cpp +++ b/silkworm/core/execution/evm.cpp @@ -299,7 +299,6 @@ CallResult EVM::deduct_entry_fees(const Transaction& txn) const { const auto owned_funds = state_.get_balance(*txn.sender()); if (owned_funds < maximum_cost + txn.value) { - Bytes data{}; std::string from = address_to_hex(*txn.sender()); std::string msg = "insufficient funds for gas * price + value: address " + from + " has " + intx::to_string(owned_funds) + " requires " + intx::to_string(maximum_cost + txn.value); return {.status = EVMC_INSUFFICIENT_BALANCE, .error_message = msg}; From 5467fb0865bef5423918da2721e507e32897f1be Mon Sep 17 00:00:00 2001 From: Bartosz Zawistowski Date: Mon, 16 Sep 2024 10:55:07 +0200 Subject: [PATCH 4/6] Missing bailout check --- silkworm/core/execution/evm.cpp | 60 +++++++++++++++++---------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/silkworm/core/execution/evm.cpp b/silkworm/core/execution/evm.cpp index 1d5c575b0f..fb0d523142 100644 --- a/silkworm/core/execution/evm.cpp +++ b/silkworm/core/execution/evm.cpp @@ -271,39 +271,41 @@ evmc::Result EVM::call(const evmc_message& message) noexcept { } CallResult EVM::deduct_entry_fees(const Transaction& txn) const { - const evmc_revision rev{revision()}; - const intx::uint256 base_fee_per_gas{block().header.base_fee_per_gas.value_or(0)}; - - // EIP-1559 normal gas cost - intx::uint256 required_funds; - if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) { - // This method should be called after check (max_fee and base_fee) present in pre_check() method - const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas) - : txn.max_priority_fee_per_gas}; - required_funds = txn.gas_limit * effective_gas_price; - } else { - required_funds = 0; - } + if (!bailout_) { + const evmc_revision rev{revision()}; + const intx::uint256 base_fee_per_gas{block().header.base_fee_per_gas.value_or(0)}; + + // EIP-1559 normal gas cost + intx::uint256 required_funds; + if (txn.max_fee_per_gas > 0 || txn.max_priority_fee_per_gas > 0) { + // This method should be called after check (max_fee and base_fee) present in pre_check() method + const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas) + : txn.max_priority_fee_per_gas}; + required_funds = txn.gas_limit * effective_gas_price; + } else { + required_funds = 0; + } - // EIP-4844 blob gas cost (calc_data_fee) - if (block().header.blob_gas_used && rev >= EVMC_CANCUN) { - // compute blob fee for eip-4844 data blobs if any - const intx::uint256 blob_gas_price{block().header.blob_gas_price().value_or(0)}; - required_funds += txn.total_blob_gas() * blob_gas_price; - } + // EIP-4844 blob gas cost (calc_data_fee) + if (block().header.blob_gas_used && rev >= EVMC_CANCUN) { + // compute blob fee for eip-4844 data blobs if any + const intx::uint256 blob_gas_price{block().header.blob_gas_price().value_or(0)}; + required_funds += txn.total_blob_gas() * blob_gas_price; + } - intx::uint512 maximum_cost = required_funds; - if (txn.type != TransactionType::kLegacy && txn.type != TransactionType::kAccessList) { - maximum_cost = txn.maximum_gas_cost(); - } + intx::uint512 maximum_cost = required_funds; + if (txn.type != TransactionType::kLegacy && txn.type != TransactionType::kAccessList) { + maximum_cost = txn.maximum_gas_cost(); + } - const auto owned_funds = state_.get_balance(*txn.sender()); - if (owned_funds < maximum_cost + txn.value) { - std::string from = address_to_hex(*txn.sender()); - std::string msg = "insufficient funds for gas * price + value: address " + from + " has " + intx::to_string(owned_funds) + " requires " + intx::to_string(maximum_cost + txn.value); - return {.status = EVMC_INSUFFICIENT_BALANCE, .error_message = msg}; + const auto owned_funds = state_.get_balance(*txn.sender()); + if (owned_funds < maximum_cost + txn.value) { + std::string from = address_to_hex(*txn.sender()); + std::string msg = "insufficient funds for gas * price + value: address " + from + " has " + intx::to_string(owned_funds) + " requires " + intx::to_string(maximum_cost + txn.value); + return {.status = EVMC_INSUFFICIENT_BALANCE, .error_message = msg}; + } + state_.subtract_from_balance(*txn.sender(), required_funds); } - state_.subtract_from_balance(*txn.sender(), required_funds); return {.status = EVMC_SUCCESS}; } From af317662b37b37eb641abfdb097843fadf5b9f7a Mon Sep 17 00:00:00 2001 From: Bartosz Zawistowski Date: Mon, 16 Sep 2024 11:12:17 +0200 Subject: [PATCH 5/6] Fixing err msg --- silkworm/core/execution/evm.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silkworm/core/execution/evm.cpp b/silkworm/core/execution/evm.cpp index fb0d523142..19ecd11a50 100644 --- a/silkworm/core/execution/evm.cpp +++ b/silkworm/core/execution/evm.cpp @@ -301,7 +301,7 @@ CallResult EVM::deduct_entry_fees(const Transaction& txn) const { const auto owned_funds = state_.get_balance(*txn.sender()); if (owned_funds < maximum_cost + txn.value) { std::string from = address_to_hex(*txn.sender()); - std::string msg = "insufficient funds for gas * price + value: address " + from + " has " + intx::to_string(owned_funds) + " requires " + intx::to_string(maximum_cost + txn.value); + std::string msg = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(owned_funds) + " want " + intx::to_string(maximum_cost + txn.value); return {.status = EVMC_INSUFFICIENT_BALANCE, .error_message = msg}; } state_.subtract_from_balance(*txn.sender(), required_funds); From 4b99a1de9e9daf1720cba5ca4a0cbfa5ad9a83a1 Mon Sep 17 00:00:00 2001 From: Bartosz Zawistowski Date: Mon, 16 Sep 2024 12:34:51 +0200 Subject: [PATCH 6/6] Revert check --- silkworm/core/execution/evm.hpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/silkworm/core/execution/evm.hpp b/silkworm/core/execution/evm.hpp index cab5147816..8ee304afe6 100644 --- a/silkworm/core/execution/evm.hpp +++ b/silkworm/core/execution/evm.hpp @@ -33,6 +33,8 @@ #include #include +#include "silkworm/core/types/address.hpp" + namespace silkworm { struct CallResult { @@ -80,7 +82,8 @@ using TransferFunc = void(IntraBlockState& state, const evmc::address& sender, c // See consensus.Transfer in Erigon inline void standard_transfer(IntraBlockState& state, const evmc::address& sender, const evmc::address& recipient, const intx::uint256& amount, bool bailout) { - if (!bailout) { + // TODO(yperbasis) why is the bailout condition different from Erigon? + if (!bailout || state.get_balance(sender) >= amount) { state.subtract_from_balance(sender, amount); } state.add_to_balance(recipient, amount);