-
Notifications
You must be signed in to change notification settings - Fork 51
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
[Perpetual]: add stop loss price #759
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
==========================================
+ Coverage 43.06% 43.08% +0.01%
==========================================
Files 637 640 +3
Lines 20900 21005 +105
==========================================
+ Hits 9001 9049 +48
- Misses 10830 10882 +52
- Partials 1069 1074 +5 |
@cryptokage1996 can you provide test results? |
I see we are only setting stop loss value, shouldn't we add logic for stop loss trigger ? when this will trigger ?
|
in the ticket it is not mentioned to close position when stop loss is hit, should i do it like we do for leveragelp @amityadav0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@cryptokage1996 @amityadav0 I agree without the ABCI logic for trigger the stop loss (closing of the position) it will be useful at all, is there any reason we are ommiting that part? |
@cosmic-vagabond there is a new ticket to implement this |
@cryptokage1996 the software upgrade test is failing due to a module migration error see here https://github.com/elys-network/elys/actions/runs/10771967355/job/29870663829?pr=759 |
@cosmic-vagabond i dont know whats the issue here , code is fine i updated the registry and added migration from v6 to v7 it is still looking for v5 to v6 migration. i think this is issue bcoz the previous pr has migration in same module(perpetual) could be the reason , also iam not totally sure. @amityadav0 @avkr003 can your help here.is there anything iam missing? Thanks! |
@cryptokage1996 Multiple migrations are happening here, and your code is skipping one version. |
Add message to update the stop loss of a given position.