Skip to content

Commit aad679b

Browse files
varex83agentvarex83
andcommitted
fix(cli/mev): address PR review comments for mev test command
- Use tokio::time::Instant instead of std::time::Instant in async code - Replace CliError::Other string literals with typed variants TimeoutInterrupted and TestCaseNotSupported - Remove TestResultError from imports; use CliError::TimeoutInterrupted directly - Simplify latest_beacon_block and fetch_proposers_for_epoch to use ? operator, leveraging existing From impls on CliError - Simplify get_block_header error conversions with .map_err(MevError::from) and .map_err(|e| MevError::Cli(e.into())) Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
1 parent 0c11a44 commit aad679b

2 files changed

Lines changed: 19 additions & 41 deletions

File tree

crates/cli/src/commands/test/mev.rs

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
11
//! MEV relay tests.
22
3-
use std::{
4-
collections::HashMap,
5-
io::Write,
6-
time::{Duration, Instant},
7-
};
3+
use std::{collections::HashMap, io::Write, time::Duration};
84

95
use reqwest::{Method, StatusCode};
10-
use tokio::task::JoinSet;
6+
use tokio::{task::JoinSet, time::Instant};
117
use tokio_util::sync::CancellationToken;
128
use tracing::info;
139

1410
use super::{
1511
AllCategoriesResult, SLOT_TIME, SLOTS_IN_EPOCH, TestCategory, TestCategoryResult,
16-
TestConfigArgs, TestResult, TestResultError, TestVerdict, calculate_score, evaluate_rtt,
12+
TestConfigArgs, TestResult, TestVerdict, calculate_score, evaluate_rtt,
1713
must_output_to_file_on_quiet, publish_result_to_obol_api, request_rtt, write_result_to_file,
1814
write_result_to_writer,
1915
};
@@ -141,7 +137,7 @@ pub async fn run(
141137
filtered
142138
};
143139
if queued_tests.is_empty() {
144-
return Err(CliError::Other("test case not supported".to_string()));
140+
return Err(CliError::TestCaseNotSupported);
145141
}
146142

147143
let token = token.child_token();
@@ -235,7 +231,7 @@ async fn test_single_mev(
235231
tokio::select! {
236232
_ = token.cancelled() => {
237233
let tr = TestResult::new(&tc_name.name);
238-
tr.fail(TestResultError::from_string("timeout/interrupted"))
234+
tr.fail(CliError::TimeoutInterrupted)
239235
}
240236
r = test_case.run(&target, &conf) => {
241237
r
@@ -404,7 +400,7 @@ async fn mev_create_block_test(target: &str, conf: &TestMevArgs) -> TestResult {
404400
}
405401

406402
if all_blocks_rtt.is_empty() {
407-
return test_res.fail(CliError::Other("timeout/interrupted".to_string()));
403+
return test_res.fail(CliError::TimeoutInterrupted);
408404
}
409405

410406
let total_rtt: Duration = all_blocks_rtt.iter().sum();
@@ -474,16 +470,9 @@ async fn latest_beacon_block(endpoint: &str) -> Result<BeaconBlockMessage> {
474470

475471
let resp = apply_basic_auth(client.get(&clean_url), creds)
476472
.send()
477-
.await
478-
.map_err(|e| CliError::Other(format!("http request do: {e}")))?;
479-
480-
let body = resp
481-
.bytes()
482-
.await
483-
.map_err(|e| CliError::Other(format!("http response body: {e}")))?;
484-
485-
let block: BeaconBlock = serde_json::from_slice(&body)
486-
.map_err(|e| CliError::Other(format!("http response json: {e}")))?;
473+
.await?;
474+
let body = resp.bytes().await?;
475+
let block: BeaconBlock = serde_json::from_slice(&body)?;
487476

488477
Ok(block.data.message)
489478
}
@@ -498,16 +487,9 @@ async fn fetch_proposers_for_epoch(
498487

499488
let resp = apply_basic_auth(client.get(&clean_url), creds)
500489
.send()
501-
.await
502-
.map_err(|e| CliError::Other(format!("http request do: {e}")))?;
503-
504-
let body = resp
505-
.bytes()
506-
.await
507-
.map_err(|e| CliError::Other(format!("http response body: {e}")))?;
508-
509-
let duties: ProposerDuties = serde_json::from_slice(&body)
510-
.map_err(|e| CliError::Other(format!("http response json: {e}")))?;
490+
.await?;
491+
let body = resp.bytes().await?;
492+
let duties: ProposerDuties = serde_json::from_slice(&body)?;
511493

512494
Ok(duties.data)
513495
}
@@ -530,8 +512,7 @@ async fn get_block_header(
530512
let url =
531513
format!("{target}/eth/v1/builder/header/{next_slot}/{block_hash}/{validator_pub_key}");
532514

533-
let (clean_url, creds) = parse_endpoint_credentials(&url)
534-
.map_err(|e| MevError::Cli(CliError::Other(format!("parse url: {e}"))))?;
515+
let (clean_url, creds) = parse_endpoint_credentials(&url).map_err(MevError::from)?;
535516

536517
let client = reqwest::Client::new();
537518
let start = Instant::now();
@@ -548,21 +529,18 @@ async fn get_block_header(
548529
)
549530
.send()
550531
.await
551-
.map_err(|e| MevError::Cli(CliError::Other(format!("http request rtt: {e}"))))?;
532+
.map_err(|e| MevError::Cli(e.into()))?;
552533

553534
let rtt = start.elapsed();
554535

555536
if resp.status() != StatusCode::OK {
556537
return Err(MevError::StatusCodeNot200);
557538
}
558539

559-
let body = resp
560-
.bytes()
561-
.await
562-
.map_err(|e| MevError::Cli(CliError::Other(format!("http response body: {e}"))))?;
540+
let body = resp.bytes().await.map_err(|e| MevError::Cli(e.into()))?;
563541

564-
let bid: BuilderBidResponse = serde_json::from_slice(&body)
565-
.map_err(|e| MevError::Cli(CliError::Other(format!("http response json: {e}"))))?;
542+
let bid: BuilderBidResponse =
543+
serde_json::from_slice(&body).map_err(|e| MevError::Cli(e.into()))?;
566544

567545
Ok((bid, rtt))
568546
}

crates/cli/src/error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ pub enum CliError {
8181

8282
/// Test timeout or interrupted.
8383
#[error("timeout/interrupted")]
84-
_TimeoutInterrupted,
84+
TimeoutInterrupted,
8585

8686
/// Test case not supported.
8787
#[error("test case not supported")]
88-
_TestCaseNotSupported,
88+
TestCaseNotSupported,
8989

9090
/// Generic error with message.
9191
#[error("{0}")]

0 commit comments

Comments
 (0)