Error in cancelOrder in TradeService class

[Tempys]

when i tried to loanch method

    /**
     * Cancel order.
     *
     * @param orderId order id
     * @return true if cancelled
     */
    boolean cancelOrder(String orderId);

in TradeService

i got the next error

2021-05-04 20:45:31,536 ERROR [boundedElastic-5] tech.cassandre.trading.bot.service.xchange.TradeServiceXChangeImplementation: TradeService - Error canceling order 130285696 : You need to provide the currency pair to cancel an order.

[straumat]

@Tempys thx! It works without currency pair on kucoin ! Which exchange are you using ? still Binance ?

[Tempys]

@straumat yes im using binance

[Tempys]

@straumat should be additional method
public boolean cancelOrder(CancelOrderParams params) throws IOException {

[Tempys]

also CancelOrderParams its interfaces and required a specific implementation ((((

[dstibbe]

But that would mean you would have the call one or the other strategy based on the exchange being used.
Wouldn't it be better to always provide the currency pair?

[straumat]

@dstibbe I will call the method with the currency pair, and my integration tests will check if it still works with an exchange not supporting this parameter.

I already do that here. Some exchanges requires currency pair as a parameter to trieve trades

[straumat]

Geez. More complicated than thought... BinanceCancelOrderParams has to be created so It forces me to add xchange-binance to the main project which is really not a good idea :(

[dstibbe]

But why retrieve the currency pair from the order repos if the strategy can simply provide it ?

And regarding xchange-binance: just implement the CancelOrderByCurrencyPair and CancelOrderByIdParams interfaces
or does it really need to be BinanceCancelOrderParams?

[straumat]

In cassandre, i don't have a dependency for any exchange library except xchange-core. And it seems I need to fulfill BinanceCancelOrderParams which forces me to add xchange-binance which is not clean

[dstibbe]

No. binance xchange implementation just checks for CancelOrderByCurrencyPair and CancelOrderByIdParams...
implement these interfaces and no extra dependency is required

[Tempys]

you need to move that logic on strategy level as i do

that code is working for me

CancelOrderParams cancelOrderParams = new BinanceCancelOrderParams(currencyPair,buyOrderId);
                            getTradeService().cancelOrder(cancelOrderParams);

in that case u ll avoid issues with adding additional libraries

[straumat]

@dstibbe but a BinanceCancelOrderParams has to be instantiated no?

[dstibbe]

NO :P

[dstibbe]

Edited : Just use the class mentioned a few posts down : DefaultCancelOrderByCurrencyPairAndIdParams

[Tempys]

@dstibbe its not only binance specific other exchange can require other parameters

so about that functionality should be responsible strategy and not bot

[dstibbe]

And I would just change the signature of cassandre's TradeService cancelOrder to : public final boolean cancelOrder(final String orderId, CurrencyPair currency)
and have the strategies call cancelOrder with the currency .
Instead of pulling it out of the order repos

[dstibbe]

@Tempys yes, it is not only binance specific, so I would say have Cassandre's tradeService always call the xchange tradeservice with a currencypair

[dstibbe]

If you don't, you are gonna have to make logic based on which exchange you are communicating with

[dstibbe]

And I just noticed , the class I was suggesting to implement already exists ! :D
Use org.knowm.xchange.service.trade.DefaultCancelOrderByCurrencyPairAndIdParams instead of BinanceCancelOrderParams

[dstibbe]

@Tempys regarding cancel orders on other exchanges: it seems Xchange has narrowed it down to 4 params:

• orderId
• currencyPair
• orderType
• userReference

based on the existing interface