Add Indicator and time based strategy

[Faouzijedidi1]

User description

Description

Add Indicator Strategy
Change Exchange init to include exchange testnets

---

PR Type

Enhancement, Tests

---

Description

• Add time-indicator trading strategy

• Persist indicator strategy executions

• Support testnet exchanges and sandbox

• Extend volume strategy with postOnly side

---

Diagram Walkthrough

flowchart LR
  app["AppModule adds IndicatorStrategyHistory + sync true"]
  enum["Add SignalType enum and Side type"]
  entity["Entity: indicator_strategy_history"]
  svcTI["Service: TimeIndicatorStrategyService (EMA/RSI)"]
  dtoTI["DTO: TimeIndicatorStrategyDto"]
  ctrl["StrategyController endpoints (+execute/start/stop indicator)"]
  exSvc["ExchangeInit: add testnets + sandbox + keepalive tweaks"]
  volEnh["StrategyService: Volume strategy rework (+postOnlySide, PnL, rebal)"]
  dtoVol["DTO: ExecuteVolumeStrategyDto (+postOnlySide)"]
  stratKey["strategyKey: add timeIndicator"]
  pkg["package.json: ccxt upgrade, yarn pm"]

  app -- "TypeORM feature include" --> entity
  ctrl -- "injects" --> svcTI
  svcTI -- "records" --> entity
  ctrl -- "new DTO" --> dtoTI
  volEnh -- "uses" --> dtoVol
  stratKey -- "supports" --> timeInd["timeIndicator key"]
  exSvc -- "initializes" --> exMap["exchanges + -testnet"]
  pkg -- "deps" --> exSvc
  enum -- "used by" --> svcTI

Loading

File Walkthrough

	Relevant files
Configuration changes	2 files app.module.ts Register indicator history entity and enable sync                +3/-1      strategy.module.ts Wire TimeIndicator service and entity into module                +10/-1
Enhancement	11 files side.ts Introduce trading side union type                                                +1/-0      indicator-strategy-history.entity.ts New entity for indicator strategy history                                +43/-0    signaltype.ts Define SignalType enum for indicators                                        +5/-0      strategyKey.ts Add timeIndicator to strategy key types                                    +1/-0      adminStrategy.service.ts Pass postOnlySide to volume strategy starter                          +2/-1      exchangeInit.service.ts Simplify init; add -testnet accounts and sandbox mode        +122/-334 strategy.controller.ts Add indicator strategy endpoints and param wiring                +70/-22  strategy.dto.ts Extend DTOs; add postOnlySide for volume                                  +18/-5    strategy.service.ts Rework volume strategy, add cancel helper, disable shutdown hooks +323/-197 time-indicator.service.ts Implement EMA/RSI time-indicator strategy with persistence +283/-0  timeIndicator.dto.ts Define DTO for time-indicator strategy                                      +106/-0
Tests	1 files adminStrategy.service.spec.ts Fix formatting in volume strategy test args                            +2/-2
Dependencies	1 files package.json Upgrade ccxt; set yarn as package manager                                +3/-2

