changes from review comments

Author: fselmo

README.md

@@ -399,25 +399,26 @@ Returns the transaction for the given hash, raising a
 transaction cannot be found.
 
 ```python
->>> t.get_transaction_by_hash('0x140c1da1370a908e4c0f7c6e33bb97182011707c6a9aff954bef1084c8a48b25')
-{'hash': '0x256baca0284f7b76238e56964dec01dce423cb11415e5f47b341a0af088bb85d',
- 'nonce': 1,
- 'block_hash': '0xc25d844e866b4f3f3126e5d5ceba949a7ee12b597a3d54115da63e8596531f58',
- 'block_number': 2,
+>>> t.get_transaction_by_hash('0x21ae665f707e12a5f1bb13ef8c706b65cc5accfd03e7067ce683d831f51122e6')
+{'type': '0x2',
+ 'hash': '0x21ae665f707e12a5f1bb13ef8c706b65cc5accfd03e7067ce683d831f51122e6',
+ 'nonce': 0,
+ 'block_hash': '0x810731efeb7498fc0ac3bc7c72a71571b672c9fdbfbfd8b435f483e368e8ef7e',
+ 'block_number': 1,
  'transaction_index': 0,
- 'from': '0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf',
- 'to': '0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF',
- 'value': 1,
- 'gas': 30000,
+ 'from': '0x2B5AD5c4795c026514f8317c7a215E218DcCD6cF',
+ 'to': '0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf',
+ 'value': 1337,
+ 'gas': 21000,
  'data': '0x',
- 'r': 113110058401990022576610285176909677614233866647576678068104192469544316041227,
- 's': 56040876284016779278390826082806410689817968450346221730224961588578254081442,
+ 'r': 1713666669454033023988006960017431058214051587080823768269189498559514600280,
+ 's': 32003859822305799628524852194521134173285969678963273753063458725692016415033,
  'chain_id': 131277322940537,
- 'max_fee_per_gas': 1000000000,
- 'max_priority_fee_per_gas': 1000000000,
+ 'max_fee_per_gas': 2000000000,
+ 'max_priority_fee_per_gas': 500000000,
  'access_list': (),
- 'y_parity': 1}
-
+ 'y_parity': 0,
+ 'gas_price': 1375000000}
 ```
 
 > Note: For unmined transaction, `transaction_index`, `block_number` and `block_hash` will all be `None`.
@@ -540,20 +541,20 @@ values.
 * `gas`: Sets the gas limit for transaction execution (integer).
 * `value`: The amount of ether in wei that should be sent with the transaction (integer).
 * `data`: The data for the transaction (hexadecimal string).
+* `chain_id`: The integer id for the chain the transaction is meant to interact with.
+
 
 In addition to the above, the following parameters are added based on the type of transaction being sent:
 
 #### Legacy transactions
 * `gas_price`: Sets the price per unit of gas in wei that will be paid for transaction execution (integer).
 
 #### Access list transactions (EIP-2930)
-* `chain_id`: The integer id for the chain the transaction is meant to interact with
 * `gas_price`: Sets the price per unit of gas in wei that will be paid for transaction execution (integer).
 * `access_list` (optional): Specifies accounts and storage slots expected to be accessed, based on the transaction, in order to 
 gain a discount on the gas for those executions (see quickstart example for usage).
 
 #### Dynamic fee transactions (EIP-1559)
-* `chain_id`: The integer id for the chain the transaction is meant to interact with
 * `max_fee_per_gas`: Sets the maximum fee per unit of gas in wei that will be paid for transaction execution (integer).
 * `max_priority_fee_per_gas`: Sets the fee per unit of gas in wei that is sent to the miner as an incentive for mining the transaction (integer).
 * `access_list` (optional): Specifies accounts and storage slots expected to be accessed, based on the transaction, in order to 

eth_tester/backends/mock/factory.py

@@ -24,7 +24,7 @@
 )
 
 from eth_tester.backends.common import merge_genesis_overrides
