-
Notifications
You must be signed in to change notification settings - Fork 116
fix(makerbot): allow more than one prices url in makerbot #2027
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
eab45fb
50c464f
7e7f5e3
21296f8
c89e67f
147144d
d8f884b
4d93e98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ use crate::mm2::{lp_ordermatch::{cancel_order, create_maker_order, | |
| update_maker_order, CancelOrderReq, MakerOrder, MakerOrderUpdateReq, | ||
| OrdermatchContext, SetPriceReq}, | ||
| lp_swap::{latest_swaps_for_pair, LatestSwapsErr}}; | ||
| use coins::lp_price::{fetch_price_tickers, Provider, RateInfos}; | ||
| use coins::lp_price::{fetch_price_tickers, Provider, RateInfos, PRICE_ENDPOINTS}; | ||
| use coins::{lp_coinfind, GetNonZeroBalance}; | ||
| use common::{executor::{SpawnFuture, Timer}, | ||
| log::{debug, error, info, warn}, | ||
|
|
@@ -23,7 +23,6 @@ use std::collections::{HashMap, HashSet}; | |
| use uuid::Uuid; | ||
|
|
||
| // !< constants | ||
| pub const KMD_PRICE_ENDPOINT: &str = "https://prices.komodian.info/api/v2/tickers"; | ||
| pub const BOT_DEFAULT_REFRESH_RATE: f64 = 30.0; | ||
| pub const PRECISION_FOR_NOTIFICATION: u64 = 8; | ||
| const LATEST_SWAPS_LIMIT: usize = 1000; | ||
|
|
@@ -110,10 +109,36 @@ impl From<std::string::String> for OrderProcessingError { | |
| fn from(error: std::string::String) -> Self { OrderProcessingError::LegacyError(error) } | ||
| } | ||
|
|
||
| #[derive(Deserialize)] | ||
| enum PriceSources { | ||
| #[serde(rename = "price_url")] | ||
| Singular(String), | ||
| #[serde(rename = "price_urls")] | ||
| Multiple(Vec<String>), | ||
| } | ||
|
|
||
| impl Default for PriceSources { | ||
| fn default() -> Self { PriceSources::Multiple(PRICE_ENDPOINTS.iter().map(ToString::to_string).collect()) } | ||
| } | ||
|
|
||
| impl PriceSources { | ||
| /// # Important | ||
| /// | ||
| /// Always use this to get the data | ||
| fn get_urls(&self) -> Vec<String> { | ||
| match self { | ||
| // TODO: deprecate price_url soon and inform the users | ||
| PriceSources::Singular(url) => vec![url.clone()], | ||
| PriceSources::Multiple(urls) => urls.clone(), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Deserialize)] | ||
| pub struct StartSimpleMakerBotRequest { | ||
| cfg: SimpleMakerBotRegistry, | ||
| price_url: Option<String>, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we please leave the singular There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No error is thrown if using the old param, looks like it uses the defaults. I'll can mark it as deprecated in docs, could we also add a log message in the bot update loop when it is used to warn users of the deprecation?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Mmm, I guess to do that I have to keep
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A third option is to revert There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I would support both at the same time with an enum. e.g.,: enum PriceSourceType {
#[serde(rename = "price_url")]
Singular(String),
#[serde(rename = "price_urls")]
Multiple(Vec<String>),
}
impl PriceSourceType {
/// Always use this to get the data
fn get_urls(&self) -> Vec<String> {
match self {
// TODO: deprecate price_url soon and inform the users
PriceSourceType::Singular(t) => vec![t.clone()],
PriceSourceType::Multiple(t) => t.clone(),
}
}
}
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| #[serde(default, flatten)] | ||
| price_sources: PriceSources, | ||
| bot_refresh_rate: Option<f64>, | ||
| } | ||
|
|
||
|
|
@@ -614,81 +639,68 @@ async fn execute_create_single_order( | |
|
|
||
| async fn process_bot_logic(ctx: &MmArc) { | ||
| let simple_market_maker_bot_ctx = TradingBotContext::from_ctx(ctx).unwrap(); | ||
| let state = simple_market_maker_bot_ctx.trading_bot_states.lock().await; | ||
| let (cfg, price_url) = if let TradingBotState::Running(running_state) = &*state { | ||
| let res = (running_state.trading_bot_cfg.clone(), running_state.price_url.clone()); | ||
| drop(state); | ||
| res | ||
| } else { | ||
| drop(state); | ||
| return; | ||
| let mut state = simple_market_maker_bot_ctx.trading_bot_states.lock().await; | ||
| let running_state = match &mut *state { | ||
| TradingBotState::Running(running_state) => running_state, | ||
| TradingBotState::Stopping(_) | TradingBotState::Stopped(_) => return, | ||
| }; | ||
| let rates_registry = match fetch_price_tickers(price_url.as_str()).await { | ||
| Ok(model) => { | ||
| info!("price successfully fetched from {price_url}"); | ||
| model | ||
| }, | ||
|
|
||
| let cfg = running_state.trading_bot_cfg.clone(); | ||
| let rates_registry = match fetch_price_tickers(&mut running_state.price_urls).await { | ||
| Ok(model) => model, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let cfg = running_state.trading_bot_cfg.clone();
let rates_registry = match fetch_price_tickers(&mut running_state.price_urls).await {
Ok(model) => model,
Err(err) => {
let nb_orders = cancel_pending_orders(ctx, &cfg).await;
error!("error fetching price: {err:?} - cancel {nb_orders} orders");
return;
},
};
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| Err(err) => { | ||
| let nb_orders = cancel_pending_orders(ctx, &cfg).await; | ||
| error!("error fetching price: {err:?} - cancel {nb_orders} orders"); | ||
| return; | ||
| }, | ||
| }; | ||
|
|
||
| drop(state); | ||
borngraced marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| let mut memoization_pair_registry: HashSet<String> = HashSet::new(); | ||
| let ordermatch_ctx = OrdermatchContext::from_ctx(ctx).unwrap(); | ||
| let maker_orders = ordermatch_ctx.maker_orders_ctx.lock().orders.clone(); | ||
| let mut futures_order_update = Vec::with_capacity(0); | ||
| // Iterating over maker orders and update order that are present in cfg as the key_trade_pair e.g KMD/LTC | ||
| for (uuid, order_mutex) in maker_orders.into_iter() { | ||
| let mut futures_order_update = Vec::with_capacity(maker_orders.len()); | ||
| for (uuid, order_mutex) in maker_orders { | ||
| let order = order_mutex.lock().await; | ||
| let key_trade_pair = TradingPair::new(order.base.clone(), order.rel.clone()); | ||
| match cfg.get(&key_trade_pair.as_combination()) { | ||
| Some(coin_cfg) => { | ||
| if !coin_cfg.enable { | ||
| continue; | ||
| } | ||
| let cloned_infos = ( | ||
| ctx.clone(), | ||
| rates_registry | ||
| .get_cex_rates(&coin_cfg.base, &coin_cfg.rel) | ||
| .unwrap_or_default(), | ||
| key_trade_pair.clone(), | ||
| coin_cfg.clone(), | ||
| ); | ||
| futures_order_update.push(execute_update_order(uuid, order.clone(), cloned_infos)); | ||
| memoization_pair_registry.insert(key_trade_pair.as_combination()); | ||
| }, | ||
| _ => continue, | ||
|
|
||
| if let Some(coin_cfg) = cfg.get(&key_trade_pair.as_combination()) { | ||
| if !coin_cfg.enable { | ||
| continue; | ||
| } | ||
| let cloned_infos = ( | ||
| ctx.clone(), | ||
| rates_registry | ||
| .get_cex_rates(&coin_cfg.base, &coin_cfg.rel) | ||
| .unwrap_or_default(), | ||
| key_trade_pair.clone(), | ||
| coin_cfg.clone(), | ||
| ); | ||
| futures_order_update.push(execute_update_order(uuid, order.clone(), cloned_infos)); | ||
| memoization_pair_registry.insert(key_trade_pair.as_combination()); | ||
| } | ||
| } | ||
|
|
||
| let all_updated_orders_tasks = futures::future::join_all(futures_order_update); | ||
| let _results_order_updates = all_updated_orders_tasks.await; | ||
| let _results_order_updates = futures::future::join_all(futures_order_update).await; | ||
|
|
||
| let mut futures_order_creation = Vec::with_capacity(0); | ||
| let mut futures_order_creation = Vec::with_capacity(cfg.len()); | ||
| // Now iterate over the registry and for every pairs that are not hit let's create an order | ||
| for (trading_pair, cur_cfg) in cfg.into_iter() { | ||
| match memoization_pair_registry.get(&trading_pair) { | ||
| Some(_) => continue, | ||
| None => { | ||
| if !cur_cfg.enable { | ||
| continue; | ||
| } | ||
| let rates_infos = rates_registry | ||
| .get_cex_rates(&cur_cfg.base, &cur_cfg.rel) | ||
| .unwrap_or_default(); | ||
| futures_order_creation.push(execute_create_single_order( | ||
| rates_infos, | ||
| cur_cfg, | ||
| trading_pair.clone(), | ||
| ctx, | ||
| )); | ||
| }, | ||
| }; | ||
| for (trading_pair, cur_cfg) in cfg { | ||
| if memoization_pair_registry.get(&trading_pair).is_some() || !cur_cfg.enable { | ||
| continue; | ||
| } | ||
| let rates_infos = rates_registry | ||
| .get_cex_rates(&cur_cfg.base, &cur_cfg.rel) | ||
| .unwrap_or_default(); | ||
| futures_order_creation.push(execute_create_single_order( | ||
| rates_infos, | ||
| cur_cfg, | ||
| trading_pair.clone(), | ||
| ctx, | ||
| )); | ||
| } | ||
| let all_created_orders_tasks = futures::future::join_all(futures_order_creation); | ||
| let _results_order_creations = all_created_orders_tasks.await; | ||
| let _results_order_creations = futures::future::join_all(futures_order_creation).await; | ||
| } | ||
|
|
||
| pub async fn lp_bot_loop(ctx: MmArc) { | ||
|
|
@@ -738,7 +750,7 @@ pub async fn start_simple_market_maker_bot(ctx: MmArc, req: StartSimpleMakerBotR | |
| *state = RunningState { | ||
| trading_bot_cfg: req.cfg, | ||
| bot_refresh_rate: refresh_rate, | ||
| price_url: req.price_url.unwrap_or_else(|| KMD_PRICE_ENDPOINT.to_string()), | ||
| price_urls: req.price_sources.get_urls(), | ||
| } | ||
| .into(); | ||
| drop(state); | ||
|
|
@@ -793,15 +805,15 @@ mod tests { | |
| let another_cloned_ctx = ctx.clone(); | ||
| let req = StartSimpleMakerBotRequest { | ||
| cfg: Default::default(), | ||
| price_url: None, | ||
| price_sources: Default::default(), | ||
| bot_refresh_rate: None, | ||
| }; | ||
| let answer = block_on(start_simple_market_maker_bot(ctx, req)).unwrap(); | ||
| assert_eq!(answer.get_result(), "Success"); | ||
|
|
||
| let req = StartSimpleMakerBotRequest { | ||
| cfg: Default::default(), | ||
| price_url: None, | ||
| price_sources: Default::default(), | ||
| bot_refresh_rate: None, | ||
| }; | ||
| let answer = block_on(start_simple_market_maker_bot(cloned_ctx, req)); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.