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

Support take profit order same as "stoploss_on_exchange" on Binance #7499

Open
liaogx opened this issue Sep 29, 2022 · 13 comments
Open

Support take profit order same as "stoploss_on_exchange" on Binance #7499

liaogx opened this issue Sep 29, 2022 · 13 comments
Labels
Enhancement Enhancements to the bot. Get lower priority than bugs by default.

Comments

@liaogx
Copy link

liaogx commented Sep 29, 2022

Describe your environment

  • Operating system: MacOS Monterey 12.5.1
  • Python Version: 3.8.5
  • CCXT version: 1.92.20
  • Freqtrade Version: freqtrade 2022.8 stable

Describe the problem

In some cases, bots may miss take profit target if prices spike or drop in very short period, 1s or 2s, shorter than default process_throttle_secs: 5, this problem could be avoided if take profit order could be set as stoploss_on_exchange order.

Describe the enhancement

Binance futures support three kinds of TP and SL orders

  • trigger price + limit price

  • trigger price + market price

  • position TP/SL

Support take profit order same as the stoploss order on Binance. 😄

@xmatthias xmatthias added the Enhancement Enhancements to the bot. Get lower priority than bugs by default. label Sep 29, 2022
@xmatthias
Copy link
Member

xmatthias commented Sep 29, 2022

While i'll mark this as enhancement - it'll probably stay in this state for quite a long time (we have enhancement issues that are 4 years and older ...) - as this will require quite some heavy changes to internal workings - and is currently not something we're actively working on.

@asuiu
Copy link
Contributor

asuiu commented Feb 21, 2023

The tested workaround can be found here:
#7414

@xmatthias
Copy link
Member

xmatthias commented Feb 22, 2023

The tested workaround can be found here: #7414

Your workaround may be tested in a standalone context - but it's not working in a freqtrade context.
Your comment there has been hidden as it's missleading freqtrade users down a path that will not work.
(the comment can still be shown - but i hope it's a sign to people to read the subsequent comment, explaining that it'll not work).

@Axel-CH
Copy link
Contributor

Axel-CH commented Jun 10, 2023

Quick update on the subject, I saw that CCXT now support TP/SL order. you can see that here
@xmatthias Do you think that could be implemented and supported in Freqtrade?
What are the key steps/things to looks in order to implement that?
I willing to work on it the next week

@xmatthias
Copy link
Member

@Axel-CH ccxt was never the blocking point for this, and it's not the case now either.

Depending on the exchange, supporting both a TP/SL order will result in either one (binnace OCO) or 2 orderid's (almost all other exchanges) that need to be tracked.

A first step can be the removal of all "open_order_id" occurances by iterating over a (to define) property of "open orders" for that trade object, using the orders we track in the database.
Implicitly, this would enable multiple orders to be open per trade - but i'd keep that limitation - at least for the moment.

I'm happy for you to work on it - but i'll not be accepting a "big bang" PR that's trying to introduce OCO / take profit/stoploss orders.

It'll be a work in steps (probably over months, as i expect each step to take time to test, as well as "settle" to iron out further bugs), where the first step is removal of open_order_id from the codebase in favor of the above logic.

Once that part is merged, we can discuss about how to progress further - but i do see that as the basis for all further discussions around how an interface for this could look like.

@henkspane
Copy link

@xmatthias What will be the next step?

@xmatthias
Copy link
Member

well do the same for stoploss_order_id (which should be simpler i assume - but didn't check).

@Axel-CH
Copy link
Contributor

Axel-CH commented Sep 13, 2023

From what I understood about this issue, the feature that we worked on (#8779) and that was merged is covering what was requested. I will do more live tests today but in my opinion we can close this

@xmatthias
Copy link
Member

xmatthias commented Sep 13, 2023

From what I understood about this issue, the feature that we worked on (#8779) and that was merged is covering what was requested. I will do more live tests today but in my opinion we can close this

well no - #8779 was a prerequisite to support this, as is the replacement / removal of stoploss_order_id.
Neither of the 2 features will enable actual take-profit orders out of the box - but are prerequisites to start working on these.

this might've been a misunderstanding or has been forgotten, but was clearly stated in #7499 (comment) - as well as in #8779 (comment)

@Axel-CH
Copy link
Contributor

Axel-CH commented Sep 13, 2023

My bad, I was refering to taking profit partially in "adjust_trade_position" context. I think it is working there, will try

@xmatthias
Copy link
Member

xmatthias commented Sep 13, 2023

using adjust-trade-position for taking partial profit is working (and was working) since this callback was introduced (or support for exiting was added, anyway).
This is about an on-exchange take-profit order - which is a different beast.

@Cristy94
Copy link

What about setting process_throttle_secs to lower than 5s? Like 1s or 3s? Would that improve the "responsiveness" of the trades?

@xmatthias
Copy link
Member

What about setting process_throttle_secs to lower than 5s? Like 1s or 3s? Would that improve the "responsiveness" of the trades?

i do struggle to see the connection to this issue at the moment.
That setting has nothing to do with what this issue is actually about.

It will increase responsiveness (and traffic / potential ratelimit problems) - however - it's not that you set it to 1s and are guaranteed 1s - it's the minimum you set and if the cycle takes less then this, it'll wait for this time.
if the combination of strategy (and external conditions, like open trades, ...) has your "1 cycle" time > your set process_throttle_secs setting, there will be no impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Enhancements to the bot. Get lower priority than bugs by default.
Projects
None yet
Development

No branches or pull requests

8 participants
@asuiu @Cristy94 @xmatthias @Axel-CH @liaogx @henkspane and others