Skip to content

Commit

Permalink
Update the effective gas price calculation + validation + transaction…
Browse files Browse the repository at this point in the history
… param cleanup
  • Loading branch information
fselmo committed Nov 3, 2021
1 parent d90ef79 commit 5b5003f
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 23 deletions.
8 changes: 0 additions & 8 deletions eth_tester/backends/mock/common.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
def extract_transaction_type(transaction):
return (
'0x2' if 'max_fee_per_gas' in transaction
else '0x1' if 'max_fee_per_gas' not in transaction and 'access_list' in transaction
else '0x0' # legacy transactions being '0x0' taken from current geth version v1.10.10
)


def calculate_effective_gas_price(transaction, block):
return (
min(
Expand Down
2 changes: 2 additions & 0 deletions eth_tester/backends/mock/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

from eth_tester.backends.mock.common import (
calculate_effective_gas_price,
)
from eth_tester.utils.transactions import (
extract_transaction_type,
)
from eth_utils import (
Expand Down
13 changes: 9 additions & 4 deletions eth_tester/backends/mock/serializers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from eth_tester.backends.mock.common import (
calculate_effective_gas_price,
)
from eth_tester.utils.transactions import (
extract_transaction_type,
)
from eth_utils.toolz import (
Expand Down Expand Up @@ -42,11 +44,14 @@ def serialize_full_transaction(transaction, block, transaction_index, is_pending
if 'gas_price' in transaction:
return serialized_transaction
else:
return assoc(
serialized_transaction,
'gas_price',
calculate_effective_gas_price(transaction, block)
# TODO: Sometime in 2022 the inclusion of gas_price may be removed from dynamic fee
# transactions and we can get rid of this behavior.
# https://github.com/ethereum/execution-specs/pull/251
gas_price = (
transaction['max_fee_per_gas'] if is_pending
else calculate_effective_gas_price(transaction, block)
)
return assoc(serialized_transaction, 'gas_price', gas_price)


def serialize_receipt(receipt, transaction, block, transaction_index, is_pending):
Expand Down
5 changes: 3 additions & 2 deletions eth_tester/backends/pyevm/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,9 @@ def get_base_fee(self, block_number='latest'):
@to_dict
def _normalize_transaction(self, transaction, block_number='latest'):
is_dynamic_fee_transaction = (
any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS)
or not any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS + ('gas_price',))
any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS) or
# if no fee params exist, default to dynamic fee transaction:
not any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS + ('gas_price',))
)

for key in transaction:
Expand Down
9 changes: 6 additions & 3 deletions eth_tester/backends/pyevm/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,13 @@ def serialize_transaction(block, transaction, transaction_index, is_pending):
'access_list': transaction.access_list or (),
'y_parity': transaction.y_parity,

# TODO: Sometime in early 2022 (the merge?), the inclusion of gas_price will be removed
# from dynamic fee transactions and we can get rid of this behavior.
# TODO: Sometime in 2022 the inclusion of gas_price may be removed from dynamic fee
# transactions and we can get rid of this behavior.
# https://github.com/ethereum/execution-specs/pull/251
'gas_price': _calculate_effective_gas_price(transaction, block, txn_type),
'gas_price': (
transaction.max_fee_per_gas if is_pending
else _calculate_effective_gas_price(transaction, block, txn_type)
),
}
else:
raise ValidationError('Invariant: code path should be unreachable')
Expand Down
31 changes: 27 additions & 4 deletions eth_tester/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
check_if_log_matches,
)
from eth_tester.utils.transactions import (
extract_transaction_type,
extract_valid_transaction_params,
remove_matching_transaction_from_list,
)
Expand All @@ -71,7 +72,7 @@ def func_wrapper(self, *args, **kwargs):
try:
transaction_hash = func(self, *args, **kwargs)
pending_transaction = self.get_transaction_by_hash(transaction_hash)
pending_transaction = _clean_pending_transaction_for_sending(pending_transaction)
pending_transaction = _clean_pending_transaction(pending_transaction)
# Remove any pending transactions with the same nonce
self._pending_transactions = remove_matching_transaction_from_list(
self._pending_transactions, pending_transaction)
Expand All @@ -80,17 +81,17 @@ def func_wrapper(self, *args, **kwargs):
self.revert_to_snapshot(snapshot)
return transaction_hash

