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

When using "Close position" function I dont get a green fill notification #5110

Closed
JonRay15 opened this issue Oct 24, 2023 · 1 comment · Fixed by #5211
Closed

When using "Close position" function I dont get a green fill notification #5110

JonRay15 opened this issue Oct 24, 2023 · 1 comment · Fixed by #5211
Assignees
Labels
bug 🐛 feedback Feedback from real-world users

Comments

@JonRay15
Copy link
Contributor

JonRay15 commented Oct 24, 2023

Description

A clear and concise description of what the bug is.

Steps to Reproduce

  1. Go to console
  2. Open a position
  3. Close it using the close position button
  4. You will see an outlandishly large TX size in your wallet
  5. Approve the transaction in your wallet
  6. You will not get a green fill notification just an orange stopped one
  7. You will see an outlandishly large size in your closed orders as per this ticket from a user

Expected behavior

You should get the normal green fill toast to indicate your order has been filled and the positon is closed.

Instead I get this "ORDER STOPPED" toast even though the order did go through and my positoin is now closed.

We think this is due to a change @macqbat made to use a massive reduce only order for this instead of using an order matching the size of the position. We think this was done to address some edge case where if an open order traded just before you closed the position you might not completely close out your full position.

In my view that is an acceptable edge case ... if you are using close position and you have open orders, you should check after you've pressed the button if there's anything left and close again if something has carried over. This minor edge case is preferable to everyone seeing these really high values and / or trying to now build handling for them. We should roll this change back.

Agreed approch (Now challenged by Barney)

  • Roll back change to use a massive reduce only order
  • Go back to original implementation where we assess the size of the position and close it out
  • IMO we should provide some info on the button ... the current tooltip says "Close position" ... I think we should extend this to "Close position and cancel all open orders for this market"

New agreed approach (Barney)

  • Toast should detect reduce only + size of 9223372036854775807 (might already do this)
  • Toast should show "close position" - it seems to actually do this already
  • Toast for an IOC should go green when it is stopped, because stopping an IOC once all available volume has filled is the correct behaviour (it is immediate or cancel), this behaviour should apply to all IOC, not just those created due to "Close position"
  • Toast for IOC should show on it how much has filled before it was stopped, just the same as a completed IOC, ie. "+0.10 @ ~ USDT"
  • The order table should also detect reduce only + size of 9223372036854775807 and instead of showing the size should show "MAX"

Image

Screenshots

image

Additional context

Add any other context about the problem here.

@JonRay15
Copy link
Contributor Author

JonRay15 commented Nov 2, 2023

Per @barnabee

My thoughts are:

  • use the reference field OR combo of size being max. possible and reduce only flag to detect close position
  • show bespoke toasts and order info for close position orders, would show amber or red if the position size after execution was non-zero
  • the “STOPPED” status for an IOC should otherwise probably always be a green toast but it should show how much is remaining

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 feedback Feedback from real-world users
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants