uniVocity / univocity-trader

open-source trading framework for java, supports backtesting and live trading with exchanges
577 stars 140 forks source link

Attachment orders missing in LiveTrading? #50

Closed wwadge closed 3 years ago

wwadge commented 3 years ago

Not sure if I'm missing something but I can't seem to find the attachment equivalent code in the Live trader mode:

https://github.com/uniVocity/univocity-trader/blob/a81fe3f154b0f1df47bda491555505964c453b06/univocity-trader-core/src/main/java/com/univocity/trader/simulation/SimulatedClientAccount.java#L112

Is this an omission or is it handled elsewhere?

jbax commented 3 years ago

Not implemented yet for live trading. Will be used to submit oco orders, trade with binance futures and interactive brokers

On Wed, Jul 15, 2020, 2:50 AM Wallace Wadge notifications@github.com wrote:

Not sure if I'm missing something but I can't seem to find the attachment equivalent code in the Live trader mode:

https://github.com/uniVocity/univocity-trader/blob/a81fe3f154b0f1df47bda491555505964c453b06/univocity-trader-core/src/main/java/com/univocity/trader/simulation/SimulatedClientAccount.java#L112

Is this an omission or is it handled elsewhere?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/uniVocity/univocity-trader/issues/50, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWFQPUOSDLCKTAJQL6W5O3R3SHXTANCNFSM4OZXIT7Q .

wwadge commented 3 years ago

Aha -- That hopefully explains why my simulator results differ a lot than my live results. Does that only require wiring in the, say, Binance API (limitBuy -> limitOCOBuy) and making minor modifications to that switch...case or is it more involved than that? I'm guessing most of the existing code remains intact and the live code should be way more simpler than the simulator code since the logic is server side.

jbax commented 3 years ago

You are right. The code for submitting orders to binance should be relatively simple. You just need to submit them to binance, update their status and hopefully the current code that tracks the orders in a trade should do the rest.

On Wed, Jul 15, 2020, 6:37 AM Wallace Wadge notifications@github.com wrote:

Aha -- That hopefully explains why my simulator results differ a lot than my live results. Does that only require wiring in the, say, Binance API (limitBuy -> limitOCOBuy) and making minor modifications to that switch...case or is it more involved than that? I'm guessing most of the existing code remains intact and the live code should be way more simpler than the simulator code since the logic is server side.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/uniVocity/univocity-trader/issues/50#issuecomment-658413819, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWFQPU4GPBFTTSL2DYHTBDR3TCHPANCNFSM4OZXIT7Q .

wwadge commented 3 years ago

FYI: I'm starting to get this to work (I hope to be able to open a MR eventually) - I can submit an OCO with the values I expect. The response though is trickier because now we get back a list instead of a single value; I'm not super sure if I should hide this behind the same interfaces or fully expose the fact that we're getting a list now (one with limit_maker, one with stop_loss), but I'm leaning towards breaking whatever needs to be broken to handle both cases rather than hack away at it. Objections?

This is what their response looks like for oco:

{
  "orderListId": 0,
  "contingencyType": "OCO",
  "listStatusType": "EXEC_STARTED",
  "listOrderStatus": "EXECUTING",
  "listClientOrderId": "JYVpp3F0f5CAG15DhtrqLp",
  "transactionTime": 1563417480525,
  "symbol": "LTCBTC",
  "orders": [
    {
      "symbol": "LTCBTC",
      "orderId": 2,
      "clientOrderId": "Kk7sqHb9J6mJWTMDVW7Vos"
    },
    {
      "symbol": "LTCBTC",
      "orderId": 3,
      "clientOrderId": "xTXKaGYd4bluPVp78IVRvl"
    }
  ],
  "orderReports": [
    {
      "symbol": "LTCBTC",
      "orderId": 2,
      "orderListId": 0,
      "clientOrderId": "Kk7sqHb9J6mJWTMDVW7Vos",
      "transactTime": 1563417480525,
      "price": "0.000000",
      "origQty": "0.624363",
      "executedQty": "0.000000",
      "cummulativeQuoteQty": "0.000000",
      "status": "NEW",
      "timeInForce": "GTC",
      "type": "STOP_LOSS",
      "side": "BUY",
      "stopPrice": "0.960664"
    },
    {
      "symbol": "LTCBTC",
      "orderId": 3,
      "orderListId": 0,
      "clientOrderId": "xTXKaGYd4bluPVp78IVRvl",
      "transactTime": 1563417480525,
      "price": "0.036435",
      "origQty": "0.624363",
      "executedQty": "0.000000",
      "cummulativeQuoteQty": "0.000000",
      "status": "NEW",
      "timeInForce": "GTC",
      "type": "LIMIT_MAKER",
      "side": "BUY"
    }
  ]
}
jbax commented 3 years ago

That's great, thanks a lot! I believe you can simply add the two orders in the list to a trade object (one to the position list, the other to the exitOrders), hopefully ignore the parent orderlist binance sends back, and let the framework poll the status of these orders as if they were individual orders.

On Thu, Jul 16, 2020, 11:36 PM Wallace Wadge notifications@github.com wrote:

FYI: I'm starting to get this to work (I hope to be able to open a MR eventually) - I can submit an OCO with the values I expect. The response though is trickier because now we get back a list instead of a single value; I'm not super sure if I should hide this behind the same interfaces or fully expose the fact that we're getting a list now (one with limit_maker, one with stop_loss), but I'm leaning towards breaking whatever needs to be broken to handle both cases rather than hack away at it. Objections?

This is what their response looks like for oco:

{ "orderListId": 0, "contingencyType": "OCO", "listStatusType": "EXEC_STARTED", "listOrderStatus": "EXECUTING", "listClientOrderId": "JYVpp3F0f5CAG15DhtrqLp", "transactionTime": 1563417480525, "symbol": "LTCBTC", "orders": [ { "symbol": "LTCBTC", "orderId": 2, "clientOrderId": "Kk7sqHb9J6mJWTMDVW7Vos" }, { "symbol": "LTCBTC", "orderId": 3, "clientOrderId": "xTXKaGYd4bluPVp78IVRvl" } ], "orderReports": [ { "symbol": "LTCBTC", "orderId": 2, "orderListId": 0, "clientOrderId": "Kk7sqHb9J6mJWTMDVW7Vos", "transactTime": 1563417480525, "price": "0.000000", "origQty": "0.624363", "executedQty": "0.000000", "cummulativeQuoteQty": "0.000000", "status": "NEW", "timeInForce": "GTC", "type": "STOP_LOSS", "side": "BUY", "stopPrice": "0.960664" }, { "symbol": "LTCBTC", "orderId": 3, "orderListId": 0, "clientOrderId": "xTXKaGYd4bluPVp78IVRvl", "transactTime": 1563417480525, "price": "0.036435", "origQty": "0.624363", "executedQty": "0.000000", "cummulativeQuoteQty": "0.000000", "status": "NEW", "timeInForce": "GTC", "type": "LIMIT_MAKER", "side": "BUY" } ] }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/uniVocity/univocity-trader/issues/50#issuecomment-659435601, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWFQPQMBQLJ54VWYZIQFD3R34COJANCNFSM4OZXIT7Q .

wwadge commented 3 years ago

Ok I have something that seems to be working. I'll let it bake for a while before opening a MR (my signals are quite slow so it takes some time to hit all the cases).

BTW During debugging I also found: https://github.com/uniVocity/univocity-trader/blob/a81fe3f154b0f1df47bda491555505964c453b06/univocity-trader-binance/src/main/java/com/univocity/trader/exchange/binance/BinanceClientAccount.java#L193

I think you meant "return order" there. I can open a separate MR for that if you want, but given it's a one liner you might just want to add that one directly ;-)

jbax commented 3 years ago

That line is correct. No order is generated if the total amount traded is less than the minimum order amount. Binance won't accept orders worth less than 10 usdt for example.

On Fri, Jul 17, 2020, 6:50 AM Wallace Wadge notifications@github.com wrote:

Ok I have something that seems to be working. I'll let it bake for a while before opening a MR (my signals are quite slow so it takes some time to hit all the cases).

BTW During debugging I also found:

https://github.com/uniVocity/univocity-trader/blob/a81fe3f154b0f1df47bda491555505964c453b06/univocity-trader-binance/src/main/java/com/univocity/trader/exchange/binance/BinanceClientAccount.java#L193

I think you meant "return order" there. I can open a separate MR for that if you want, but given it's a one liner you might just want to add that one directly ;-)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/uniVocity/univocity-trader/issues/50#issuecomment-659679440, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWFQPVHHVCS5HNIJFMOULLR35VIDANCNFSM4OZXIT7Q .

wwadge commented 3 years ago

I'm tracking my changes in this branch: https://github.com/wwadge/univocity-trader-1/tree/feature/add-OCO-orders-to-live

Code seems to be working fine, but I lack unit tests at the moment so consider this as a pre-cleanup branch for now.

There's also a small bug I detected: during order processing, it does a try..finalize to update balances, but that balances update code in AccountManager is guarded by a check to avoid fetching balances if not enough tine has passed. So this sometimes leads to: a SELL is triggered, an order is placed, the updateBalance check returns stale data and the next SELL order results in an insufficient funds exception from Binance. I've fixed this in the same branch by adding a "force" flag so skip the cache check.

jbax commented 3 years ago

Thanks a lot! I also think the "force" flag will be required there. Once I get the time I'll check it out.

wwadge commented 3 years ago

Hmmm actually I don't think I did this right -- I'm only doing a single OCO order of say BUY if signal returned BUY.

In interactive brokers, we send 3 simultaneous orders: BUY, then an OCO of SELLs. But as far as I can tell, Binance does not support this behaviour, instead it allows you to only do OCO or a market buy or limit; to replicate the behavior of the simulator in Binance I think I have to send a manual market order first to force a buy, then immediately do an OCO with SELLs. Or alternately watch for the "filled" and then trigger the OCO but this will probably need some re-architecting work. Did I think it right?

jbax commented 3 years ago

Yes the oco in Binance is just an order targeting a price range. So it's either 2 buys or 2 sells.

If the idea is to emulate what IB does then ideally we should watch the buy order to fill, then submit the oco sell to binance. Market orders might fail (order quantity might be too much if price went up too quickly) so watching the order is the way to go imo.

On Tue, Jul 28, 2020, 1:35 AM Wallace Wadge notifications@github.com wrote:

Hmmm actually I don't think I did this right -- I'm only doing a single OCO order of say BUY if signal returned BUY.

In interactive brokers, we send 3 simultaneous orders: BUY, then an OCO of SELLs. But as far as I can tell, Binance does not support this behaviour, instead it allows you to only do OCO or a market buy or limit; to replicate the behavior of the simulator in Binance I think I have to send a manual market order first to force a buy, then immediately do an OCO with SELLs. Or alternately watch for the "filled" and then trigger the OCO but this will probably need some re-architecting work. Did I think it right?

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/uniVocity/univocity-trader/issues/50#issuecomment-664487726, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWFQPUXIJBADS5SPQEHCTDR5WQUHANCNFSM4OZXIT7Q .

wwadge commented 3 years ago

OK - I'll work on this and open a new MR