Skip to content
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

feat: allow to configure coordinator leverage #2618

Closed
wants to merge 2 commits into from

Conversation

bonomat
Copy link
Contributor

@bonomat bonomat commented Jun 6, 2024

With this patch we introduce a new table: coordinator_leverages.
It contains two fields:

  • trader_leverage
  • coordinator_leverage

i.e. we can define a coordinator leverage for each possible trader_leverage. For simplcitiy reasons we only have round numbers in the database and no floating points, hence, when looking up a coordinator leverage for a trader leverage, we need to round. If no match is found, we fall back and take the max allowed leverage of the coordinator. This allows us to not have an entry from 1 - 100 but stop at e.g. 10 with a max coordinator leverage of 10. If a user then goes a 100x, we still pick leverage 10.

There is one downside of this patch: The coordinator's reserve now depends on the trader's leverage which we might not want.

bonomat added 2 commits June 6, 2024 21:28
With this patch we introduce a new table: `coordinator_leverages`.
It contains two fields:
- trader_leverage
- coordinator_leverage

i.e. we can define a coordinator leverage for each possible trader_leverage. For simplcitiy reasons we only have round numbers in the database and no floating points, hence, when looking up a coordinator leverage for a trader leverage, we need to round. If no match is found, we fall back and take the max allowed leverage of the coordinator. This allows us to not have an entry from 1 - 100 but stop at e.g. 10 with a max coordinator leverage of 10. If a user then goes a 100x, we still pick leverage 10.

There is one downside of this patch: The coordinator's reserve now depends on the trader's leverage which we might not want.
@bonomat bonomat requested review from holzeis and luckysori June 6, 2024 11:30
Base automatically changed from feat/higher-leverage to main June 6, 2024 23:21
Copy link
Contributor

@luckysori luckysori left a comment

Choose a reason for hiding this comment

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

I like the simplicity of this approach, but I don't get why we need the default coordinator leverage.

Comment on lines +44 to +45
tracing::error!("Failed at loading leverage from db. Falling back to 2.0 {err:#}");
2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

❓I guess we don't need this?

// TODO(bonomat): we should make this configurable as well. Ideally, we
// can differentiate between a _leverage_ when opening a position and a
// _multiplier_ when opening a channel.
let default_coordinator_leverage =
Copy link
Contributor

Choose a reason for hiding this comment

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

❓Why do we need a default coordinator leverage?

///
/// Unfortunately our version of flutter_rust_bridge does not support hashmaps yet
pub coordinator_leverages: Vec<CoordinatorLeverage>,
/// The leverage the coordinator will take if none is provided in `coordinator_leverages`
Copy link
Contributor

Choose a reason for hiding this comment

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

❓Do we really need to cater for this situation by adding a default? If coordinator_leverages is empty I think something went very wrong and we shouldn't allow trading anyway.

@github-actions github-actions bot added the Stale label Jul 8, 2024
@github-actions github-actions bot closed this Jul 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants