more refactoring before next team review

eth_tester/backends/mock/factory.py

@@ -91,52 +91,45 @@ def _fill_transaction(transaction, block, transaction_index, is_pending, overrid
     if overrides is None:
         overrides = {}
 
-    is_dynamic_fee_transaction = (
-        'gas_price' not in transaction and any(
-            _ in transaction for _ in ('max_fee_per_gas', 'max_priority_fee_per_gas')
-        )
-    )
-
-    if 'hash' in overrides:
+    if 'hash' in overrides:  # else calculate hash after all fields are filled
         yield 'hash', overrides['hash']
-    else:
-        # calculate hash after all fields are filled
-        pass
 
-    # this pattern isn't the nicest to read but it keeps things clean. Here, we yield the overrides
-    # value if it exists, else either the transaction value if that exists, or a default value
+    # Here, we yield the key with the overrides value if it exists, else either the transaction
+    # value if it exists or a default value
     yield 'nonce', overrides.get('nonce', 0)
     yield 'from', overrides.get('from', transaction.get('from'))
     yield 'to', overrides.get('to', transaction.get('to', b''))
     yield 'data', overrides.get('data', transaction.get('data', b''))
     yield 'value', overrides.get('value', transaction.get('value', 0))
     yield 'gas', overrides.get('gas', transaction.get('gas'))
+    yield 'r', overrides.get('r', transaction.get('r', 12345))
+    yield 's', overrides.get('s', transaction.get('s', 67890))
 
-    if 'gas_price' in transaction or 'gas_price' in overrides:
-        # gas price is 1 gwei in order to be at least the base fee at the (London) genesis block
-        yield 'gas_price', overrides.get('gas_price', transaction.get('gas_price', 1000000000))
-    else:
-        # if no gas_price specified, default to dynamic fee txn parameters
-        is_dynamic_fee_transaction = True
-
-    if is_dynamic_fee_transaction:
+    if 'gas_price' not in transaction:
+        # dynamic fee transaction (type = 2)
         yield 'max_fee_per_gas', overrides.get(
             'max_fee_per_gas', transaction.get('max_fee_per_gas', 1000000000)
         )
         yield 'max_priority_fee_per_gas', overrides.get(
             'max_priority_fee_per_gas', transaction.get('max_priority_fee_per_gas', 1000000000)
         )
+        yield from _yield_typed_transaction_fields(overrides, transaction)
 
-    if is_dynamic_fee_transaction or 'access_list' in transaction:
-        # if is typed transaction (dynamic fee or access list transaction)
-        yield 'chain_id', overrides.get('chain_id', transaction.get('chain_id', 131277322940537))
-        yield 'access_list', overrides.get('access_list', transaction.get('access_list', []))
-        yield 'y_parity', overrides.get('y_parity', transaction.get('y_parity', 0))
     else:
-        yield 'v', overrides.get('v', transaction.get('v', 27))
+        yield 'gas_price', overrides.get('gas_price', transaction.get('gas_price'))
+        if 'access_list' in transaction:
+            # access list transaction (type = 1)
+            yield from _yield_typed_transaction_fields(overrides, transaction)
 
-    yield 'r', overrides.get('r', transaction.get('r', 12345))
-    yield 's', overrides.get('s', transaction.get('s', 67890))
+        else:
+            # legacy transaction
+            yield 'v', overrides.get('v', transaction.get('v', 27))
+
+
+def _yield_typed_transaction_fields(overrides, transaction):
+    yield 'chain_id', overrides.get('chain_id', transaction.get('chain_id', 131277322940537))
+    yield 'access_list', overrides.get('access_list', transaction.get('access_list', []))
+    yield 'y_parity', overrides.get('y_parity', transaction.get('y_parity', 0))
 
 
 @to_dict

eth_tester/backends/pyevm/main.py

@@ -438,7 +438,8 @@ def get_base_fee(self, block_number='latest'):
     #
     @to_dict
     def _normalize_transaction(self, transaction, block_number='latest'):
-        is_dynamic_fee_transaction = False
+        is_dynamic_fee_transaction = 'gas_price' not in transaction  # default to dynamic fee txn
+
         for key in transaction:
             if key == 'from':
                 continue
@@ -447,19 +448,21 @@ def _normalize_transaction(self, transaction, block_number='latest'):
             yield 'nonce', self.get_nonce(transaction['from'], block_number)
         if 'data' not in transaction:
             yield 'data', b''
-        if 'gas_price' not in transaction:
-            is_dynamic_fee_transaction = True
+        if 'value' not in transaction:
+            yield 'value', 0
+        if 'to' not in transaction:
+            yield 'to', b''
+
+        if is_dynamic_fee_transaction:
             if not any(_ in transaction for _ in ('max_fee_per_gas', 'max_priority_fee_per_gas')):
                 yield 'max_fee_per_gas', 1 * 10**9
                 yield 'max_priority_fee_per_gas', 1 * 10**9
             elif 'max_priority_fee_per_gas' in transaction and 'max_fee_per_gas' not in transaction:
                 base_fee = self.get_base_fee(block_number)
                 yield 'max_fee_per_gas', transaction['max_priority_fee_per_gas'] + 2 * base_fee
-        if 'value' not in transaction:
-            yield 'value', 0
-        if 'to' not in transaction:
-            yield 'to', b''
+
         if is_dynamic_fee_transaction or 'access_list' in transaction:
+            # typed transaction
             if 'access_list' not in transaction:
                 yield 'access_list', []
             if 'chain_id' not in transaction:

eth_tester/utils/backend_testing.py

@@ -348,7 +348,7 @@ def test_send_access_list_transaction(self, eth_tester):
         }
         self._send_and_check_transaction(eth_tester, access_list_transaction, accounts[0])
 
