Skip to content

Commit

Permalink
Refactor solvers Configuration (#2351)
Browse files Browse the repository at this point in the history
# Description

As mentioned
[here](#1853 (comment))
we are moving the `infra` dependencies out of the `domain` module to
follow domain driven design principles.

# Changes

Misc:

- ~~Reduced standard output from tests by disabling doctests (there
didn't seem to be any)~~
- ~~Formatted the code by running `cargo fmt` (this affected a lot of
files)~~

Related Issue:

- Moved the configs from the `infra` modules into the `domain` modules
- Changed the `infra` modules, `baseline` and `naive`, to be files
instead of directories since the directories aren't needed here, and to
follow the `legacy` structure
- Updated the `run` module to 
  - Accept the configs that have been moved into `domain`
- Changed the paths to `load` the configs given that the directories
have been changed into files

## How to test

In the README.md there is a testing section.

The following sections have been run:

1. Unit Tests
2. DB Tests
3. E2E Tests - Local Node

## Related Issues

Closes #1853 

## Comments

I have refrained from doing any refactoring after the configs have been
moved into `domain` despite the similarity of certain private types
simply because I'm not familiar enough with the codebase to deem whether
such a refactor would be apt as part of this issue.

---------

Co-authored-by: Felix Leupold <felixleupold90@gmail.com>
  • Loading branch information
Braqzen and fleupold authored Feb 2, 2024
1 parent e6e1495 commit 4ca9a35
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 30 deletions.
11 changes: 9 additions & 2 deletions crates/solvers/src/domain/solver/baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use {
order::{self, UserOrder},
solution,
},
infra::config,
},
ethereum_types::U256,
std::{cmp, collections::HashSet, sync::Arc},
Expand All @@ -28,6 +27,14 @@ pub struct Baseline(Arc<Inner>);
/// reached.
const DEADLINE_SLACK: chrono::Duration = chrono::Duration::milliseconds(500);

pub struct Config {
pub weth: eth::WethAddress,
pub base_tokens: Vec<eth::TokenAddress>,
pub max_hops: usize,
pub max_partial_attempts: usize,
pub risk: domain::Risk,
}

struct Inner {
weth: eth::WethAddress,

Expand Down Expand Up @@ -56,7 +63,7 @@ struct Inner {

impl Baseline {
/// Creates a new baseline solver for the specified configuration.
pub fn new(config: config::baseline::Config) -> Self {
pub fn new(config: Config) -> Self {
Self(Arc::new(Inner {
weth: config.weth,
base_tokens: config.base_tokens.into_iter().collect(),
Expand Down
7 changes: 5 additions & 2 deletions crates/solvers/src/domain/solver/naive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,22 @@ use {
crate::{
boundary,
domain::{self, auction, eth, liquidity, order, solution},
infra::config,
},
std::collections::HashMap,
};

pub struct Config {
pub risk: domain::Risk,
}

pub struct Naive {
/// Parameters used to calculate the revert risk of a solution.
risk: domain::Risk,
}

impl Naive {
/// Creates a new naive solver for the specified configuration.
pub fn new(config: config::naive::Config) -> Self {
pub fn new(config: Config) -> Self {
Self { risk: config.risk }
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use {
crate::{
domain::{eth, Risk},
domain::{eth, solver::baseline, Risk},
infra::{config::unwrap_or_log, contracts},
util::serialize,
},
Expand Down Expand Up @@ -48,7 +48,7 @@ struct Config {
/// # Panics
///
/// This method panics if the config is invalid or on I/O errors.
pub async fn load(path: &Path) -> super::Config {
pub async fn load(path: &Path) -> baseline::Config {
let data = fs::read_to_string(path)
.await
.unwrap_or_else(|e| panic!("I/O error while reading {path:?}: {e:?}"));
Expand All @@ -66,7 +66,7 @@ pub async fn load(path: &Path) -> super::Config {
),
};

super::Config {
baseline::Config {
weth,
base_tokens: config
.base_tokens
Expand Down
11 changes: 0 additions & 11 deletions crates/solvers/src/infra/config/baseline/mod.rs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use {
crate::{domain::Risk, infra::config::unwrap_or_log},
crate::{
domain::{solver::naive, Risk},
infra::config::unwrap_or_log,
},
serde::Deserialize,
serde_with::serde_as,
std::path::Path,
Expand All @@ -20,13 +23,13 @@ struct Config {
/// # Panics
///
/// This method panics if the config is invalid or on I/O errors.
pub async fn load(path: &Path) -> super::Config {
pub async fn load(path: &Path) -> naive::Config {
let data = fs::read_to_string(path)
.await
.unwrap_or_else(|e| panic!("I/O error while reading {path:?}: {e:?}"));
// Not printing detailed error because it could potentially leak secrets.
let config = unwrap_or_log(toml::de::from_str::<Config>(&data), &path);
super::Config {
naive::Config {
risk: Risk {
gas_amount_factor: config.risk_parameters.0,
gas_price_factor: config.risk_parameters.1,
Expand Down
7 changes: 0 additions & 7 deletions crates/solvers/src/infra/config/naive/mod.rs

This file was deleted.

4 changes: 2 additions & 2 deletions crates/solvers/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ async fn run_with(args: cli::Args, bind: Option<oneshot::Sender<SocketAddr>>) {

let solver = match args.command {
cli::Command::Baseline { config } => {
let config = config::baseline::file::load(&config).await;
let config = config::baseline::load(&config).await;
Solver::Baseline(solver::Baseline::new(config))
}
cli::Command::Naive { config } => {
let config = config::naive::file::load(&config).await;
let config = config::naive::load(&config).await;
Solver::Naive(solver::Naive::new(config))
}
cli::Command::Legacy { config } => {
Expand Down

0 comments on commit 4ca9a35

Please sign in to comment.