general fix to Arb orders

Faouzijedidi1 commented 2 months ago

Type

enhancement

Description

Refactored ArbitrageOrder and MMOrder entities to support nullable fields for increased flexibility in order handling.
Renamed fields in ArbitrageOrder entity from buyExchange and sellExchange to exchangeAName and exchangeBName for clarity.
Updated StrategyService to align with the renaming of exchange fields in the ArbitrageOrder entity.

Changes walkthrough

Relevant files

Enhancement

arbitrage-order.entity.ts `Refactor ArbitrageOrder Entity for Flexibility` server/src/common/entities/arbitrage-order.entity.ts Made `buyExchange` and `sellExchange` columns nullable and renamed them to `exchangeAName` and `exchangeBName`. Made `amount`, `buyPrice`, `sellPrice`, `profit`, `executedAt`, `status`, and `strategy` columns nullable.	+11/-11
mm-order.entity.ts `Update MMOrder Entity to Support Optional Fields` server/src/common/entities/mm-order.entity.ts Made `amount` and `price` columns nullable to allow for flexibility in order handling. Updated `strategy` column to be nullable, aligning with changes in arbitrage orders.	+3/-3
strategy.service.ts `Align Strategy Service with Entity Renaming` server/src/modules/strategy/strategy.service.ts Updated references from `buyExchange` and `sellExchange` to `exchangeAName` and `exchangeBName` in strategy service.	+2/-2

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

vercel[bot] commented 2 months ago

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

Name	Status	Preview	Updated (UTC)
mr-market	✅ Ready (Inspect)	Visit Preview	Apr 11, 2024 2:14am

railway-app[bot] commented 2 months ago

This PR is being deployed to Railway 🚅

github-actions[bot] commented 2 months ago

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

github-actions[bot] commented 2 months ago

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward, involving renaming and making fields nullable. The logic seems not to be heavily altered, which simplifies the review process. However, understanding the context and implications of these changes on the system's behavior requires some domain knowledge.
🧪 Relevant tests	No
🔍 Possible issues	Nullable Fields: Making several fields nullable in entities might introduce unexpected behavior or errors if the rest of the application does not handle null values properly. This is especially critical for fields like `amount`, `price`, `profit`, which are essential for financial calculations.
🔒 Security concerns	No

Code feedback:

relevant file	server/src/common/entities/arbitrage-order.entity.ts
suggestion	Consider implementing validation logic for nullable fields to ensure that when they are provided, they meet the expected format or range. This can prevent potential issues with data integrity and application logic. [important]
relevant line	@Column({ nullable: true })

relevant file	server/src/common/entities/mm-order.entity.ts
suggestion	Ensure that the application logic that interacts with `MMOrder` entities correctly handles cases where `amount` and `price` are null, to avoid runtime errors during financial calculations. [important]
relevant line	@Column('decimal', { precision: 10, scale: 2, nullable: true })

relevant file	server/src/modules/strategy/strategy.service.ts
suggestion	After renaming fields in `ArbitrageOrder`, verify that all references to these fields across the application are updated to prevent any issues with undefined properties. This includes checking any serialization/deserialization logic that might rely on the old field names. [important]
relevant line	exchangeAName: exchangeA.name,

relevant file	server/src/common/entities/arbitrage-order.entity.ts
suggestion	For fields that are now nullable, consider adding logic to handle default values or fallback mechanisms where it makes sense, to maintain the robustness of the application's functionality. [medium]
relevant line	@Column({ nullable: true })

✨ Review tool usage guide:

