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

evm: drop bailout option from validate_call_funds() #2652

Merged
merged 1 commit into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion silkworm/core/execution/processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,10 @@ CallResult ExecutionProcessor::call(const Transaction& txn, const std::vector<st
SILKWORM_ASSERT(sender);

SILKWORM_ASSERT(protocol::validate_call_precheck(txn, evm_) == ValidationResult::kOk);
SILKWORM_ASSERT(protocol::validate_call_funds(txn, evm_, state_.get_balance(*txn.sender()), bailout) == ValidationResult::kOk);

if (!bailout) {
SILKWORM_ASSERT(protocol::validate_call_funds(txn, evm_, state_.get_balance(*txn.sender())) == ValidationResult::kOk);
}

const BlockHeader& header{evm_.block().header};
const intx::uint256 base_fee_per_gas{header.base_fee_per_gas.value_or(0)};
Expand Down
26 changes: 12 additions & 14 deletions silkworm/core/protocol/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,20 +222,18 @@
return ValidationResult::kOk;
}

ValidationResult validate_call_funds(const Transaction& txn, const EVM& evm, const intx::uint256& owned_funds, bool bailout) noexcept {
if (!bailout) {
const intx::uint256 base_fee{evm.block().header.base_fee_per_gas.value_or(0)};
const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= evm.block().header.base_fee_per_gas ? txn.effective_gas_price(base_fee)
: txn.max_priority_fee_per_gas};

const auto required_funds = 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();
}
if (owned_funds < maximum_cost + txn.value) {
return ValidationResult::kInsufficientFunds;
}
ValidationResult validate_call_funds(const Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) noexcept {
const intx::uint256 base_fee{evm.block().header.base_fee_per_gas.value_or(0)};
const intx::uint256 effective_gas_price{txn.max_fee_per_gas >= evm.block().header.base_fee_per_gas ? txn.effective_gas_price(base_fee)
: txn.max_priority_fee_per_gas};

const auto required_funds = 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();
}

Check warning on line 234 in silkworm/core/protocol/validation.cpp

View check run for this annotation

Codecov / codecov/patch

silkworm/core/protocol/validation.cpp#L233-L234

Added lines #L233 - L234 were not covered by tests
if (owned_funds < maximum_cost + txn.value) {
return ValidationResult::kInsufficientFunds;
}
return ValidationResult::kOk;
}
Expand Down
2 changes: 1 addition & 1 deletion silkworm/core/protocol/validation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ namespace protocol {

ValidationResult pre_validate_common_forks(const Transaction& txn, evmc_revision rev, const std::optional<intx::uint256>& blob_gas_price) noexcept;

ValidationResult validate_call_funds(const Transaction& txn, const EVM& evm, const intx::uint256& owned_funds, bool bailout) noexcept;
ValidationResult validate_call_funds(const Transaction& txn, const EVM& evm, const intx::uint256& owned_funds) noexcept;

intx::uint256 compute_call_cost(const Transaction& txn, const intx::uint256& effective_gas_price, const EVM& evm);

Expand Down
4 changes: 2 additions & 2 deletions silkworm/rpc/core/evm_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ ExecutionResult EVMExecutor::call(

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);
result != ValidationResult::kOk) {
if (const auto result = protocol::validate_call_funds(txn, evm, owned_funds);
!bailout && result != ValidationResult::kOk) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The application is a big awkward because we call the function even if we don't need the result. But considering the bailout is not enabled often this somehow guarantees the validate_call_funds still work correctly without crashes or assert failures.

return convert_validated_funds(block, txn, evm, owned_funds);
}

Expand Down
Loading