-        # with access list
+        # with non-empty access list
         access_list_transaction['access_list'] = (
             {
                 'address': '0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae',
@@ -376,11 +376,10 @@ def test_send_dynamic_fee_transaction(self, eth_tester):
             'gas': 40000,
             'max_fee_per_gas': 2000000000,
             'max_priority_fee_per_gas': 1000000000,
-            'access_list': [],
         }
         self._send_and_check_transaction(eth_tester, dynamic_fee_transaction, accounts[0])
 
-        # with access list
+        # with non-empty access list
         dynamic_fee_transaction['access_list'] = (
             {
                 'address': '0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae',

eth_tester/validation/common.py

@@ -34,7 +34,7 @@ def validate_positive_integer(value):
     error_message = "Value must be positive integers.  Got: {}".format(
         value,
     )
-    if not is_integer(value) or is_boolean(value):
+    if not is_integer(value):
         raise ValidationError(error_message)
     elif value < 0:
         raise ValidationError(error_message)

eth_tester/validation/inbound.py

@@ -147,8 +147,6 @@ def validate_private_key(value):
 
 
 TRANSACTION_KEYS = {
-    # note that the transaction 'type' is not included, though it is a valid param, because it will
-    # never be passed to the backend and thus is not necessary.
     'chain_id',
     'from',
     'to',
@@ -165,8 +163,8 @@ def validate_private_key(value):
 SIGNED_TRANSACTION_KEYS = {
     'r',
     's',
-    'v',
-    'y_parity',
+    'v',  # legacy transactions only
+    'y_parity',  # typed transactions only, instead of 'v'
 }
 
 TRANSACTION_INTERNAL_TYPE_INFO = {
@@ -179,6 +177,12 @@ def validate_private_key(value):
 ALLOWED_TRANSACTION_INTERNAL_TYPES = set(TRANSACTION_INTERNAL_TYPE_INFO.keys())
 
 
+def _is_typed_transaction(txn_dict: dict) -> bool:
+    return any(_ in txn_dict.keys() for _ in (
+        'max_fee_per_gas', 'max_priority_fee_per_gas', 'access_list'
+    ))
+
+
 def validate_transaction(value, txn_internal_type):
     if txn_internal_type not in ALLOWED_TRANSACTION_INTERNAL_TYPES:
         raise TypeError("the `txn_internal_type` parameter must be one of send/call/estimate")
@@ -201,12 +205,11 @@ def validate_transaction(value, txn_internal_type):
 
     elif txn_internal_type == 'send_signed':
         signed_transaction_keys = SIGNED_TRANSACTION_KEYS.copy()
-        if any(_ in value.keys() for _ in (
-            'max_fee_per_gas', 'max_priority_fee_per_gas', 'access_list'
-        )):
-            signed_transaction_keys.remove('v')  # typed txn
+        if _is_typed_transaction(value):
+            signed_transaction_keys.remove('v')
         else:
-            signed_transaction_keys.remove('y_parity')  # legacy txn
+            # legacy transaction
+            signed_transaction_keys.remove('y_parity')
         required_keys = {'from', 'gas'} | signed_transaction_keys
 
     elif txn_internal_type in {'estimate', 'call'}:
@@ -272,10 +275,11 @@ def validate_transaction(value, txn_internal_type):
     if txn_internal_type == 'send_signed':
         validate_uint256(value['r'])
         validate_uint256(value['s'])
-        try:
-            validate_uint8(value['v'])  # legacy transaction
-        except KeyError:
-            validate_uint8(value['y_parity'])  # typed transaction
+        if _is_typed_transaction(value):
+            validate_uint8(value['y_parity'])
+        else:
+            # legacy transaction
+            validate_uint8(value['v'])
 
 
 def _validate_inbound_access_list(access_list):