**Overview:** The `review` tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be [added](https://pr-agent-docs.codium.ai/tools/review/#general-configurations) by configuring the tool. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L23) related to the review tool (`pr_reviewer` section), use the following template: ``` /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_reviewer] some_config1=... some_config2=... ``` See the review [usage page](https://pr-agent-docs.codium.ai/tools/review/) for a comprehensive guide on using this tool.

github-actions[bot] commented 2 months ago

PR Code Suggestions

Category	Suggestions
Best practice	Add explicit type annotations for new entity columns. ___ Consider adding explicit type annotations for the new columns to ensure clarity and maintainability. TypeScript can infer types, but explicit types for entity columns can make the code more robust and easier to understand for new developers or when revisiting the code after some time. [server/src/common/entities/arbitrage-order.entity.ts [18-25]](https://github.com/Hu-Fi/Mr.Market/pull/146/files#diff-ff5a2e5f3af4797869b2a42e0e031f8e629b36f0e4e628d5819683c1a44c79a6R18-R25) ```diff -@Column({ nullable: true }) +@Column({ type: 'varchar', nullable: true }) exchangeAName: string; -@Column({ nullable: true }) +@Column({ type: 'varchar', nullable: true }) exchangeBName: string; -@Column('decimal', { precision: 10, scale: 2, nullable: true }) +@Column({ type: 'decimal', precision: 10, scale: 2, nullable: true }) amount: number; ```
	Use enums for fields with specific allowed values. ___ Given that `status` and `strategy` fields have specific allowed values ('open', 'closed', 'canceled' for `status` and 'arbitrage', 'market-making', etc. for `strategy`), consider using TypeScript enums to enforce these constraints at the type level. This approach enhances code readability and maintainability. [server/src/common/entities/arbitrage-order.entity.ts [39-43]](https://github.com/Hu-Fi/Mr.Market/pull/146/files#diff-ff5a2e5f3af4797869b2a42e0e031f8e629b36f0e4e628d5819683c1a44c79a6R39-R43) ```diff -@Column({ nullable: true }) -status: string; // 'open', 'closed', 'canceled' +enum Status { + Open = 'open', + Closed = 'closed', + Canceled = 'canceled', +} -@Column({ nullable: true }) -strategy: string; // 'arbitrage', 'market-making', etc. +enum Strategy { + Arbitrage = 'arbitrage', + MarketMaking = 'market-making', +} +@Column({ type: 'enum', enum: Status, nullable: true }) +status: Status; + +@Column({ type: 'enum', enum: Strategy, nullable: true }) +strategy: Strategy; + ```
	Use enums for the `strategy` field to enforce allowed values. ___ For the `strategy` column, similar to the suggestion for the `arbitrage-order.entity.ts` file, consider defining a TypeScript enum for the strategy types. This will ensure consistency across different parts of the application and make the codebase more type-safe. [server/src/common/entities/mm-order.entity.ts [38-39]](https://github.com/Hu-Fi/Mr.Market/pull/146/files#diff-db3678ceab2b08be6527186ae21cae7c0bdb4eeedb4f035144aced59dbd28729R38-R39) ```diff -@Column({ nullable: true }) -strategy: string; // 'arbitrage', 'market-making', etc. +enum Strategy { + Arbitrage = 'arbitrage', + MarketMaking = 'market-making', +} +@Column({ type: 'enum', enum: Strategy, nullable: true }) +strategy: Strategy; + ```
Enhancement	Ensure `amount` and `price` columns accept only positive values. ___ For the `amount` and `price` columns, consider validating the input to ensure that negative values are not accepted. This can be done using custom validation logic or by integrating a library like class-validator to enforce positive values. [server/src/common/entities/mm-order.entity.ts [23-27]](https://github.com/Hu-Fi/Mr.Market/pull/146/files#diff-db3678ceab2b08be6527186ae21cae7c0bdb4eeedb4f035144aced59dbd28729R23-R27) ```diff @Column('decimal', { precision: 10, scale: 2, nullable: true }) +@Min(0) amount: number; @Column('decimal', { precision: 10, scale: 2, nullable: true }) +@Min(0) price: number; ```
Security	Validate or sanitize exchange names to prevent security vulnerabilities. ___ Ensure that `exchangeA.name` and `exchangeB.name` are validated or sanitized before being used to prevent potential security vulnerabilities, such as injection attacks. Consider implementing a validation layer if not already present. [server/src/modules/strategy/strategy.service.ts [669-670]](https://github.com/Hu-Fi/Mr.Market/pull/146/files#diff-413cb1b28e0d47a46768f97d10145a8e14d9e46b0a195768786127305916d944R669-R670) ```diff -exchangeAName: exchangeA.name, -exchangeBName: exchangeB.name, +exchangeAName: sanitizeExchangeName(exchangeA.name), +exchangeBName: sanitizeExchangeName(exchangeB.name), ```

✨ Improve tool usage guide:

**Overview:** The `improve` tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on a PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L78) related to the improve tool (`pr_code_suggestions` section), use the following template: ``` /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_code_suggestions] some_config1=... some_config2=... ``` See the improve [usage page](https://pr-agent-docs.codium.ai/tools/improve/) for a comprehensive guide on using this tool.

Hu-Fi / Mr.Market