Skip to content

Commit 534c09b

Browse files
committed
implemented mario feedback
1 parent 5ed0304 commit 534c09b

File tree

3 files changed

+31
-32
lines changed

3 files changed

+31
-32
lines changed

src/ethereum_test_rpc/rpc.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
)
2727

2828
logger = get_logger(__name__)
29-
3029
BlockNumberType = int | Literal["latest", "earliest", "pending"]
3130

3231

@@ -81,14 +80,17 @@ def post_request(
8180
self,
8281
*,
8382
method: str,
84-
params: Any | None = None,
83+
params: List[Any] | None = None,
8584
extra_headers: Dict | None = None,
8685
request_id: int | str | None = None,
8786
timeout: int | None = None,
8887
) -> Any:
8988
"""Send JSON-RPC POST request to the client RPC server at port defined in the url."""
9089
if extra_headers is None:
9190
extra_headers = {}
91+
if params is None:
92+
params = []
93+
9294
assert self.namespace, "RPC namespace not set"
9395

9496
next_request_id_counter = next(self.request_id_counter)
@@ -106,8 +108,7 @@ def post_request(
106108
}
107109
headers = base_header | extra_headers
108110

109-
# print(f"Sending RPC request to {self.url}, timeout is set to {timeout}...")
110-
print(f"Sending RPC request, timeout is set to {timeout}...") # don't leak url in logs
111+
logger.debug(f"Sending RPC request, timeout is set to {timeout}...")
111112
response = requests.post(self.url, json=json, headers=headers, timeout=timeout)
112113
response.raise_for_status()
113114
response_json = response.json()
@@ -145,7 +146,7 @@ def config(self, timeout: int | None = None):
145146
try:
146147
response = self.post_request(method="config", timeout=timeout)
147148
if response is None:
148-
print("eth_config request: failed to get response")
149+
logger.warning("eth_config request: failed to get response")
149150
return None
150151
return EthConfigResponse.model_validate(
151152
response, context=self.response_validation_context
@@ -154,7 +155,7 @@ def config(self, timeout: int | None = None):
154155
pprint(e.errors())
155156
raise e
156157
except Exception as e:
157-
print(f"exception occurred when sending JSON-RPC request: {e}")
158+
logger.error(f"exception occurred when sending JSON-RPC request: {e}")
158159
raise e
159160

160161
def chain_id(self) -> int:
@@ -211,7 +212,7 @@ def get_transaction_by_hash(self, transaction_hash: Hash) -> TransactionByHashRe
211212
"""`eth_getTransactionByHash`: Returns transaction details."""
212213
try:
213214
response = self.post_request(
214-
method="getTransactionByHash", params=f"{transaction_hash}"
215+
method="getTransactionByHash", params=[f"{transaction_hash}"]
215216
)
216217
if response is None:
217218
return None
@@ -245,7 +246,7 @@ def send_raw_transaction(
245246
try:
246247
response = self.post_request(
247248
method="sendRawTransaction",
248-
params=f"{transaction_rlp.hex()}",
249+
params=[transaction_rlp.hex()],
249250
request_id=request_id, # noqa: E501
250251
)
251252

@@ -261,7 +262,7 @@ def send_transaction(self, transaction: Transaction) -> Hash:
261262
try:
262263
response = self.post_request(
263264
method="sendRawTransaction",
264-
params=f"{transaction.rlp().hex()}",
265+
params=[transaction.rlp().hex()],
265266
request_id=transaction.metadata_string(), # noqa: E501
266267
)
267268

@@ -431,7 +432,7 @@ def forkchoice_updated(
431432
if payload_attributes is None:
432433
params = [to_json(forkchoice_state)]
433434
else:
434-
params = [to_json(forkchoice_state), to_json(payload_attributes)]
435+
params = [to_json(forkchoice_state), None]
435436

436437
return ForkchoiceUpdateResponse.model_validate(
437438
self.post_request(
@@ -456,7 +457,7 @@ def get_payload(
456457
return GetPayloadResponse.model_validate(
457458
self.post_request(
458459
method=method,
459-
params=f"{payload_id}",
460+
params=[f"{payload_id}"],
460461
),
461462
context=self.response_validation_context,
462463
)
@@ -499,4 +500,4 @@ class AdminRPC(BaseRPC):
499500

500501
def add_peer(self, enode: str) -> bool:
501502
"""`admin_addPeer`: Add a peer by enode URL."""
502-
return self.post_request(method="addPeer", params=enode)
503+
return self.post_request(method="addPeer", params=[enode])

src/pytest_plugins/execute/eth_config/eth_config.py

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ def pytest_addoption(parser):
5858
action="store",
5959
dest="clients",
6060
type=str,
61-
default="besu,erigon,geth,nethermind,reth",
62-
help="Comma-separated list of clients to be tested in majority mode. This flag will be "
63-
"ignored when you pass a value for the network-config-file flag. Default: "
64-
"besu,erigon,geth,nethermind,reth",
61+
default=None,
62+
help="Comma-separated list of clients to be tested in majority mode. Example: "
63+
'"besu,erigon,geth,nethermind,nimbusel,reth"\nIf you do not pass a value, majority mode '
64+
"testing will be disabled.",
6565
)
6666
eth_config_group.addoption(
6767
"--genesis-config-file",
@@ -167,15 +167,15 @@ def pytest_configure(config: pytest.Config) -> None:
167167
# Test out the RPC endpoint to be able to fail fast if it's not working
168168
eth_rpc = EthRPC(rpc_endpoint)
169169
try:
170-
print("Will now perform a connection check (request chain_id)..")
170+
logger.debug("Will now perform a connection check (request chain_id)..")
171171
chain_id = eth_rpc.chain_id()
172-
print(f"Connection check ok (successfully got chain id {chain_id})")
172+
logger.debug(f"Connection check ok (successfully got chain id {chain_id})")
173173
except Exception as e:
174174
pytest.exit(f"Could not connect to RPC endpoint {rpc_endpoint}: {e}")
175175
try:
176-
print("Will now briefly check whether eth_config is supported by target rpc..")
176+
logger.debug("Will now briefly check whether eth_config is supported by target rpc..")
177177
eth_rpc.config()
178-
print("Connection check ok (successfully got eth_config response)")
178+
logger.debug("Connection check ok (successfully got eth_config response)")
179179
except Exception as e:
180180
pytest.exit(f"RPC endpoint {rpc_endpoint} does not support `eth_config`: {e}")
181181

@@ -186,12 +186,6 @@ def rpc_endpoint(request) -> str:
186186
return request.config.getoption("rpc_endpoint")
187187

188188

189-
# @pytest.fixture(autouse=True, scope="session")
190-
# def eth_rpc(rpc_endpoint: str) -> EthRPC:
191-
# """Initialize ethereum RPC client for the execution client under test."""
192-
# return EthRPC(rpc_endpoint)
193-
194-
195189
def all_rpc_endpoints(config) -> Dict[str, List[EthRPC]]:
196190
"""Derive a mapping of exec clients to the RPC URLs they are reachable at."""
197191
rpc_endpoint = config.getoption("rpc_endpoint")
@@ -231,7 +225,9 @@ def pytest_generate_tests(metafunc: pytest.Metafunc):
231225
if metafunc.definition.name == "test_eth_config_majority":
232226
if len(all_rpc_endpoints_dict) < 2:
233227
# The test function is not run because we only have a single client, so no majority comparison # noqa: E501
234-
print("Skipping eth_config majority because less than 2 exec clients were passed")
228+
logger.info(
229+
"Skipping eth_config majority because less than 2 exec clients were passed"
230+
)
235231
metafunc.parametrize(
236232
["all_rpc_endpoints"],
237233
[],

src/pytest_plugins/execute/eth_config/execute_eth_config.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@
88
import pytest
99

1010
from ethereum_test_rpc import EthConfigResponse, EthRPC
11+
from pytest_plugins.logging import get_logger
1112

1213
from .types import NetworkConfig
1314

15+
logger = get_logger(__name__)
16+
1417

1518
@pytest.fixture(scope="function")
1619
def eth_config_response(eth_rpc: List[EthRPC]) -> EthConfigResponse | None:
@@ -34,7 +37,6 @@ def current_time() -> int:
3437
@pytest.fixture(scope="function")
3538
def expected_eth_config(network: NetworkConfig, current_time: int) -> EthConfigResponse:
3639
"""Calculate the current fork value to verify against the client's response."""
37-
print(f"Network provided: {network}, Type: {type(network)}")
3840
return network.get_eth_config(current_time)
3941

4042

@@ -191,7 +193,7 @@ def test_eth_config_majority(
191193
response = eth_rpc_target.config(timeout=10)
192194
if response is None:
193195
# safely split url to not leak rpc_endpoint in logs
194-
print(
196+
logger.warning(
195197
f"When trying to get eth_config from {eth_rpc_target} a problem occurred" # problem itself is logged by .config() call # noqa: E501
196198
)
197199
continue
@@ -201,7 +203,7 @@ def test_eth_config_majority(
201203
client_to_url_used_dict[exec_client] = (
202204
eth_rpc_target.url
203205
) # remember which cl+el combination was used # noqa: E501
204-
print(f"Response of {exec_client}: {response_str}\n\n")
206+
logger.info(f"Response of {exec_client}: {response_str}\n\n")
205207

206208
break # no need to gather more responses for this client
207209

@@ -218,7 +220,7 @@ def test_eth_config_majority(
218220
for client in responses.keys():
219221
response_bytes = json.dumps(responses[client], sort_keys=True).encode("utf-8")
220222
response_hash = sha256(response_bytes).digest().hex()
221-
print(f"Response hash of client {client}: {response_hash}")
223+
logger.info(f"Response hash of client {client}: {response_hash}")
222224
client_to_hash_dict[client] = response_hash
223225

224226
# if not all responses have the same hash there is a critical consensus issue
@@ -236,4 +238,4 @@ def test_eth_config_majority(
236238
)
237239
assert expected_hash != ""
238240

239-
print("All clients returned the same eth_config response. Test has been passed!")
241+
logger.info("All clients returned the same eth_config response. Test has been passed!")

0 commit comments

Comments
 (0)