def _clean_pending_transaction_for_sending(pending_transaction):
def _clean_pending_transaction(pending_transaction):
cleaned_transaction = dissoc(pending_transaction, 'type')

# TODO: Sometime in early 2022 (the merge?), the inclusion of gas_price will be removed
# from dynamic fee transactions and we can get rid of this behavior.
# https://github.com/ethereum/execution-specs/pull/251
# remove gas_price for dynamic fee transactions
if 'gas_price' and 'max_fee_per_gas' in pending_transaction:
cleaned_transaction = dissoc(cleaned_transaction, 'gas_price')

return cleaned_transaction

return func_wrapper


Expand Down Expand Up @@ -264,6 +265,27 @@ def get_nonce(self, account, block_number="latest"):
# Blocks, Transactions, Receipts
#

@staticmethod
def _normalize_pending_transaction(pending_transaction):
"""
Add the transaction type and, if a dynamic fee transaction, add gas_price =
max_fee_per_gas as highlighted in the execution-specs link below.
"""
_type = extract_transaction_type(pending_transaction)
pending_transaction = assoc(pending_transaction, 'type', _type)

# TODO: Sometime in 2022 the inclusion of gas_price may be removed from dynamic fee
# transactions and we can get rid of this behavior.
# https://github.com/ethereum/execution-specs/pull/251
# add gas_price = max_fee_per_gas to pending dynamic fee transactions
if _type == '0x2':
pending_transaction = assoc(
pending_transaction,
'gas_price',
pending_transaction['max_fee_per_gas']
)
return pending_transaction

def _get_pending_transaction_by_hash(self, transaction_hash):
for transaction in self._pending_transactions:
if transaction['hash'] == transaction_hash:
Expand All @@ -275,7 +297,8 @@ def _get_pending_transaction_by_hash(self, transaction_hash):
def get_transaction_by_hash(self, transaction_hash):
self.validator.validate_inbound_transaction_hash(transaction_hash)
try:
return self._get_pending_transaction_by_hash(transaction_hash)
pending_transaction = self._get_pending_transaction_by_hash(transaction_hash)
return self._normalize_pending_transaction(pending_transaction)
except TransactionNotFound:
raw_transaction_hash = self.normalizer.normalize_inbound_transaction_hash(
transaction_hash,
Expand Down
8 changes: 8 additions & 0 deletions eth_tester/utils/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ def extract_valid_transaction_params(transaction_params):
for key in VALID_TRANSACTION_PARAMS if key in transaction_params}


def extract_transaction_type(transaction):
return (
'0x2' if 'max_fee_per_gas' in transaction
else '0x1' if 'max_fee_per_gas' not in transaction and 'access_list' in transaction
else '0x0' # legacy transactions being '0x0' taken from current geth version v1.10.10
)


@to_list
def remove_matching_transaction_from_list(transaction_list, transaction):
for tx in transaction_list:
Expand Down
4 changes: 2 additions & 2 deletions tests/core/validation/test_outbound_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def _make_access_list_txn(chain_id=131277322940537, access_list=[], y_parity=0,

# This is an outbound transaction so we still keep the gas_price for now since the gas_price is
# the min(max_fee_per_gas, base_fee_per_gas + max_priority_fee_per_gas).
# TODO: Sometime in early 2022 (the merge?), the inclusion of gas_price will be removed
# from dynamic fee transactions and we can get rid of this behavior.
# TODO: Sometime in 2022 the inclusion of gas_price may be removed from dynamic fee
# transactions and we can get rid of this behavior.
# https://github.com/ethereum/execution-specs/pull/251
def _make_dynamic_fee_txn(
chain_id=131277322940537,
Expand Down

0 comments on commit 5b5003f

Please sign in to comment.