Liquidation price #5348
Liquidation price #5348
Conversation
|
What's the difference between this and #5339? |
I changed the name of the branch, and then when I came back to the PRs I couldn't see it so I thought it was deleted. Then I created this one. |
|
|
||
| def run(self): | ||
| # TODO-mg: implement a thread that constantly updates with every price change, | ||
| # TODO-mg: must update at least every second |
There was a problem hiding this comment.
If this is a requirement - assume this as impossible.
Maybe add a safety margin next to liquidation to manually liquidate - but i'd handle it in a similar way than stoploss-on-exchange - which is checked once every iteration (no matter how long that takes).
with exchange-ratelimiting and strategy processing, you'll not get this done.
Also - we'll definitely not spin out into different threads for this - so it'll be "in-line" with the regular bot processing - which can't guarantee any runtime, really (as it depends on the strategy) - and everything < 5s will get you blocked in the long run (especially on kraken).
There was a problem hiding this comment.
It's not a requirement, every 5s might be enough. The problem is that with cross margin, there is one liquidation price, which depends on the current price of every trade your in. If you can see that in the last minute that a few coins went down 5% or something, and then came back up, that doesn't mean that they all did it at the same time, so maybe your maintenance margin dropped below the liquidation price, but maybe one of your coins went up at the right time within that minute to prevent the maintenance margin from dropping low enough.
With isolated margin, there's only one price to look at, so its obvious if you drop below the liquidation price.
This is why I think isolated futures would be the easiest, it wouldn't require any transferring either (like isolated margin would), I don't believe that there is any dust with futures either
It looks like on binance, an isolated futures order would be something like
binance_usdm.set_leverage(symbol='ADA/USDT', leverage=5)
order = binance_usdm.create_order(
symbol='ADA/USDT',
type='market',
side='buy',
amount=amount
)
and I'm not sure about FTX, and this is why I don't want to add much more support for Kraken. I see no reason to remove the tests and formulas which have been written for Kraken already, but we could just block leverage support for it until someone is willing to get a maintenance margin class that works well
There was a problem hiding this comment.
see - my main problem with looking at binance is that they have like 4 or 5 methods to do leveraged trades (margin, isolated margin, futures, apparently isolated futures, and futures_coin).
Each of these works slightly differently - so only adding support for binance is risky, as it might lead us down a wrong path.
If you want to support only one exchange, then pick another one (not binance) - as binance probably the most complicated / complex ones. I don't really care if it's kraken or FTX - but binance will definitely not be the only one (or if we go with one only, it will not be part of the first row).
There was a problem hiding this comment.
That just means they have more options, they don't all need to be supported. The only one that's really that complicated is isolated margin, so I'll avoid that one for sure.
FTX supports cross margin and perpetual cross futures, and kraken supports cross margin.
Isolated futures is really the least complicated of all of them, it only involves that extra set_leverage command, which I think will also work for FTX, with cross margin and cross futures though there is the extra part of keeping track of the maintenance margin, which i'm worried we won't be able to do very accurately if we're not updating it that often,
There was a problem hiding this comment.
for the moment - i'm not decided on which exchange / market kind (margin / futures / ...) will be added initially (neither are you, i think) - but i also don't think it's too important at this point.
Looking at a file like liqformula.py however i do have some strong concerns.
Having all these (either exception or TODO) if/else statements in there is basically a technical depth (before it's even merged).
There's no point in having TradingMode.ISOLATED_MARGIN in the enum if we don't plan on implementing it.
Should someone see this and implement just the calculation, it'd potentially be wasted time/effort both on the implementation/research, as well as on the review side.
Also - it's a burden for every refactoring that might potentially happen - so i see no point in doing it this way.
If the calculation is added (with the intend to support this mode by the author) - then sure - add it as if/elif ... otherwise, don't even include it in the enum.
There was a problem hiding this comment.
I separated TradingMode into TradingMode and Collateral with Collateral being Isolated and Cross. The logic of when both were used didn't have much overlap.
LiqFormula is an enum because it's used similarly to InterestMode. It's called in LocalTrade, but differs based on exchange. I took most of the functions out of the LiqFormula class and have them as independent functions though if you like that better
There was a problem hiding this comment.
Basically any isolated liquidation price would need to be stored on the Trade/LocalTrade object, and I think that object should be able to handle the logic also
ah, yeah, github will do that if you rename branches on open PR's - as for github, it looks like it's gone / deleted (i guess). |
808fcb1 to
9b088c6
Compare
2045999 to
2dff06e
Compare
… reorganized liqformula
…t of liqformula class
…alue to change based on trading mode
…rgin to freqtradebot
…t of liqformula class
2dff06e to
6c489f1
Compare
There was a problem hiding this comment.
As you've included this class the same way in #5364 - maybe i wasn't clear before.
Please remove all logic from the freqtrade.enums and move it to seperate files in a different module.
This also applies to the merged interestMode - however this can (and probably should) be done in a seperate "cleanup" PR, refactoring the logic out of the enums class.
|
I'm going to close this pull request and reopen it when it's ready |
Adds maintenance margin and liquidation formula classes