Add rewards calculation

[Faouzijedidi1]

PR Type

enhancement

---

Description

• Added a new entity UserExchangeReward to manage user rewards based on trading activities.
• Implemented StrategyRewardsService to calculate rewards, complete with a daily cron job for automation.
• Updated StrategyController to include endpoints for executing arbitrage and market making strategies.
• Enhanced testing and module configurations to support the new features.

---

Changes walkthrough 📝

	Relevant files
Enhancement	app.module.ts Update AppModule to Include UserExchangeReward                      server/src/app.module.ts - Added `UserExchangeReward` entity to the module imports. +2/-0      user-rewards.ts Add New Entity for User Exchange Rewards                                  server/src/common/entities/user-rewards.ts Created a new entity UserExchangeReward with fields for user ID, exchange, score, start and end dates, and trading pair. +25/-0    strategy-rewards.service.ts Implement Strategy Rewards Calculation Service                      server/src/modules/strategy/strategy-rewards.service.ts Implemented StrategyRewardsService with methods to calculate and handle rewards. Added a cron job to trigger reward calculation daily. +107/-0  strategy.controller.ts Extend Strategy Controller with New Endpoints                        server/src/modules/strategy/strategy.controller.ts Added endpoints to execute and manage arbitrage and market making strategies. +38/-0    strategy.module.ts Update Strategy Module to Include Rewards Service                server/src/modules/strategy/strategy.module.ts - Registered `StrategyRewardsService` in the strategy module. +2/-1
Tests	health.service.spec.ts Simplify Mock Implementation in Health Service Tests          server/src/modules/health/health.service.spec.ts Simplified the mock implementation for exchangeMock.fetchBalance for 'mexc' case. +1/-3      strategy-rewards.service.spec.ts Add Tests for StrategyRewardsService                                          server/src/modules/strategy/strategy-rewards.service.spec.ts - Added tests for `StrategyRewardsService`. +18/-0

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mr-market	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 13, 2024 0:32am

[railway-app[bot]]

🚅 Deployed to the Mr.Market-pr-176 environment in quiet-desk	Service	Status	Web	Updated (UTC)

[github-actions[bot]]

