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

fix: stop loss with highest priority #589

Merged

Conversation

uhliksk
Copy link
Contributor

@uhliksk uhliksk commented Jan 14, 2023

Description

  • Stop-loss is higher priority than buy order.
  • Added stop-loss highlighting in closed trades.
  • It will still allow to finish already opened buy order before stop-loss is triggered to check if stop-loss is unavoidable.

Related Issue

#424

Motivation and Context

I was highly motivated when it lost -0.77% with stop-loss set to -0.1% (Max loss percentage 0.999). It was much higher overshoot than what I'm able to accept as tolerable margin of error. It should not overshoot even on so tight percentages.

How Has This Been Tested?

Validated by tests but also in production.

Screenshots (if appropriate):

image

@chrisleekr chrisleekr added the bug Something isn't working label Jan 15, 2023
@uhliksk uhliksk force-pushed the fix/stop-loss-with-highest-priority branch from ab89ccd to a8cead0 Compare January 15, 2023 01:03
@chrisleekr
Copy link
Owner

@uhliksk
This bug is definitely something I would not be able to catch by myself. Nice catch!

Could you check my test case and see if that makes sense?

And is it good to merge?

@uhliksk
Copy link
Contributor Author

uhliksk commented Jan 15, 2023

@chrisleekr
Thank you. I think this test case is enough. I tested with previous version of code and it tried to buy which means this scenario is exactly what we need to have tested. It's good to merge. 👍

@chrisleekr chrisleekr merged commit 450e07c into chrisleekr:master Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants