Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Feature: market close order #3244

Closed
wants to merge 5 commits into from
Closed

Feature: market close order #3244

wants to merge 5 commits into from

Conversation

bonomat
Copy link
Collaborator

@bonomat bonomat commented Nov 9, 2022

The taker will market close his position.

Resolves #2924

Note: this patch should go against 0.7.0 and can be released as a patch release from there.

Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
@klochowicz
Copy link
Contributor

@bonomat would you mind creating a branch that branches from release/0.7.0 and point this PR against that? It would be easier to review on github 🙏

.get(&(cfd.contract_symbol(), cfd.position().counter_position()))
.context("Cannot propose settlement without price")?;

if !offer.is_safe_to_take(OffsetDateTime::now_utc()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the frequent protocol failures the user might see some annoyance of "outdated offer" here when triggering collab settlement. This is unrelated to this fix, nothing to change here, but I wanted to point it out :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The timeout is currently set to 10 minutes . That's plenty of time to hopefully get an offer :D

Copy link
Contributor

@da-kami da-kami Nov 9, 2022

Choose a reason for hiding this comment

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

But that would mean that the only thing that protects us from using an outdated price is the automation's logic of "what is acceptable" as price. Is this fixing the problem of using an outdated price? Is the problem not that the price in the automation is potentially not up to date?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That should be fine. The taker simply proposes the latest price what he got from the maker in here. The logic whether to accept or decline this price is outside of this scope.
We simply spare the roundtrip and potentially protect the user by sending a settlement request with an outdated price.


let cfd = self.db.load_open_cfd::<Cfd>(order_id, ()).await?;

let proposal_closing_price = market_closing_price(bid, ask, Role::Taker, cfd.position());
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ We should remove the function market_closing_price and also use the maker's price in the projection - otherwise what we show in the UI does not match what is actually used!

@bonomat bonomat changed the base branch from master to release/0.7.0 November 9, 2022 03:05
@bonomat
Copy link
Collaborator Author

bonomat commented Nov 9, 2022

@bonomat would you mind creating a branch that branches from release/0.7.0 and point this PR against that? It would be easier to review on github 🙏

done :)

Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Copy link
Contributor

@da-kami da-kami left a comment

Choose a reason for hiding this comment

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

I think we can actually remove the model::market_closing_price - I think the now replaced usage in projection should have been the last usage of that function or did I miss something?

@bonomat
Copy link
Collaborator Author

bonomat commented Sep 12, 2023

This project is unmaintained now.

@bonomat bonomat closed this Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: market close order
3 participants