-from eth_tester.constants import EIP_1559_TRANSACTION_PARAMS
+from eth_tester.constants import DYNAMIC_FEE_TRANSACTION_PARAMS
 from eth_tester.utils.address import (
     generate_contract_address,
 )
@@ -94,8 +94,8 @@ def create_transaction(transaction, block, transaction_index, is_pending, overri
 @to_dict
 def _fill_transaction(transaction, block, transaction_index, is_pending, overrides=None):
     is_dynamic_fee_transaction = (
-        any(_ in transaction for _ in EIP_1559_TRANSACTION_PARAMS)
-        or not any(_ in transaction for _ in EIP_1559_TRANSACTION_PARAMS + ('gas_price',))
+        any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS)
+        or not any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS + ('gas_price',))
     )
 
     if overrides is None:

eth_tester/backends/pyevm/main.py

@@ -26,7 +26,7 @@
 
 from eth_keys import KeyAPI
 
-from eth_tester.constants import EIP_1559_TRANSACTION_PARAMS
+from eth_tester.constants import DYNAMIC_FEE_TRANSACTION_PARAMS
 from eth_tester.exceptions import (
     BackendDistributionNotFound,
     BlockNotFound,
@@ -165,6 +165,7 @@ def setup_tester_chain(
         no_proof_vms = consensus_applier.amend_vm_configuration(vm_configuration)
 
     class MainnetTesterNoProofChain(MiningChain):
+        chain_id = 131277322940537
         vm_configuration = no_proof_vms
 
         def create_header_from_parent(self, parent_header, **header_params):
@@ -191,7 +192,6 @@ def get_transaction_builder(self):
     base_db = get_db_backend()
 
     chain = MainnetTesterNoProofChain.from_genesis(base_db, genesis_params, genesis_state)
-    chain.chain_id = 131277322940537  # typed transactions need a chain_id
     return account_keys, chain
 
 
@@ -436,11 +436,9 @@ def get_base_fee(self, block_number='latest'):
     #
     @to_dict
     def _normalize_transaction(self, transaction, block_number='latest'):
-        base_fee = self.get_base_fee(block_number)
-
         is_dynamic_fee_transaction = (
-            any(_ in transaction for _ in EIP_1559_TRANSACTION_PARAMS)
-            or not any(_ in transaction for _ in EIP_1559_TRANSACTION_PARAMS + ('gas_price',))
+            any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS)
+            or not any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS + ('gas_price',))
         )
 
         for key in transaction:
@@ -458,16 +456,19 @@ def _normalize_transaction(self, transaction, block_number='latest'):
             yield 'to', b''
 
         if is_dynamic_fee_transaction:
-            if not any(_ in transaction for _ in EIP_1559_TRANSACTION_PARAMS):
+            if not any(_ in transaction for _ in DYNAMIC_FEE_TRANSACTION_PARAMS):
                 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:
-                yield 'max_fee_per_gas', transaction['max_priority_fee_per_gas'] + 2 * base_fee
+                yield (
+                    'max_fee_per_gas',
+                    transaction['max_priority_fee_per_gas'] + 2 * self.get_base_fee(block_number)
+                )
 
         if is_dynamic_fee_transaction or 'access_list' in transaction:
             # typed transaction
             if 'access_list' not in transaction:
-                yield 'access_list', []
+                yield 'access_list', ()
             if 'chain_id' not in transaction:
                 yield 'chain_id', self.chain.chain_id
 
@@ -492,7 +493,7 @@ def _create_type_aware_unsigned_transaction(self, normalized_txn):
             return self.chain.get_transaction_builder().new_unsigned_access_list_transaction(
                 **normalized_txn
             )
-        elif all(_ in normalized_txn for _ in ("max_fee_per_gas", "max_priority_fee_per_gas")):
+        elif all(_ in normalized_txn for _ in DYNAMIC_FEE_TRANSACTION_PARAMS):
             return self.chain.get_transaction_builder().new_unsigned_dynamic_fee_transaction(
                 **normalized_txn
             )
