Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion crates/driver/src/infra/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub struct Api {
pub mempools: Mempools,
pub addr: SocketAddr,
pub bad_token_detector: risk_detector::bad_tokens::Detector,
pub disable_settlement_balance_fetcher: bool,
/// If this channel is specified, the bound address will be sent to it. This
/// allows the driver to bind to 0.0.0.0:0 during testing.
pub addr_sender: Option<oneshot::Sender<SocketAddr>>,
Expand All @@ -61,7 +62,11 @@ impl Api {
self.eth.current_block().clone(),
);

let tokens = tokens::Fetcher::new(&self.eth);
let tokens = if self.disable_settlement_balance_fetcher {
tokens::Fetcher::disabled()
} else {
tokens::Fetcher::new(&self.eth)
};
let fetcher = Arc::new(domain::competition::DataAggregator::new(
self.eth.clone(),
app_data_retriever.clone(),
Expand Down
1 change: 1 addition & 0 deletions crates/driver/src/infra/config/file/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,5 +391,6 @@ pub async fn load(chain: Chain, path: &Path) -> infra::Config {
simulation_bad_token_max_age: config.simulation_bad_token_max_age,
app_data_fetching: config.app_data_fetching,
tx_gas_limit: config.tx_gas_limit,
disable_settlement_balance_fetcher: config.disable_settlement_balance_fetcher,
}
}
6 changes: 6 additions & 0 deletions crates/driver/src/infra/config/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ struct Config {
#[serde(default)]
flashloans_enabled: bool,

/// Skip settlement contract token balance fetching (background and
/// on-demand). Balances default to 0. Useful for local testing to
/// avoid RPC calls.
#[serde(default)]
disable_settlement_balance_fetcher: bool,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is applied wholesale to the driver, as the driver is intended to handle different solvers this is a setting that should be handled on a per-solver level.


#[serde_as(as = "HexOrDecimalU256")]
tx_gas_limit: eth::U256,
}
Expand Down
1 change: 1 addition & 0 deletions crates/driver/src/infra/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,5 @@ pub struct Config {
pub simulation_bad_token_max_age: Duration,
pub app_data_fetching: AppDataFetching,
pub tx_gas_limit: eth::U256,
pub disable_settlement_balance_fetcher: bool,
}
35 changes: 30 additions & 5 deletions crates/driver/src/infra/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,31 @@ pub struct Metadata {
}

#[derive(Clone)]
pub struct Fetcher(Arc<Inner>);
pub enum Fetcher {
/// Fetches token metadata and settlement contract balances from the node.
Rpc(Arc<Inner>),
/// Returns zero balances without making any RPC calls.
Disabled,
}

impl Fetcher {
pub fn new(eth: &Ethereum) -> Self {
let eth = eth.with_metric_label("tokenInfos".into());
let block_stream = eth.current_block().clone();
let inner = Arc::new(Inner {
eth,
cache: RwLock::new(HashMap::new()),
requests: BoxRequestSharing::labelled("token_info".into()),
});
let block_stream = inner.eth.current_block().clone();
tokio::task::spawn(
update_task(block_stream, Arc::downgrade(&inner))
.instrument(tracing::info_span!("token_fetcher")),
);
Self(inner)
Self::Rpc(inner)
}

pub fn disabled() -> Self {
Self::Disabled
}

/// Returns the `Metadata` for the given tokens. Note that the result will
Expand All @@ -50,7 +59,23 @@ impl Fetcher {
&self,
addresses: &[eth::TokenAddress],
) -> HashMap<eth::TokenAddress, Metadata> {
self.0.get(addresses).await
match self {
Self::Rpc(inner) => inner.get(addresses).await,
Self::Disabled => addresses
.iter()
.filter(|address| address.0.0 != BUY_ETH_ADDRESS)
.map(|address| {
(
*address,
Metadata {
decimals: None,
symbol: None,
balance: 0.into(),
},
)
})
.collect(),
}
}
}

Expand Down Expand Up @@ -120,7 +145,7 @@ async fn update_balances(inner: Arc<Inner>) -> Result<(), blockchain::Error> {
}

/// Provides metadata of tokens.
struct Inner {
pub struct Inner {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pub and Inner should never go in the "same line" as Inner implies internal implementation and should not be exposed.

I like the enum approach as it has some advantages in terms of speed, but it brings more complexities in maintenance; since we're not chasing sub-ms speeds yet, this is unnecessary.

One reason it doesn't scale well in terms of maintenance is the burden of handling different branches, for example, in get we'd need matching on every enum variant we add. Leading to a big match statement that to be eventually be made ergonomic will require more "inner structs" that implement a given trait/interface, which is just a less ergonomic trait implementation.

I think the best way to go about this right now is:

  • Split the enum into two structs — RpcFetcher & DisabledFetcher
  • Extract the get into a trait called SettlementBalanceFetching
  • Implement the trait on the previous structs
  • Update the type of let tokens in the crates/driver/src/infra/api/mod.rs to be Arc<dyn SettlementBalanceFetching>
  • Update where necessary to Arc too (old tokens::Fetcher basically)

eth: Ethereum,
cache: RwLock<HashMap<eth::TokenAddress, Metadata>>,
requests: BoxRequestSharing<eth::TokenAddress, Option<(eth::TokenAddress, Metadata)>>,
Expand Down
1 change: 1 addition & 0 deletions crates/driver/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ async fn run_with(args: cli::Args, addr_sender: Option<oneshot::Sender<SocketAdd
config.simulation_bad_token_max_age,
&eth,
),
disable_settlement_balance_fetcher: config.disable_settlement_balance_fetcher,
eth,
addr: args.addr,
addr_sender,
Expand Down
Loading