Skip to content

Commit

Permalink
rework comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@m-lord-renkse
m-lord-renkse committed Oct 17, 2024
1 parent 17dbb88 commit bec1368
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 134 deletions.
4 changes: 2 additions & 2 deletions crates/autopilot/src/infra/solvers/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct Driver {
// winning solution should be discarded if it contains at least one order, which
// another driver solved with surplus exceeding this driver's surplus by `threshold`
pub fairness_threshold: Option<eth::Ether>,
pub submission_address: Option<eth::Address>,
pub submission_address: eth::Address,
client: Client,
}

Expand All @@ -28,7 +28,7 @@ impl Driver {
url: Url,
name: String,
fairness_threshold: Option<eth::Ether>,
submission_address: Option<eth::Address>,
submission_address: eth::Address,
) -> Self {
Self {
name,
Expand Down
4 changes: 2 additions & 2 deletions crates/autopilot/src/run.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ pub async fn run(args: Arguments) {
driver.url,
driver.name,
driver.fairness_threshold.map(Into::into),
driver.submission_address.map(Into::into),
driver.submission_address.into(),
))
})
.collect(),
Expand Down Expand Up @@ -570,7 +570,7 @@ async fn shadow_mode(args: Arguments) -> ! {
driver.url,
driver.name,
driver.fairness_threshold.map(Into::into),
driver.submission_address.map(Into::into),
driver.submission_address.into(),
))
})
.collect();
Expand Down
120 changes: 40 additions & 80 deletions crates/autopilot/src/run_loop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,97 +721,57 @@ impl RunLoop {
request: &solve::Request,
) -> Result<Vec<Result<competition::Solution, domain::competition::SolutionError>>, SolveError>
{
// @TODO: to be deleted once the submission address in the driver configuration
// is not optional
if let Some(submission_address) = driver.submission_address {
let authenticator = self.eth.contracts().authenticator();
let is_allowed = authenticator
.is_solver(submission_address.into())
.call()
.await;

// Do not send the request to the driver if the solver is denied
match is_allowed {
Ok(true) => {}
Ok(false) => return Err(SolveError::SolverDenyListed),
Err(err) => {
// log warning but discard solution anyway to be on the safe side
tracing::warn!(
driver = driver.name,
?driver.submission_address,
?err,
"failed to check if solver is deny listed"
);
return Err(SolveError::SolverDenyListed);
}
}
let authenticator = self.eth.contracts().authenticator();
let is_allowed = authenticator
.is_solver(driver.submission_address.into())
.call()
.await;

let response = tokio::time::timeout(self.config.solve_deadline, driver.solve(request))
.await
.map_err(|_| SolveError::Timeout)?
.map_err(SolveError::Failure)?;
if response.solutions.is_empty() {
return Err(SolveError::NoSolutions);
// Do not send the request to the driver if the solver is denied
match is_allowed {
Ok(true) => {}
Ok(false) => return Err(SolveError::SolverDenyListed),
Err(err) => {
tracing::warn!(
driver = driver.name,
?driver.submission_address,
?err,
"failed to check if solver is deny listed"
);
return Err(SolveError::SolverDenyListed);
}
// Filter the responses
Ok(response
.into_domain()
.into_iter()
.filter(|solution| {
solution
.as_ref()
.ok()
.map(|solution| solution.solver() == submission_address)
.unwrap_or_default()
})
.collect())
} else {
self.try_solve_legacy(driver, request).await
}
}

// @TODO: Legacy try_solve, to be deleted once the submission address in the
// driver configuration is not optional
async fn try_solve_legacy(
&self,
driver: &infra::Driver,
request: &solve::Request,
) -> Result<Vec<Result<competition::Solution, domain::competition::SolutionError>>, SolveError>
{
let response = tokio::time::timeout(self.config.solve_deadline, driver.solve(request))
.await
.map_err(|_| SolveError::Timeout)?
.map_err(SolveError::Failure)?;
if response.solutions.is_empty() {
return Err(SolveError::NoSolutions);
}
let solutions = response.into_domain();

// TODO: remove this workaround when implementing #2780
// Discard any solutions from solvers that got deny listed in the mean time.
let futures = solutions.into_iter().map(|solution| async {
let solution = solution?;
let solver = solution.solver();
let authenticator = self.eth.contracts().authenticator();
let is_allowed = authenticator.is_solver(solver.into()).call().await;

match is_allowed {
Ok(true) => Ok(solution),
Ok(false) => Err(domain::competition::SolutionError::SolverDenyListed),
Err(err) => {
// log warning but discard solution anyway to be on the safe side
tracing::warn!(
driver = driver.name,
?solver,
?err,
"failed to check if solver is deny listed"
);
Err(domain::competition::SolutionError::SolverDenyListed)
}
}
});

Ok(futures::future::join_all(futures).await)
// Filter the responses
Ok(response
.into_domain()
.into_iter()
.filter(|solution| {
solution
.as_ref()
.ok()
.map(|solution| {
let is_solution_from_driver =
solution.solver() == driver.submission_address;
if !is_solution_from_driver {
tracing::warn!(
driver = driver.name,
?driver.submission_address,
"the solution is not received from the driver submission address"
);
}
is_solution_from_driver
})
.unwrap_or_default()
})
.collect())
}

/// Execute the solver's solution. Returns Ok when the corresponding
Expand Down
64 changes: 14 additions & 50 deletions crates/shared/src/arguments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ macro_rules! logging_args_with_default_filter {
pub struct ExternalSolver {
pub name: String,
pub url: Url,
pub submission_address: Option<H160>,
pub submission_address: H160,
pub fairness_threshold: Option<U256>,
}

Expand Down Expand Up @@ -480,36 +480,17 @@ impl FromStr for ExternalSolver {

fn from_str(solver: &str) -> Result<Self> {
let parts: Vec<&str> = solver.split('|').collect();
ensure!(parts.len() >= 2, "not enough arguments for external solver");
ensure!(parts.len() >= 3, "not enough arguments for external solver");
let (name, url) = (parts[0], parts[1]);
let url: Url = url.parse()?;
let mut submission_address = None;
let mut fairness_threshold = None;

// @TODO: To remove the complex logic one submission address is not optional
if parts.len() == 4 {
submission_address = Some(
H160::from_str(parts.get(2).unwrap())
.context("failed to parse submission address")?,
);
fairness_threshold = Some(
U256::from_dec_str(parts.get(3).unwrap())
.context("failed to parse fairness threshold")?,
);
}
if parts.len() == 3 {
let part = parts.get(2).unwrap();
if let Ok(value) = H160::from_str(part) {
submission_address = Some(value);
}
if let Ok(value) = U256::from_dec_str(part) {
fairness_threshold = Some(value);
let submission_address =
H160::from_str(parts[2]).context("failed to parse submission address")?;
let fairness_threshold = match parts.get(3) {
Some(value) => {
Some(U256::from_dec_str(value).context("failed to parse fairness threshold")?)
}
ensure!(
submission_address.is_some() || fairness_threshold.is_some(),
"failed to parse external solver arguments"
);
}
None => None,
};

Ok(Self {
name: name.to_owned(),
Expand All @@ -526,55 +507,38 @@ mod test {

#[test]
fn parse_driver() {
let argument = "name1|http://localhost:8080";
let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
let driver = ExternalSolver::from_str(argument).unwrap();
let expected = ExternalSolver {
name: "name1".into(),
url: Url::parse("http://localhost:8080").unwrap(),
fairness_threshold: None,
submission_address: None,
submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")),
};
assert_eq!(driver, expected);
}

#[test]
fn parse_driver_with_threshold() {
let argument = "name1|http://localhost:8080|1000000000000000000";
let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000";
let driver = ExternalSolver::from_str(argument).unwrap();
let expected = ExternalSolver {
name: "name1".into(),
url: Url::parse("http://localhost:8080").unwrap(),
submission_address: None,
submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")),
fairness_threshold: Some(U256::exp10(18)),
};
assert_eq!(driver, expected);
}

#[test]
fn parse_driver_with_submission_address() {
let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
let driver = ExternalSolver::from_str(argument).unwrap();
let expected = ExternalSolver {
name: "name1".into(),
url: Url::parse("http://localhost:8080").unwrap(),
submission_address: Some(H160::from_slice(&hex!(
"C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"
))),
fairness_threshold: None,
};
assert_eq!(driver, expected);
}

#[test]
fn parse_driver_with_submission_address_and_threshold() {
let argument = "name1|http://localhost:8080|0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2|1000000000000000000";
let driver = ExternalSolver::from_str(argument).unwrap();
let expected = ExternalSolver {
name: "name1".into(),
url: Url::parse("http://localhost:8080").unwrap(),
submission_address: Some(H160::from_slice(&hex!(
"C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"
))),
submission_address: H160::from_slice(&hex!("C02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2")),
fairness_threshold: Some(U256::exp10(18)),
};
assert_eq!(driver, expected);
Expand Down

0 comments on commit bec1368

Please sign in to comment.