@@ -515,7 +516,7 @@ def _create_type_aware_signed_transaction(self, normalized_txn):
             return self.chain.get_transaction_builder().new_access_list_transaction(
                 **normalized_txn
             )
-        elif all(_ in normalized_txn for _ in ("max_fee_per_gas", "max_priority_fee_per_gas")):
+        elif all(_ in normalized_txn for _ in DYNAMIC_FEE_TRANSACTION_PARAMS):
             return self.chain.get_transaction_builder().new_dynamic_fee_transaction(
                 **normalized_txn
             )

eth_tester/backends/pyevm/serializers.py

@@ -120,7 +120,7 @@ def serialize_transaction(block, transaction, transaction_index, is_pending):
             'gas_price': _calculate_effective_gas_price(transaction, block, txn_type),
         }
     else:
-        raise ValidationError('Transaction serialization error')
+        raise ValidationError('Invariant: code path should be unreachable')
 
     return merge(common_transaction_params, type_specific_params)
 

eth_tester/constants.py

@@ -58,4 +58,4 @@
 #
 # EIP CONSTANTS
 #
-EIP_1559_TRANSACTION_PARAMS = ('max_fee_per_gas', 'max_priority_fee_per_gas')
+DYNAMIC_FEE_TRANSACTION_PARAMS = ('max_fee_per_gas', 'max_priority_fee_per_gas')

eth_tester/normalization/inbound.py

@@ -97,6 +97,7 @@ def _normalize_inbound_access_list(access_list):
 
 
 TRANSACTION_NORMALIZERS = {
+    'type': identity,  # type is not required so the type is inferred from the txn params
     'chain_id': identity,
     'from': to_canonical_address,
     'to': to_empty_or_canonical_address,

eth_tester/utils/backend_testing.py

@@ -51,10 +51,6 @@
     _make_call_throws_transaction,
     _decode_throws_result,
 )
-from .. import (
-    PyEVMBackend,
-)
-
 
 PK_A = '0x58d23b55bc9cdce1f18c2500f40ff4ab7245df9a89505e9b1fa4851f623d241d'
 PK_A_ADDRESS = '0xdc544d1aa88ff8bbd2f2aec754b1f1e99e1812fd'
@@ -123,23 +119,24 @@ def _send_and_check_transaction(self, eth_tester, test_transaction, _from):
         self._check_transactions(transaction, txn)
 
     @staticmethod
-    def _check_transactions(expected_transaction, actual_transaction):
-        assert is_same_address(actual_transaction['from'], expected_transaction['from'])
-        if 'to' not in expected_transaction or expected_transaction['to'] == '':
+    def _check_transactions(sent_transaction, actual_transaction):
+        assert is_same_address(actual_transaction['from'], sent_transaction['from'])
+        if 'to' not in sent_transaction or sent_transaction['to'] == '':
             assert actual_transaction['to'] == ''
         else:
-            assert is_same_address(actual_transaction['to'], expected_transaction['to'])
+            assert is_same_address(actual_transaction['to'], sent_transaction['to'])
 
-        if expected_transaction.get('gas_price'):
-            assert actual_transaction['gas_price'] == expected_transaction['gas_price']
+        assert actual_transaction['gas'] == sent_transaction['gas']
+        assert actual_transaction['value'] == sent_transaction['value']
+
+        if sent_transaction.get('gas_price') is not None:
+            assert actual_transaction['gas_price'] == sent_transaction['gas_price']
         else:
-            assert actual_transaction['max_fee_per_gas'] == expected_transaction['max_fee_per_gas']
+            assert actual_transaction['max_fee_per_gas'] == sent_transaction['max_fee_per_gas']
             assert (
                 actual_transaction['max_priority_fee_per_gas'] ==
-                expected_transaction['max_priority_fee_per_gas']
+                sent_transaction['max_priority_fee_per_gas']
             )
-        assert actual_transaction['gas'] == expected_transaction['gas']
-        assert actual_transaction['value'] == expected_transaction['value']
 
     #
     # Testing Flags
