Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rpcdaemon: remove double validation from processor and executor #2696

Merged
merged 9 commits into from
Feb 6, 2025
Merged
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
Loading