rpc should use named parameters, added timeout option for sending requests, wip

Author: felix314159

src/ethereum_test_rpc/rpc.py

@@ -77,10 +77,12 @@ def __init_subclass__(cls) -> None:
 
     def post_request(
         self,
+        *,
         method: str,
-        *params: Any,
+        params: 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:
@@ -90,7 +92,8 @@ def post_request(
         next_request_id_counter = next(self.request_id_counter)
         if request_id is None:
             request_id = next_request_id_counter
-        payload = {
+
+        json = {
             "jsonrpc": "2.0",
             "method": f"{self.namespace}_{method}",
             "params": params,
@@ -101,7 +104,9 @@ def post_request(
         }
         headers = base_header | self.extra_headers | extra_headers
 
-        response = requests.post(self.url, json=payload, headers=headers)
+        print(f"Sending RPC request to {self.url}, timeout is set to {timeout}...")
+        # WHY IS TIMEOUT ALWAYS NONE IT SHOULD HAVE RECEIVED AN ACTUAL VALUE
+        response = requests.post(self.url, json=json, headers=headers, timeout=timeout)
         response.raise_for_status()
         response_json = response.json()
 
@@ -139,53 +144,79 @@ def __init__(
         )
         self.transaction_wait_timeout = transaction_wait_timeout
 
-    def config(self):
+    def config(self, timeout: int | None = None):
         """`eth_config`: Returns information about a fork configuration of the client."""
         try:
-            response = self.post_request("config")
+            response = self.post_request(method="config", timeout=timeout)
             if response is None:
+                print("eth_config request: failed to get response")
                 return None
             return EthConfigResponse.model_validate(
                 response, context=self.response_validation_context
             )
         except ValidationError as e:
             pprint(e.errors())
             raise e
+        except Exception as e:
+            print(f"exception occurred when sending JSON-RPC request: {e}")
+            raise e
 
     def chain_id(self) -> int:
         """`eth_chainId`: Returns the current chain id."""
-        return int(self.post_request("chainId"), 16)
+        response = self.post_request(method="chainId")
+
+        return int(response, 16)
 
     def get_block_by_number(self, block_number: BlockNumberType = "latest", full_txs: bool = True):
         """`eth_getBlockByNumber`: Returns information about a block by block number."""
         block = hex(block_number) if isinstance(block_number, int) else block_number
-        return self.post_request("getBlockByNumber", block, full_txs)
+        params = [block, full_txs]
+        response = self.post_request(method="getBlockByNumber", params=params)
+
+        return response
 
     def get_block_by_hash(self, block_hash: Hash, full_txs: bool = True):
         """`eth_getBlockByHash`: Returns information about a block by hash."""
-        return self.post_request("getBlockByHash", f"{block_hash}", full_txs)
+        params = [f"{block_hash}", full_txs]
+        response = self.post_request(method="getBlockByHash", params=params)
+
+        return response
 
     def get_balance(self, address: Address, block_number: BlockNumberType = "latest") -> int:
         """`eth_getBalance`: Returns the balance of the account of given address."""
         block = hex(block_number) if isinstance(block_number, int) else block_number
-        return int(self.post_request("getBalance", f"{address}", block), 16)
+        params = [f"{address}", block]
+
+        response = self.post_request(method="getBalance", params=params)
+
+        return int(response, 16)
 
     def get_code(self, address: Address, block_number: BlockNumberType = "latest") -> Bytes:
         """`eth_getCode`: Returns code at a given address."""
         block = hex(block_number) if isinstance(block_number, int) else block_number
-        return Bytes(self.post_request("getCode", f"{address}", block))
+        params = [f"{address}", block]
+
+        response = self.post_request(method="getCode", params=params)
+
+        return Bytes(response)
 
     def get_transaction_count(
         self, address: Address, block_number: BlockNumberType = "latest"
     ) -> int:
         """`eth_getTransactionCount`: Returns the number of transactions sent from an address."""
         block = hex(block_number) if isinstance(block_number, int) else block_number
-        return int(self.post_request("getTransactionCount", f"{address}", block), 16)
+        params = [f"{address}", block]
+
+        response = self.post_request(method="getTransactionCount", params=params)
+
+        return int(response, 16)
 
     def get_transaction_by_hash(self, transaction_hash: Hash) -> TransactionByHashResponse | None:
         """`eth_getTransactionByHash`: Returns transaction details."""
         try:
-            response = self.post_request("getTransactionByHash", f"{transaction_hash}")
+            response = self.post_request(
+                method="getTransactionByHash", params=f"{transaction_hash}"
+            )
             if response is None:
                 return None
             return TransactionByHashResponse.model_validate(
@@ -200,37 +231,45 @@ def get_storage_at(
     ) -> Hash:
         """`eth_getStorageAt`: Returns the value from a storage position at a given address."""
         block = hex(block_number) if isinstance(block_number, int) else block_number
-        return Hash(self.post_request("getStorageAt", f"{address}", f"{position}", block))
+        params = [f"{address}", f"{position}", block]
+
+        response = self.post_request(method="getStorageAt", params=params)
+        return Hash(response)
 
     def gas_price(self) -> int:
         """`eth_gasPrice`: Returns the number of transactions sent from an address."""
-        return int(self.post_request("gasPrice"), 16)
+        response = self.post_request(method="gasPrice")
+
+        return int(response, 16)
 
     def send_raw_transaction(
         self, transaction_rlp: Bytes, request_id: int | str | None = None
     ) -> Hash:
         """`eth_sendRawTransaction`: Send a transaction to the client."""
         try:
-            result_hash = Hash(
-                self.post_request(
-                    "sendRawTransaction", f"{transaction_rlp.hex()}", request_id=request_id
-                ),
+            response = self.post_request(
+                method="sendRawTransaction",
+                params=f"{transaction_rlp.hex()}",
+                request_id=request_id,  # noqa: E501
             )
+
+            result_hash = Hash(response)
             assert result_hash is not None
             return result_hash
         except Exception as e:
             raise SendTransactionExceptionError(str(e), tx_rlp=transaction_rlp) from e
 
     def send_transaction(self, transaction: Transaction) -> Hash:
         """`eth_sendRawTransaction`: Send a transaction to the client."""
+        # TODO: is this a copypaste error from above?
         try:
-            result_hash = Hash(
-                self.post_request(
-                    "sendRawTransaction",
-                    f"{transaction.rlp().hex()}",
-                    request_id=transaction.metadata_string(),
-                )
+            response = self.post_request(
+                method="sendRawTransaction",
+                params=f"{transaction.rlp().hex()}",
+                request_id=transaction.metadata_string(),  # noqa: E501
             )
+
+            result_hash = Hash(response)
             assert result_hash == transaction.hash
             assert result_hash is not None
             return transaction.hash
@@ -322,7 +361,8 @@ class DebugRPC(EthRPC):
 
     def trace_call(self, tr: dict[str, str], block_number: str):
         """`debug_traceCall`: Returns pre state required for transaction."""
-        return self.post_request("traceCall", tr, block_number, {"tracer": "prestateTracer"})
+        params = [tr, block_number, {"tracer": "prestateTracer"}]
+        return self.post_request(method="traceCall", params=params)
 
 
 class EngineRPC(BaseRPC):
@@ -333,10 +373,12 @@ class EngineRPC(BaseRPC):
 
     def post_request(
         self,
+        *,
         method: str,
-        *params: Any,
+        params: 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:
@@ -349,14 +391,22 @@ def post_request(
         extra_headers = {
             "Authorization": f"Bearer {jwt_token}",
         } | extra_headers
+
         return super().post_request(
-            method, *params, extra_headers=extra_headers, request_id=request_id
+            method=method,
+            params=params,
+            extra_headers=extra_headers,
+            timeout=timeout,
+            request_id=request_id,
         )
 
     def new_payload(self, *params: Any, version: int) -> PayloadStatus:
         """`engine_newPayloadVX`: Attempts to execute the given payload on an execution client."""
+        method = f"newPayloadV{version}"
+        params_list = [to_json(param) for param in params]
+
         return PayloadStatus.model_validate(
-            self.post_request(f"newPayloadV{version}", *[to_json(param) for param in params]),
+            self.post_request(method=method, params=params_list),
             context=self.response_validation_context,
         )
 
@@ -368,11 +418,17 @@ def forkchoice_updated(
         version: int,
     ) -> ForkchoiceUpdateResponse:
         """`engine_forkchoiceUpdatedVX`: Updates the forkchoice state of the execution client."""
+        method = f"forkchoiceUpdatedV{version}"
+
+        if payload_attributes is None:
+            params = [to_json(forkchoice_state)]
+        else:
+            params = [to_json(forkchoice_state), to_json(payload_attributes)]
+
         return ForkchoiceUpdateResponse.model_validate(
             self.post_request(
-                f"forkchoiceUpdatedV{version}",
-                to_json(forkchoice_state),
-                to_json(payload_attributes) if payload_attributes is not None else None,
+                method=method,
+                params=params,
             ),
             context=self.response_validation_context,
         )
@@ -387,10 +443,12 @@ def get_payload(
         `engine_getPayloadVX`: Retrieves a payload that was requested through
         `engine_forkchoiceUpdatedVX`.
         """
+        method = f"getPayloadV{version}"
+
         return GetPayloadResponse.model_validate(
             self.post_request(
-                f"getPayloadV{version}",
-                f"{payload_id}",
+                method=method,
+                params=f"{payload_id}",
             ),
             context=self.response_validation_context,
         )
@@ -402,10 +460,13 @@ def get_blobs(
         version: int,
     ) -> GetBlobsResponse:
         """`engine_getBlobsVX`: Retrieves blobs from an execution layers tx pool."""
+        method = f"getBlobsV{version}"
+        params = [f"{h}" for h in versioned_hashes]
+
         return GetBlobsResponse.model_validate(
             self.post_request(
-                f"getBlobsV{version}",
-                [f"{h}" for h in versioned_hashes],
+                method=method,
+                params=params,
             ),
             context=self.response_validation_context,
         )
@@ -416,7 +477,7 @@ class NetRPC(BaseRPC):
 
     def peer_count(self) -> int:
         """`net_peerCount`: Get the number of peers connected to the client."""
-        response = self.post_request("peerCount")
+        response = self.post_request(method="peerCount")
         return int(response, 16)  # hex -> int
 
 
@@ -425,4 +486,4 @@ class AdminRPC(BaseRPC):
 
     def add_peer(self, enode: str) -> bool:
         """`admin_addPeer`: Add a peer by enode URL."""
-        return self.post_request("addPeer", enode)
+        return self.post_request(method="addPeer", params=enode)

src/pytest_plugins/execute/eth_config/eth_config.py

@@ -139,7 +139,7 @@ def eth_rpc(rpc_endpoint: str) -> EthRPC:
     return EthRPC(rpc_endpoint)
 
 
-def get_eth_config(url: str) -> Tuple[bool, str]:  # success, response
+def request_eth_config(*, url: str, timeout: int = 10) -> Tuple[bool, str]:  # success, response
     """Request data from devnet node via JSON_RPC."""
     payload = {
         "jsonrpc": "2.0",
@@ -152,7 +152,7 @@ def get_eth_config(url: str) -> Tuple[bool, str]:  # success, response
 
     try:
         # Make the request
-        response = requests.post(url, json=payload, headers=headers, timeout=20)
+        response = requests.post(url, json=payload, headers=headers, timeout=timeout)
 
         # Return JSON response
         return True, response.json()

src/pytest_plugins/execute/eth_config/execute_eth_config.py

@@ -9,14 +9,14 @@
 
 from ethereum_test_rpc import EthConfigResponse, EthRPC
 
-from .eth_config import get_eth_config, get_rpc_url_combinations_el_cl
+from .eth_config import get_rpc_url_combinations_el_cl, request_eth_config
 from .types import NetworkConfig
 
 
 @pytest.fixture(scope="session")
 def eth_config_response(eth_rpc: EthRPC) -> EthConfigResponse | None:
     """Get the `eth_config` response from the client to be verified by all tests."""
-    return eth_rpc.config()
+    return eth_rpc.config(timeout=10)
 
 
 @pytest.fixture(scope="session")
@@ -93,8 +93,6 @@ def test_eth_config_next(
     config = request.config
     if config.getoption("network_config_file") is None:
         pytest.skip("Skipping test because no 'network_config_file' was specified")
-    else:
-        print("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")
 
     assert eth_config_response is not None, "Client did not return a valid `eth_config` response."
     expected_next = expected_eth_config.next
@@ -226,12 +224,13 @@ def test_eth_config_majority(
         el_clients=el_clients, rpc_endpoint=rpc_endpoint
     )
     assert url_dict is not None
+
     responses = dict()  # noqa: C408
     for exec_client in url_dict.keys():
         # try only as many consensus+exec client combinations until you receive a response
         # if all combinations fail we panic
         for url in url_dict[exec_client]:
-            success, response = get_eth_config(url)
+            success, response = request_eth_config(url=url, timeout=9)
             if not success:
                 # safely split url to not leak rpc_endpoint in logs
                 print(