@@ -347,12 +344,12 @@ def test_send_transaction_raises_with_legacy_and_dynamic_fee_fields(
             "max_fee_per_gas": 1000000000,
             "max_priority_fee_per_gas": 1000000000,
         }
-        with pytest.raises(ValidationError):
+        with pytest.raises(ValidationError, match="legacy and dynamic fee transaction values"):
             self._send_and_check_transaction(eth_tester, test_transaction, accounts[0])
 
     def test_send_transaction_no_gas_price_or_dynamic_fees(self, eth_tester):
         # test that we default to a dynamic fee transaction which, while pending, will include the
-        # gas_price as equal to the max_fee_per_gas until that is removed.
+        # gas_price as equal to the effective gas price paid.
         accounts = eth_tester.get_accounts()
         assert accounts, "No accounts available for transaction sending"
 
@@ -362,10 +359,11 @@ def test_send_transaction_no_gas_price_or_dynamic_fees(self, eth_tester):
         txn_hash = eth_tester.send_transaction(test_transaction)
         sent_transaction = eth_tester.get_transaction_by_hash(txn_hash)
 
-        assert sent_transaction.get('gas_price') == 1000000000
+        assert sent_transaction.get('type') == '0x2'
         assert sent_transaction.get('max_fee_per_gas') == 1000000000
         assert sent_transaction.get('max_priority_fee_per_gas') == 1000000000
         assert sent_transaction.get('access_list') == ()
+        assert sent_transaction.get('gas_price') == 1000000000
 
     def test_send_access_list_transaction(self, eth_tester):
         accounts = eth_tester.get_accounts()
@@ -382,7 +380,7 @@ def test_send_access_list_transaction(self, eth_tester):
         }
         txn_hash = eth_tester.send_transaction(access_list_transaction)
         txn = eth_tester.get_transaction_by_hash(txn_hash)
-        assert eth_tester.get_transaction_receipt(txn_hash)['type'] == '0x1'
+        assert txn.get('type') == '0x1'
         self._check_transactions(access_list_transaction, txn)
 
         # with non-empty access list
@@ -401,7 +399,7 @@ def test_send_access_list_transaction(self, eth_tester):
         )
         txn_hash = eth_tester.send_transaction(access_list_transaction)
         txn = eth_tester.get_transaction_by_hash(txn_hash)
-        assert eth_tester.get_transaction_receipt(txn_hash)['type'] == '0x1'
+        assert txn.get('type') == '0x1'
         self._check_transactions(access_list_transaction, txn)
 
     def test_send_dynamic_fee_transaction(self, eth_tester):
@@ -419,7 +417,7 @@ def test_send_dynamic_fee_transaction(self, eth_tester):
         }
         txn_hash = eth_tester.send_transaction(dynamic_fee_transaction)
         txn = eth_tester.get_transaction_by_hash(txn_hash)
-        assert eth_tester.get_transaction_receipt(txn_hash)['type'] == '0x2'
+        assert txn.get('type') == '0x2'
         self._check_transactions(dynamic_fee_transaction, txn)
 
         # with non-empty access list
@@ -438,7 +436,7 @@ def test_send_dynamic_fee_transaction(self, eth_tester):
         )
         txn_hash = eth_tester.send_transaction(dynamic_fee_transaction)
         txn = eth_tester.get_transaction_by_hash(txn_hash)
-        assert eth_tester.get_transaction_receipt(txn_hash)['type'] == '0x2'
+        assert txn.get('type') == '0x2'
         self._check_transactions(dynamic_fee_transaction, txn)
 
     def test_block_number_auto_mine_transactions_enabled(self, eth_tester):
@@ -648,7 +646,6 @@ def test_get_block_by_hash(self, eth_tester):
             block = eth_tester.get_block_by_hash(block_hash)
             assert block['number'] == block_number
             assert block['hash'] == block_hash
-            assert block['base_fee_per_gas'] is not None
 
     def test_get_block_by_hash_full_transactions(self, eth_tester):
         eth_tester.mine_blocks(2)
@@ -730,30 +727,17 @@ def test_get_transaction_by_hash_for_unmined_transaction(self, eth_tester):
         assert transaction['hash'] == transaction_hash
         assert transaction['block_hash'] is None
 
