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

Add missing functionality for the gas-fee algorithm #131

Merged
merged 14 commits into from
Mar 20, 2024
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
2 changes: 1 addition & 1 deletion cmd/state-transition/state_transition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ void StateTransition::run() {

auto pre_block_validation = ruleSet->pre_validate_block_body(block, *state);
auto block_validation = ruleSet->validate_block_header(block.header, *state, true);
auto pre_txn_validation = protocol::pre_validate_transaction(txn, rev, config.chain_id, block.header.base_fee_per_gas, block.header.data_gas_price());
auto pre_txn_validation = protocol::pre_validate_transaction(txn, rev, config.chain_id, block.header.base_fee_per_gas, block.header.data_gas_price(), 0, {});
auto txn_validation = protocol::validate_transaction(txn, processor.evm().state(), processor.available_gas());

// std::cout << "pre: " << std::endl;
Expand Down
4 changes: 2 additions & 2 deletions cmd/test/ethereum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ RunResults transaction_test(const nlohmann::json& j) {

if (ValidationResult err{
pre_validate_transaction(txn, rev, config.chain_id, /*base_fee_per_gas=*/std::nullopt,
/*data_gas_price=*/std::nullopt)};
/*data_gas_price=*/std::nullopt, 0, {})};
err != ValidationResult::kOk) {
if (should_be_valid) {
std::cout << "Validation error " << magic_enum::enum_name<ValidationResult>(err) << std::endl;
Expand Down Expand Up @@ -405,7 +405,7 @@ RunResults transaction_test(const nlohmann::json& j) {
}

const auto expected_intrinsic_gas{intx::from_string<intx::uint256>(test["intrinsicGas"].get<std::string>())};
const auto calculated_intrinsic_gas{intrinsic_gas(txn, rev)};
const auto calculated_intrinsic_gas{intrinsic_gas(txn, rev, 0, {})};
if (calculated_intrinsic_gas != expected_intrinsic_gas) {
std::cout << "Intrinsic gas mismatch for " << entry.key() << ":\n"
<< intx::to_string(calculated_intrinsic_gas, /*base=*/16)
Expand Down
18 changes: 10 additions & 8 deletions silkworm/core/execution/evm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,9 +217,7 @@ evmc::Result EVM::call(const evmc_message& message) noexcept {
const auto snapshot{state_.take_snapshot()};

const evmc_revision rev{revision()};
bool recipient_is_dead = !is_reserved_address(message.recipient) &&
!precompile::is_precompile(message.recipient, rev) &&
state_.is_dead(message.recipient);
bool recipient_exists = account_exists(message.recipient);

if (message.kind == EVMC_CALL) {
if (message.flags & EVMC_STATIC) {
Expand Down Expand Up @@ -259,7 +257,7 @@ evmc::Result EVM::call(const evmc_message& message) noexcept {
} else {

evmc_message revisted_message{message};
if(eos_evm_version_ > 0 && revisted_message.depth == 0 && recipient_is_dead) {
if(eos_evm_version_ > 0 && revisted_message.depth == 0 && !is_zero(evmc::bytes32{revisted_message.value}) && !recipient_exists) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems will cause a functionality change if recipient is precompiled or reserved address?
What's the reason behind this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We wanted to be consistent with what the CALL instruction is doing when trying to check if the account exists.
The instruction implementation its just calling the host function account_exists on the destination address, so we are doing the same when processing the call message coming from a transaction.

if ((res.gas_left -= static_cast<int64_t>(gas_params_.G_txnewaccount)) < 0) {
// If we run out of gas lets do everything here
state_.revert_to_snapshot(snapshot);
Expand Down Expand Up @@ -366,16 +364,20 @@ void EVM::add_tracer(EvmTracer& tracer) noexcept {
tracers_.push_back(std::ref(tracer));
}

bool EvmHost::account_exists(const evmc::address& address) const noexcept {
const evmc_revision rev{evm_.revision()};
bool EVM::account_exists(const evmc::address& address) const noexcept {
const evmc_revision rev{revision()};

if (rev >= EVMC_SPURIOUS_DRAGON) {
return !evm_.state().is_dead(address);
return !state_.is_dead(address);
} else {
return evm_.state().exists(address);
return state_.exists(address);
}
}

bool EvmHost::account_exists(const evmc::address& address) const noexcept {
return evm_.account_exists(address);
}

evmc_access_status EvmHost::access_account(const evmc::address& address) noexcept {
const evmc_revision rev{evm_.revision()};

Expand Down
10 changes: 10 additions & 0 deletions silkworm/core/execution/evm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ class EVM {
void add_tracer(EvmTracer& tracer) noexcept;
[[nodiscard]] const EvmTracers& tracers() const noexcept { return tracers_; };

bool account_exists(const evmc::address& address) const noexcept;

AnalysisCache* analysis_cache{nullptr}; // provide one for better performance
ObjectPool<evmone::ExecutionState>* state_pool{nullptr}; // ditto

Expand All @@ -107,6 +109,14 @@ class EVM {
gas_params_ = gas_params;
}

const evmone::gas_parameters& get_gas_params() {
return gas_params_;
}

uint64_t get_eos_evm_version() {
return eos_evm_version_;
}

private:
friend class EvmHost;

Expand Down
10 changes: 5 additions & 5 deletions silkworm/core/execution/evm_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,8 @@ TEST_CASE("EOS EVM G_txnewaccount") {
block.header.nonce = eosevm::version_to_nonce(1);

evmc::address sender{0x0a6bb546b9208cfab9e8fa2b9b2c042b18df7030_address};
evmc::address receiver{0x1a6bb546b9208cfab9e8fa2b9b2c042b18df7030_address};
evmc::address receiver1{0x1a6bb546b9208cfab9e8fa2b9b2c042b18df7030_address};
evmc::address receiver2{0x1000000000000000000000000000000000000001_address};

evmone::gas_parameters gas_params;
gas_params.G_txnewaccount = 0;
Expand All @@ -769,14 +770,15 @@ TEST_CASE("EOS EVM G_txnewaccount") {

Transaction txn{};
txn.from = sender;
txn.to = receiver;
txn.value = intx::uint256{0};
txn.to = receiver1;
txn.value = intx::uint256{1};

CallResult res = evm.execute(txn, 0);
CHECK(res.status == EVMC_SUCCESS);
CHECK(res.gas_left == 0);
CHECK(res.gas_refund == 0);

txn.to = receiver2;
gas_params.G_txnewaccount = 1;
evm.update_gas_params(gas_params);

Expand All @@ -787,6 +789,4 @@ TEST_CASE("EOS EVM G_txnewaccount") {

}



} // namespace silkworm
2 changes: 1 addition & 1 deletion silkworm/core/execution/processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void ExecutionProcessor::execute_transaction(const Transaction& txn, Receipt& re
const intx::uint256 data_gas_price{evm_.block().header.data_gas_price().value_or(0)};
state_.subtract_from_balance(*txn.from, txn.total_data_gas() * data_gas_price);

const intx::uint128 g0{protocol::intrinsic_gas(txn, rev)};
const intx::uint128 g0{protocol::intrinsic_gas(txn, rev, evm_.get_eos_evm_version(), evm_.get_gas_params())};
assert(g0 <= UINT64_MAX); // true due to the precondition (transaction must be valid)

const CallResult vm_res{evm_.execute(txn, txn.gas_limit - static_cast<uint64_t>(g0))};
Expand Down
2 changes: 0 additions & 2 deletions silkworm/core/execution/processor_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,4 @@ TEST_CASE("EVM message filter revert") {
CHECK(filtered_messages2[0].data == *from_hex("0xf781185b0000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000003796573000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010100000000000000000000000000000000000000000000000000000000000000"));
}



} // namespace silkworm
4 changes: 3 additions & 1 deletion silkworm/core/protocol/base_rule_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ ValidationResult BaseRuleSet::pre_validate_block_body(const Block& block, const
return ValidationResult::kWrongTransactionsRoot;
}

if (ValidationResult err{pre_validate_transactions(block, chain_config_)}; err != ValidationResult::kOk) {
// NOTE: TrustRuleSet inherits directly from IRuleSet and it has `pre_validate_block_body` overrided that always return kOk
yarkinwho marked this conversation as resolved.
Show resolved Hide resolved
// So here we are sure that another RuleSet (other than trust) is being used, so we can call `pre_validate_transactions` with version=0 and gas_parameters={}
if (ValidationResult err{pre_validate_transactions(block, chain_config_, 0, {})}; err != ValidationResult::kOk) {
return err;
}

Expand Down
4 changes: 2 additions & 2 deletions silkworm/core/protocol/intrinsic_gas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@

namespace silkworm::protocol {

intx::uint128 intrinsic_gas(const UnsignedTransaction& txn, const evmc_revision rev) noexcept {
intx::uint128 intrinsic_gas(const UnsignedTransaction& txn, const evmc_revision rev, uint64_t eos_evm_version, const evmone::gas_parameters& gas_params) noexcept {
intx::uint128 gas{fee::kGTransaction};

const bool contract_creation{!txn.to};
if (contract_creation && rev >= EVMC_HOMESTEAD) {
gas += fee::kGTxCreate;
gas += eos_evm_version > 0 ? gas_params.G_txcreate : fee::kGTxCreate;
}

// EIP-2930: Optional access lists
Expand Down
5 changes: 2 additions & 3 deletions silkworm/core/protocol/intrinsic_gas.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@
#include <intx/intx.hpp>

#include <silkworm/core/types/transaction.hpp>

#include <evmone/execution_state.hpp>
namespace silkworm::protocol {

// Returns the intrinsic gas of a transaction.
// Refer to g0 in Section 6.2 "Execution" of the Yellow Paper
// and EIP-3860 "Limit and meter initcode".
intx::uint128 intrinsic_gas(const UnsignedTransaction& txn, evmc_revision rev) noexcept;

intx::uint128 intrinsic_gas(const UnsignedTransaction& txn, const evmc_revision rev, uint64_t eos_evm_version, const evmone::gas_parameters& gas_params ) noexcept;
} // namespace silkworm::protocol
20 changes: 18 additions & 2 deletions silkworm/core/protocol/intrinsic_gas_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <catch2/catch.hpp>

#include "param.hpp"

#include <eosevm/version.hpp>
namespace silkworm::protocol {

TEST_CASE("EIP-2930 intrinsic gas") {
Expand All @@ -43,8 +43,24 @@ TEST_CASE("EIP-2930 intrinsic gas") {
.value = 2 * kEther,
.access_list = access_list};

intx::uint128 g0{intrinsic_gas(txn, EVMC_ISTANBUL)};
intx::uint128 g0{intrinsic_gas(txn, EVMC_ISTANBUL, 0, {})};
CHECK(g0 == fee::kGTransaction + 2 * fee::kAccessListAddressCost + 2 * fee::kAccessListStorageKeyCost);
}

TEST_CASE("G_txcreate intrinsic gas") {
uint64_t eos_evm_version = 0;
evmone::gas_parameters gas_params;

UnsignedTransaction txn{};

intx::uint128 g0{intrinsic_gas(txn, eosevm::version_to_evmc_revision(eos_evm_version), eos_evm_version, gas_params)};
CHECK(g0 == fee::kGTransaction + fee::kGTxCreate);

eos_evm_version = 1;
gas_params.G_txcreate = 33333;
g0 = intrinsic_gas(txn, eosevm::version_to_evmc_revision(eos_evm_version), eos_evm_version, gas_params);
CHECK(g0 == fee::kGTransaction + 33333);
}


} // namespace silkworm::protocol
9 changes: 5 additions & 4 deletions silkworm/core/protocol/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ bool transaction_type_is_supported(TransactionType type, evmc_revision rev) {

ValidationResult pre_validate_transaction(const Transaction& txn, const evmc_revision rev, const uint64_t chain_id,
const std::optional<intx::uint256>& base_fee_per_gas,
const std::optional<intx::uint256>& data_gas_price) {
const std::optional<intx::uint256>& data_gas_price,
uint64_t eos_evm_version, const evmone::gas_parameters& gas_params) {
if (txn.chain_id.has_value()) {
if (rev < EVMC_SPURIOUS_DRAGON) {
// EIP-155 transaction before EIP-155 was activated
Expand Down Expand Up @@ -70,7 +71,7 @@ ValidationResult pre_validate_transaction(const Transaction& txn, const evmc_rev
}
}

const intx::uint128 g0{intrinsic_gas(txn, rev)};
const intx::uint128 g0{intrinsic_gas(txn, rev, eos_evm_version, gas_params)};
if (txn.gas_limit < g0) {
return ValidationResult::kIntrinsicGas;
}
Expand Down Expand Up @@ -143,14 +144,14 @@ ValidationResult validate_transaction(const Transaction& txn, const IntraBlockSt
return ValidationResult::kOk;
}

ValidationResult pre_validate_transactions(const Block& block, const ChainConfig& config) {
ValidationResult pre_validate_transactions(const Block& block, const ChainConfig& config, uint64_t eos_evm_version, const evmone::gas_parameters& gas_params) {
const BlockHeader& header{block.header};
const evmc_revision rev{config.revision(header)};
const std::optional<intx::uint256> data_gas_price{header.data_gas_price()};

for (const Transaction& txn : block.transactions) {
ValidationResult err{pre_validate_transaction(txn, rev, config.chain_id,
header.base_fee_per_gas, data_gas_price)};
header.base_fee_per_gas, data_gas_price, eos_evm_version, gas_params)};
if (err != ValidationResult::kOk) {
return err;
}
Expand Down
6 changes: 4 additions & 2 deletions silkworm/core/protocol/validation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <silkworm/core/chain/config.hpp>
#include <silkworm/core/state/intra_block_state.hpp>
#include <silkworm/core/types/transaction.hpp>
#include <evmone/execution_state.hpp>

namespace silkworm {

Expand Down Expand Up @@ -109,9 +110,10 @@ namespace protocol {
//! \remarks These function is agnostic to whole block validity
ValidationResult pre_validate_transaction(const Transaction& txn, evmc_revision revision, uint64_t chain_id,
const std::optional<intx::uint256>& base_fee_per_gas,
const std::optional<intx::uint256>& data_gas_price);
const std::optional<intx::uint256>& data_gas_price,
uint64_t eos_evm_version, const evmone::gas_parameters& gas_params);

ValidationResult pre_validate_transactions(const Block& block, const ChainConfig& config);
ValidationResult pre_validate_transactions(const Block& block, const ChainConfig& config, uint64_t eos_evm_version, const evmone::gas_parameters& gas_params);

//! \brief Final part of transaction validation that requires access to the state.
//!
Expand Down
32 changes: 16 additions & 16 deletions silkworm/core/protocol/validation_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,35 @@ TEST_CASE("Validate transaction types") {

Transaction txn;
txn.type = TransactionType::kLegacy;
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kUnsupportedTransactionType);

txn.type = static_cast<TransactionType>(0x03); // unsupported transaction type
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kUnsupportedTransactionType);

txn.type = TransactionType::kAccessList;
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kUnsupportedTransactionType);

txn.type = TransactionType::kDynamicFee;
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_ISTANBUL, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_BERLIN, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kUnsupportedTransactionType);
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kUnsupportedTransactionType);
}

Expand All @@ -70,22 +70,22 @@ TEST_CASE("Validate max_fee_per_gas") {

txn.max_priority_fee_per_gas = 500'000'000;
txn.max_fee_per_gas = 700'000'000;
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kMaxFeeLessThanBase);

txn.max_priority_fee_per_gas = 3'000'000'000;
txn.max_fee_per_gas = 2'000'000'000;
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) ==
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) ==
ValidationResult::kMaxPriorityFeeGreaterThanMax);

txn.max_priority_fee_per_gas = 2'000'000'000;
txn.max_fee_per_gas = 2'000'000'000;
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kMaxPriorityFeeGreaterThanMax);

txn.max_priority_fee_per_gas = 1'000'000'000;
txn.max_fee_per_gas = 2'000'000'000;
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price) !=
CHECK(pre_validate_transaction(txn, EVMC_LONDON, 1, base_fee_per_gas, data_gas_price, 0, {}) !=
ValidationResult::kMaxPriorityFeeGreaterThanMax);
}

Expand Down
1 change: 0 additions & 1 deletion silkworm/core/types/block.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ struct Block : public BlockBody {
BlockHeader header;

bool irreversible{false};
std::optional<eosevm::ConsensusParameters> consensus_parameters_cache;
void recover_senders();
};

Expand Down
1 change: 0 additions & 1 deletion silkworm/node/db/access_layer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -963,5 +963,4 @@ TEST_CASE("ConsensusParameters") {
CHECK(read_consensus_parameters(txn, value2.hash()) == value2);
}


} // namespace silkworm::db
Loading
Loading