New strategies

[Faouzijedidi1]

Description

fix issues with the testnet not working

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 Security concerns Sensitive information exposure: loadMarketsWithRetry logs e?.message from ccxt errors; depending on the exchange/client, error messages can sometimes include request details. Consider sanitizing/whitelisting logged error fields. Also, enabling TypeORM synchronize: true can cause unintended schema changes in production environments, which is a deployment safety concern.

⚡ Recommended focus areas for review Risky Config The ORM setting synchronize was switched to enabled, which can unintentionally mutate/drop schema in non-development environments and makes DB changes implicit rather than controlled via migrations. This should be guarded by environment (e.g., only in local/test) and/or replaced with migrations in deployed environments. SimplyGrowOrder, SpotdataTradingPair, GrowdataExchange, GrowdataSimplyGrowToken, GrowdataArbitragePair, GrowdataMarketMakingPair, IndicatorStrategyHistory, ], synchronize: true, ssl: process.env.POSTGRES_SSL === 'true', }), Data Model Drift The startPrice column was removed from StrategyInstance while multiple strategy creation flows now have startPrice logic commented out. Verify no runtime paths, analytics, reporting, or UI rely on startPrice, and ensure the DB migration/cleanup and any backward compatibility concerns are handled. userId: string; @Column() clientId: string; @Column() strategyType: string; @Column('json') parameters: Record<string, any>; @Column() Retry Semantics loadMarketsWithRetry adds retries and logging, but it may still behave poorly under rate limiting / transient network failures (fixed backoff, no jitter, no timeout/cancel, and it retries all errors). Consider narrowing retryable errors (e.g., network/timeout/429), adding jitter, and ensuring logging won’t leak sensitive request details if ccxt error messages contain them. private async loadMarketsWithRetry( exchange: ccxt.Exchange, exName: string, label: string, maxRetries = 3, ) { for (let attempt = 1; attempt <= maxRetries; attempt++) { try { await exchange.loadMarkets(); return; } catch (e: any) { this.logger.warn( `[${exName}:${label}] loadMarkets attempt ${attempt}/${maxRetries} failed: ${ e?.message ?? e }`, ); if (attempt === maxRetries) throw e; await new Promise((r) => setTimeout(r, 2000 * attempt)); } } }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Gate schema sync by environment Enabling automatic schema synchronization can unintentionally alter or drop tables in non-dev environments. Gate synchronize behind an explicit env flag (or restrict it to non-production) so production DBs aren’t modified by application startup. server/src/app.module.ts [132] -synchronize: true, +synchronize: + process.env.TYPEORM_SYNCHRONIZE === 'true' && + process.env.NODE_ENV !== 'production', Suggestion importance[1-10]: 8 __ Why: Setting TypeORM synchronize: true can be destructive in non-dev environments (implicit schema changes/drops on startup). Gating synchronize behind an explicit env flag (and/or disabling in production) is a high-impact safety improvement.	Medium
	Ensure pricing exchange name fallback Passing oracleExchangeName directly can be undefined when no oracle is provided, causing pricing to fail or select the wrong exchange. Compute a fallback to executionExchangeName (or a known default) before calling getPriceSource. server/src/modules/strategy/strategy.service.ts [1791-1795] +const priceExchangeName = oracleExchangeName ?? executionExchangeName; + const priceSource = await this.getPriceSource( - oracleExchangeName, // use priceExchange for data + priceExchangeName, pair, priceSourceType, ); Suggestion importance[1-10]: 7 __ Why: The new code passes oracleExchangeName directly into getPriceSource, which can be undefined when no oracle is provided, potentially breaking price fetching. Adding a fallback to executionExchangeName preserves the previous behavior where a pricing exchange was always selected.	Medium
General	Reduce exchange rate-limit hammering Retrying loadMarkets without considering rateLimit can hammer the exchange and trigger bans/429s during outages. Short-circuit if markets are already loaded, and base backoff on exchange.rateLimit to reduce the chance of repeated throttling. server/src/modules/exchangeInit/exchangeInit.service.ts [91-111] private async loadMarketsWithRetry( exchange: ccxt.Exchange, exName: string, label: string, maxRetries = 3, ) { + if (exchange.markets && Object.keys(exchange.markets).length > 0) return; + + const baseDelay = Math.max(Number((exchange as any).rateLimit) || 0, 2000); + for (let attempt = 1; attempt <= maxRetries; attempt++) { try { await exchange.loadMarkets(); return; } catch (e: any) { this.logger.warn( `[${exName}:${label}] loadMarkets attempt ${attempt}/${maxRetries} failed: ${ - e?.message ?? e + e?.message ?? String(e) }`, ); if (attempt === maxRetries) throw e; - await new Promise((r) => setTimeout(r, 2000 * attempt)); + await new Promise((r) => setTimeout(r, baseDelay * attempt)); } } } Suggestion importance[1-10]: 5 __ Why: Using exchange.rateLimit for backoff and skipping retries when exchange.markets is already populated can reduce unnecessary load and mitigate 429/bans during outages. This is a reasonable resiliency tweak, though the current fixed backoff is already functional.	Low