implemented mario feedback

Author: felix314159

src/ethereum_test_rpc/rpc.py

@@ -11,6 +11,7 @@
 
 from ethereum_test_base_types import Address, Bytes, Hash, to_json
 from ethereum_test_types import Transaction
+from pytest_plugins.logging import get_logger
 
 from .types import (
     EthConfigResponse,
@@ -24,6 +25,7 @@
     TransactionByHashResponse,
 )
 
+logger = get_logger(__name__)
 BlockNumberType = int | Literal["latest", "earliest", "pending"]
 
 
@@ -79,14 +81,17 @@ def post_request(
         self,
         *,
         method: str,
-        params: Any | None = None,
+        params: List[Any] | None = None,
         extra_headers: Dict | None = None,
         request_id: int | str | None = None,
         timeout: int | None = None,
     ) -> Any:
         """Send JSON-RPC POST request to the client RPC server at port defined in the url."""
         if extra_headers is None:
             extra_headers = {}
+        if params is None:
+            params = []
+
         assert self.namespace, "RPC namespace not set"
 
         next_request_id_counter = next(self.request_id_counter)
@@ -104,8 +109,7 @@ def post_request(
         }
         headers = base_header | self.extra_headers | extra_headers
 
-        # print(f"Sending RPC request to {self.url}, timeout is set to {timeout}...")
-        print(f"Sending RPC request, timeout is set to {timeout}...")  # don't leak url in logs
+        logger.debug(f"Sending RPC request, timeout is set to {timeout}...")
         response = requests.post(self.url, json=json, headers=headers, timeout=timeout)
         response.raise_for_status()
         response_json = response.json()
@@ -149,7 +153,7 @@ def config(self, timeout: int | None = None):
         try:
             response = self.post_request(method="config", timeout=timeout)
             if response is None:
