Lean icon indicating copy to clipboard operation
Lean copied to clipboard

CryptoFutureMarginModel - GetInitialMarginRequirement doesn't consider order type

Open fishstoryyy opened this issue 1 year ago • 3 comments

Expected Behavior

GetInitialMarginRequirement calculates the margin required to open a position and in cases of a limit order, it should calculate the position value based on the limit price instead of the security's current price.

Actual Behavior

GetInitialMarginRequirement calculates the margin based on the security's latest price, which leads to inaccurate buying power calculation, especially when the order type is a limit order where the limit price is much lower than the current price. See here https://github.com/QuantConnect/Lean/blob/5611b5508ada70dda96cc2b917e9475400f781e3/Common/Securities/CryptoFuture/CryptoFutureMarginModel.cs#L77

Potential Solution

Maybe the InitialMarginParameters should take the orderticket as an input?https://github.com/QuantConnect/Lean/blob/6e2744af7cf46090bafb166be016ea350df2cb8f/Common/Securities/InitialMarginParameters.cs#L23-L33

Reproducing the Problem

N/A (can provide a failing limit order due to insufficient buying power due to this issue if needed, but the link to the implementation code explains the issue)

System Information

N/A

Checklist

  • [x] I have completely filled out this template
  • [x] I have confirmed that this issue exists on the current master branch
  • [x] I have confirmed that this is not a duplicate issue by searching issues
  • [x] I have provided detailed steps to reproduce the issue

fishstoryyy avatar Jan 26 '24 08:01 fishstoryyy

I could be wrong but this issue appears to exist in the virtual GetInitialMarginRequirement method of the BuyingPowerModel class (parent of CryptoFutureMarginModel), where the initial margin calculation fails to consider the limit price of the order.

https://github.com/QuantConnect/Lean/blob/410956bf9ff36446e7c744f0cc9778623833ff5a/Common/Securities/BuyingPowerModel.cs#L230-L239

fishstoryyy avatar Jan 26 '24 08:01 fishstoryyy

I could be wrong but this issue appears to exist in the virtual GetInitialMarginRequirement method of the BuyingPowerModel class (parent of CryptoFutureMarginModel), where the initial margin calculation fails to consider the limit price of the order.

https://github.com/QuantConnect/Lean/blob/410956bf9ff36446e7c744f0cc9778623833ff5a/Common/Securities/BuyingPowerModel.cs#L230-L239

Hey @fishstoryyy! believe this is based on the actual behavior we've seen on brokerages, which base the margin requirements based on the current value, even if the limit price is 100x up/down.

Limit price 1 image

Limit price 1000 image

Martin-Molinero avatar Jan 30 '24 23:01 Martin-Molinero

It seems the initial margin requirement does actually depends on the limit price for limit orders. I'm attaching screenrecordings from Binance and Bybit Futures testnets. I couldn't see the actual initial margin shown but the max quantity for the orders does change with the limit price, which might mean the initial margin is different for each case.

Binance: image

Bybit: image

jhonabreul avatar Jan 31 '24 14:01 jhonabreul