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(vpool): Add check for maximum leverage for open positions #793

Merged
merged 12 commits into from
Aug 9, 2022

Conversation

matthiasmatt
Copy link
Contributor

@matthiasmatt matthiasmatt commented Aug 4, 2022

Fix #781

tasks:

  • Create the vpool parameters and add unit tests
  • Create tests for maintenance margin ratio / max leverage interaction
  • Check maximum leverage on open position in perp module
  • Tests behavior of open positions

@matthiasmatt matthiasmatt marked this pull request as ready for review August 4, 2022 12:54
@matthiasmatt matthiasmatt requested a review from a team as a code owner August 4, 2022 12:54
@matthiasmatt matthiasmatt changed the title feat(vpool/perp): Add check for maximum leverage for open positions feat(vpool): Add check for maximum leverage for open positions Aug 4, 2022
app.VpoolKeeper.CreatePool(
ctx,
common.PairBTCStable,
sdk.OneDec(),
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) would be nice to add argument comments for the other parameters too.

x/vpool/keeper/keeper_test.go Show resolved Hide resolved
x/vpool/keeper/keeper_test.go Outdated Show resolved Hide resolved
@@ -66,5 +66,13 @@ func (m *CreatePoolProposal) ValidateBasic() error {
return fmt.Errorf("maintenance margin ratio ratio must be 0 <= ratio <= 1")
}

if m.MaxLeverage.LT(sdk.OneDec()) {
return fmt.Errorf("Max leverage must be >= 1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be possible for users to open positions with < 1 leverage? They would essentially be opening an over-collateralized position because they maybe think the asset will fluctuate a lot in the short term, but long term will move in their position favorably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave that as a behavior, i think it causes no harm to let the user open those positions

@NibiruHeisenberg
Copy link
Contributor

Can you check the integration tests? They seem to have failed for this PR.

@matthiasmatt matthiasmatt merged commit 93d8963 into master Aug 9, 2022
@matthiasmatt matthiasmatt deleted the mat/add-leverage-check branch August 9, 2022 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(perp): Add a leverage for each vpool as a parameter
3 participants