From 899989df43ff7cb4805774db36ef16703ef6cdb6 Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Tue, 4 Feb 2025 07:02:00 +0100 Subject: [PATCH 1/9] remove double validation from processor and executor --- silkworm/core/execution/evm.hpp | 2 ++ silkworm/core/execution/processor.cpp | 10 ++++++++-- silkworm/rpc/core/evm_executor.cpp | 11 +++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/silkworm/core/execution/evm.hpp b/silkworm/core/execution/evm.hpp index fc657d05bd..2f3583de41 100644 --- a/silkworm/core/execution/evm.hpp +++ b/silkworm/core/execution/evm.hpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -39,6 +40,7 @@ namespace silkworm { struct CallResult { + ValidationResult validation_result{ValidationResult::kOk}; evmc_status_code status{EVMC_SUCCESS}; uint64_t gas_left{0}; uint64_t gas_refund{0}; diff --git a/silkworm/core/execution/processor.cpp b/silkworm/core/execution/processor.cpp index e6da257b0c..2986a605d6 100644 --- a/silkworm/core/execution/processor.cpp +++ b/silkworm/core/execution/processor.cpp @@ -264,7 +264,10 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector sender{txn.sender()}; SILKWORM_ASSERT(sender); - SILKWORM_ASSERT(protocol::validate_call_precheck(txn, evm_) == ValidationResult::kOk); + ValidationResult validation_result = protocol::validate_call_precheck(txn, evm_); + if (validation_result != ValidationResult::kOk) { + return {validation_result, EVMC_SUCCESS, 0, {}, {}}; + } const BlockHeader& header{evm_.block().header}; const intx::uint256 base_fee_per_gas{header.base_fee_per_gas.value_or(0)}; @@ -289,7 +292,10 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector(g0)); diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index 13f7a247ef..2d7b957db6 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -233,6 +233,7 @@ ExecutionResult EVMExecutor::call( return {std::nullopt, txn.gas_limit, Bytes{}, "malformed transaction: cannot recover sender"}; } +#ifdef notdef if (const auto result = protocol::validate_call_precheck(txn, evm); result != ValidationResult::kOk) { return convert_validated_precheck(result, block, txn, evm); @@ -244,8 +245,18 @@ ExecutionResult EVMExecutor::call( !bailout && result != ValidationResult::kOk) { return convert_validated_funds(block, txn, evm, owned_funds); } +#endif const auto result = execution_processor_.call(txn, tracers, refund); + if (result.validation_result != ValidationResult::kOk) { + return convert_validated_precheck(result.validation_result, block, txn, evm); + } + + if (0) { + const auto owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); + + return convert_validated_funds(block, txn, evm, owned_funds); + } ExecutionResult exec_result{result.status, result.gas_left, result.data}; From 8ba53120648324d86735d0a9c79cf400d4de6dcc Mon Sep 17 00:00:00 2001 From: GitHub Date: Tue, 4 Feb 2025 06:03:40 +0000 Subject: [PATCH 2/9] make fmt --- silkworm/core/execution/processor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silkworm/core/execution/processor.cpp b/silkworm/core/execution/processor.cpp index 2986a605d6..b345202196 100644 --- a/silkworm/core/execution/processor.cpp +++ b/silkworm/core/execution/processor.cpp @@ -266,7 +266,7 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector Date: Tue, 4 Feb 2025 07:14:00 +0100 Subject: [PATCH 3/9] initialize also ValidationResult --- silkworm/core/execution/evm.cpp | 2 +- silkworm/core/execution/evm.hpp | 1 + silkworm/core/execution/processor.cpp | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/silkworm/core/execution/evm.cpp b/silkworm/core/execution/evm.cpp index e9a5ba4a6b..1f312d8f39 100644 --- a/silkworm/core/execution/evm.cpp +++ b/silkworm/core/execution/evm.cpp @@ -98,7 +98,7 @@ CallResult EVM::execute(const Transaction& txn, uint64_t gas) noexcept { const auto gas_left = static_cast(res.gas_left); const auto gas_refund = static_cast(res.gas_refund); - return {res.status_code, gas_left, gas_refund, {res.output_data, res.output_size}}; + return {ValidationResult::kOk, res.status_code, gas_left, gas_refund, {res.output_data, res.output_size}}; } evmc::Result EVM::create(const evmc_message& message) noexcept { diff --git a/silkworm/core/execution/evm.hpp b/silkworm/core/execution/evm.hpp index 2f3583de41..d4e3e04020 100644 --- a/silkworm/core/execution/evm.hpp +++ b/silkworm/core/execution/evm.hpp @@ -39,6 +39,7 @@ namespace silkworm { + struct CallResult { ValidationResult validation_result{ValidationResult::kOk}; evmc_status_code status{EVMC_SUCCESS}; diff --git a/silkworm/core/execution/processor.cpp b/silkworm/core/execution/processor.cpp index b345202196..9891329012 100644 --- a/silkworm/core/execution/processor.cpp +++ b/silkworm/core/execution/processor.cpp @@ -321,7 +321,7 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector Date: Tue, 4 Feb 2025 19:00:30 +0100 Subject: [PATCH 4/9] implement single convert_validated_result() --- silkworm/core/execution/evm.hpp | 1 - silkworm/core/execution/evm_test.cpp | 2 +- silkworm/core/protocol/validation.cpp | 1 + silkworm/core/protocol/validation.hpp | 3 +- silkworm/rpc/core/evm_executor.cpp | 52 +++++++++------------------ 5 files changed, 21 insertions(+), 38 deletions(-) diff --git a/silkworm/core/execution/evm.hpp b/silkworm/core/execution/evm.hpp index d4e3e04020..2f3583de41 100644 --- a/silkworm/core/execution/evm.hpp +++ b/silkworm/core/execution/evm.hpp @@ -39,7 +39,6 @@ namespace silkworm { - struct CallResult { ValidationResult validation_result{ValidationResult::kOk}; evmc_status_code status{EVMC_SUCCESS}; diff --git a/silkworm/core/execution/evm_test.cpp b/silkworm/core/execution/evm_test.cpp index 519be42697..1a170c7aab 100644 --- a/silkworm/core/execution/evm_test.cpp +++ b/silkworm/core/execution/evm_test.cpp @@ -569,7 +569,7 @@ class TestTracer : public EvmTracer { execution_end_called_ = true; const auto gas_left = static_cast(res.gas_left); const auto gas_refund = static_cast(res.gas_refund); - result_ = {res.status_code, gas_left, gas_refund, {res.output_data, res.output_size}}; + result_ = {ValidationResult::kOk, res.status_code, gas_left, gas_refund, {res.output_data, res.output_size}}; if (contract_address_ && !pc_stack_.empty()) { const auto pc = pc_stack_.back(); storage_stack_[pc] = diff --git a/silkworm/core/protocol/validation.cpp b/silkworm/core/protocol/validation.cpp index aeee16d5f1..65c94756f3 100644 --- a/silkworm/core/protocol/validation.cpp +++ b/silkworm/core/protocol/validation.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include diff --git a/silkworm/core/protocol/validation.hpp b/silkworm/core/protocol/validation.hpp index 4bc6405857..1d0e03eac8 100644 --- a/silkworm/core/protocol/validation.hpp +++ b/silkworm/core/protocol/validation.hpp @@ -20,13 +20,14 @@ #include -#include #include #include #include namespace silkworm { +class EVM; + // Classification of invalid transactions and blocks. enum class [[nodiscard]] ValidationResult { kOk, // All checks passed diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index 2d7b957db6..f95ff7188d 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -166,7 +166,7 @@ void EVMExecutor::reset() { execution_processor_.reset(); } -ExecutionResult convert_validated_precheck(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm) { +ExecutionResult convert_validated_result(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) { std::string from = address_to_hex(*txn.sender()); switch (result) { case ValidationResult::kMaxPriorityFeeGreaterThanMax: { @@ -192,6 +192,21 @@ ExecutionResult convert_validated_precheck(const ValidationResult& result, const std::string error = "eip-1559 transactions require london"; return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kIsNotLondon}; } + + case ValidationResult::kInsufficientFunds: { + const intx::uint256 base_fee_per_gas{block.header.base_fee_per_gas.value_or(0)}; + + 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}; + const auto required_funds = protocol::compute_call_cost(txn, effective_gas_price, evm); + intx::uint512 maximum_cost = required_funds; + if (txn.type != TransactionType::kLegacy && txn.type != TransactionType::kAccessList) { + maximum_cost = txn.maximum_gas_cost(); + } + std::string error = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(owned_funds) + " want " + intx::to_string(maximum_cost + txn.value); + return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kInsufficientFunds}; + } + default: { std::string error = "internal failure"; return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kInternalError}; @@ -199,21 +214,6 @@ ExecutionResult convert_validated_precheck(const ValidationResult& result, const } } -ExecutionResult convert_validated_funds(const Block& block, const silkworm::Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) { - std::string from = address_to_hex(*txn.sender()); - const intx::uint256 base_fee_per_gas{block.header.base_fee_per_gas.value_or(0)}; - - 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}; - const auto required_funds = protocol::compute_call_cost(txn, effective_gas_price, evm); - intx::uint512 maximum_cost = required_funds; - if (txn.type != TransactionType::kLegacy && txn.type != TransactionType::kAccessList) { - maximum_cost = txn.maximum_gas_cost(); - } - std::string error = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(owned_funds) + " want " + intx::to_string(maximum_cost + txn.value); - return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kInsufficientFunds}; -} - ExecutionResult EVMExecutor::call( const silkworm::Block& block, const silkworm::Transaction& txn, @@ -233,29 +233,11 @@ ExecutionResult EVMExecutor::call( return {std::nullopt, txn.gas_limit, Bytes{}, "malformed transaction: cannot recover sender"}; } -#ifdef notdef - if (const auto result = protocol::validate_call_precheck(txn, evm); - result != ValidationResult::kOk) { - return convert_validated_precheck(result, block, txn, evm); - } - - const auto owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); - - if (const auto result = protocol::validate_call_funds(txn, evm, owned_funds, bailout); - !bailout && result != ValidationResult::kOk) { - return convert_validated_funds(block, txn, evm, owned_funds); - } -#endif - const auto result = execution_processor_.call(txn, tracers, refund); if (result.validation_result != ValidationResult::kOk) { - return convert_validated_precheck(result.validation_result, block, txn, evm); - } - - if (0) { const auto owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); - return convert_validated_funds(block, txn, evm, owned_funds); + return convert_validated_result(result.validation_result, block, txn, evm, owned_funds); } ExecutionResult exec_result{result.status, result.gas_left, result.data}; From 59a08f3c9693f89302e030433cff00d877d8f0e4 Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Tue, 4 Feb 2025 21:44:03 +0100 Subject: [PATCH 5/9] fix test --- silkworm/rpc/core/evm_executor_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/silkworm/rpc/core/evm_executor_test.cpp b/silkworm/rpc/core/evm_executor_test.cpp index 9f23251393..05531d7d0d 100644 --- a/silkworm/rpc/core/evm_executor_test.cpp +++ b/silkworm/rpc/core/evm_executor_test.cpp @@ -111,7 +111,7 @@ TEST_CASE_METHOD(EVMExecutorTest, "EVMExecutor") { SECTION("failed if transaction cost greater user amount") { auto cursor = std::make_shared(); - EXPECT_CALL(transaction, get_as_of(_)).WillOnce(Invoke([=](Unused) -> Task { + EXPECT_CALL(transaction, get_as_of(_)).WillRepeatedly(Invoke([=](Unused) -> Task { db::kv::api::GetAsOfResult response{ .success = true, .value = Bytes{}}; From 199dd4cccaef77abd8a677faefbcb9492ecb055d Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Tue, 4 Feb 2025 21:44:58 +0100 Subject: [PATCH 6/9] retrieve funds only if necessary --- silkworm/rpc/core/evm_executor.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index f95ff7188d..3569db7f1b 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -235,7 +235,10 @@ ExecutionResult EVMExecutor::call( const auto result = execution_processor_.call(txn, tracers, refund); if (result.validation_result != ValidationResult::kOk) { - const auto owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); + intx::uint256 owned_funds{0}; + if (result.validation_result == ValidationResult::kInsufficientFunds) { + owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); + } return convert_validated_result(result.validation_result, block, txn, evm, owned_funds); } From ae6473beabd5e4bf1bd116321557f53302c88f0c Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Tue, 4 Feb 2025 22:22:48 +0100 Subject: [PATCH 7/9] renamed convert_validated_result() into convert_validation_result() --- silkworm/rpc/core/evm_executor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index 3569db7f1b..472e03a96f 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -166,7 +166,7 @@ void EVMExecutor::reset() { execution_processor_.reset(); } -ExecutionResult convert_validated_result(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) { +ExecutionResult convert_validation_result(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) { std::string from = address_to_hex(*txn.sender()); switch (result) { case ValidationResult::kMaxPriorityFeeGreaterThanMax: { @@ -240,7 +240,7 @@ ExecutionResult EVMExecutor::call( owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); } - return convert_validated_result(result.validation_result, block, txn, evm, owned_funds); + return convert_validation_result(result.validation_result, block, txn, evm, owned_funds); } ExecutionResult exec_result{result.status, result.gas_left, result.data}; From aad77aeefd82f26ac2308da40d60d8c698ef8a2f Mon Sep 17 00:00:00 2001 From: lupin012 <58134934+lupin012@users.noreply.github.com.> Date: Wed, 5 Feb 2025 06:56:38 +0100 Subject: [PATCH 8/9] convert_validation_result from routine to method --- silkworm/rpc/core/evm_executor.cpp | 10 +++------- silkworm/rpc/core/evm_executor.hpp | 2 ++ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index 472e03a96f..b608f5c7a3 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -166,7 +166,7 @@ void EVMExecutor::reset() { execution_processor_.reset(); } -ExecutionResult convert_validation_result(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) { +ExecutionResult EVMExecutor::convert_validation_result(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm) { std::string from = address_to_hex(*txn.sender()); switch (result) { case ValidationResult::kMaxPriorityFeeGreaterThanMax: { @@ -194,6 +194,7 @@ ExecutionResult convert_validation_result(const ValidationResult& result, const } case ValidationResult::kInsufficientFunds: { + auto owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); const intx::uint256 base_fee_per_gas{block.header.base_fee_per_gas.value_or(0)}; const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= base_fee_per_gas ? txn.effective_gas_price(base_fee_per_gas) @@ -235,12 +236,7 @@ ExecutionResult EVMExecutor::call( const auto result = execution_processor_.call(txn, tracers, refund); if (result.validation_result != ValidationResult::kOk) { - intx::uint256 owned_funds{0}; - if (result.validation_result == ValidationResult::kInsufficientFunds) { - owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); - } - - return convert_validation_result(result.validation_result, block, txn, evm, owned_funds); + return convert_validation_result(result.validation_result, block, txn, evm); } ExecutionResult exec_result{result.status, result.gas_left, result.data}; diff --git a/silkworm/rpc/core/evm_executor.hpp b/silkworm/rpc/core/evm_executor.hpp index 5ccad3dbf9..eb9c40144e 100644 --- a/silkworm/rpc/core/evm_executor.hpp +++ b/silkworm/rpc/core/evm_executor.hpp @@ -132,6 +132,8 @@ class EVMExecutor { void reset(); private: + ExecutionResult convert_validation_result(const ValidationResult& result, const Block& block, const silkworm::Transaction& txn, const EVM& evm); + const silkworm::ChainConfig& config_; WorkerPool& workers_; std::shared_ptr state_; From 58961087bdb754af6d9c6ece14f72ca36365566a Mon Sep 17 00:00:00 2001 From: canepat <16927169+canepat@users.noreply.github.com> Date: Thu, 6 Feb 2025 09:04:59 +0100 Subject: [PATCH 9/9] Update evm_executor.cpp --- silkworm/rpc/core/evm_executor.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/silkworm/rpc/core/evm_executor.cpp b/silkworm/rpc/core/evm_executor.cpp index b608f5c7a3..7a72a74594 100644 --- a/silkworm/rpc/core/evm_executor.cpp +++ b/silkworm/rpc/core/evm_executor.cpp @@ -192,7 +192,6 @@ ExecutionResult EVMExecutor::convert_validation_result(const ValidationResult& r std::string error = "eip-1559 transactions require london"; return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kIsNotLondon}; } - case ValidationResult::kInsufficientFunds: { auto owned_funds = execution_processor_.intra_block_state().get_balance(*txn.sender()); const intx::uint256 base_fee_per_gas{block.header.base_fee_per_gas.value_or(0)}; @@ -207,7 +206,6 @@ ExecutionResult EVMExecutor::convert_validation_result(const ValidationResult& r std::string error = "insufficient funds for gas * price + value: address " + from + " have " + intx::to_string(owned_funds) + " want " + intx::to_string(maximum_cost + txn.value); return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kInsufficientFunds}; } - default: { std::string error = "internal failure"; return {std::nullopt, txn.gas_limit, {}, error, PreCheckErrorCode::kInternalError};