Skip to content

Commit

Permalink
rpcdaemon: remove double validation from processor and executor (#2696)
Browse files Browse the repository at this point in the history
  • Loading branch information
lupin012 authored Feb 6, 2025
1 parent c9e32d1 commit b3a6357
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 35 deletions.
2 changes: 1 addition & 1 deletion silkworm/core/execution/evm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ CallResult EVM::execute(const Transaction& txn, uint64_t gas) noexcept {

const auto gas_left = static_cast<uint64_t>(res.gas_left);
const auto gas_refund = static_cast<uint64_t>(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 {
Expand Down
2 changes: 2 additions & 0 deletions silkworm/core/execution/evm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <silkworm/core/common/lru_cache.hpp>
#include <silkworm/core/common/object_pool.hpp>
#include <silkworm/core/common/util.hpp>
#include <silkworm/core/protocol/validation.hpp>
#include <silkworm/core/state/intra_block_state.hpp>
#include <silkworm/core/types/block.hpp>

Expand All @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion silkworm/core/execution/evm_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ class TestTracer : public EvmTracer {
execution_end_called_ = true;
const auto gas_left = static_cast<uint64_t>(res.gas_left);
const auto gas_refund = static_cast<uint64_t>(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] =
Expand Down
12 changes: 9 additions & 3 deletions silkworm/core/execution/processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,10 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector<st
const std::optional<evmc::address> 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)};
Expand All @@ -289,7 +292,10 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector<st
state_.add_to_balance(*txn.sender(), required_funds);
}

SILKWORM_ASSERT(protocol::validate_call_funds(txn, evm_, state_.get_balance(*txn.sender()), evm().bailout) == ValidationResult::kOk);
validation_result = protocol::validate_call_funds(txn, evm_, state_.get_balance(*txn.sender()), evm().bailout);
if (validation_result != ValidationResult::kOk) {
return {validation_result, EVMC_SUCCESS, 0, {}, {}};
}
state_.subtract_from_balance(*txn.sender(), required_funds);
const intx::uint128 g0{protocol::intrinsic_gas(txn, evm_.revision())};
const auto result = evm_.execute(txn, txn.gas_limit - static_cast<uint64_t>(g0));
Expand All @@ -315,7 +321,7 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector<st

evm_.remove_tracers();

return {result.status, gas_left, gas_used, result.data, result.error_message};
return {ValidationResult::kOk, result.status, gas_left, gas_used, result.data, result.error_message};
}

void ExecutionProcessor::reset() {
Expand Down
1 change: 1 addition & 0 deletions silkworm/core/protocol/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <silkworm/core/common/empty_hashes.hpp>
#include <silkworm/core/crypto/secp256k1n.hpp>
#include <silkworm/core/execution/evm.hpp>
#include <silkworm/core/rlp/encode_vector.hpp>
#include <silkworm/core/trie/vector_root.hpp>

Expand Down
3 changes: 2 additions & 1 deletion silkworm/core/protocol/validation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@

#include <evmc/evmc.h>

#include <silkworm/core/execution/evm.hpp>
#include <silkworm/core/state/intra_block_state.hpp>
#include <silkworm/core/types/block.hpp>
#include <silkworm/core/types/transaction.hpp>

namespace silkworm {

class EVM;

// Classification of invalid transactions and blocks.
enum class [[nodiscard]] ValidationResult {
kOk, // All checks passed
Expand Down
46 changes: 18 additions & 28 deletions silkworm/rpc/core/evm_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 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: {
Expand All @@ -192,28 +192,27 @@ 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: {
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)
: 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};
}
}
}

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,
Expand All @@ -233,19 +232,10 @@ ExecutionResult EVMExecutor::call(
return {std::nullopt, txn.gas_limit, Bytes{}, "malformed transaction: cannot recover sender"};
}

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);
}

const auto result = execution_processor_.call(txn, tracers, refund);
if (result.validation_result != ValidationResult::kOk) {
return convert_validation_result(result.validation_result, block, txn, evm);
}

ExecutionResult exec_result{result.status, result.gas_left, result.data};

Expand Down
2 changes: 2 additions & 0 deletions silkworm/rpc/core/evm_executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> state_;
Expand Down
2 changes: 1 addition & 1 deletion silkworm/rpc/core/evm_executor_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ TEST_CASE_METHOD(EVMExecutorTest, "EVMExecutor") {

SECTION("failed if transaction cost greater user amount") {
auto cursor = std::make_shared<silkworm::db::test_util::MockCursor>();
EXPECT_CALL(transaction, get_as_of(_)).WillOnce(Invoke([=](Unused) -> Task<db::kv::api::GetAsOfResult> {
EXPECT_CALL(transaction, get_as_of(_)).WillRepeatedly(Invoke([=](Unused) -> Task<db::kv::api::GetAsOfResult> {
db::kv::api::GetAsOfResult response{
.success = true,
.value = Bytes{}};
Expand Down

0 comments on commit b3a6357

Please sign in to comment.