Closed Faouzijedidi1 closed 7 months ago
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 | Apr 11, 2024 0:56am |
This PR is being deployed to Railway 🚅
This PR is being deployed to Railway 🚅
Mr.Market: ◻️ REMOVED
PR Description updated to latest commit (https://github.com/Hu-Fi/Mr.Market/commit/d386d87a860e7f9748df772f8852631ebc280fe3)
⏱️ Estimated effort to review [1-5] | 3, because the PR introduces significant changes across multiple files, including new entities, service logic, and controller endpoints. The complexity of the changes, especially around the trading logic and database interactions, requires a thorough review to ensure correctness, performance, and security. |
🧪 Relevant tests | No |
🔍 Possible issues | Possible Bug: In `strategy.service.ts`, the `side` property is incorrectly set to 'buy' for a sell order entity creation. This could lead to incorrect order tracking and reporting. |
Performance Concern: The current implementation of order and arbitrage order creation within the trading functions might not be the most efficient, especially under high load. Consider batching or async processing to improve performance. | |
🔒 Security concerns | No |
relevant file | server/src/modules/strategy/strategy.service.ts |
suggestion | Correct the `side` property for the sell order entity creation to 'sell'. This ensures accurate representation of the order's intent in the database. [important] |
relevant line | side: 'buy', |
relevant file | server/src/common/entities/arbitrage-order.entity.ts |
suggestion | Consider adding index decorators to frequently queried columns like `userId`, `clientId`, and `pair` to improve query performance. [medium] |
relevant line | @Column() |
relevant file | server/src/modules/strategy/strategy.service.ts |
suggestion | Implement error handling for database operations (e.g., `save` method) to gracefully manage failures and ensure system stability. [important] |
relevant line | await this.orderRepository.save(orderEntity); |
relevant file | server/src/modules/strategy/strategy.service.ts |
suggestion | Optimize the arbitrage trading logic by considering parallel execution of buy and sell orders where possible, to reduce execution time and potentially increase profitability. [medium] |
relevant line | await this.executeArbitrageTradeWithLimitOrders( |
Category | Suggestions |
Enhancement |
Add a check for successful trade execution before saving the order entity.___ **Consider checking for the execution result of the limit trade before creating and savingthe order entity. This ensures that the order entity is only saved if the trade was successfully executed.** [server/src/modules/strategy/strategy.service.ts [367-392]](https://github.com/Hu-Fi/Mr.Market/pull/144/files#diff-413cb1b28e0d47a46768f97d10145a8e14d9e46b0a195768786127305916d944R367-R392) ```diff const order = await this.tradeService.executeLimitTrade({ ... -await this.orderRepository.save(orderEntity); +if (order && order.id) { + await this.orderRepository.save(orderEntity); +} ``` |
Use enum types for
___
**Consider using enum types for the | |
Make the
___
**For consistency and to avoid potential errors, consider making the | |
Bug |
Correct the order side for sell orders when saving the order entity.___ **For theside property in the order entity for sell orders, it should be set to 'sell' instead of 'buy'. This ensures the correct order side is recorded.** [server/src/modules/strategy/strategy.service.ts [427]](https://github.com/Hu-Fi/Mr.Market/pull/144/files#diff-413cb1b28e0d47a46768f97d10145a8e14d9e46b0a195768786127305916d944R427-R427) ```diff -side: 'buy', +side: 'sell', ``` |
Fix the typo in the route for fetching market making orders.___ **There's a typo in the route decorator for getting user orders. It should be 'marketmaking'instead of 'maretmaking'.** [server/src/modules/strategy/orders.controller.ts [12]](https://github.com/Hu-Fi/Mr.Market/pull/144/files#diff-0069d0715a0f071e72068a2530df2fa7b1663ab751cc3b8f9a90d343369b516fR12-R12) ```diff -@Get('/maretmaking/:userId') +@Get('/marketmaking/:userId') ``` |
Type
enhancement, bug_fix
Description
Order
andArbitrageOrder
to manage trading orders and arbitrage orders, respectively.OrdersController
for fetching market-making and arbitrage orders for a user.StrategyModule
to include new entities and controller for handling orders.StrategyService
to support creation and retrieval of order entities, improving the order management system.Changes walkthrough
app.module.ts
Import and Synchronize New Order Entities
server/src/app.module.ts
Order
andArbitrageOrder
entities to the module imports anddatabase synchronization list.
strategy.module.ts
Strategy Module Updates for Orders Handling
server/src/modules/strategy/strategy.module.ts
Order
entity in module imports.OrdersController
to the module's controllers.arbitrage-order.entity.ts
New Arbitrage Order Entity
server/src/common/entities/arbitrage-order.entity.ts
ArbitrageOrder
with fields for managing arbitrageorders, including user and client IDs, trading pair, exchanges,
amounts, prices, profit, execution time, status, and strategy.
order.entity.ts
New General Order Entity
server/src/common/entities/order.entity.ts
Order
for handling general orders, includingfields for user and client IDs, exchange, trading pair, side, amount,
price, order ID, execution time, status, and strategy.
orders.controller.ts
Orders Fetching Endpoints
server/src/modules/strategy/orders.controller.ts
OrdersController
with endpoints to fetch market-making andarbitrage orders for a user.
strategy.service.ts
Enhancements in Strategy Service for Order Handling
server/src/modules/strategy/strategy.service.ts
Order
andArbitrageOrder
entities.upon trade execution.