-    def test_get_transaction_receipt_for_unmined_transaction_raises(self, eth_tester):
-        eth_tester.disable_auto_mine_transactions()
-        transaction_hash = eth_tester.send_transaction({
-            "from": eth_tester.get_accounts()[0],
-            "to": BURN_ADDRESS,
-            "gas": 21000,
-        })
-        with pytest.raises(TransactionNotFound):
-            eth_tester.get_transaction_receipt(transaction_hash)
-
     def test_get_transaction_receipt_for_mined_transaction(self, eth_tester):
         transaction_hash = eth_tester.send_transaction({
             "from": eth_tester.get_accounts()[0],
             "to": BURN_ADDRESS,
             "gas": 21000,
         })
-        mined_transaction = eth_tester.get_transaction_by_hash(transaction_hash)
-        max_fee = mined_transaction['max_fee_per_gas']
-        assert max_fee == 1000000000
 
         receipt = eth_tester.get_transaction_receipt(transaction_hash)
         assert receipt['transaction_hash'] == transaction_hash
         assert receipt['type'] == '0x2'
-        assert receipt['effective_gas_price'] == max_fee
+        assert receipt['effective_gas_price'] == 1000000000
 
     @pytest.mark.parametrize(
         'type_specific_params,_type',
@@ -815,7 +799,7 @@ def test_receipt_effective_gas_price_for_mined_transaction_legacy(self, eth_test
         assert receipt['type'] == '0x0'
         assert receipt['effective_gas_price'] == gas_price
 
-    def test_receipt_effective_gas_price_for_mined_transaction_base_fee_limit(self, eth_tester):
+    def test_receipt_effective_gas_price_for_mined_transaction_base_fee_minimum(self, eth_tester):
         priority_fee = 1500000000
 
         transaction_hash = eth_tester.send_transaction({
@@ -838,7 +822,7 @@ def test_receipt_effective_gas_price_for_mined_transaction_base_fee_limit(self,
         # effective gas price
         assert receipt['effective_gas_price'] == base_fee + priority_fee
 
-    def test_receipt_effective_gas_price_for_mined_transaction_max_fee_limit(self, eth_tester):
+    def test_receipt_effective_gas_price_for_mined_transaction_max_fee_minimum(self, eth_tester):
         priority_fee = 1500000000
         max_fee = priority_fee + 1  # arbitrary, to differentiate from the priority fee
 
@@ -862,6 +846,16 @@ def test_receipt_effective_gas_price_for_mined_transaction_max_fee_limit(self, e
         # effective gas price
         assert receipt['effective_gas_price'] == max_fee
 
+    def test_get_transaction_receipt_for_unmined_transaction_raises(self, eth_tester):
+        eth_tester.disable_auto_mine_transactions()
+        transaction_hash = eth_tester.send_transaction({
+            "from": eth_tester.get_accounts()[0],
+            "to": BURN_ADDRESS,
+            "gas": 21000,
+        })
+        with pytest.raises(TransactionNotFound):
+            eth_tester.get_transaction_receipt(transaction_hash)
+
     def test_call_return13(self, eth_tester):
         self.skip_if_no_evm_execution()
 
@@ -1525,16 +1519,8 @@ def test_delete_filter(self, eth_tester):
     def test_receipt_gas_used_computation(self, eth_tester):
         eth_tester.disable_auto_mine_transactions()
 
-        if isinstance(eth_tester.backend, PyEVMBackend):
-            chain_id = eth_tester.backend.chain.chain_id
-
         tx_hashes = []
         for i in range(4):
-            # TODO: PyEvm backend was not consistent in keeping the chain_id, so reset it. This
-            #  needs to be investigated further.
-            if isinstance(eth_tester.backend, PyEVMBackend):
-                eth_tester.backend.chain.chain_id = chain_id
-
             tx = {
                 'from': eth_tester.get_accounts()[i],
                 'to': eth_tester.get_accounts()[i + 1],