-                print("eth_config request: failed to get response")
+                logger.warning("eth_config request: failed to get response")
                 return None
             return EthConfigResponse.model_validate(
                 response, context=self.response_validation_context
@@ -158,7 +162,7 @@ def config(self, timeout: int | None = None):
             pprint(e.errors())
             raise e
         except Exception as e:
-            print(f"exception occurred when sending JSON-RPC request: {e}")
+            logger.error(f"exception occurred when sending JSON-RPC request: {e}")
             raise e
 
     def chain_id(self) -> int:
@@ -215,7 +219,7 @@ def get_transaction_by_hash(self, transaction_hash: Hash) -> TransactionByHashRe
         """`eth_getTransactionByHash`: Returns transaction details."""
         try:
             response = self.post_request(
-                method="getTransactionByHash", params=f"{transaction_hash}"
+                method="getTransactionByHash", params=[f"{transaction_hash}"]
             )
             if response is None:
                 return None
@@ -249,7 +253,7 @@ def send_raw_transaction(
         try:
             response = self.post_request(
                 method="sendRawTransaction",
-                params=f"{transaction_rlp.hex()}",
+                params=[transaction_rlp.hex()],
                 request_id=request_id,  # noqa: E501
             )
 
@@ -265,7 +269,7 @@ def send_transaction(self, transaction: Transaction) -> Hash:
         try:
             response = self.post_request(
                 method="sendRawTransaction",
-                params=f"{transaction.rlp().hex()}",
+                params=[transaction.rlp().hex()],
                 request_id=transaction.metadata_string(),  # noqa: E501
             )
 
@@ -423,7 +427,7 @@ def forkchoice_updated(
         if payload_attributes is None:
             params = [to_json(forkchoice_state)]
         else:
-            params = [to_json(forkchoice_state), to_json(payload_attributes)]
+            params = [to_json(forkchoice_state), None]
 
         return ForkchoiceUpdateResponse.model_validate(
             self.post_request(
@@ -448,7 +452,7 @@ def get_payload(
         return GetPayloadResponse.model_validate(
             self.post_request(
                 method=method,
-                params=f"{payload_id}",
+                params=[f"{payload_id}"],
             ),
             context=self.response_validation_context,
         )
@@ -466,7 +470,7 @@ def get_blobs(
         return GetBlobsResponse.model_validate(
             self.post_request(
                 method=method,
-                params=params,
+                params=[params],
             ),
             context=self.response_validation_context,
         )
@@ -486,4 +490,4 @@ class AdminRPC(BaseRPC):
 
     def add_peer(self, enode: str) -> bool:
         """`admin_addPeer`: Add a peer by enode URL."""
-        return self.post_request(method="addPeer", params=enode)
+        return self.post_request(method="addPeer", params=[enode])

src/pytest_plugins/execute/eth_config/eth_config.py

@@ -8,6 +8,7 @@
 import pytest
 
 from ethereum_test_rpc import EthRPC
+from pytest_plugins.logging import get_logger
 
 from .types import NetworkConfigFile
 
@@ -20,6 +21,8 @@
 EXECUTION_CLIENTS = ["besu", "erigon", "geth", "nethermind", "nimbusel", "reth"]
 CONSENSUS_CLIENTS = ["grandine", "lighthouse", "lodestar", "nimbus", "prysm", "teku"]
 
+logger = get_logger(__name__)
+
 
 def pytest_addoption(parser):
     """Add command-line options to pytest."""
@@ -54,10 +57,10 @@ def pytest_addoption(parser):
         action="store",
         dest="clients",
         type=str,
-        default="besu,erigon,geth,nethermind,reth",
-        help="Comma-separated list of clients to be tested in majority mode. This flag will be "
-        "ignored when you pass a value for the network-config-file flag. Default: "
-        "besu,erigon,geth,nethermind,reth",
+        default=None,
+        help="Comma-separated list of clients to be tested in majority mode. Example: "
+        '"besu,erigon,geth,nethermind,nimbusel,reth"\nIf you do not pass a value, majority mode '
+        "testing will be disabled.",
     )
     eth_config_group.addoption(
         "--rpc-endpoint",
@@ -97,36 +100,37 @@ def pytest_configure(config: pytest.Config) -> None:
     config.network = network_configs.root[network_name]  # type: ignore
 
     # prepare majority test mode
-    #   parse clients list
-    clients.replace(" ", "")
-    clients = clients.split(",")
-    for c in clients:
-        if c not in EXECUTION_CLIENTS:
-            pytest.exit(f"Unsupported client was passed: {c}")
-    print(f"Provided client list: {clients}")
-
-    # store majority mode configuration
-    if ".ethpandaops.io" in rpc_endpoint:
-        print("Ethpandaops RPC detected, toggling majority test on")
-        config.option.majority_clients = clients  # List[str]
+    #   parse clients list if something was passed for it
+    if clients:
+        clients.replace(" ", "")
+        clients = clients.split(",")
+        for c in clients:
+            if c not in EXECUTION_CLIENTS:
+                pytest.exit(f"Unsupported client was passed: {c}")
+        logger.info(f"Provided client list: {clients}")
+        # activate majority mode if also URL condition is met
+        if ".ethpandaops.io" in rpc_endpoint:
+            logger.info("Ethpandaops RPC detected")
+            logger.info("Toggling majority test on")
+            config.option.majority_clients = clients  # List[str]
     else:
-        print("No ethpandaops RPC detected, majority test will not be toggled on")
+        logger.info("Majority test mode is disabled because no --clients value was passed.")
 
     if config.getoption("collectonly", default=False):
         return
 
     # Test out the RPC endpoint to be able to fail fast if it's not working
     eth_rpc = EthRPC(rpc_endpoint)
     try:
-        print("Will now perform a connection check (request chain_id)..")
+        logger.debug("Will now perform a connection check (request chain_id)..")
         chain_id = eth_rpc.chain_id()
-        print(f"Connection check ok (successfully got chain id {chain_id})")
+        logger.debug(f"Connection check ok (successfully got chain id {chain_id})")
     except Exception as e:
         pytest.exit(f"Could not connect to RPC endpoint {rpc_endpoint}: {e}")
     try:
-        print("Will now briefly check whether eth_config is supported by target rpc..")
+        logger.debug("Will now briefly check whether eth_config is supported by target rpc..")
         eth_rpc.config()
-        print("Connection check ok (successfully got eth_config response)")
+        logger.debug("Connection check ok (successfully got eth_config response)")
     except Exception as e:
         pytest.exit(f"RPC endpoint {rpc_endpoint} does not support `eth_config`: {e}")
 
@@ -137,12 +141,6 @@ def rpc_endpoint(request) -> str:
     return request.config.getoption("rpc_endpoint")
 
 
-# @pytest.fixture(autouse=True, scope="session")
-# def eth_rpc(rpc_endpoint: str) -> EthRPC:
-#     """Initialize ethereum RPC client for the execution client under test."""
-#     return EthRPC(rpc_endpoint)
-
-
 def all_rpc_endpoints(config) -> Dict[str, List[EthRPC]]:
     """Derive a mapping of exec clients to the RPC URLs they are reachable at."""
     rpc_endpoint = config.getoption("rpc_endpoint")
@@ -182,7 +180,9 @@ def pytest_generate_tests(metafunc: pytest.Metafunc):
     if metafunc.definition.name == "test_eth_config_majority":
         if len(all_rpc_endpoints_dict) < 2:
             # The test function is not run because we only have a single client, so no majority comparison  # noqa: E501
-            print("Skipping eth_config majority because less than 2 exec clients were passed")
+            logger.info(
+                "Skipping eth_config majority because less than 2 exec clients were passed"
+            )
             metafunc.parametrize(
                 ["all_rpc_endpoints"],
                 [],

src/pytest_plugins/execute/eth_config/execute_eth_config.py

@@ -8,9 +8,12 @@
 import pytest
 
 from ethereum_test_rpc import EthConfigResponse, EthRPC
+from pytest_plugins.logging import get_logger
 
 from .types import NetworkConfig
 
+logger = get_logger(__name__)
+
 
 @pytest.fixture(scope="function")
 def eth_config_response(eth_rpc: List[EthRPC]) -> EthConfigResponse | None:
@@ -34,7 +37,6 @@ def current_time() -> int:
 @pytest.fixture(scope="function")
 def expected_eth_config(network: NetworkConfig, current_time: int) -> EthConfigResponse:
     """Calculate the current fork value to verify against the client's response."""
-    print(f"Network provided: {network}, Type: {type(network)}")
     return network.get_eth_config(current_time)
 
 
@@ -191,7 +193,7 @@ def test_eth_config_majority(
             response = eth_rpc_target.config(timeout=10)
             if response is None:
                 # safely split url to not leak rpc_endpoint in logs
-                print(
+                logger.warning(
                     f"When trying to get eth_config from {eth_rpc_target} a problem occurred"  # problem itself is logged by .config() call # noqa: E501
                 )
                 continue
@@ -201,7 +203,7 @@ def test_eth_config_majority(
             client_to_url_used_dict[exec_client] = (
                 eth_rpc_target.url
             )  # remember which cl+el combination was used  # noqa: E501
-            print(f"Response of {exec_client}: {response_str}\n\n")
+            logger.info(f"Response of {exec_client}: {response_str}\n\n")
 
             break  # no need to gather more responses for this client
 
@@ -218,7 +220,7 @@ def test_eth_config_majority(
     for client in responses.keys():
         response_bytes = json.dumps(responses[client], sort_keys=True).encode("utf-8")
         response_hash = sha256(response_bytes).digest().hex()
-        print(f"Response hash of client {client}: {response_hash}")
+        logger.info(f"Response hash of client {client}: {response_hash}")
         client_to_hash_dict[client] = response_hash
 
     # if not all responses have the same hash there is a critical consensus issue
@@ -236,4 +238,4 @@ def test_eth_config_majority(
         )
     assert expected_hash != ""
 
-    print("All clients returned the same eth_config response. Test has been passed!")
+    logger.info("All clients returned the same eth_config response. Test has been passed!")