cassandre-tech / cassandre-trading-bot

Create your Java crypto trading bot in minutes. Our Spring boot starter takes care of exchange connections, accounts, orders, trades, and positions so you can focus on building your strategies.
https://trading-bot.cassandre.tech
GNU General Public License v3.0
583 stars 166 forks source link

The problem that the order obtained by the exchange is empty #932

Closed hlevel closed 2 years ago

hlevel commented 2 years ago

TradeServiceXChangeImplementation#getOrders This method can get orders in the simulated environment, but it can't find any orders in the real environment. As a result, the order stays in the new status during operation. The improvements are as follows (the binance exchange has been tested)

Before change:

public Set<OrderDTO> getOrders() {
        logger.debug("Getting orders from exchange");
        try {
            // We check if we have some local orders to push.
            // When we create them in local, they have the PENDING_NEW status.
            // We retrieve them all to return them in localOrders.
            // And for each, we update their status to NEW.
            final Set<OrderDTO> localOrders = orderRepository.findByStatus(PENDING_NEW)
                    .stream()
                    .map(Base.ORDER_MAPPER::mapToOrderDTO)
                    .sorted(Comparator.comparing(OrderDTO::getTimestamp))
                    .peek(orderDTO -> logger.debug("Local order retrieved: {}", orderDTO))
                    .peek(orderDTO -> orderDTO.updateStatus(NEW))
                    .collect(Collectors.toCollection(LinkedHashSet::new));

            // If we have local orders to push, we return them.
            if (!localOrders.isEmpty()) {
                localOrders.stream().forEach(order -> System.out.println("repo=" + order));
                return localOrders;
            } else {
                try {
                    // Consume a token from the token bucket.
                    // If a token is not available this method will block until the refill adds one to the bucket.
                    bucket.asBlocking().consume(1);
                    return tradeService.getOpenOrders()
                            .getOpenOrders()
                            .stream()
                            .map(Base.ORDER_MAPPER::mapToOrderDTO)
                            .peek(orderDTO -> logger.debug("Remote order retrieved: {}", orderDTO))
                            .collect(Collectors.toCollection(LinkedHashSet::new));
                } catch (NotAvailableFromExchangeException e) {
                    // If the classical call to getOpenOrders() is not implemented, we use the specific parameters that asks for currency pair.
                    Set<OrderDTO> orders = new LinkedHashSet<>();
                    orderRepository.findAll()
                            .stream()
                            .map(Base.ORDER_MAPPER::mapToOrderDTO)
                            // We only ask for currency pairs of the non fulfilled orders.
                            .filter(orderDTO -> !orderDTO.isFulfilled())
                            .map(OrderDTO::getCurrencyPair)
                            .distinct()
                            .forEach(currencyPairDTO -> {
                                try {
                                    // Consume a token from the token bucket.
                                    // If a token is not available this method will block until the refill adds one to the bucket.
                                    bucket.asBlocking().consume(1);
                                    orders.addAll(tradeService.getOpenOrders(new DefaultOpenOrdersParamCurrencyPair(Base.CURRENCY_MAPPER.mapToCurrencyPair(currencyPairDTO)))
                                            .getOpenOrders()
                                            .stream()
                                            .map(Base.ORDER_MAPPER::mapToOrderDTO)
                                            .peek(orderDTO -> logger.debug("Remote order retrieved: {}", orderDTO))
                                            .collect(Collectors.toCollection(LinkedHashSet::new)));
                                } catch (IOException | InterruptedException specificOrderException) {
                                    logger.error("Error retrieving orders: {}", specificOrderException.getMessage());
                                }
                            });
                    return orders;
                } catch (CurrencyPairNotValidException e) {
                    logger.error("Error retrieving orders(If it is a simulated environment, please configure simulated.json): {}", e.getMessage());
                    return Collections.emptySet();
                } catch (IOException e) {
                    logger.error("Error retrieving orders: {}", e.getMessage());
                    return Collections.emptySet();
                }
            }
        } catch (InterruptedException e) {
            return Collections.emptySet();
        }

After modification:

public Set<OrderDTO> getOrders() {
        logger.debug("Getting orders from exchange");
        try {
            // We check if we have some local orders to push.
            // When we create them in local, they have the PENDING_NEW status.
            // We retrieve them all to return them in localOrders.
            // And for each, we update their status to NEW.
            final Set<OrderDTO> localOrders = orderRepository.findByStatus(PENDING_NEW)
                    .stream()
                    .map(Base.ORDER_MAPPER::mapToOrderDTO)
                    .sorted(Comparator.comparing(OrderDTO::getTimestamp))
                    .peek(orderDTO -> logger.debug("Local order retrieved: {}", orderDTO))
                    .peek(orderDTO -> orderDTO.updateStatus(NEW))
                    .collect(Collectors.toCollection(LinkedHashSet::new));

            // If we have local orders to push, we return them.
            if (!localOrders.isEmpty()) {
                localOrders.stream().forEach(order -> System.out.println("repo=" + order));
                return localOrders;
            } else {
                try {
                    // Consume a token from the token bucket.
                    // If a token is not available this method will block until the refill adds one to the bucket.
                    bucket.asBlocking().consume(1);

                    final List<Order> orders = orderRepository.findByStatus(NEW);
                    if(!orders.isEmpty()) {
                        Map<String, OrderStatusDTO> exchangeOrderStatus = new HashMap<>();
                        Map<String, OrderStatusDTO> repoOrderStatus = new HashMap<>();

                        //Exchange query conditions
                        List<DefaultQueryOrderParamCurrencyPair> orderQueryParamCurrencyPairList = orders
                                .stream()
                                .peek(order -> repoOrderStatus.put(order.getOrderId(), order.getStatus()))
                                .map(order -> new DefaultQueryOrderParamCurrencyPair(new CurrencyPair(order.getCurrencyPair()), order.getOrderId()))
                                .collect(Collectors.toList());
                        //Query the status of the order in the exchange
                        tradeService.getOrder(orderQueryParamCurrencyPairList.toArray(new DefaultQueryOrderParamCurrencyPair[]{}))
                                .stream()
                                .filter(order -> repoOrderStatus.containsKey(order.getId()))
                                .filter(order -> Base.UTIL_MAPPER.mapToOrderStatusDTO(order.getStatus()) != repoOrderStatus.get(order.getId()))
                                .forEach(order -> {
                                    exchangeOrderStatus.put(order.getId(), Base.UTIL_MAPPER.mapToOrderStatusDTO(order.getStatus()));
                                });

                        if(!exchangeOrderStatus.isEmpty()) {
                            return orders.stream()
                                    .map(Base.ORDER_MAPPER::mapToOrderDTO)
                                    .sorted(Comparator.comparing(OrderDTO::getTimestamp))
                                    .peek(orderDTO -> logger.debug("Local order retrieved: {}", orderDTO))
                                    .peek(orderDTO -> orderDTO.updateStatus(exchangeOrderStatus.get(orderDTO.getOrderId())))
                                    .collect(Collectors.toCollection(LinkedHashSet::new));
                        }
                    }
                    return tradeService.getOpenOrders()
                            .getOpenOrders()
                            .stream()
                            .map(Base.ORDER_MAPPER::mapToOrderDTO)
                            .peek(orderDTO -> logger.debug("Remote order retrieved: {}", orderDTO))
                            .collect(Collectors.toCollection(LinkedHashSet::new));
                } catch (NotAvailableFromExchangeException e) {
                    // If the classical call to getOpenOrders() is not implemented, we use the specific parameters that asks for currency pair.
                    Set<OrderDTO> orders = new LinkedHashSet<>();
                    orderRepository.findAll()
                            .stream()
                            .map(Base.ORDER_MAPPER::mapToOrderDTO)
                            // We only ask for currency pairs of the non fulfilled orders.
                            .filter(orderDTO -> !orderDTO.isFulfilled())
                            .map(OrderDTO::getCurrencyPair)
                            .distinct()
                            .forEach(currencyPairDTO -> {
                                try {
                                    // Consume a token from the token bucket.
                                    // If a token is not available this method will block until the refill adds one to the bucket.
                                    bucket.asBlocking().consume(1);
                                    orders.addAll(tradeService.getOpenOrders(new DefaultOpenOrdersParamCurrencyPair(Base.CURRENCY_MAPPER.mapToCurrencyPair(currencyPairDTO)))
                                            .getOpenOrders()
                                            .stream()
                                            .map(Base.ORDER_MAPPER::mapToOrderDTO)
                                            .peek(orderDTO -> logger.debug("Remote order retrieved: {}", orderDTO))
                                            .collect(Collectors.toCollection(LinkedHashSet::new)));
                                } catch (IOException | InterruptedException specificOrderException) {
                                    logger.error("Error retrieving orders: {}", specificOrderException.getMessage());
                                }
                            });
                    return orders;
                } catch (CurrencyPairNotValidException e) {
                    logger.error("Error retrieving orders(If it is a simulated environment, please configure simulated.json): {}", e.getMessage());
                    return Collections.emptySet();
                } catch (IOException e) {
                    logger.error("Error retrieving orders: {}", e.getMessage());
                    return Collections.emptySet();
                }
            }
        } catch (InterruptedException e) {
            return Collections.emptySet();
        }
straumat commented 2 years ago

@hlevel can you make a pull request to see if tests are still working ?

hlevel commented 2 years ago

@straumat Do you mean that I need to test the revised suggestion by myself and tell you the result? This is a piece of code that has been verified by me. It is still running in the real environment.

straumat commented 2 years ago

@hlevel Ok, I will integrate this but in the next release because I have to update Cassandre mocks to make tests pass and I have to test the behavior on the exchanges I support with production tests (coinbase, kucoin and Binance)