---

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Risky Config TypeORM synchronize is enabled in production module initialization, which can lead to unintended schema changes and data loss. Consider reverting to migrations for safety. MarketMakingOrder, ArbitrageOrder, SimplyGrowOrder, SpotdataTradingPair, GrowdataExchange, GrowdataSimplyGrowToken, GrowdataArbitragePair, GrowdataMarketMakingPair, IndicatorStrategyHistory, ], synchronize: true, ssl: process.env.POSTGRES_SSL === 'true', Possible Issue The keep-alive ProBit check uses "startsWith('probit')" after introducing '-testnet' names elsewhere; verify naming consistency for ProBit and that default accounts map keys include the '-testnet' suffix where intended. setInterval(async () => { this.logger.log('Running keep-alive checks for all exchanges...'); for (const [exchangeName, accounts] of this.exchanges) { if (exchangeName.startsWith('probit')) { for (const [label, exchange] of accounts) { try { if (!exchange.has['signIn']) { this.logger.log( `ProBit ${label} does not support signIn. Skipping...`, ); continue; } const openOrders = await exchange.fetchOpenOrders(); if (openOrders.length > 0) { this.logger.log( `ProBit ${label} has open orders. Skipping signIn to avoid resetting them.`, ); continue; } await exchange.signIn(); this.logger.log(`ProBit ${label} re-signed in successfully.`); } catch (error) { this.logger.error( `ProBit ${label} keep-alive signIn failed: ${error.message}`, ); } } } } Data Update Logic In cancelAllOrders, DB updates reference a 'strategy' field using strategyKey; ensure this matches your schema (e.g., 'strategy' vs strategyKey vs strategyType) to avoid silent update failures. private async cancelAllOrders( exchange: ccxt.Exchange, pair: string, strategyKey: string, ) { this.logger.log( `Canceling leftover orders for [${strategyKey}] on ${exchange.id}`, ); try { const orders = await exchange.fetchOpenOrders(pair); for (const order of orders) { try { await exchange.cancelOrder(order.id, pair); // Optionally update DB if you're tracking orders by strategy await this.orderRepository.update( { orderId: order.id, exchange: exchange.id, pair, strategy: strategyKey, // or "volume" if you store by strategy name }, { status: 'canceled', }, ); } catch (error) { this.logger.error( `Failed to cancel order ${order.id} on ${exchange.id}: ${error.message}`, ); } } } catch (error) { this.logger.error( `Error fetching open orders for ${pair} on ${exchange.id}: ${error.message}`, ); } }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Disable dangerous auto-sync Enabling TypeORM synchronize: true in a non-dev environment can drop/alter columns unintentionally and cause data loss. Revert to false and rely on migrations for schema changes. server/src/app.module.ts [131-133] ], - synchronize: true, + synchronize: false, ssl: process.env.POSTGRES_SSL === 'true', }), Suggestion importance[1-10]: 8 __ Why: Changing TypeORM synchronize to true in PR introduces potential data loss in non-dev; reverting to false is correct and important for production safety.	Medium
	Ensure default account presence Skipping accounts without keys silently makes exchanges maps exist with missing default accounts, breaking getExchange. If keys are absent, log and skip the whole exName, or ensure a default exists before setting maps to avoid runtime errors. server/src/modules/exchangeInit/exchangeInit.service.ts [49-160] const exchangeNames = [config.name, `${config.name}-testnet`]; for (const exName of exchangeNames) { const exchangeMap = new Map<string, ccxt.Exchange>(); const isTestnet = exName.endsWith('-testnet'); const accounts = [ - { - label: 'default', - apiKey: - process.env[ - `${config.name.toUpperCase()}${ - isTestnet ? '_TESTNET' : '' - }_API_KEY` - ], - secret: - process.env[ - `${config.name.toUpperCase()}${ - isTestnet ? '_TESTNET' : '' - }_SECRET` - ], - }, - ... + { label: 'default', apiKey: process.env[`${config.name.toUpperCase()}${isTestnet ? '_TESTNET' : ''}_API_KEY`], secret: process.env[`${config.name.toUpperCase()}${isTestnet ? '_TESTNET' : ''}_SECRET`] }, + { label: 'account2', apiKey: process.env[`${config.name.toUpperCase()}${isTestnet ? '_TESTNET' : ''}_API_KEY_2`], secret: process.env[`${config.name.toUpperCase()}${isTestnet ? '_TESTNET' : ''}_SECRET_2`] }, + { label: 'read-only', apiKey: process.env[`${config.name.toUpperCase()}${isTestnet ? '_TESTNET' : ''}_API_KEY_READ_ONLY`], secret: process.env[`${config.name.toUpperCase()}${isTestnet ? '_TESTNET' : ''}_SECRET_READ_ONLY`] }, ]; + const validAccounts = accounts.filter(a => a.apiKey && a.secret); + if (!validAccounts.some(a => a.label === 'default')) { + this.logger.warn(`Missing default credentials for ${exName}. Skipping initialization.`); + continue; + } + await Promise.all( - accounts.map(async (account) => { + validAccounts.map(async (account) => { try { - if (!account.apiKey || !account.secret) { - return; + const exchange = new config.class({ apiKey: account.apiKey, secret: account.secret }); + if (isTestnet && typeof exchange.setSandboxMode === 'function') { + exchange.setSandboxMode(true); + this.logger.log(`${exName} ${account.label} sandbox mode activated.`); } + await exchange.loadMarkets(); + if (config.name === 'probit' && exchange.has['signIn']) { + try { + await exchange.signIn(); + this.logger.log(`${exName} ${account.label} signed in successfully.`); + } catch (error) { + this.logger.error(`${exName} ${account.label} sign-in failed: ${error.message}`); + } + } + exchangeMap.set(account.label, exchange); + if (account.label === 'default') { + this.defaultAccounts.set(exName, exchange); + } + this.logger.log(`${exName} ${account.label} initialized successfully.`); + } catch (error) { + this.logger.error(`Failed to initialize ${exName} ${account.label}: ${error.message}`); + } + }), + ); - const exchange = new config.class({ - apiKey: account.apiKey, - secret: account.secret, - }); + this.exchanges.set(exName, exchangeMap); + } - // 🔹 Enable sandbox if testnet - if ( - isTestnet && - typeof exchange.setSandboxMode === 'function' - ) { - exchange.setSandboxMode(true); - this.logger.log( - `${exName} ${account.label} sandbox mode activated.`, - ); - } - - await exchange.loadMarkets(); - Suggestion importance[1-10]: 7 __ Why: The PR silently skips accounts without keys and still registers exchange maps, which can break getExchange; enforcing a default account improves robustness though it's not a critical bug.	Medium
	Fix strategy key mismatch Updating DB with strategy: strategyKey changes semantics versus existing code that uses strategy names (e.g., 'pureMarketMaking'). This will fail to match rows and leave orders in open state. Use a consistent strategy identifier when updating status. server/src/modules/strategy/strategy.service.ts [663-699] private async cancelAllOrders( exchange: ccxt.Exchange, pair: string, strategyKey: string, ) { - this.logger.log( - `Canceling leftover orders for [${strategyKey}] on ${exchange.id}`, - ); - + this.logger.log(`Canceling leftover orders for [${strategyKey}] on ${exchange.id}`); try { const orders = await exchange.fetchOpenOrders(pair); for (const order of orders) { try { await exchange.cancelOrder(order.id, pair); - // Optionally update DB if you're tracking orders by strategy await this.orderRepository.update( { orderId: order.id, exchange: exchange.id, pair, - strategy: strategyKey, // or "volume" if you store by strategy name + // store by strategy name to match existing records + strategy: 'pureMarketMaking', }, - { - status: 'canceled', - }, + { status: 'canceled' }, ); } catch (error) { - this.logger.error( - `Failed to cancel order ${order.id} on ${exchange.id}: ${error.message}`, - ); + this.logger.error(`Failed to cancel order ${order.id} on ${exchange.id}: ${error.message}`); } } } catch (error) { - this.logger.error( - `Error fetching open orders for ${pair} on ${exchange.id}: ${error.message}`, - ); + this.logger.error(`Error fetching open orders for ${pair} on ${exchange.id}: ${error.message}`); } } Suggestion importance[1-10]: 5 __ Why: The PR's updated cancelAllOrders uses strategy: strategyKey, which may not match existing rows that store strategy names; suggestion to standardize is plausible but assumes storage semantics not fully evident from the diff.	Low