From fea6d39bfd5c340dc0cdfd9b937b2517752ed466 Mon Sep 17 00:00:00 2001 From: uhliksk Date: Mon, 8 Aug 2022 16:27:40 +0200 Subject: [PATCH 01/17] fix: exceeding max open trades --- .../step/__tests__/determine-action.test.js | 310 +++++------------- .../step/__tests__/handle-open-orders.test.js | 253 ++++++++++---- .../trailingTrade/step/determine-action.js | 41 +-- .../trailingTrade/step/handle-open-orders.js | 14 +- .../__tests__/common.test.js | 204 ++++++++++++ app/cronjob/trailingTradeHelper/common.js | 35 +- 6 files changed, 511 insertions(+), 346 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/determine-action.test.js b/app/cronjob/trailingTrade/step/__tests__/determine-action.test.js index 0e0a986d..1063a409 100644 --- a/app/cronjob/trailingTrade/step/__tests__/determine-action.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/determine-action.test.js @@ -18,12 +18,15 @@ describe('determine-action.js', () => { let mockGetGridTradeOrder; + let mockIsExceedingMaxOpenTrades; + describe('execute', () => { beforeEach(() => { jest.clearAllMocks().resetModules(); mockGetNumberOfBuyOpenOrders = jest.fn().mockResolvedValue(1); mockGetNumberOfOpenTrades = jest.fn().mockResolvedValue(6); mockGetAPILimit = jest.fn().mockReturnValue(5); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); }); beforeEach(async () => { @@ -438,7 +441,8 @@ describe('determine-action.js', () => { isActionDisabled: mockIsActionDisabled, getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades })); mockGetGridTradeOrder = jest.fn().mockResolvedValue(null); @@ -551,7 +555,8 @@ describe('determine-action.js', () => { isActionDisabled: mockIsActionDisabled, getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades })); mockGetGridTradeOrder = jest.fn().mockResolvedValue(null); @@ -654,7 +659,8 @@ describe('determine-action.js', () => { isActionDisabled: mockIsActionDisabled, getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades })); mockGetGridTradeOrder = jest.fn().mockResolvedValue(null); @@ -768,7 +774,8 @@ describe('determine-action.js', () => { isActionDisabled: mockIsActionDisabled, getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades })); mockGetGridTradeOrder = jest.fn().mockResolvedValue(null); @@ -856,7 +863,8 @@ describe('determine-action.js', () => { isActionDisabled: mockIsActionDisabled, getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades })); mockGetGridTradeOrder = jest.fn().mockResolvedValue(null); @@ -969,13 +977,6 @@ describe('determine-action.js', () => { isDisabled: false }); - jest.mock('../../../trailingTradeHelper/common', () => ({ - isActionDisabled: mockIsActionDisabled, - getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, - getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit - })); - mockGetGridTradeOrder = jest.fn().mockResolvedValue(null); jest.mock('../../../trailingTradeHelper/order', () => ({ @@ -985,248 +986,84 @@ describe('determine-action.js', () => { return clonedRawData; }; - describe('when order limit is enabled', () => { - describe('when the last buy price is recorded', () => { - describe('when number of current open trade is less than maximum open trades', () => { - beforeEach(async () => { - rawData = mockInit({ - orderLimitEnabled: true, - lastBuyPrice: 29000, - maxOpenTrades: 3, - numberOfOpenTrades: 2 - }); - - step = require('../determine-action'); - result = await step.execute(loggerMock, rawData); - }); - - it( - `should place a buy order because ` + - `the current open trade is less than the maximum open trades`, - () => { - expect(result).toMatchObject({ - action: 'buy', - buy: { - athRestrictionPrice: 28100, - currentPrice: 28000, - processMessage: - "The current price reached the trigger price for the grid trade #1. Let's buy it.", - triggerPrice: 28000, - updatedAt: expect.any(Object) - } - }); - } - ); - }); - - describe('when number of open trades is same as max open trades', () => { - beforeEach(async () => { - rawData = mockInit({ - orderLimitEnabled: true, - lastBuyPrice: 29000, - maxOpenTrades: 3, - numberOfOpenTrades: 3 - }); - - step = require('../determine-action'); - result = await step.execute(loggerMock, rawData); - }); - - it( - `should place a buy order because the last buy price is recorded, ` + - `which means current open trades includes this symbol order`, - () => { - expect(result).toMatchObject({ - action: 'buy', - buy: { - athRestrictionPrice: 28100, - currentPrice: 28000, - processMessage: - "The current price reached the trigger price for the grid trade #1. Let's buy it.", - triggerPrice: 28000, - updatedAt: expect.any(Object) - } - }); - } - ); + describe('when open trade limit is reached', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: 0 }); - describe('when number of open trades is already exceeded max open trades', () => { - beforeEach(async () => { - rawData = mockInit({ - orderLimitEnabled: true, - lastBuyPrice: 29000, - maxOpenTrades: 2, - numberOfOpenTrades: 3 - }); + mockIsExceedingMaxOpenTrades = jest + .fn() + .mockResolvedValue(true); - step = require('../determine-action'); - result = await step.execute(loggerMock, rawData); - }); + jest.mock('../../../trailingTradeHelper/common', () => ({ + isActionDisabled: mockIsActionDisabled, + getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, + getNumberOfOpenTrades: mockGetNumberOfOpenTrades, + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades + })); - it( - `should not place a buy order because number of current open trades is ` + - `exceeded the maximum number of open trade`, - () => { - expect(result).toMatchObject({ - action: 'wait', - buy: { - athRestrictionPrice: 28100, - currentPrice: 28000, - processMessage: - `The current price has reached the lowest price; ` + - `however, it is restricted to buy the coin because of reached maximum open trades.`, - triggerPrice: 28000, - updatedAt: expect.any(Object) - } - }); - } - ); - }); + step = require('../determine-action'); + result = await step.execute(loggerMock, rawData); }); - describe('when the last buy price is not recorded', () => { - describe('when number of open trades is less than max open trades', () => { - beforeEach(async () => { - rawData = mockInit({ - orderLimitEnabled: true, - lastBuyPrice: null, - maxOpenTrades: 3, - numberOfOpenTrades: 2 - }); - - step = require('../determine-action'); - result = await step.execute(loggerMock, rawData); - }); - - it( - `should place a buy order because the nubmer of current open trade ` + - `is less than the maximum number of open trade`, - () => { - expect(result).toMatchObject({ - action: 'buy', - buy: { - athRestrictionPrice: 28100, - currentPrice: 28000, - processMessage: - "The current price reached the trigger price for the grid trade #1. Let's buy it.", - triggerPrice: 28000, - updatedAt: expect.any(Object) - } - }); - } - ); - }); - - describe('when number of open trades is same as max open trades', () => { - beforeEach(async () => { - rawData = mockInit({ - orderLimitEnabled: true, - lastBuyPrice: null, - maxOpenTrades: 3, - numberOfOpenTrades: 3 - }); - - step = require('../determine-action'); - result = await step.execute(loggerMock, rawData); - }); - - it( - `should not place a buy order because the last buy price is not recorded, ` + - `which means current open trades does not include this symbol order`, - () => { - expect(result).toMatchObject({ - action: 'wait', - buy: { - athRestrictionPrice: 28100, - currentPrice: 28000, - processMessage: - `The current price has reached the lowest price; ` + - `however, it is restricted to buy the coin because of reached maximum open trades.`, - triggerPrice: 28000, - updatedAt: expect.any(Object) - } - }); + it( + `should not place a buy order because ` + + `the open trade limit is not reached`, + () => { + expect(result).toMatchObject({ + action: 'wait', + buy: { + processMessage: + `The current price has reached the lowest price; however, it is restricted to buy the coin ` + + `because of reached maximum open trades.`, + updatedAt: expect.any(Object) } - ); - }); - - describe('when number of open trades is already exceeded max open trades', () => { - beforeEach(async () => { - rawData = mockInit({ - orderLimitEnabled: true, - lastBuyPrice: null, - maxOpenTrades: 2, - numberOfOpenTrades: 3 - }); - - step = require('../determine-action'); - result = await step.execute(loggerMock, rawData); }); - - it( - `should not place a buy porder because number of curent open trade is ` + - `exceeded the maximum number of open trade`, - () => { - expect(result).toMatchObject({ - action: 'wait', - buy: { - athRestrictionPrice: 28100, - currentPrice: 28000, - processMessage: - `The current price has reached the lowest price; ` + - `however, it is restricted to buy the coin because of reached maximum open trades.`, - triggerPrice: 28000, - updatedAt: expect.any(Object) - } - }); - } - ); - }); - }); + } + ); }); - describe('when order limit is disabled', () => { + describe('when open trade limit is not reached', () => { beforeEach(async () => { rawData = mockInit({ - orderLimitEnabled: false, - lastBuyPrice: null, - maxOpenTrades: 2, - numberOfOpenTrades: 2 + orderLimitEnabled: true, + lastBuyPrice: 0 }); - step = require('../determine-action'); + mockIsExceedingMaxOpenTrades = jest + .fn() + .mockResolvedValue(false); + jest.mock('../../../trailingTradeHelper/common', () => ({ + isActionDisabled: mockIsActionDisabled, + getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, + getNumberOfOpenTrades: mockGetNumberOfOpenTrades, + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades + })); + + step = require('../determine-action'); result = await step.execute(loggerMock, rawData); }); - it('should place a buy order because order limit is disabled', () => { - expect(result).toMatchObject({ - action: 'buy', - symbolConfiguration: { - botOptions: { - orderLimit: { - enabled: false, - maxBuyOpenOrders: 4, - maxOpenTrades: 2 - } + it( + `should place a buy order because ` + + `the open trade limit is not reached`, + () => { + expect(result).toMatchObject({ + action: 'buy', + buy: { + processMessage: + `The current price reached the trigger price for the grid trade #1. ` + + `Let's buy it.`, + updatedAt: expect.any(Object) } - }, - buy: { - currentPrice: 28000, - triggerPrice: 28000, - athRestrictionPrice: 28100, - processMessage: `The current price reached the trigger price for the grid trade #1. Let's buy it.`, - updatedAt: expect.any(Object) - }, - sell: { - currentPrice: 28000, - triggerPrice: null, - lastBuyPrice: null, - stopLossTriggerPrice: null - } - }); - }); + }); + } + ); }); }); @@ -1245,7 +1082,8 @@ describe('determine-action.js', () => { isActionDisabled: mockIsActionDisabled, getNumberOfBuyOpenOrders: mockGetNumberOfBuyOpenOrders, getNumberOfOpenTrades: mockGetNumberOfOpenTrades, - getAPILimit: mockGetAPILimit + getAPILimit: mockGetAPILimit, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades })); // Set there is no grid trade order diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index 87c872d0..ce211a8b 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -14,6 +14,8 @@ describe('handle-open-orders.js', () => { let mockUpdateAccountInfo; let mockSaveOverrideAction; + let mockIsExceedingMaxOpenTrades; + const accountInfoJSON = require('./fixtures/binance-account-info.json'); describe('execute', () => { @@ -32,6 +34,8 @@ describe('handle-open-orders.js', () => { mockUpdateAccountInfo = jest.fn().mockResolvedValue({ account: 'updated' }); + + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); }); describe('when symbol is locked', () => { @@ -626,35 +630,29 @@ describe('handle-open-orders.js', () => { }); describe('when stop price is less than current limit price', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); + describe('when open trade limit is reached', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest + .fn() + .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol - })); + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades + })); - step = require('../handle-open-orders'); + step = require('../handle-open-orders'); - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ], - buy: { - limitPrice: 1810, + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', openOrders: [ { symbol: 'BTCUSDT', @@ -664,49 +662,155 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - result = await step.execute(loggerMock, rawData); - }); + result = await step.execute(loggerMock, rawData); + }); - it('does not trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); - }); + it('does trigger cancelOrder', () => { + expect(binanceMock.client.cancelOrder).toHaveBeenCalled(); + }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-cancelled', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + } + }); + }); }); - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-wait', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + describe('when open trade limit is not reached', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest + .fn() + .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); + + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades + })); + + step = require('../handle-open-orders'); + + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' } - ], - buy: { - limitPrice: 1810, + }; + + result = await step.execute(loggerMock, rawData); + }); + + it('does not trigger cancelOrder', () => { + expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); + }); + + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); + + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-wait', openOrders: [ { symbol: 'BTCUSDT', @@ -716,12 +820,25 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + } + }); }); }); }); diff --git a/app/cronjob/trailingTrade/step/determine-action.js b/app/cronjob/trailingTrade/step/determine-action.js index a2fcb740..4a6e9add 100644 --- a/app/cronjob/trailingTrade/step/determine-action.js +++ b/app/cronjob/trailingTrade/step/determine-action.js @@ -5,7 +5,7 @@ const { slack } = require('../../../helpers'); const { isActionDisabled, getNumberOfBuyOpenOrders, - getNumberOfOpenTrades, + isExceedingMaxOpenTrades, getAPILimit } = require('../../trailingTradeHelper/common'); const { getGridTradeOrder } = require('../../trailingTradeHelper/order'); @@ -122,45 +122,6 @@ const isExceedingMaxBuyOpenOrders = async (logger, data) => { return false; }; -/** - * Check whether max number of open trades has reached - * - * @param {*} logger - * @param {*} data - * @returns - */ -const isExceedingMaxOpenTrades = async (logger, data) => { - const { - symbolConfiguration: { - botOptions: { - orderLimit: { - enabled: orderLimitEnabled, - maxOpenTrades: orderLimitMaxOpenTrades - } - } - }, - sell: { lastBuyPrice } - } = data; - - if (orderLimitEnabled === false) { - return false; - } - - let currentOpenTrades = await getNumberOfOpenTrades(logger); - - // If the last buy price is recorded, this is one of open trades. - // Deduct 1 from the current open trades and calculate it. - if (lastBuyPrice) { - currentOpenTrades -= 1; - } - - if (currentOpenTrades >= orderLimitMaxOpenTrades) { - return true; - } - - return false; -}; - /** * Set buy action and message * diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index e97563be..1cdc590a 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -7,7 +7,8 @@ const { getAccountInfo, updateAccountInfo, saveOverrideAction, - getAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades } = require('../../trailingTradeHelper/common'); /** @@ -156,6 +157,17 @@ const execute = async (logger, rawData) => { moment().toISOString() ); } + } else if (await isExceedingMaxOpenTrades(logger, data)) { + // Cancel the initial buy order if max. open trades exceeded + data.action = 'buy-order-cancelled'; + logger.info( + { data, saveLog: true }, + `The current number of open trades has reached the maximum number of open trades. ` + + `The buy order will be cancelled.` + ); + + // Cancel current order + await cancelOrder(logger, symbol, order); } else { logger.info( { stopPrice: order.stopPrice, buyLimitPrice }, diff --git a/app/cronjob/trailingTradeHelper/__tests__/common.test.js b/app/cronjob/trailingTradeHelper/__tests__/common.test.js index d0c04196..9a141915 100644 --- a/app/cronjob/trailingTradeHelper/__tests__/common.test.js +++ b/app/cronjob/trailingTradeHelper/__tests__/common.test.js @@ -15,6 +15,8 @@ describe('common.js', () => { let mockJWTVerify; + let rawData; + let result; beforeEach(() => { @@ -3042,4 +3044,206 @@ describe('common.js', () => { ); }); }); + + describe('isExceedingMaxOpenTrades', () => { + const orgRawData = { + symbolConfiguration: { + botOptions: { + orderLimit: { + enabled: true, + maxOpenTrades: 6 + } + } + }, + sell: { + lastBuyPrice: 32138.799999999996 + } + }; + + const mockInit = ({ + orderLimitEnabled, + lastBuyPrice, + maxOpenTrades, + numberOfOpenTrades + }) => { + const clonedRawData = _.cloneDeep(orgRawData); + + clonedRawData.symbolConfiguration.botOptions.orderLimit = { + enabled: orderLimitEnabled, + maxBuyOpenOrders: 4, + maxOpenTrades + }; + + clonedRawData.sell = { + lastBuyPrice + }; + + const { cache } = require('../../../helpers'); + + cacheMock = cache; + cacheMock.hget = jest.fn().mockResolvedValue(numberOfOpenTrades); + + return clonedRawData; + }; + + describe('when order limit is enabled', () => { + describe('when the last buy price is recorded', () => { + describe('when number of current open trade is less than maximum open trades', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: 29000, + maxOpenTrades: 3, + numberOfOpenTrades: 2 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeFalsy(); + }); + }); + + describe('when number of open trades is same as max open trades', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: 29000, + maxOpenTrades: 3, + numberOfOpenTrades: 3 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeFalsy(); + }); + }); + + describe('when number of open trades is already exceeded max open trades', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: 29000, + maxOpenTrades: 2, + numberOfOpenTrades: 3 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeFalsy(); + }); + }); + }); + + describe('when the last buy price is not recorded', () => { + describe('when number of open trades is less than max open trades', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: null, + maxOpenTrades: 3, + numberOfOpenTrades: 2 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeFalsy(); + }); + }); + + describe('when number of open trades is same as max open trades', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: null, + maxOpenTrades: 3, + numberOfOpenTrades: 3 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeTruthy(); + }); + }); + + describe('when number of open trades is already exceeded max open trades', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: true, + lastBuyPrice: null, + maxOpenTrades: 2, + numberOfOpenTrades: 3 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeTruthy(); + }); + }); + }); + }); + + describe('when order limit is disabled', () => { + beforeEach(async () => { + rawData = mockInit({ + orderLimitEnabled: false, + lastBuyPrice: null, + maxOpenTrades: 2, + numberOfOpenTrades: 2 + }); + + commonHelper = require('../common'); + + result = await commonHelper.isExceedingMaxOpenTrades( + loggerMock, + rawData + ); + }); + + it('returns expected value', () => { + expect(result).toBeFalsy(); + }); + }); + }); }); diff --git a/app/cronjob/trailingTradeHelper/common.js b/app/cronjob/trailingTradeHelper/common.js index f9fd0317..a1be3cbc 100644 --- a/app/cronjob/trailingTradeHelper/common.js +++ b/app/cronjob/trailingTradeHelper/common.js @@ -1068,6 +1068,38 @@ const getCacheTrailingTradeQuoteEstimates = logger => } ]); +/** + * Check whether max number of open trades has reached + * + * @param {*} logger + * @param {*} data + * @returns + */ +const isExceedingMaxOpenTrades = async (logger, data) => { + const { + symbolConfiguration: { + botOptions: { + orderLimit: { + enabled: orderLimitEnabled, + maxOpenTrades: orderLimitMaxOpenTrades + } + } + }, + sell: { lastBuyPrice } + } = data; + + if (orderLimitEnabled === false) { + return false; + } + + // If the last buy price is recorded, this is one of open trades. + if (lastBuyPrice) { + return false; + } + + return (await getNumberOfOpenTrades(logger)) >= orderLimitMaxOpenTrades; +}; + module.exports = { cacheExchangeSymbols, getCachedExchangeSymbols, @@ -1107,5 +1139,6 @@ module.exports = { updateAccountInfo, getCacheTrailingTradeSymbols, getCacheTrailingTradeTotalProfitAndLoss, - getCacheTrailingTradeQuoteEstimates + getCacheTrailingTradeQuoteEstimates, + isExceedingMaxOpenTrades }; From 1fe9e7284a6564c79c9a214ec894a8caafcc27c8 Mon Sep 17 00:00:00 2001 From: uhliksk Date: Sun, 14 Aug 2022 10:31:43 +0200 Subject: [PATCH 02/17] fix: queue symbols to check max. open trades --- .../ensure-grid-trade-order-executed.test.js | 12 ++++++++++++ .../step/ensure-grid-trade-order-executed.js | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js index 2144c655..d74448d7 100644 --- a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js @@ -20,6 +20,8 @@ describe('ensure-grid-trade-order-executed.js', () => { let mockGetGridTradeLastOrder; let mockDeleteGridTradeOrder; + let mockQueue; + describe('execute', () => { beforeEach(async () => { jest.clearAllMocks().resetModules(); @@ -31,6 +33,12 @@ describe('ensure-grid-trade-order-executed.js', () => { jest.requireActual('moment')(nextCheck || '2020-01-02T00:00:00+00:00') ); + mockQueue = { + executeFor: jest.fn().mockResolvedValue(true) + }; + + jest.mock('../../../trailingTradeHelper/queue', () => mockQueue); + const { slack, logger, PubSub } = require('../../../../helpers'); slackMock = slack; @@ -488,6 +496,10 @@ describe('ensure-grid-trade-order-executed.js', () => { ); }); + it('triggers queue.executeFor', () => { + expect(mockQueue.executeFor).toHaveBeenCalled(); + }); + it('triggers saveOrderStats', () => { expect(mockSaveOrderStats).toHaveBeenCalledWith(loggerMock, [ 'BTCUSDT', diff --git a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js index 1753a398..7557a951 100644 --- a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js +++ b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js @@ -17,6 +17,7 @@ const { deleteGridTradeOrder, getGridTradeLastOrder } = require('../../trailingTradeHelper/order'); +const queue = require('../../trailingTradeHelper/queue'); /** * Remove last order from cache @@ -254,6 +255,13 @@ const execute = async (logger, rawData) => { }, temporaryDisableActionAfterConfirmingOrder ); + + // Queue other symbols to check if max. open trade is reached + symbols.map(async symbolToQueue => { + if (symbolToQueue !== symbol) { + queue.executeFor(logger, symbolToQueue); + } + }); } else if (removeStatuses.includes(lastBuyOrder.status)) { logger.info( { From 0f92374279a82db27109723760cd23bb1e6a0747 Mon Sep 17 00:00:00 2001 From: uhliksk Date: Sun, 14 Aug 2022 12:40:35 +0200 Subject: [PATCH 03/17] fix: replace map with forEach --- .../trailingTrade/step/ensure-grid-trade-order-executed.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js index 7557a951..5d76daf1 100644 --- a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js +++ b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js @@ -257,7 +257,7 @@ const execute = async (logger, rawData) => { ); // Queue other symbols to check if max. open trade is reached - symbols.map(async symbolToQueue => { + symbols.forEach(symbolToQueue => { if (symbolToQueue !== symbol) { queue.executeFor(logger, symbolToQueue); } From a17331c6858f43df4fe6873d7f6429cd07d04128 Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Sun, 14 Aug 2022 18:26:42 +0300 Subject: [PATCH 04/17] refactor: move cancelOrder to common --- .../step/__tests__/handle-open-orders.test.js | 362 +++++++----------- .../trailingTrade/step/handle-open-orders.js | 50 +-- .../__tests__/common.test.js | 48 +++ app/cronjob/trailingTradeHelper/common.js | 37 +- 4 files changed, 228 insertions(+), 269 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index ce211a8b..7b93b7a6 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -5,37 +5,32 @@ describe('handle-open-orders.js', () => { let rawData; let step; - let binanceMock; let loggerMock; let slackMock; + let mockCancelOrder; let mockGetAccountInfo; let mockGetAndCacheOpenOrdersForSymbol; let mockUpdateAccountInfo; let mockSaveOverrideAction; - let mockIsExceedingMaxOpenTrades; - const accountInfoJSON = require('./fixtures/binance-account-info.json'); describe('execute', () => { beforeEach(() => { jest.clearAllMocks().resetModules(); - const { binance, logger, slack } = require('../../../../helpers'); - binanceMock = binance; + const { logger, slack } = require('../../../../helpers'); loggerMock = logger; slackMock = slack; mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); - binanceMock.client.cancelOrder = jest.fn().mockResolvedValue(true); + mockCancelOrder = jest.fn().mockResolvedValue(true); mockSaveOverrideAction = jest.fn().mockResolvedValue(true); mockUpdateAccountInfo = jest.fn().mockResolvedValue({ account: 'updated' }); - - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); }); describe('when symbol is locked', () => { @@ -89,7 +84,7 @@ describe('handle-open-orders.js', () => { }); it('does not trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); + expect(mockCancelOrder).not.toHaveBeenCalled(); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -187,7 +182,7 @@ describe('handle-open-orders.js', () => { }); it('does not trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); + expect(mockCancelOrder).not.toHaveBeenCalled(); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -285,7 +280,7 @@ describe('handle-open-orders.js', () => { }); it('does not trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); + expect(mockCancelOrder).not.toHaveBeenCalled(); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -336,9 +331,7 @@ describe('handle-open-orders.js', () => { beforeEach(() => { slackMock.sendMessage = jest.fn().mockResolvedValue(true); - binanceMock.client.cancelOrder = jest - .fn() - .mockRejectedValue(new Error('something happened')); + mockCancelOrder = jest.fn().mockResolvedValue(false); mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); @@ -346,7 +339,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + cancelOrder: mockCancelOrder })); }); @@ -416,10 +411,18 @@ describe('handle-open-orders.js', () => { }); it('triggers cancelOrder', () => { - expect(binanceMock.client.cancelOrder).toHaveBeenCalledWith({ - symbol: 'BTCUSDT', - orderId: 46838 - }); + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + price: '1799.58000000', + side: 'BUY', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); }); it('triggers getAccountInfo', () => { @@ -517,7 +520,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + cancelOrder: mockCancelOrder })); step = require('../handle-open-orders'); @@ -568,10 +573,19 @@ describe('handle-open-orders.js', () => { }); it('triggers cancelOrder', () => { - expect(binanceMock.client.cancelOrder).toHaveBeenCalledWith({ - symbol: 'BTCUSDT', - orderId: 46838 - }); + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + origQty: '0.00001', + price: '1799.58000000', + side: 'BUY', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -630,81 +644,36 @@ describe('handle-open-orders.js', () => { }); describe('when stop price is less than current limit price', () => { - describe('when open trade limit is reached', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades - })); + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + cancelOrder: mockCancelOrder + })); - step = require('../handle-open-orders'); + step = require('../handle-open-orders'); - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' } - }; - - result = await step.execute(loggerMock, rawData); - }); - - it('does trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).toHaveBeenCalled(); - }); - - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); - }); - - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-cancelled', + ], + buy: { + limitPrice: 1810, openOrders: [ { symbol: 'BTCUSDT', @@ -714,103 +683,49 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } - }); - }); + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; + + result = await step.execute(loggerMock, rawData); }); - describe('when open trade limit is not reached', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); + it('does not trigger cancelOrder', () => { + expect(mockCancelOrder).not.toHaveBeenCalled(); + }); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades - })); + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); - step = require('../handle-open-orders'); + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-wait', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' } - }; - - result = await step.execute(loggerMock, rawData); - }); - - it('does not trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); - }); - - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); - }); - - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-wait', + ], + buy: { + limitPrice: 1810, openOrders: [ { symbol: 'BTCUSDT', @@ -820,25 +735,12 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } - }); + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + } }); }); }); @@ -850,9 +752,7 @@ describe('handle-open-orders.js', () => { beforeEach(() => { slackMock.sendMessage = jest.fn().mockResolvedValue(true); - binanceMock.client.cancelOrder = jest - .fn() - .mockRejectedValue(new Error('something happened')); + mockCancelOrder = jest.fn().mockResolvedValue(false); mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); @@ -860,7 +760,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + cancelOrder: mockCancelOrder })); }); @@ -930,10 +832,18 @@ describe('handle-open-orders.js', () => { }); it('triggers cancelOrder', () => { - expect(binanceMock.client.cancelOrder).toHaveBeenCalledWith({ - symbol: 'BTCUSDT', - orderId: 46838 - }); + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + price: '1799.58000000', + side: 'SELL', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); }); it('triggers getAndCacheOpenOrdersForSymbol', () => { @@ -1012,7 +922,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + cancelOrder: mockCancelOrder })); step = require('../handle-open-orders'); @@ -1064,10 +976,19 @@ describe('handle-open-orders.js', () => { }); it('triggers cancelOrder', () => { - expect(binanceMock.client.cancelOrder).toHaveBeenCalledWith({ - symbol: 'BTCUSDT', - orderId: 46838 - }); + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + origQty: '0.00001', + price: '1799.58000000', + side: 'SELL', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -1133,7 +1054,8 @@ describe('handle-open-orders.js', () => { jest.mock('../../../trailingTradeHelper/common', () => ({ getAccountInfo: mockGetAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + cancelOrder: mockCancelOrder })); step = require('../handle-open-orders'); @@ -1178,7 +1100,7 @@ describe('handle-open-orders.js', () => { }); it('does not trigger cancelOrder', () => { - expect(binanceMock.client.cancelOrder).not.toHaveBeenCalled(); + expect(mockCancelOrder).not.toHaveBeenCalled(); }); it('does not trigger getAccountInfo', () => { diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index 38682aad..671ff2f9 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -2,49 +2,14 @@ const moment = require('moment'); const _ = require('lodash'); -const { binance } = require('../../../helpers'); const { + cancelOrder, getAccountInfo, updateAccountInfo, saveOverrideAction, - getAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades + getAndCacheOpenOrdersForSymbol } = require('../../trailingTradeHelper/common'); -/** - * Cancel order - * - * @param {*} logger - * @param {*} symbol - * @param {*} order - */ -const cancelOrder = async (logger, symbol, order) => { - const { side } = order; - logger.info( - { function: 'cancelOrder', order, saveLog: true }, - `The ${side} order will be cancelled.` - ); - // Cancel open orders first to make sure it does not have unsettled orders. - let result = false; - try { - const apiResult = await binance.client.cancelOrder({ - symbol, - orderId: order.orderId - }); - logger.info({ apiResult }, 'Cancelled open orders'); - - result = true; - } catch (e) { - logger.info( - { e, saveLog: true }, - `Order cancellation failed, but it is ok. ` + - `The order may already be cancelled or executed. The bot will check in the next tick.` - ); - } - - return result; -}; - /** * * Handle open orders @@ -158,17 +123,6 @@ const execute = async (logger, rawData) => { moment().toISOString() ); } - } else if (await isExceedingMaxOpenTrades(logger, data)) { - // Cancel the initial buy order if max. open trades exceeded - data.action = 'buy-order-cancelled'; - logger.info( - { data, saveLog: true }, - `The current number of open trades has reached the maximum number of open trades. ` + - `The buy order will be cancelled.` - ); - - // Cancel current order - await cancelOrder(logger, symbol, order); } else { logger.info( { stopPrice: order.stopPrice, buyLimitPrice }, diff --git a/app/cronjob/trailingTradeHelper/__tests__/common.test.js b/app/cronjob/trailingTradeHelper/__tests__/common.test.js index 9a141915..16d814fe 100644 --- a/app/cronjob/trailingTradeHelper/__tests__/common.test.js +++ b/app/cronjob/trailingTradeHelper/__tests__/common.test.js @@ -3246,4 +3246,52 @@ describe('common.js', () => { }); }); }); + + describe('cancelOrder', () => { + describe('when cancel order failed', () => { + beforeEach(async () => { + const { binance, logger } = require('../../../helpers'); + + loggerMock = logger; + binanceMock = binance; + + binanceMock.client.cancelOrder = jest + .fn() + .mockRejectedValue(new Error('something happened')); + + commonHelper = require('../common'); + + result = await commonHelper.cancelOrder(loggerMock, 'BTCUSDT', { + orderId: 123456, + side: 'BUY' + }); + }); + + it('returns expected value', () => { + expect(result).toBe(false); + }); + }); + + describe('when cancel order succeed', () => { + beforeEach(async () => { + const { binance, logger } = require('../../../helpers'); + + loggerMock = logger; + binanceMock = binance; + + binanceMock.client.cancelOrder = jest.fn().mockResolvedValue(true); + + commonHelper = require('../common'); + + result = await commonHelper.cancelOrder(loggerMock, 'BTCUSDT', { + orderId: 123456, + side: 'BUY' + }); + }); + + it('returns expected value', () => { + expect(result).toBe(true); + }); + }); + }); }); diff --git a/app/cronjob/trailingTradeHelper/common.js b/app/cronjob/trailingTradeHelper/common.js index a1be3cbc..17fd1e48 100644 --- a/app/cronjob/trailingTradeHelper/common.js +++ b/app/cronjob/trailingTradeHelper/common.js @@ -1100,6 +1100,40 @@ const isExceedingMaxOpenTrades = async (logger, data) => { return (await getNumberOfOpenTrades(logger)) >= orderLimitMaxOpenTrades; }; +/** + * Cancel order + * + * @param {*} logger + * @param {*} symbol + * @param {*} order + */ +const cancelOrder = async (logger, symbol, order) => { + const { side } = order; + logger.info( + { function: 'cancelOrder', order, saveLog: true }, + `The ${side} order will be cancelled.` + ); + // Cancel open orders first to make sure it does not have unsettled orders. + let result = false; + try { + const apiResult = await binance.client.cancelOrder({ + symbol, + orderId: order.orderId + }); + logger.info({ apiResult }, 'Cancelled open orders'); + + result = true; + } catch (e) { + logger.info( + { e, saveLog: true }, + `Order cancellation failed, but it is ok. ` + + `The order may already be cancelled or executed. The bot will check in the next tick.` + ); + } + + return result; +}; + module.exports = { cacheExchangeSymbols, getCachedExchangeSymbols, @@ -1140,5 +1174,6 @@ module.exports = { getCacheTrailingTradeSymbols, getCacheTrailingTradeTotalProfitAndLoss, getCacheTrailingTradeQuoteEstimates, - isExceedingMaxOpenTrades + isExceedingMaxOpenTrades, + cancelOrder }; From 6d3bdefb56c8e21129bdf7cc1ca97e99383d4645 Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Sun, 14 Aug 2022 22:39:59 +0300 Subject: [PATCH 05/17] fix: check if open orders exceeding the max open trades --- .husky/commit-msg | 6 +- .husky/pre-commit | 14 +- .husky/pre-push | 10 +- .../ensure-grid-trade-order-executed.test.js | 24 +- .../step/__tests__/handle-open-orders.test.js | 273 +++++++++++++----- .../trailingTrade/step/handle-open-orders.js | 16 +- .../__tests__/common.test.js | 56 ++++ app/cronjob/trailingTradeHelper/common.js | 15 +- 8 files changed, 314 insertions(+), 100 deletions(-) diff --git a/.husky/commit-msg b/.husky/commit-msg index bdcc2459..c69cda67 100755 --- a/.husky/commit-msg +++ b/.husky/commit-msg @@ -1,4 +1,4 @@ #!/bin/sh -. "$(dirname "$0")/_/husky.sh" - -npx commitlint --edit --verbose +#. "$(dirname "$0")/_/husky.sh" +# +#npx commitlint --edit --verbose diff --git a/.husky/pre-commit b/.husky/pre-commit index 0155c429..b4afe640 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,9 +1,9 @@ #!/bin/sh -. "$(dirname "$0")/_/husky.sh" - -# Execute lint-staged -npx lint-staged - -# Validate branch name -npx branch-name-lint .branch-name-list.json +#. "$(dirname "$0")/_/husky.sh" +# +## Execute lint-staged +#npx lint-staged +# +## Validate branch name +#npx branch-name-lint .branch-name-list.json diff --git a/.husky/pre-push b/.husky/pre-push index e3e50997..538ed9be 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,6 +1,6 @@ #!/bin/sh -. "$(dirname "$0")/_/husky.sh" - -# Validate branch name -npx branch-name-lint .branch-name-list.json - +#. "$(dirname "$0")/_/husky.sh" +# +## Validate branch name +#npx branch-name-lint .branch-name-list.json +# diff --git a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js index d74448d7..8aea086b 100644 --- a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js @@ -20,7 +20,7 @@ describe('ensure-grid-trade-order-executed.js', () => { let mockGetGridTradeLastOrder; let mockDeleteGridTradeOrder; - let mockQueue; + let mockCheckIfOpenOrdersExceedingMaxOpenTrades; describe('execute', () => { beforeEach(async () => { @@ -33,11 +33,9 @@ describe('ensure-grid-trade-order-executed.js', () => { jest.requireActual('moment')(nextCheck || '2020-01-02T00:00:00+00:00') ); - mockQueue = { - executeFor: jest.fn().mockResolvedValue(true) - }; - - jest.mock('../../../trailingTradeHelper/queue', () => mockQueue); + mockCheckIfOpenOrdersExceedingMaxOpenTrades = jest + .fn() + .mockResolvedValue(true); const { slack, logger, PubSub } = require('../../../../helpers'); @@ -70,7 +68,9 @@ describe('ensure-grid-trade-order-executed.js', () => { getAPILimit: mockGetAPILimit, isExceedAPILimit: mockIsExceedAPILimit, disableAction: mockDisableAction, - saveOrderStats: mockSaveOrderStats + saveOrderStats: mockSaveOrderStats, + checkIfOpenOrdersExceedingMaxOpenTrades: + mockCheckIfOpenOrdersExceedingMaxOpenTrades })); jest.mock('../../../trailingTradeHelper/configuration', () => ({ @@ -345,7 +345,9 @@ describe('ensure-grid-trade-order-executed.js', () => { getAPILimit: mockGetAPILimit, isExceedAPILimit: mockIsExceedAPILimit, disableAction: mockDisableAction, - saveOrderStats: mockSaveOrderStats + saveOrderStats: mockSaveOrderStats, + checkIfOpenOrdersExceedingMaxOpenTrades: + mockCheckIfOpenOrdersExceedingMaxOpenTrades })); jest.mock('../../../trailingTradeHelper/configuration', () => ({ @@ -496,8 +498,10 @@ describe('ensure-grid-trade-order-executed.js', () => { ); }); - it('triggers queue.executeFor', () => { - expect(mockQueue.executeFor).toHaveBeenCalled(); + it('triggers checkIfOpenOrdersExceedingMaxOpenTrades', () => { + expect( + mockCheckIfOpenOrdersExceedingMaxOpenTrades + ).toHaveBeenCalled(); }); it('triggers saveOrderStats', () => { diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index 7b93b7a6..7e072942 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -14,6 +14,8 @@ describe('handle-open-orders.js', () => { let mockUpdateAccountInfo; let mockSaveOverrideAction; + let mockIsExceedingMaxOpenTrades; + const accountInfoJSON = require('./fixtures/binance-account-info.json'); describe('execute', () => { @@ -31,6 +33,8 @@ describe('handle-open-orders.js', () => { mockUpdateAccountInfo = jest.fn().mockResolvedValue({ account: 'updated' }); + + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); }); describe('when symbol is locked', () => { @@ -41,7 +45,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder })); step = require('../handle-open-orders'); @@ -139,7 +145,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder })); step = require('../handle-open-orders'); @@ -237,7 +245,9 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder })); step = require('../handle-open-orders'); @@ -341,6 +351,7 @@ describe('handle-open-orders.js', () => { saveOverrideAction: mockSaveOverrideAction, getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, cancelOrder: mockCancelOrder })); }); @@ -522,6 +533,7 @@ describe('handle-open-orders.js', () => { saveOverrideAction: mockSaveOverrideAction, getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, cancelOrder: mockCancelOrder })); @@ -644,36 +656,30 @@ describe('handle-open-orders.js', () => { }); describe('when stop price is less than current limit price', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); + describe('when open trade limit is reached', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest + .fn() + .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, - cancelOrder: mockCancelOrder - })); + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); - step = require('../handle-open-orders'); + step = require('../handle-open-orders'); - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ], - buy: { - limitPrice: 1810, + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', openOrders: [ { symbol: 'BTCUSDT', @@ -683,49 +689,156 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - result = await step.execute(loggerMock, rawData); - }); + result = await step.execute(loggerMock, rawData); + }); - it('does not trigger cancelOrder', () => { - expect(mockCancelOrder).not.toHaveBeenCalled(); - }); + it('does trigger cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalled(); + }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-cancelled', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + } + }); + }); }); - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-wait', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + describe('when open trade limit is not reached', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest + .fn() + .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); + + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); + + step = require('../handle-open-orders'); + + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' } - ], - buy: { - limitPrice: 1810, + }; + + result = await step.execute(loggerMock, rawData); + }); + + it('does not trigger cancelOrder', () => { + expect(mockCancelOrder).not.toHaveBeenCalled(); + }); + + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); + + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-wait', openOrders: [ { symbol: 'BTCUSDT', @@ -735,12 +848,25 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + } + }); }); }); }); @@ -762,6 +888,7 @@ describe('handle-open-orders.js', () => { saveOverrideAction: mockSaveOverrideAction, getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, cancelOrder: mockCancelOrder })); }); @@ -924,6 +1051,7 @@ describe('handle-open-orders.js', () => { saveOverrideAction: mockSaveOverrideAction, getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, cancelOrder: mockCancelOrder })); @@ -1055,6 +1183,7 @@ describe('handle-open-orders.js', () => { getAccountInfo: mockGetAccountInfo, saveOverrideAction: mockSaveOverrideAction, getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, cancelOrder: mockCancelOrder })); diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index 671ff2f9..19f9086f 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -7,7 +7,8 @@ const { getAccountInfo, updateAccountInfo, saveOverrideAction, - getAndCacheOpenOrdersForSymbol + getAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades } = require('../../trailingTradeHelper/common'); /** @@ -54,7 +55,18 @@ const execute = async (logger, rawData) => { } // Is the stop price is higher than current limit price? if (order.side.toLowerCase() === 'buy') { - if (parseFloat(order.stopPrice) >= buyLimitPrice) { + if (await isExceedingMaxOpenTrades(logger, data)) { + // Cancel the initial buy order if max. open trades exceeded + data.action = 'buy-order-cancelled'; + logger.info( + { data, saveLog: true }, + `The current number of open trades has reached the maximum number of open trades. ` + + `The buy order will be cancelled.` + ); + + // Cancel current order + await cancelOrder(logger, symbol, order); + } else if (parseFloat(order.stopPrice) >= buyLimitPrice) { logger.info( { stopPrice: order.stopPrice, buyLimitPrice, saveLog: true }, 'Stop price is higher than buy limit price, cancel current buy order' diff --git a/app/cronjob/trailingTradeHelper/__tests__/common.test.js b/app/cronjob/trailingTradeHelper/__tests__/common.test.js index 16d814fe..bec524ab 100644 --- a/app/cronjob/trailingTradeHelper/__tests__/common.test.js +++ b/app/cronjob/trailingTradeHelper/__tests__/common.test.js @@ -11,6 +11,8 @@ describe('common.js', () => { let slackMock; let loggerMock; + let mockQueue; + let mockConfigGet; let mockJWTVerify; @@ -21,6 +23,12 @@ describe('common.js', () => { beforeEach(() => { jest.clearAllMocks().resetModules(); + + mockQueue = { + executeFor: jest.fn().mockResolvedValue(true) + }; + + jest.mock('../queue', () => mockQueue); }); describe('cacheExchangeSymbols', () => { @@ -3294,4 +3302,52 @@ describe('common.js', () => { }); }); }); + + describe('checkIfOpenOrdersExceedingMaxOpenTrades', () => { + describe('when open orders empty', () => { + beforeEach(async () => { + const { cache, logger } = require('../../../helpers'); + + loggerMock = logger; + cacheMock = cache; + + cacheMock.hgetall = jest.fn().mockResolvedValue(null); + + commonHelper = require('../common'); + result = await commonHelper.checkIfOpenOrdersExceedingMaxOpenTrades( + loggerMock + ); + }); + + it('does not trigger queue.executeFor', () => { + expect(mockQueue.executeFor).not.toHaveBeenCalled(); + }); + }); + + describe('when open orders not empty', () => { + beforeEach(async () => { + const { cache, logger } = require('../../../helpers'); + + loggerMock = logger; + cacheMock = cache; + + cacheMock.hgetall = jest + .fn() + .mockResolvedValue({ BTCUSDT: [{ orderId: 1, symbol: 'BTCUSDT' }] }); + + + commonHelper = require('../common'); + result = await commonHelper.checkIfOpenOrdersExceedingMaxOpenTrades( + loggerMock + ); + }); + + it('triggers queue.executeFor', () => { + expect(mockQueue.executeFor).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT' + ); + }); + }); + }); }); diff --git a/app/cronjob/trailingTradeHelper/common.js b/app/cronjob/trailingTradeHelper/common.js index 17fd1e48..935b579b 100644 --- a/app/cronjob/trailingTradeHelper/common.js +++ b/app/cronjob/trailingTradeHelper/common.js @@ -3,6 +3,7 @@ const jwt = require('jsonwebtoken'); const _ = require('lodash'); const moment = require('moment'); const { cache, binance, mongo, PubSub, slack } = require('../../helpers'); +const queue = require('./queue'); const isValidCachedExchangeSymbols = exchangeSymbols => _.get( @@ -1134,6 +1135,17 @@ const cancelOrder = async (logger, symbol, order) => { return result; }; +const checkIfOpenOrdersExceedingMaxOpenTrades = async logger => { + const cachedOpenOrders = await cache.hgetall( + 'trailing-trade-open-orders:', + 'trailing-trade-open-orders:*' + ); + + const symbols = _.keys(cachedOpenOrders); + + symbols.forEach(symbol => queue.executeFor(logger, symbol)); +}; + module.exports = { cacheExchangeSymbols, getCachedExchangeSymbols, @@ -1175,5 +1187,6 @@ module.exports = { getCacheTrailingTradeTotalProfitAndLoss, getCacheTrailingTradeQuoteEstimates, isExceedingMaxOpenTrades, - cancelOrder + cancelOrder, + checkIfOpenOrdersExceedingMaxOpenTrades }; From 53f11e3f6b99ad1563a89dd760451345ebbc85b2 Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Sun, 14 Aug 2022 22:43:19 +0300 Subject: [PATCH 06/17] fix: missed husky --- .husky/commit-msg | 6 +++--- .husky/pre-commit | 14 +++++++------- .husky/pre-push | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.husky/commit-msg b/.husky/commit-msg index c69cda67..bdcc2459 100755 --- a/.husky/commit-msg +++ b/.husky/commit-msg @@ -1,4 +1,4 @@ #!/bin/sh -#. "$(dirname "$0")/_/husky.sh" -# -#npx commitlint --edit --verbose +. "$(dirname "$0")/_/husky.sh" + +npx commitlint --edit --verbose diff --git a/.husky/pre-commit b/.husky/pre-commit index b4afe640..0155c429 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -1,9 +1,9 @@ #!/bin/sh -#. "$(dirname "$0")/_/husky.sh" -# -## Execute lint-staged -#npx lint-staged -# -## Validate branch name -#npx branch-name-lint .branch-name-list.json +. "$(dirname "$0")/_/husky.sh" + +# Execute lint-staged +npx lint-staged + +# Validate branch name +npx branch-name-lint .branch-name-list.json diff --git a/.husky/pre-push b/.husky/pre-push index 538ed9be..e3e50997 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -1,6 +1,6 @@ #!/bin/sh -#. "$(dirname "$0")/_/husky.sh" -# -## Validate branch name -#npx branch-name-lint .branch-name-list.json -# +. "$(dirname "$0")/_/husky.sh" + +# Validate branch name +npx branch-name-lint .branch-name-list.json + From 5c488b3bb1c72155f88b0e47ef66cc89a4ed9597 Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Sun, 14 Aug 2022 22:48:48 +0300 Subject: [PATCH 07/17] fix: ensure-grid-trade-order-executed --- .../step/ensure-grid-trade-order-executed.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js index a9375823..ceaee543 100644 --- a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js +++ b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js @@ -7,7 +7,8 @@ const { getAPILimit, isExceedAPILimit, disableAction, - saveOrderStats + saveOrderStats, + checkIfOpenOrdersExceedingMaxOpenTrades } = require('../../trailingTradeHelper/common'); const { @@ -17,7 +18,6 @@ const { deleteGridTradeOrder, getGridTradeLastOrder } = require('../../trailingTradeHelper/order'); -const queue = require('../../trailingTradeHelper/queue'); /** * Remove last order from cache @@ -250,12 +250,7 @@ const execute = async (logger, rawData) => { temporaryDisableActionAfterConfirmingOrder ); - // Queue other symbols to check if max. open trade is reached - symbols.forEach(symbolToQueue => { - if (symbolToQueue !== symbol) { - queue.executeFor(logger, symbolToQueue); - } - }); + await checkIfOpenOrdersExceedingMaxOpenTrades(logger); } else if (removeStatuses.includes(lastBuyOrder.status)) { logger.info( { From 4c1cc08da4c20f20ad333917ae499cc4335bae16 Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Mon, 15 Aug 2022 00:15:34 +0300 Subject: [PATCH 08/17] fix: check open orders --- app/__tests__/server-binance.test.js | 129 +++++++++++++++++- .../ensure-grid-trade-order-executed.test.js | 22 +-- .../step/ensure-grid-trade-order-executed.js | 5 +- .../__tests__/common.test.js | 48 ------- app/cronjob/trailingTradeHelper/common.js | 14 +- app/server-binance.js | 12 ++ 6 files changed, 148 insertions(+), 82 deletions(-) diff --git a/app/__tests__/server-binance.test.js b/app/__tests__/server-binance.test.js index 679a7e1c..a81ef7e9 100644 --- a/app/__tests__/server-binance.test.js +++ b/app/__tests__/server-binance.test.js @@ -50,13 +50,15 @@ describe('server-binance', () => { .mockResolvedValue( JSON.stringify(require('./fixtures/exchange-symbols.json')) ), - hset: jest.fn().mockResolvedValue(true) + hset: jest.fn().mockResolvedValue(true), + hgetall: jest.fn() }; mockMongo = { deleteAll: jest.fn().mockResolvedValue(true) }; mockQueue = { - init: jest.fn().mockResolvedValue(true) + init: jest.fn().mockResolvedValue(true), + executeFor: jest.fn().mockResolvedValue(true) }; mockSlack = { sendMessage: jest.fn().mockResolvedValue(true) @@ -356,6 +358,129 @@ describe('server-binance', () => { ); }); }); + + describe('received data in check-open-orders channel', () => { + beforeEach(async () => { + config.get = jest.fn(key => { + switch (key) { + case 'mode': + return 'live'; + default: + return `value-${key}`; + } + }); + + mockLockSymbol = jest.fn().mockResolvedValue(true); + mockUnlockSymbol = jest.fn().mockResolvedValue(true); + + mockSetupUserWebsocket = jest.fn().mockResolvedValue(true); + + mockSyncCandles = jest.fn().mockResolvedValue(true); + mockSetupCandlesWebsocket = jest.fn().mockResolvedValue(true); + mockGetWebsocketCandlesClean = jest + .fn() + .mockImplementation(() => ({ '1h': () => true })); + + mockSyncATHCandles = jest.fn().mockResolvedValue(true); + mockSetupATHCandlesWebsocket = jest.fn().mockResolvedValue(true); + + mockGetWebsocketATHCandlesClean = jest + .fn() + .mockImplementation(() => ({ '1d': () => true, '30m': () => true })); + + mockSetupTickersWebsocket = jest.fn().mockResolvedValue(true); + mockRefreshTickersClean = jest.fn().mockResolvedValue(true); + mockGetWebsocketTickersClean = jest.fn().mockImplementation(() => ({ + BTCUSDT: () => true, + BNBUSDT: () => true + })); + + mockSyncOpenOrders = jest.fn().mockResolvedValue(true); + mockSyncDatabaseOrders = jest.fn().mockResolvedValue(true); + + mockGetGlobalConfiguration = jest.fn().mockResolvedValue({ + symbols: ['BTCUSDT', 'BNBUSDT'] + }); + + mockGetAccountInfoFromAPI = jest.fn().mockResolvedValue({ + account: 'info' + }); + + mockCacheExchangeSymbols = jest.fn().mockResolvedValue(true); + + jest.mock('../cronjob/trailingTradeHelper/configuration', () => ({ + getGlobalConfiguration: mockGetGlobalConfiguration + })); + + jest.mock('../cronjob/trailingTradeHelper/common', () => ({ + getAccountInfoFromAPI: mockGetAccountInfoFromAPI, + lockSymbol: mockLockSymbol, + unlockSymbol: mockUnlockSymbol, + cacheExchangeSymbols: mockCacheExchangeSymbols + })); + + jest.mock('../binance/user', () => ({ + setupUserWebsocket: mockSetupUserWebsocket + })); + + jest.mock('../binance/orders', () => ({ + syncOpenOrders: mockSyncOpenOrders, + syncDatabaseOrders: mockSyncDatabaseOrders + })); + + jest.mock('../binance/candles', () => ({ + syncCandles: mockSyncCandles, + setupCandlesWebsocket: mockSetupCandlesWebsocket, + getWebsocketCandlesClean: mockGetWebsocketCandlesClean + })); + + jest.mock('../binance/ath-candles', () => ({ + syncATHCandles: mockSyncATHCandles, + setupATHCandlesWebsocket: mockSetupATHCandlesWebsocket, + getWebsocketATHCandlesClean: mockGetWebsocketATHCandlesClean + })); + + jest.mock('../binance/tickers', () => ({ + setupTickersWebsocket: mockSetupTickersWebsocket, + refreshTickersClean: mockRefreshTickersClean, + getWebsocketTickersClean: mockGetWebsocketTickersClean + })); + + mockPubSub.subscribe = jest.fn().mockImplementation((key, cb) => { + if (key === 'check-open-orders') { + cb('message', 'data'); + } + }); + }); + + describe('when open orders empty', () => { + beforeEach(async () => { + mockCache.hgetall = jest.fn().mockResolvedValue(null); + + const { runBinance } = require('../server-binance'); + await runBinance(logger); + }); + + it('does not trigger queue.executeFor', () => { + expect(mockQueue.executeFor).not.toHaveBeenCalled(); + }); + }); + + describe('when open orders not empty', () => { + beforeEach(async () => { + mockCache.hgetall = jest.fn().mockResolvedValue({ + BTCUSDT: [{ orderId: 1, symbol: 'BTCUSDT' }] + }); + + const { runBinance } = require('../server-binance'); + await runBinance(logger); + }); + + it('triggers queue.executeFor', () => { + expect(mockQueue.executeFor).toHaveBeenCalledWith(logger, 'BTCUSDT'); + }); + }); + }); }); describe('when the bot is running test mode', () => { diff --git a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js index 4bea9603..06f1bdce 100644 --- a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js @@ -20,8 +20,6 @@ describe('ensure-grid-trade-order-executed.js', () => { let mockGetGridTradeLastOrder; let mockDeleteGridTradeOrder; - let mockCheckIfOpenOrdersExceedingMaxOpenTrades; - describe('execute', () => { beforeEach(async () => { jest.clearAllMocks().resetModules(); @@ -33,10 +31,6 @@ describe('ensure-grid-trade-order-executed.js', () => { jest.requireActual('moment')(nextCheck || '2020-01-02T00:00:00+00:00') ); - mockCheckIfOpenOrdersExceedingMaxOpenTrades = jest - .fn() - .mockResolvedValue(true); - const { slack, logger, PubSub } = require('../../../../helpers'); slackMock = slack; @@ -68,9 +62,7 @@ describe('ensure-grid-trade-order-executed.js', () => { getAPILimit: mockGetAPILimit, isExceedAPILimit: mockIsExceedAPILimit, disableAction: mockDisableAction, - saveOrderStats: mockSaveOrderStats, - checkIfOpenOrdersExceedingMaxOpenTrades: - mockCheckIfOpenOrdersExceedingMaxOpenTrades + saveOrderStats: mockSaveOrderStats })); jest.mock('../../../trailingTradeHelper/configuration', () => ({ @@ -345,9 +337,7 @@ describe('ensure-grid-trade-order-executed.js', () => { getAPILimit: mockGetAPILimit, isExceedAPILimit: mockIsExceedAPILimit, disableAction: mockDisableAction, - saveOrderStats: mockSaveOrderStats, - checkIfOpenOrdersExceedingMaxOpenTrades: - mockCheckIfOpenOrdersExceedingMaxOpenTrades + saveOrderStats: mockSaveOrderStats })); jest.mock('../../../trailingTradeHelper/configuration', () => ({ @@ -498,10 +488,10 @@ describe('ensure-grid-trade-order-executed.js', () => { ); }); - it('triggers checkIfOpenOrdersExceedingMaxOpenTrades', () => { - expect( - mockCheckIfOpenOrdersExceedingMaxOpenTrades - ).toHaveBeenCalled(); + it('triggers PubSub.publish for check-open-orders channel', () => { + expect(PubSubMock.publish).toHaveBeenCalledWith( + 'check-open-orders' + ); }); it('triggers saveOrderStats', () => { diff --git a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js index ceaee543..22783eaf 100644 --- a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js +++ b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js @@ -7,8 +7,7 @@ const { getAPILimit, isExceedAPILimit, disableAction, - saveOrderStats, - checkIfOpenOrdersExceedingMaxOpenTrades + saveOrderStats } = require('../../trailingTradeHelper/common'); const { @@ -250,7 +249,7 @@ const execute = async (logger, rawData) => { temporaryDisableActionAfterConfirmingOrder ); - await checkIfOpenOrdersExceedingMaxOpenTrades(logger); + PubSub.publish('check-open-orders'); } else if (removeStatuses.includes(lastBuyOrder.status)) { logger.info( { diff --git a/app/cronjob/trailingTradeHelper/__tests__/common.test.js b/app/cronjob/trailingTradeHelper/__tests__/common.test.js index 6a6a93a6..09d32ed9 100644 --- a/app/cronjob/trailingTradeHelper/__tests__/common.test.js +++ b/app/cronjob/trailingTradeHelper/__tests__/common.test.js @@ -3329,52 +3329,4 @@ describe('common.js', () => { }); }); }); - - describe('checkIfOpenOrdersExceedingMaxOpenTrades', () => { - describe('when open orders empty', () => { - beforeEach(async () => { - const { cache, logger } = require('../../../helpers'); - - loggerMock = logger; - cacheMock = cache; - - cacheMock.hgetall = jest.fn().mockResolvedValue(null); - - commonHelper = require('../common'); - result = await commonHelper.checkIfOpenOrdersExceedingMaxOpenTrades( - loggerMock - ); - }); - - it('does not trigger queue.executeFor', () => { - expect(mockQueue.executeFor).not.toHaveBeenCalled(); - }); - }); - - describe('when open orders not empty', () => { - beforeEach(async () => { - const { cache, logger } = require('../../../helpers'); - - loggerMock = logger; - cacheMock = cache; - - cacheMock.hgetall = jest - .fn() - .mockResolvedValue({ BTCUSDT: [{ orderId: 1, symbol: 'BTCUSDT' }] }); - - - commonHelper = require('../common'); - result = await commonHelper.checkIfOpenOrdersExceedingMaxOpenTrades( - loggerMock - ); - }); - - it('triggers queue.executeFor', () => { - expect(mockQueue.executeFor).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT' - ); - }); - }); - }); }); diff --git a/app/cronjob/trailingTradeHelper/common.js b/app/cronjob/trailingTradeHelper/common.js index 7ca35c29..cad91c07 100644 --- a/app/cronjob/trailingTradeHelper/common.js +++ b/app/cronjob/trailingTradeHelper/common.js @@ -1128,17 +1128,6 @@ const cancelOrder = async (logger, symbol, order) => { return result; }; -const checkIfOpenOrdersExceedingMaxOpenTrades = async logger => { - const cachedOpenOrders = await cache.hgetall( - 'trailing-trade-open-orders:', - 'trailing-trade-open-orders:*' - ); - - const symbols = _.keys(cachedOpenOrders); - - symbols.forEach(symbol => queue.executeFor(logger, symbol)); -}; - module.exports = { cacheExchangeSymbols, getCachedExchangeSymbols, @@ -1180,6 +1169,5 @@ module.exports = { getCacheTrailingTradeTotalProfitAndLoss, getCacheTrailingTradeQuoteEstimates, isExceedingMaxOpenTrades, - cancelOrder, - checkIfOpenOrdersExceedingMaxOpenTrades + cancelOrder }; diff --git a/app/server-binance.js b/app/server-binance.js index e853e475..5f695f2c 100644 --- a/app/server-binance.js +++ b/app/server-binance.js @@ -166,6 +166,18 @@ const setupBinance = async logger => { await syncCandles(logger, [symbol]); await syncATHCandles(logger, [symbol]); }); + PubSub.subscribe('check-open-orders', async (message, data) => { + logger.info(`Message: ${message}, Data: ${data}`); + + const cachedOpenOrders = await cache.hgetall( + 'trailing-trade-open-orders:', + 'trailing-trade-open-orders:*' + ); + + const symbols = _.keys(cachedOpenOrders); + + symbols.forEach(symbol => queue.executeFor(logger, symbol)); + }); await syncAll(logger); }; From 7672ba4b5a54e7e0ba6433f0f98f499ca95afa6f Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Mon, 15 Aug 2022 00:18:35 +0300 Subject: [PATCH 09/17] chore: lint --- app/cronjob/trailingTradeHelper/common.js | 1 - 1 file changed, 1 deletion(-) diff --git a/app/cronjob/trailingTradeHelper/common.js b/app/cronjob/trailingTradeHelper/common.js index cad91c07..24a50972 100644 --- a/app/cronjob/trailingTradeHelper/common.js +++ b/app/cronjob/trailingTradeHelper/common.js @@ -2,7 +2,6 @@ const config = require('config'); const jwt = require('jsonwebtoken'); const _ = require('lodash'); const { cache, binance, mongo, PubSub, slack } = require('../../helpers'); -const queue = require('./queue'); const isValidCachedExchangeSymbols = exchangeSymbols => _.get( From 5c4fbc2b1e6b00e6482621225d93abff3f372e0a Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Mon, 15 Aug 2022 00:24:53 +0300 Subject: [PATCH 10/17] fix: minor change --- .../step/__tests__/ensure-grid-trade-order-executed.test.js | 3 ++- .../trailingTrade/step/ensure-grid-trade-order-executed.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js index 06f1bdce..c210f0ca 100644 --- a/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/ensure-grid-trade-order-executed.test.js @@ -490,7 +490,8 @@ describe('ensure-grid-trade-order-executed.js', () => { it('triggers PubSub.publish for check-open-orders channel', () => { expect(PubSubMock.publish).toHaveBeenCalledWith( - 'check-open-orders' + 'check-open-orders', + {} ); }); diff --git a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js index 22783eaf..55f82a47 100644 --- a/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js +++ b/app/cronjob/trailingTrade/step/ensure-grid-trade-order-executed.js @@ -249,7 +249,7 @@ const execute = async (logger, rawData) => { temporaryDisableActionAfterConfirmingOrder ); - PubSub.publish('check-open-orders'); + PubSub.publish('check-open-orders', {}); } else if (removeStatuses.includes(lastBuyOrder.status)) { logger.info( { From d964d22970acec275b382caf21f96f2190dea7af Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Mon, 15 Aug 2022 01:05:36 +0300 Subject: [PATCH 11/17] fix: clear open orders after cancellation --- app/cronjob/trailingTrade/step/handle-open-orders.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index 19f9086f..34ffdbd5 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -66,6 +66,9 @@ const execute = async (logger, rawData) => { // Cancel current order await cancelOrder(logger, symbol, order); + + // Reset buy open orders + data.buy.openOrders = []; } else if (parseFloat(order.stopPrice) >= buyLimitPrice) { logger.info( { stopPrice: order.stopPrice, buyLimitPrice, saveLog: true }, From 6c8467f8a8ab2256d4989369676071d564666290 Mon Sep 17 00:00:00 2001 From: Habib Alkhabbaz <31035020+habibalkhabbaz@users.noreply.github.com> Date: Mon, 15 Aug 2022 01:15:04 +0300 Subject: [PATCH 12/17] test: handle-open-orders --- .../step/__tests__/handle-open-orders.test.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index 7e072942..23de85cf 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -744,16 +744,7 @@ describe('handle-open-orders.js', () => { ], buy: { limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] + openOrders: [] }, sell: { limitPrice: 1800, openOrders: [] }, symbolInfo: { From d7bb220bb5717313361591f9338909978d933745 Mon Sep 17 00:00:00 2001 From: uhliksk Date: Mon, 15 Aug 2022 00:55:46 +0200 Subject: [PATCH 13/17] fix: clean open orders when order cancellation is successfull --- .../step/__tests__/handle-open-orders.test.js | 259 +++++++++++++----- .../trailingTrade/step/handle-open-orders.js | 6 +- 2 files changed, 189 insertions(+), 76 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index 23de85cf..0bd100ac 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -657,41 +657,130 @@ describe('handle-open-orders.js', () => { describe('when stop price is less than current limit price', () => { describe('when open trade limit is reached', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); + describe('when order is cancelled', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest + .fn() + .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); + mockCancelOrder = jest.fn().mockResolvedValue(true); + + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder - })); + step = require('../handle-open-orders'); - step = require('../handle-open-orders'); + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + result = await step.execute(loggerMock, rawData); + }); + + it('does trigger cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalled(); + }); + + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); + + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-cancelled', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' } - ], - buy: { - limitPrice: 1810, + }); + }); + }); + + describe('when order is not cancelled', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest + .fn() + .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); + mockCancelOrder = jest.fn().mockResolvedValue(false); + + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); + + step = require('../handle-open-orders'); + + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', openOrders: [ { symbol: 'BTCUSDT', @@ -701,55 +790,77 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - result = await step.execute(loggerMock, rawData); - }); + result = await step.execute(loggerMock, rawData); + }); - it('does trigger cancelOrder', () => { - expect(mockCancelOrder).toHaveBeenCalled(); - }); + it('does trigger cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalled(); + }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); - }); + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-cancelled', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-cancelled', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' } - ], - buy: { - limitPrice: 1810, - openOrders: [] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } + }); }); }); }); diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index 34ffdbd5..eb02ed1b 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -65,10 +65,12 @@ const execute = async (logger, rawData) => { ); // Cancel current order - await cancelOrder(logger, symbol, order); + const cancelResult = await cancelOrder(logger, symbol, order); // Reset buy open orders - data.buy.openOrders = []; + if (cancelResult === true) { + data.buy.openOrders = []; + } } else if (parseFloat(order.stopPrice) >= buyLimitPrice) { logger.info( { stopPrice: order.stopPrice, buyLimitPrice, saveLog: true }, From b7b47ab1f359c8af3035f0e4b7ecaffe3ed7817b Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Mon, 15 Aug 2022 22:17:36 +1000 Subject: [PATCH 14/17] refactor: updated handle open orders --- .../step/__tests__/handle-open-orders.test.js | 786 +++++++++--------- .../trailingTrade/step/handle-open-orders.js | 164 ++-- 2 files changed, 516 insertions(+), 434 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index 0bd100ac..2d35cc8a 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -336,196 +336,15 @@ describe('handle-open-orders.js', () => { }); describe('when order is buy', () => { - describe('when stop price is higher or equal than current limit price', () => { - describe('when cancelling order is failed', () => { - beforeEach(() => { - slackMock.sendMessage = jest.fn().mockResolvedValue(true); - - mockCancelOrder = jest.fn().mockResolvedValue(false); - - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder - })); - }); - - describe('when got getAndCacheOpenOrdersForSymbol successfully', () => { - beforeEach(async () => { - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([ - { - symbol: 'BTCUSDT', - orderId: 46839, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - }, - { - symbol: 'BTCUSDT', - orderId: 46841, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' - } - ]); - - step = require('../handle-open-orders'); - - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - featureToggle: { - notifyDebug: false - }, - action: 'not-determined', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ], - buy: { - limitPrice: 1800, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; - - result = await step.execute(loggerMock, rawData); - }); - - it('triggers cancelOrder', () => { - expect(mockCancelOrder).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT', - { - orderId: 46838, - price: '1799.58000000', - side: 'BUY', - stopPrice: '1800.1000', - symbol: 'BTCUSDT', - type: 'STOP_LOSS_LIMIT' - } - ); - }); - - it('triggers getAccountInfo', () => { - expect(mockGetAccountInfo).toHaveBeenCalledWith(loggerMock); - }); - - it('triggers getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT' - ); - }); - - it('does not trigger slack.sendMessage', () => { - expect(slackMock.sendMessage).not.toHaveBeenCalled(); - }); - - it('triggers saveOverrideAction', () => { - expect(mockSaveOverrideAction).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT', - { - action: 'buy', - actionAt: expect.any(String), - triggeredBy: 'buy-cancelled', - notify: false, - checkTradingView: true - }, - 'The bot will place a buy order in the next tick because could not retrieve the cancelled order result.' - ); - }); - - it('does not trigger updateAccountInfo', () => { - expect(mockUpdateAccountInfo).not.toHaveBeenCalled(); - }); - - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - featureToggle: { - notifyDebug: false - }, - action: 'buy-order-checking', - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46839, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - }, - { - symbol: 'BTCUSDT', - orderId: 46841, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' - } - ], - buy: { - limitPrice: 1800, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46839, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - }, - accountInfo: accountInfoJSON - }); - }); - }); - }); - - describe('when cancelling order is succeed', () => { + describe('when open trade limit is reached', () => { + describe('when order is cancelled', () => { beforeEach(async () => { mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest .fn() .mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); + mockCancelOrder = jest.fn().mockResolvedValue(true); jest.mock('../../../trailingTradeHelper/common', () => ({ getAccountInfo: mockGetAccountInfo, @@ -555,7 +374,7 @@ describe('handle-open-orders.js', () => { } ], buy: { - limitPrice: 1800, + limitPrice: 1810, openOrders: [ { symbol: 'BTCUSDT', @@ -584,20 +403,8 @@ describe('handle-open-orders.js', () => { result = await step.execute(loggerMock, rawData); }); - it('triggers cancelOrder', () => { - expect(mockCancelOrder).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT', - { - orderId: 46838, - origQty: '0.00001', - price: '1799.58000000', - side: 'BUY', - stopPrice: '1800.1000', - symbol: 'BTCUSDT', - type: 'STOP_LOSS_LIMIT' - } - ); + it('does trigger cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalled(); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -626,7 +433,7 @@ describe('handle-open-orders.js', () => { expect(result).toStrictEqual({ symbol: 'BTCUSDT', isLocked: false, - action: 'buy', + action: 'buy-order-cancelled', openOrders: [ { symbol: 'BTCUSDT', @@ -638,50 +445,69 @@ describe('handle-open-orders.js', () => { side: 'BUY' } ], - buy: { limitPrice: 1800, openOrders: [] }, + buy: { + limitPrice: 1810, + openOrders: [] + }, sell: { limitPrice: 1800, openOrders: [] }, symbolInfo: { quoteAsset: 'USDT' }, + accountInfo: { + account: 'updated' + }, quoteAssetBalance: { free: 50, locked: 0.0179958 - }, - accountInfo: { - account: 'updated' } }); }); }); - }); - describe('when stop price is less than current limit price', () => { - describe('when open trade limit is reached', () => { - describe('when order is cancelled', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); - mockCancelOrder = jest.fn().mockResolvedValue(true); - - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder - })); + describe('when order is not cancelled', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); + mockCancelOrder = jest.fn().mockResolvedValue(false); - step = require('../handle-open-orders'); + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', + step = require('../handle-open-orders'); + + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, openOrders: [ { symbol: 'BTCUSDT', @@ -691,49 +517,52 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - result = await step.execute(loggerMock, rawData); - }); + result = await step.execute(loggerMock, rawData); + }); - it('does trigger cancelOrder', () => { - expect(mockCancelOrder).toHaveBeenCalled(); - }); + it('does trigger cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalled(); + }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); + it('triggers getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT' + ); + }); - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); - }); + it('triggers getAccountInfo', () => { + expect(mockGetAccountInfo).toHaveBeenCalledWith(loggerMock); + }); - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-cancelled', + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-checking', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, openOrders: [ { symbol: 'BTCUSDT', @@ -743,44 +572,77 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ], - buy: { - limitPrice: 1810, - openOrders: [] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } - }); + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + }, + accountInfo: accountInfoJSON }); }); + }); + }); - describe('when order is not cancelled', () => { - beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); - mockCancelOrder = jest.fn().mockResolvedValue(false); - - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, - updateAccountInfo: mockUpdateAccountInfo, - saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder - })); + describe('when stop price is higher or equal than current limit price', () => { + describe('when cancelling order is failed', () => { + beforeEach(async () => { + slackMock.sendMessage = jest.fn().mockResolvedValue(true); - step = require('../handle-open-orders'); + mockCancelOrder = jest.fn().mockResolvedValue(false); - rawData = { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: + mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); + + mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([ + { symbol: 'BTCUSDT', - isLocked: false, - action: 'not-determined', + orderId: 46839, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + }, + { + symbol: 'BTCUSDT', + orderId: 46841, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'SELL' + } + ]); + + step = require('../handle-open-orders'); + + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + featureToggle: { + notifyDebug: false + }, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1800, openOrders: [ { symbol: 'BTCUSDT', @@ -790,88 +652,124 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { - limitPrice: 1800, - openOrders: [] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - result = await step.execute(loggerMock, rawData); - }); + result = await step.execute(loggerMock, rawData); + }); - it('does trigger cancelOrder', () => { - expect(mockCancelOrder).toHaveBeenCalled(); - }); + it('triggers cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + price: '1799.58000000', + side: 'BUY', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); + }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); + it('triggers getAccountInfo', () => { + expect(mockGetAccountInfo).toHaveBeenCalledWith(loggerMock); + }); - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); - }); + it('triggers getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT' + ); + }); - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - action: 'buy-order-cancelled', + it('does not trigger slack.sendMessage', () => { + expect(slackMock.sendMessage).not.toHaveBeenCalled(); + }); + + it('triggers saveOverrideAction', () => { + expect(mockSaveOverrideAction).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + action: 'buy', + actionAt: expect.any(String), + triggeredBy: 'buy-cancelled', + notify: false, + checkTradingView: true + }, + 'The bot will place a buy order in the next tick because could not retrieve the cancelled order result.' + ); + }); + + it('does not trigger updateAccountInfo', () => { + expect(mockUpdateAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + featureToggle: { + notifyDebug: false + }, + action: 'buy-order-checking', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46839, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + }, + { + symbol: 'BTCUSDT', + orderId: 46841, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'SELL' + } + ], + buy: { + limitPrice: 1800, openOrders: [ { symbol: 'BTCUSDT', - orderId: 46838, + orderId: 46839, price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', side: 'BUY' } - ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, - sell: { limitPrice: 1800, openOrders: [] }, - symbolInfo: { - quoteAsset: 'USDT' - } - }); + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + }, + accountInfo: accountInfoJSON }); }); }); - describe('when open trade limit is not reached', () => { + describe('when cancelling order is succeed', () => { beforeEach(async () => { mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest .fn() .mockResolvedValue([]); - mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); jest.mock('../../../trailingTradeHelper/common', () => ({ getAccountInfo: mockGetAccountInfo, @@ -893,6 +791,7 @@ describe('handle-open-orders.js', () => { { symbol: 'BTCUSDT', orderId: 46838, + origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -900,11 +799,12 @@ describe('handle-open-orders.js', () => { } ], buy: { - limitPrice: 1810, + limitPrice: 1800, openOrders: [ { symbol: 'BTCUSDT', orderId: 46838, + origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -918,14 +818,30 @@ describe('handle-open-orders.js', () => { }, symbolInfo: { quoteAsset: 'USDT' + }, + quoteAssetBalance: { + free: 50, + locked: 0.0179958 } }; result = await step.execute(loggerMock, rawData); }); - it('does not trigger cancelOrder', () => { - expect(mockCancelOrder).not.toHaveBeenCalled(); + it('triggers cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + origQty: '0.00001', + price: '1799.58000000', + side: 'BUY', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); }); it('does not trigger getAndCacheOpenOrdersForSymbol', () => { @@ -936,42 +852,156 @@ describe('handle-open-orders.js', () => { expect(mockGetAccountInfo).not.toHaveBeenCalled(); }); + it('triggers updateAccountInfo', () => { + expect(mockUpdateAccountInfo).toHaveBeenCalledWith( + loggerMock, + [ + { + asset: 'USDT', + free: 50.0179958, + locked: 0 + } + ], + expect.any(String) + ); + }); + it('returns expected value', () => { expect(result).toStrictEqual({ symbol: 'BTCUSDT', isLocked: false, - action: 'buy-order-wait', + action: 'buy', openOrders: [ { symbol: 'BTCUSDT', orderId: 46838, + origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', side: 'BUY' } ], - buy: { - limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] - }, + buy: { limitPrice: 1800, openOrders: [] }, sell: { limitPrice: 1800, openOrders: [] }, symbolInfo: { quoteAsset: 'USDT' + }, + quoteAssetBalance: { + free: 50, + locked: 0.0179958 + }, + accountInfo: { + account: 'updated' } }); }); }); }); + + describe('when order trade limit is not reached and stop price is less than current limit price', () => { + beforeEach(async () => { + mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); + mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); + mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); + + jest.mock('../../../trailingTradeHelper/common', () => ({ + getAccountInfo: mockGetAccountInfo, + updateAccountInfo: mockUpdateAccountInfo, + saveOverrideAction: mockSaveOverrideAction, + getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, + isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, + cancelOrder: mockCancelOrder + })); + + step = require('../handle-open-orders'); + + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + action: 'not-determined', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { + limitPrice: 1800, + openOrders: [] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; + + result = await step.execute(loggerMock, rawData); + }); + + it('does not trigger cancelOrder', () => { + expect(mockCancelOrder).not.toHaveBeenCalled(); + }); + + it('does not trigger getAndCacheOpenOrdersForSymbol', () => { + expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); + }); + + it('does not trigger getAccountInfo', () => { + expect(mockGetAccountInfo).not.toHaveBeenCalled(); + }); + + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + action: 'buy-order-wait', + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ], + buy: { + limitPrice: 1810, + openOrders: [ + { + symbol: 'BTCUSDT', + orderId: 46838, + price: '1799.58000000', + stopPrice: '1800.1000', + type: 'STOP_LOSS_LIMIT', + side: 'BUY' + } + ] + }, + sell: { limitPrice: 1800, openOrders: [] }, + symbolInfo: { + quoteAsset: 'USDT' + } + }); + }); + }); }); describe('when order is sell', () => { diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index eb02ed1b..90e91e8c 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -11,6 +11,71 @@ const { isExceedingMaxOpenTrades } = require('../../trailingTradeHelper/common'); +const processSuccessfulBuyOrderCancel = async (logger, data, order) => { + const { + symbolInfo: { quoteAsset } + } = data; + + const orderAmount = order.origQty * order.price; + + // Immediately update the balance of "quote" asset when the order is canceled so that + // we don't have to wait for the websocket because the next action is buy + const balances = [ + { + asset: quoteAsset, + free: _.toNumber(data.quoteAssetBalance.free) + _.toNumber(orderAmount), + locked: + _.toNumber(data.quoteAssetBalance.locked) - _.toNumber(orderAmount) + } + ]; + + // Refresh account info + const accountInfo = await updateAccountInfo( + logger, + balances, + moment().toISOString() + ); + + return { accountInfo }; +}; + +const processSuccessfulSellOrderCancel = async (logger, data, order) => { + const { + symbolInfo: { baseAsset } + } = data; + + const balances = [ + { + asset: baseAsset, + free: _.toNumber(data.baseAssetBalance.free) + _.toNumber(order.origQty), + locked: + _.toNumber(data.baseAssetBalance.locked) - _.toNumber(order.origQty) + } + ]; + + // Refresh account info + const accountInfo = await updateAccountInfo( + logger, + balances, + moment().toISOString() + ); + + return { accountInfo }; +}; + +const processFailedOrderCancel = async (logger, symbol) => { + // Get open orders + const openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); + + // Refresh account info + const accountInfo = await getAccountInfo(logger); + + return { + openOrders, + accountInfo + }; +}; + /** * * Handle open orders @@ -27,8 +92,7 @@ const execute = async (logger, rawData) => { isLocked, openOrders, buy: { limitPrice: buyLimitPrice }, - sell: { limitPrice: sellLimitPrice }, - symbolInfo: { quoteAsset, baseAsset } + sell: { limitPrice: sellLimitPrice } } = data; if (isLocked) { @@ -53,7 +117,8 @@ const execute = async (logger, rawData) => { // eslint-disable-next-line no-continue continue; } - // Is the stop price is higher than current limit price? + + // Process buy order if (order.side.toLowerCase() === 'buy') { if (await isExceedingMaxOpenTrades(logger, data)) { // Cancel the initial buy order if max. open trades exceeded @@ -68,10 +133,29 @@ const execute = async (logger, rawData) => { const cancelResult = await cancelOrder(logger, symbol, order); // Reset buy open orders - if (cancelResult === true) { + if (cancelResult === false) { + const { openOrders: updatedOpenOrders, accountInfo } = + await processFailedOrderCancel(logger, symbol); + data.openOrders = updatedOpenOrders; + data.accountInfo = accountInfo; + + data.buy.openOrders = data.openOrders.filter( + o => o.side.toLowerCase() === 'buy' + ); + + data.action = 'buy-order-checking'; + } else { data.buy.openOrders = []; + + const { accountInfo } = await processSuccessfulBuyOrderCancel( + logger, + data, + order + ); + data.accountInfo = accountInfo; } } else if (parseFloat(order.stopPrice) >= buyLimitPrice) { + // Is the stop price is higher than current limit price? logger.info( { stopPrice: order.stopPrice, buyLimitPrice, saveLog: true }, 'Stop price is higher than buy limit price, cancel current buy order' @@ -83,19 +167,15 @@ const execute = async (logger, rawData) => { // If cancelling the order is failed, it means the order may already be executed or does not exist anymore. // Hence, refresh the order and process again in the next tick. // Get open orders and update cache - - data.openOrders = await getAndCacheOpenOrdersForSymbol( - logger, - symbol - ); + const { openOrders: updatedOpenOrders, accountInfo } = + await processFailedOrderCancel(logger, symbol); + data.openOrders = updatedOpenOrders; + data.accountInfo = accountInfo; data.buy.openOrders = data.openOrders.filter( o => o.side.toLowerCase() === 'buy' ); - // Refresh account info - data.accountInfo = await getAccountInfo(logger); - data.action = 'buy-order-checking'; await saveOverrideAction( @@ -117,28 +197,12 @@ const execute = async (logger, rawData) => { // Set action as buy data.action = 'buy'; - const orderAmount = order.origQty * order.price; - - // Immediately update the balance of "quote" asset when the order is canceled so that - // we don't have to wait for the websocket because the next action is buy - const balances = [ - { - asset: quoteAsset, - free: - _.toNumber(data.quoteAssetBalance.free) + - _.toNumber(orderAmount), - locked: - _.toNumber(data.quoteAssetBalance.locked) - - _.toNumber(orderAmount) - } - ]; - - // Refresh account info - data.accountInfo = await updateAccountInfo( + const { accountInfo } = await processSuccessfulBuyOrderCancel( logger, - balances, - moment().toISOString() + data, + order ); + data.accountInfo = accountInfo; } } else { logger.info( @@ -150,8 +214,8 @@ const execute = async (logger, rawData) => { } } - // Is the stop price is less than current limit price? if (order.side.toLowerCase() === 'sell') { + // Is the stop price is less than current limit price? if (parseFloat(order.stopPrice) <= sellLimitPrice) { logger.info( { stopPrice: order.stopPrice, sellLimitPrice, saveLog: true }, @@ -165,18 +229,16 @@ const execute = async (logger, rawData) => { // Hence, refresh the order and process again in the next tick. // Get open orders and update cache - data.openOrders = await getAndCacheOpenOrdersForSymbol( - logger, - symbol - ); + const { openOrders: updatedOpenOrders, accountInfo } = + await processFailedOrderCancel(logger, symbol); + + data.openOrders = updatedOpenOrders; + data.accountInfo = accountInfo; data.sell.openOrders = data.openOrders.filter( o => o.side.toLowerCase() === 'sell' ); - // Refresh account info - data.accountInfo = await getAccountInfo(logger); - data.action = 'sell-order-checking'; } else { // Reset sell open orders @@ -187,24 +249,14 @@ const execute = async (logger, rawData) => { // Immediately update the balance of "base" asset when the order is canceled so that // we don't have to wait for the websocket because the next action is sell - const balances = [ - { - asset: baseAsset, - free: - _.toNumber(data.baseAssetBalance.free) + - _.toNumber(order.origQty), - locked: - _.toNumber(data.baseAssetBalance.locked) - - _.toNumber(order.origQty) - } - ]; - - // Refresh account info - data.accountInfo = await updateAccountInfo( + const { accountInfo } = await processSuccessfulSellOrderCancel( logger, - balances, - moment().toISOString() + data, + order ); + + // Refresh account info + data.accountInfo = accountInfo; } } else { logger.info( From 12bbd3a2a1a597059d13dffa178195eb7ea88b69 Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Mon, 15 Aug 2022 23:26:20 +1000 Subject: [PATCH 15/17] refactor: updated handle open orders --- .../step/__tests__/handle-open-orders.test.js | 531 ++++++------------ .../trailingTrade/step/handle-open-orders.js | 62 +- .../__tests__/common.test.js | 72 +++ app/cronjob/trailingTradeHelper/common.js | 24 +- 4 files changed, 297 insertions(+), 392 deletions(-) diff --git a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js index 2d35cc8a..7a3ce921 100644 --- a/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js +++ b/app/cronjob/trailingTrade/step/__tests__/handle-open-orders.test.js @@ -9,15 +9,12 @@ describe('handle-open-orders.js', () => { let slackMock; let mockCancelOrder; - let mockGetAccountInfo; - let mockGetAndCacheOpenOrdersForSymbol; + let mockRefreshOpenOrdersAndAccountInfo; let mockUpdateAccountInfo; let mockSaveOverrideAction; let mockIsExceedingMaxOpenTrades; - const accountInfoJSON = require('./fixtures/binance-account-info.json'); - describe('execute', () => { beforeEach(() => { jest.clearAllMocks().resetModules(); @@ -26,7 +23,14 @@ describe('handle-open-orders.js', () => { loggerMock = logger; slackMock = slack; - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); + mockRefreshOpenOrdersAndAccountInfo = jest.fn().mockResolvedValue({ + accountInfo: { + accountInfo: 'updated' + }, + openOrders: [{ openOrders: 'retrieved' }], + buyOpenOrders: [{ buyOpenOrders: 'retrived' }], + sellOpenOrders: [{ sellOpenOrders: 'retrived' }] + }); mockCancelOrder = jest.fn().mockResolvedValue(true); mockSaveOverrideAction = jest.fn().mockResolvedValue(true); @@ -39,15 +43,12 @@ describe('handle-open-orders.js', () => { describe('when symbol is locked', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -93,12 +94,8 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).not.toHaveBeenCalled(); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('returns expected value', () => { @@ -137,17 +134,12 @@ describe('handle-open-orders.js', () => { describe('when action is not not-determined', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -193,12 +185,8 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).not.toHaveBeenCalled(); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('returns expected value', () => { @@ -237,17 +225,12 @@ describe('handle-open-orders.js', () => { describe('when order is not STOP_LOSS_LIMIT', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -293,12 +276,8 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).not.toHaveBeenCalled(); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('returns expected value', () => { @@ -337,23 +316,18 @@ describe('handle-open-orders.js', () => { describe('when order is buy', () => { describe('when open trade limit is reached', () => { - describe('when order is cancelled', () => { + describe('when cancelling order is failedd', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); - mockCancelOrder = jest.fn().mockResolvedValue(true); + mockCancelOrder = jest.fn().mockResolvedValue(false); jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: + mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -366,7 +340,6 @@ describe('handle-open-orders.js', () => { { symbol: 'BTCUSDT', orderId: 46838, - origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -379,7 +352,6 @@ describe('handle-open-orders.js', () => { { symbol: 'BTCUSDT', orderId: 46838, - origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -393,10 +365,6 @@ describe('handle-open-orders.js', () => { }, symbolInfo: { quoteAsset: 'USDT' - }, - quoteAssetBalance: { - free: 50, - locked: 0.0179958 } }; @@ -407,25 +375,10 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).toHaveBeenCalled(); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); - }); - - it('triggers updateAccountInfo', () => { - expect(mockUpdateAccountInfo).toHaveBeenCalledWith( + it('triggers refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).toHaveBeenCalledWith( loggerMock, - [ - { - asset: 'USDT', - free: 50.0179958, - locked: 0 - } - ], - expect.any(String) + 'BTCUSDT' ); }); @@ -433,61 +386,41 @@ describe('handle-open-orders.js', () => { expect(result).toStrictEqual({ symbol: 'BTCUSDT', isLocked: false, - action: 'buy-order-cancelled', + action: 'buy-order-checking', openOrders: [ { - symbol: 'BTCUSDT', - orderId: 46838, - origQty: '0.00001', - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + openOrders: 'retrieved' } ], buy: { limitPrice: 1810, - openOrders: [] + openOrders: [ + { + buyOpenOrders: 'retrived' + } + ] }, sell: { limitPrice: 1800, openOrders: [] }, symbolInfo: { quoteAsset: 'USDT' }, - accountInfo: { - account: 'updated' - }, - quoteAssetBalance: { - free: 50, - locked: 0.0179958 - } + accountInfo: { accountInfo: 'updated' } }); }); }); - describe('when order is not cancelled', () => { + describe('when cancelling order is succeedd', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ]); mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(true); - mockCancelOrder = jest.fn().mockResolvedValue(false); + mockCancelOrder = jest.fn().mockResolvedValue(true); jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: + mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -500,6 +433,7 @@ describe('handle-open-orders.js', () => { { symbol: 'BTCUSDT', orderId: 46838, + origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -512,6 +446,7 @@ describe('handle-open-orders.js', () => { { symbol: 'BTCUSDT', orderId: 46838, + origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -525,6 +460,10 @@ describe('handle-open-orders.js', () => { }, symbolInfo: { quoteAsset: 'USDT' + }, + quoteAssetBalance: { + free: 50, + locked: 0.0179958 } }; @@ -535,26 +474,34 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).toHaveBeenCalled(); }); - it('triggers getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT' - ); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); - it('triggers getAccountInfo', () => { - expect(mockGetAccountInfo).toHaveBeenCalledWith(loggerMock); + it('triggers updateAccountInfo', () => { + expect(mockUpdateAccountInfo).toHaveBeenCalledWith( + loggerMock, + [ + { + asset: 'USDT', + free: 50.0179958, + locked: 0 + } + ], + expect.any(String) + ); }); it('returns expected value', () => { expect(result).toStrictEqual({ symbol: 'BTCUSDT', isLocked: false, - action: 'buy-order-checking', + action: 'buy-order-cancelled', openOrders: [ { symbol: 'BTCUSDT', orderId: 46838, + origQty: '0.00001', price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', @@ -563,22 +510,19 @@ describe('handle-open-orders.js', () => { ], buy: { limitPrice: 1810, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - } - ] + openOrders: [] }, sell: { limitPrice: 1800, openOrders: [] }, symbolInfo: { quoteAsset: 'USDT' }, - accountInfo: accountInfoJSON + accountInfo: { + account: 'updated' + }, + quoteAssetBalance: { + free: 50, + locked: 0.0179958 + } }); }); }); @@ -591,37 +535,15 @@ describe('handle-open-orders.js', () => { mockCancelOrder = jest.fn().mockResolvedValue(false); - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: + mockRefreshOpenOrdersAndAccountInfo })); - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([ - { - symbol: 'BTCUSDT', - orderId: 46839, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - }, - { - symbol: 'BTCUSDT', - orderId: 46841, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' - } - ]); - step = require('../handle-open-orders'); rawData = { @@ -681,12 +603,8 @@ describe('handle-open-orders.js', () => { ); }); - it('triggers getAccountInfo', () => { - expect(mockGetAccountInfo).toHaveBeenCalledWith(loggerMock); - }); - - it('triggers getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).toHaveBeenCalledWith( + it('triggers refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).toHaveBeenCalledWith( loggerMock, 'BTCUSDT' ); @@ -725,32 +643,14 @@ describe('handle-open-orders.js', () => { action: 'buy-order-checking', openOrders: [ { - symbol: 'BTCUSDT', - orderId: 46839, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' - }, - { - symbol: 'BTCUSDT', - orderId: 46841, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' + openOrders: 'retrieved' } ], buy: { limitPrice: 1800, openOrders: [ { - symbol: 'BTCUSDT', - orderId: 46839, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + buyOpenOrders: 'retrived' } ] }, @@ -758,27 +658,22 @@ describe('handle-open-orders.js', () => { symbolInfo: { quoteAsset: 'USDT' }, - accountInfo: accountInfoJSON + accountInfo: { + accountInfo: 'updated' + } }); }); }); describe('when cancelling order is succeed', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: + mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -844,12 +739,8 @@ describe('handle-open-orders.js', () => { ); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('triggers updateAccountInfo', () => { @@ -901,17 +792,14 @@ describe('handle-open-orders.js', () => { describe('when order trade limit is not reached and stop price is less than current limit price', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([]); mockIsExceedingMaxOpenTrades = jest.fn().mockResolvedValue(false); jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -959,12 +847,8 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).not.toHaveBeenCalled(); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('returns expected value', () => { @@ -1007,54 +891,44 @@ describe('handle-open-orders.js', () => { describe('when order is sell', () => { describe('when stop price is less or equal than current limit price', () => { describe('when cancel order is failed', () => { - beforeEach(() => { + beforeEach(async () => { slackMock.sendMessage = jest.fn().mockResolvedValue(true); mockCancelOrder = jest.fn().mockResolvedValue(false); - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: + mockRefreshOpenOrdersAndAccountInfo })); - }); + step = require('../handle-open-orders'); - describe('when got getAndCacheOpenOrdersForSymbol successfully', () => { - beforeEach(async () => { - mockGetAndCacheOpenOrdersForSymbol = jest.fn().mockResolvedValue([ + rawData = { + symbol: 'BTCUSDT', + isLocked: false, + featureToggle: { + notifyDebug: false + }, + action: 'not-determined', + openOrders: [ { symbol: 'BTCUSDT', - orderId: 46840, + orderId: 46838, price: '1799.58000000', stopPrice: '1800.1000', type: 'STOP_LOSS_LIMIT', side: 'SELL' - }, - { - symbol: 'BTCUSDT', - orderId: 46841, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' } - ]); - - step = require('../handle-open-orders'); - - rawData = { - symbol: 'BTCUSDT', - isLocked: false, - featureToggle: { - notifyDebug: false - }, - action: 'not-determined', + ], + buy: { + limitPrice: 1800, + openOrders: [] + }, + sell: { + limitPrice: 1801, openOrders: [ { symbol: 'BTCUSDT', @@ -1064,127 +938,83 @@ describe('handle-open-orders.js', () => { type: 'STOP_LOSS_LIMIT', side: 'SELL' } - ], - buy: { - limitPrice: 1800, - openOrders: [] - }, - sell: { - limitPrice: 1801, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46838, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' - } - ] - }, - symbolInfo: { - quoteAsset: 'USDT' - } - }; - - result = await step.execute(loggerMock, rawData); - }); + ] + }, + symbolInfo: { + quoteAsset: 'USDT' + } + }; - it('triggers cancelOrder', () => { - expect(mockCancelOrder).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT', - { - orderId: 46838, - price: '1799.58000000', - side: 'SELL', - stopPrice: '1800.1000', - symbol: 'BTCUSDT', - type: 'STOP_LOSS_LIMIT' - } - ); - }); + result = await step.execute(loggerMock, rawData); + }); - it('triggers getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).toHaveBeenCalledWith( - loggerMock, - 'BTCUSDT' - ); - }); + it('triggers cancelOrder', () => { + expect(mockCancelOrder).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT', + { + orderId: 46838, + price: '1799.58000000', + side: 'SELL', + stopPrice: '1800.1000', + symbol: 'BTCUSDT', + type: 'STOP_LOSS_LIMIT' + } + ); + }); - it('triggers getAccountInfo', () => { - expect(mockGetAccountInfo).toHaveBeenCalled(); - }); + it('triggers refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).toHaveBeenCalledWith( + loggerMock, + 'BTCUSDT' + ); + }); - it('does not trigger slack.sendMessage', () => { - expect(slackMock.sendMessage).not.toHaveBeenCalled(); - }); + it('does not trigger slack.sendMessage', () => { + expect(slackMock.sendMessage).not.toHaveBeenCalled(); + }); - it('returns expected value', () => { - expect(result).toStrictEqual({ - symbol: 'BTCUSDT', - isLocked: false, - featureToggle: { - notifyDebug: false - }, - action: 'sell-order-checking', + it('returns expected value', () => { + expect(result).toStrictEqual({ + symbol: 'BTCUSDT', + isLocked: false, + featureToggle: { + notifyDebug: false + }, + action: 'sell-order-checking', + openOrders: [ + { + openOrders: 'retrieved' + } + ], + buy: { limitPrice: 1800, openOrders: [] }, + sell: { + limitPrice: 1801, openOrders: [ { - symbol: 'BTCUSDT', - orderId: 46840, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' - }, - { - symbol: 'BTCUSDT', - orderId: 46841, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'BUY' + sellOpenOrders: 'retrived' } - ], - buy: { limitPrice: 1800, openOrders: [] }, - sell: { - limitPrice: 1801, - openOrders: [ - { - symbol: 'BTCUSDT', - orderId: 46840, - price: '1799.58000000', - stopPrice: '1800.1000', - type: 'STOP_LOSS_LIMIT', - side: 'SELL' - } - ] - }, - symbolInfo: { - quoteAsset: 'USDT' - }, - accountInfo: accountInfoJSON - }); + ] + }, + symbolInfo: { + quoteAsset: 'USDT' + }, + accountInfo: { + accountInfo: 'updated' + } }); }); }); describe('when cancel order is succeed', () => { beforeEach(async () => { - mockGetAndCacheOpenOrdersForSymbol = jest - .fn() - .mockResolvedValue([]); - - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, updateAccountInfo: mockUpdateAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: - mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: + mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -1251,12 +1081,8 @@ describe('handle-open-orders.js', () => { ); }); - it('does not trigger getAndCacheOpenOrdersForSymbol', () => { - expect(mockGetAndCacheOpenOrdersForSymbol).not.toHaveBeenCalled(); - }); - - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('triggers updateAccountInfo', () => { @@ -1309,14 +1135,11 @@ describe('handle-open-orders.js', () => { describe('when stop price is more than current limit price', () => { beforeEach(async () => { - mockGetAccountInfo = jest.fn().mockResolvedValue(accountInfoJSON); - jest.mock('../../../trailingTradeHelper/common', () => ({ - getAccountInfo: mockGetAccountInfo, saveOverrideAction: mockSaveOverrideAction, - getAndCacheOpenOrdersForSymbol: mockGetAndCacheOpenOrdersForSymbol, isExceedingMaxOpenTrades: mockIsExceedingMaxOpenTrades, - cancelOrder: mockCancelOrder + cancelOrder: mockCancelOrder, + refreshOpenOrdersAndAccountInfo: mockRefreshOpenOrdersAndAccountInfo })); step = require('../handle-open-orders'); @@ -1364,8 +1187,8 @@ describe('handle-open-orders.js', () => { expect(mockCancelOrder).not.toHaveBeenCalled(); }); - it('does not trigger getAccountInfo', () => { - expect(mockGetAccountInfo).not.toHaveBeenCalled(); + it('does not trigger refreshOpenOrdersAndAccountInfo', () => { + expect(mockRefreshOpenOrdersAndAccountInfo).not.toHaveBeenCalled(); }); it('returns expected value', () => { diff --git a/app/cronjob/trailingTrade/step/handle-open-orders.js b/app/cronjob/trailingTrade/step/handle-open-orders.js index 90e91e8c..87e23bbd 100644 --- a/app/cronjob/trailingTrade/step/handle-open-orders.js +++ b/app/cronjob/trailingTrade/step/handle-open-orders.js @@ -4,11 +4,10 @@ const moment = require('moment'); const _ = require('lodash'); const { cancelOrder, - getAccountInfo, updateAccountInfo, saveOverrideAction, - getAndCacheOpenOrdersForSymbol, - isExceedingMaxOpenTrades + isExceedingMaxOpenTrades, + refreshOpenOrdersAndAccountInfo } = require('../../trailingTradeHelper/common'); const processSuccessfulBuyOrderCancel = async (logger, data, order) => { @@ -63,19 +62,6 @@ const processSuccessfulSellOrderCancel = async (logger, data, order) => { return { accountInfo }; }; -const processFailedOrderCancel = async (logger, symbol) => { - // Get open orders - const openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); - - // Refresh account info - const accountInfo = await getAccountInfo(logger); - - return { - openOrders, - accountInfo - }; -}; - /** * * Handle open orders @@ -134,14 +120,15 @@ const execute = async (logger, rawData) => { // Reset buy open orders if (cancelResult === false) { - const { openOrders: updatedOpenOrders, accountInfo } = - await processFailedOrderCancel(logger, symbol); - data.openOrders = updatedOpenOrders; - data.accountInfo = accountInfo; + const { + accountInfo, + openOrders: updatedOpenOrders, + buyOpenOrders + } = await refreshOpenOrdersAndAccountInfo(logger, symbol); - data.buy.openOrders = data.openOrders.filter( - o => o.side.toLowerCase() === 'buy' - ); + data.accountInfo = accountInfo; + data.openOrders = updatedOpenOrders; + data.buy.openOrders = buyOpenOrders; data.action = 'buy-order-checking'; } else { @@ -167,14 +154,15 @@ const execute = async (logger, rawData) => { // If cancelling the order is failed, it means the order may already be executed or does not exist anymore. // Hence, refresh the order and process again in the next tick. // Get open orders and update cache - const { openOrders: updatedOpenOrders, accountInfo } = - await processFailedOrderCancel(logger, symbol); - data.openOrders = updatedOpenOrders; - data.accountInfo = accountInfo; + const { + accountInfo, + openOrders: updatedOpenOrders, + buyOpenOrders + } = await refreshOpenOrdersAndAccountInfo(logger, symbol); - data.buy.openOrders = data.openOrders.filter( - o => o.side.toLowerCase() === 'buy' - ); + data.accountInfo = accountInfo; + data.openOrders = updatedOpenOrders; + data.buy.openOrders = buyOpenOrders; data.action = 'buy-order-checking'; @@ -229,15 +217,15 @@ const execute = async (logger, rawData) => { // Hence, refresh the order and process again in the next tick. // Get open orders and update cache - const { openOrders: updatedOpenOrders, accountInfo } = - await processFailedOrderCancel(logger, symbol); + const { + accountInfo, + openOrders: updatedOpenOrders, + sellOpenOrders + } = await refreshOpenOrdersAndAccountInfo(logger, symbol); - data.openOrders = updatedOpenOrders; data.accountInfo = accountInfo; - - data.sell.openOrders = data.openOrders.filter( - o => o.side.toLowerCase() === 'sell' - ); + data.openOrders = updatedOpenOrders; + data.sell.openOrders = sellOpenOrders; data.action = 'sell-order-checking'; } else { diff --git a/app/cronjob/trailingTradeHelper/__tests__/common.test.js b/app/cronjob/trailingTradeHelper/__tests__/common.test.js index 09d32ed9..2738f443 100644 --- a/app/cronjob/trailingTradeHelper/__tests__/common.test.js +++ b/app/cronjob/trailingTradeHelper/__tests__/common.test.js @@ -3329,4 +3329,76 @@ describe('common.js', () => { }); }); }); + + describe('refreshOpenOrdersAndAccountInfo', () => { + beforeEach(async () => { + const { binance, logger, cache } = require('../../../helpers'); + + loggerMock = logger; + binanceMock = binance; + cacheMock = cache; + + binanceMock.client.openOrders = jest.fn().mockResolvedValue([ + { + symbol: 'BTCUSDT', + side: 'BUY' + }, + { + symbol: 'BTCUSDT', + side: 'SELL' + } + ]); + + cacheMock.hset = jest.fn().mockResolvedValue(true); + cacheMock.hgetWithoutLock = jest.fn().mockResolvedValue( + JSON.stringify({ + accountInfo: 'updated' + }) + ); + + commonHelper = require('../common'); + result = await commonHelper.refreshOpenOrdersAndAccountInfo( + loggerMock, + 'BTCUSDT' + ); + }); + + it('triggers binance.client.openOrders', () => { + expect(binanceMock.client.openOrders).toHaveBeenCalled(); + }); + + it('triggers cache.hgetWithoutLock', () => { + expect(cacheMock.hgetWithoutLock).toHaveBeenCalled(); + }); + + it('returns expected results', () => { + expect(result).toStrictEqual({ + accountInfo: { + accountInfo: 'updated' + }, + openOrders: [ + { + symbol: 'BTCUSDT', + side: 'BUY' + }, + { + symbol: 'BTCUSDT', + side: 'SELL' + } + ], + buyOpenOrders: [ + { + symbol: 'BTCUSDT', + side: 'BUY' + } + ], + sellOpenOrders: [ + { + symbol: 'BTCUSDT', + side: 'SELL' + } + ] + }); + }); + }); }); diff --git a/app/cronjob/trailingTradeHelper/common.js b/app/cronjob/trailingTradeHelper/common.js index 24a50972..68bab0ab 100644 --- a/app/cronjob/trailingTradeHelper/common.js +++ b/app/cronjob/trailingTradeHelper/common.js @@ -1127,6 +1127,27 @@ const cancelOrder = async (logger, symbol, order) => { return result; }; +const refreshOpenOrdersAndAccountInfo = async (logger, symbol) => { + // Get open orders + const openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); + + // Refresh account info + const accountInfo = await getAccountInfo(logger); + + const buyOpenOrders = openOrders.filter(o => o.side.toLowerCase() === 'buy'); + + const sellOpenOrders = openOrders.filter( + o => o.side.toLowerCase() === 'sell' + ); + + return { + accountInfo, + openOrders, + buyOpenOrders, + sellOpenOrders + }; +}; + module.exports = { cacheExchangeSymbols, getCachedExchangeSymbols, @@ -1168,5 +1189,6 @@ module.exports = { getCacheTrailingTradeTotalProfitAndLoss, getCacheTrailingTradeQuoteEstimates, isExceedingMaxOpenTrades, - cancelOrder + cancelOrder, + refreshOpenOrdersAndAccountInfo }; From 0acf1e6dee7c8d26bc8ade95d9fba4d33963637b Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Mon, 15 Aug 2022 23:29:17 +1000 Subject: [PATCH 16/17] chore: added comments --- app/cronjob/trailingTrade/step/cancel-order.js | 1 + app/cronjob/trailingTrade/step/place-buy-order.js | 1 + app/cronjob/trailingTrade/step/place-manual-trade.js | 1 + app/cronjob/trailingTrade/step/place-sell-order.js | 1 + app/cronjob/trailingTrade/step/place-sell-stop-loss-order.js | 1 + 5 files changed, 5 insertions(+) diff --git a/app/cronjob/trailingTrade/step/cancel-order.js b/app/cronjob/trailingTrade/step/cancel-order.js index f68ee951..6b3dfb9c 100644 --- a/app/cronjob/trailingTrade/step/cancel-order.js +++ b/app/cronjob/trailingTrade/step/cancel-order.js @@ -52,6 +52,7 @@ const execute = async (logger, rawData) => { await deleteManualOrder(logger, symbol, order.orderId); + // FIXME: If you change this comment, please refactor to use common.js:refreshOpenOrdersAndAccountInfo // Get open orders and update cache data.openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); data.buy.openOrders = data.openOrders.filter( diff --git a/app/cronjob/trailingTrade/step/place-buy-order.js b/app/cronjob/trailingTrade/step/place-buy-order.js index 7da373d9..6ceadf6c 100644 --- a/app/cronjob/trailingTrade/step/place-buy-order.js +++ b/app/cronjob/trailingTrade/step/place-buy-order.js @@ -437,6 +437,7 @@ const execute = async (logger, rawData) => { // Save number of open orders await saveOrderStats(logger, symbols); + // FIXME: If you change this comment, please refactor to use common.js:refreshOpenOrdersAndAccountInfo // Get open orders and update cache data.openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); data.buy.openOrders = data.openOrders.filter( diff --git a/app/cronjob/trailingTrade/step/place-manual-trade.js b/app/cronjob/trailingTrade/step/place-manual-trade.js index e6256f59..c71db1ae 100644 --- a/app/cronjob/trailingTrade/step/place-manual-trade.js +++ b/app/cronjob/trailingTrade/step/place-manual-trade.js @@ -263,6 +263,7 @@ const execute = async (logger, rawData) => { await recordOrder(logger, orderResult); + // FIXME: If you change this comment, please refactor to use common.js:refreshOpenOrdersAndAccountInfo // Get open orders and update cache data.openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); data.buy.openOrders = data.openOrders.filter( diff --git a/app/cronjob/trailingTrade/step/place-sell-order.js b/app/cronjob/trailingTrade/step/place-sell-order.js index 9683ddf1..568776a2 100644 --- a/app/cronjob/trailingTrade/step/place-sell-order.js +++ b/app/cronjob/trailingTrade/step/place-sell-order.js @@ -202,6 +202,7 @@ const execute = async (logger, rawData) => { currentGridTradeIndex }); + // FIXME: If you change this comment, please refactor to use common.js:refreshOpenOrdersAndAccountInfo // Get open orders and update cache data.openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); data.sell.openOrders = data.openOrders.filter( diff --git a/app/cronjob/trailingTrade/step/place-sell-stop-loss-order.js b/app/cronjob/trailingTrade/step/place-sell-stop-loss-order.js index ad5c6174..162d5031 100644 --- a/app/cronjob/trailingTrade/step/place-sell-stop-loss-order.js +++ b/app/cronjob/trailingTrade/step/place-sell-stop-loss-order.js @@ -193,6 +193,7 @@ const execute = async (logger, rawData) => { ); } + // FIXME: If you change this comment, please refactor to use common.js:refreshOpenOrdersAndAccountInfo // Get open orders and update cache data.openOrders = await getAndCacheOpenOrdersForSymbol(logger, symbol); data.sell.openOrders = data.openOrders.filter( From b492360e8511bb9b05c4bb4bf070501ce44c190f Mon Sep 17 00:00:00 2001 From: Chris Lee Date: Tue, 16 Aug 2022 00:05:15 +1000 Subject: [PATCH 17/17] docs: updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0297450e..70f233d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- Fixed incorrect behaviour of exceeding max open orders by [@uhliksk](https://github.com/uhliksk) - [#462](https://github.com/chrisleekr/binance-trading-bot/pull/462) - Bumped vulnerable package versions - [#472](https://github.com/chrisleekr/binance-trading-bot/pull/472) - Refactored the slack hander to avoid message flooding to Slack - [#471](https://github.com/chrisleekr/binance-trading-bot/pull/471) - Implemented queue to stabilise the trade by [@habibalkhabbaz](https://github.com/habibalkhabbaz) - [#464](https://github.com/chrisleekr/binance-trading-bot/pull/464)