PR Description updated to latest commit (https://github.com/Hu-Fi/Mr.Market/commit/0ef15e2a1be0c494bd326ac24bbf88f3925978a7)

[github-actions[bot]]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	4, because the PR introduces significant changes across multiple files including new entities, service logic, and controller endpoints. The complexity of the reward calculation logic and integration with existing modules increases the review effort.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The `calculateRewardsForPeriod` method in `strategy-rewards.service.ts` assumes the existence of methods `getUserOrders` and `getUserArbitrageHistorys` which are not shown in the PR. If these methods do not handle exceptions or if they are not implemented efficiently, it could lead to performance issues or runtime errors.
	Data Integrity: The reward calculation does not handle potential concurrency issues. Multiple instances of the cron job could lead to duplicate entries or race conditions when updating the `UserExchangeReward`.
🔒 Security concerns	No

Code feedback:

---

relevant file	server/src/modules/strategy/strategy-rewards.service.ts
suggestion	Consider implementing transaction management for the database operations within the `calculateRewardsForPeriod` method to ensure data consistency and handle errors more effectively. This is important to prevent partial updates in the database which can lead to data integrity issues. [important]
relevant line	await this.userExchangeRewardRepository.save(userReward);

---

relevant file	server/src/modules/strategy/strategy-rewards.service.ts
suggestion	Introduce error handling for the `getAllUsers` method to manage exceptions that may occur during database queries. This will improve the robustness of your service by ensuring that the service can gracefully handle situations where the database is temporarily unavailable or the query fails. [important]
relevant line	const mmUsers = await this.marketMakingRepository

---

relevant file	server/src/common/entities/user-rewards.ts
suggestion	Add index annotations to frequently queried columns such as `userId`, `exchange`, and `pair` in the `UserExchangeReward` entity to improve query performance. This is particularly beneficial as the size of data grows and queries become more complex. [medium]
relevant line	@Column()

---

relevant file	server/src/modules/strategy/strategy-rewards.service.ts
suggestion	Optimize the reward calculation by reducing the number of database queries. For instance, you can fetch all orders once and filter them in the service rather than executing separate queries for market making and arbitrage orders. This will reduce the load on the database and improve the performance of the reward calculation process. [important]
relevant line	const arbUsers = await this.arbitrageRepository

---

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for filtering orders by date and pair ___ **Consider handling potential exceptions when filtering orders by date and pair to ensure robust error handling and avoid runtime exceptions.** [server/src/modules/strategy/strategy-rewards.service.ts [48-50]](https://github.com/Hu-Fi/Mr.Market/pull/176/files#diff-9f1102428568a379f9a8804f6c74a929417c46b77f9fde274f251b559c0a8634R48-R50) ```diff -const mmOrders = (await this.strategyService.getUserOrders(userId)).filter(order => - new Date(order.executedAt) >= startDate && new Date(order.executedAt) <= endDate && order.pair === pair -); +let mmOrders; +try { + mmOrders = (await this.strategyService.getUserOrders(userId)).filter(order => + new Date(order.executedAt) >= startDate && new Date(order.executedAt) <= endDate && order.pair === pair + ); +} catch (error) { + this.logger.error(`Error filtering market making orders for user ${userId}:`, error); + mmOrders = []; +} ``` Suggestion importance[1-10]: 8 Why: Adding error handling around filtering orders by date and pair is crucial to prevent runtime exceptions and ensure robustness, especially given the asynchronous nature of the operations.	8
	Add null checks for order properties to prevent runtime errors ___ **Implement a mechanism to handle potential null values when accessing properties of orders in the reward calculation logic.** [server/src/modules/strategy/strategy-rewards.service.ts [58-60]](https://github.com/Hu-Fi/Mr.Market/pull/176/files#diff-9f1102428568a379f9a8804f6c74a929417c46b77f9fde274f251b559c0a8634R58-R60) ```diff mmOrders.forEach(order => { - const key = `${userId}-${order.exchange}`; - rewardsMap.set(key, (rewardsMap.get(key) || 0) + order.amount); + if (order.exchange && order.amount) { + const key = `${userId}-${order.exchange}`; + rewardsMap.set(key, (rewardsMap.get(key) || 0) + order.amount); + } else { + this.logger.warn(`Order missing critical information: ${JSON.stringify(order)}`); + } }); ``` Suggestion importance[1-10]: 8 Why: Implementing null checks on order properties is essential to prevent potential runtime errors and ensure data integrity during the reward calculation process.	8
Performance	Combine user ID queries into a single database query for performance improvement ___ **To improve performance, consider combining the database queries for distinct user IDs from market making and arbitrage orders into a single query.** [server/src/modules/strategy/strategy-rewards.service.ts [26-35]](https://github.com/Hu-Fi/Mr.Market/pull/176/files#diff-9f1102428568a379f9a8804f6c74a929417c46b77f9fde274f251b559c0a8634R26-R35) ```diff -const mmUsers = await this.marketMakingRepository +const allUsers = await this.marketMakingRepository .createQueryBuilder("order") .select("distinct(order.userId)", "userId") - .getRawMany(); -const arbUsers = await this.arbitrageRepository - .createQueryBuilder("order") - .select("distinct(order.userId)", "userId") + .union( + this.arbitrageRepository.createQueryBuilder("order") + .select("distinct(order.userId)", "userId") + ) .getRawMany(); ``` Suggestion importance[1-10]: 7 Why: Combining database queries into a single operation can significantly improve performance by reducing the number of round trips to the database.	7
	Use batch operations for database updates to enhance performance ___ **Optimize the creation and updating of user rewards by batching database operations instead of handling them one by one in a loop.** [server/src/modules/strategy/strategy-rewards.service.ts [72-81]](https://github.com/Hu-Fi/Mr.Market/pull/176/files#diff-9f1102428568a379f9a8804f6c74a929417c46b77f9fde274f251b559c0a8634R72-R81) ```diff +const rewardPromises = []; for (const [key, score] of rewardsMap) { const [userId, exchange] = key.split('-'); - let userReward = await this.userExchangeRewardRepository.findOne({ where: { userId, exchange, startDate, endDate, pair } }); - if (userReward) { - userReward.score += score; - } else { - userReward = this.userExchangeRewardRepository.create({ userId, exchange, score, startDate, endDate, pair }); - } - await this.userExchangeRewardRepository.save(userReward); + rewardPromises.push(this.userExchangeRewardRepository.upsert({ userId, exchange, score, startDate, endDate, pair }, ['userId', 'exchange', 'startDate', 'endDate', 'pair'])); } +await Promise.all(rewardPromises); ``` Suggestion importance[1-10]: 7 Why: Using batch operations for database updates is a performance optimization that can reduce the overhead of multiple individual write operations, especially in a loop.	7