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

[PY] v0.6.6 only cancel Short-Term orders, break stateful order cancellations #38

Merged
merged 1 commit into from
Sep 20, 2023

Conversation

lucas-dydx
Copy link
Contributor

@lucas-dydx lucas-dydx commented Sep 20, 2023

This is a quick hack to get MM testnet onboarding unblocked. This should be fine since the Python client is not used by the frontend, and therefore doesn't send stateful order cancellations.

Slack thread for context.

@@ -155,8 +155,7 @@ def compose_msg_cancel_order(
)
return MsgCancelOrder(
order_id=order_id,
good_til_block=good_til_block,
good_til_block_time=good_til_block_time
good_til_block=good_til_block
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid overwriting the good_til_block attribute, the good_til_block_time attribute is unset.

This temporarily breaks stateful order cancellations, the tradeoff is considered worth it since the Python client shouldn't be used for stateful order placements or cancellations.

Copy link

@teddyding teddyding Sep 20, 2023

Choose a reason for hiding this comment

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

Is there a discussion thread somewhere? If so can you link
Do we have a tracking item to fix stateful order cancellations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's in the slack thread. This python client isn't used for stateful order cancellations at all, it should only be used by programmatic traders.

@lucas-dydx lucas-dydx marked this pull request as ready for review September 20, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants