Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update test constants #1785

Merged
merged 1 commit into from
Jun 26, 2024
Merged

update test constants #1785

merged 1 commit into from
Jun 26, 2024

Conversation

jayy04
Copy link
Contributor

@jayy04 jayy04 commented Jun 26, 2024

Changelist

[Describe or list the changes made in this PR]

Test Plan

[Describe how this PR was tested (if applicable)]

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

Summary by CodeRabbit

  • Refactor
    • Improved numerical precision and handling of large numbers for perpetual and asset positions by using enhanced utility functions.

Copy link
Contributor

coderabbitai bot commented Jun 26, 2024

Walkthrough

The changes involve refactoring the creation of perpetual and asset positions in the protocol/testutil/constants package. Instead of using dtypes.NewInt, the code now employs big.NewInt within newly introduced testutil functions. This enhances numerical precision and the ability to handle large numbers.

Changes

Files Summary
protocol/testutil/constants/positions.go Refactored the initialization of perpetual and asset positions to use big.NewInt via testutil functions.
protocol/testutil/constants/subaccounts.go Replaced direct initialization of PerpetualPositions with testutil.CreateSinglePerpetualPosition using big.Int.

Sequence Diagram(s)

No complex control flow changes warranting a sequence diagram were introduced.

Poem

A rabbit hopped with boundless cheer,
Numbers now so very clear,
big.NewInt to hold the might,
Precision soaring to new height.
Positions strong, they hold their ground,
In digits deep, they're safe and sound.
The code now runs without a hitch,
With every byte, a perfect pitch! 🎵


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 75829b3 and 1476923.

Files selected for processing (2)
  • protocol/testutil/constants/positions.go (1 hunks)
  • protocol/testutil/constants/subaccounts.go (24 hunks)
Additional comments not posted (51)
protocol/testutil/constants/positions.go (39)

12-16: Review of Perpetual Position Initialization

The initialization of Long_Perp_1BTC_PositiveFunding uses big.NewInt correctly to handle large numbers. Ensure that the CreateSinglePerpetualPosition function is implemented to handle these parameters effectively.


17-21: Review of Perpetual Position Initialization

The use of negative values in Short_Perp_1ETH_NegativeFunding is appropriate for representing short positions. Confirm that the negative values are handled as expected within the CreateSinglePerpetualPosition function.


22-26: Review of Perpetual Position Initialization

PerpetualPosition_OneBTCLong is correctly initialized. Consistency in using big.NewInt for all perpetual positions enhances precision handling.


27-31: Review of Perpetual Position Initialization

Similar to previous comments, PerpetualPosition_OneBTCShort uses negative values correctly. It is crucial that the system processes these values accurately.


32-36: Review of Perpetual Position Initialization

The smaller notional value in PerpetualPosition_OneTenthBTCLong is handled using big.NewInt, which is good practice for maintaining precision with smaller and fractional values.


37-41: Review of Perpetual Position Initialization

PerpetualPosition_OneTenthBTCShort demonstrates consistent use of negative values for short positions across different scales of notional values.


42-46: Review of Perpetual Position Initialization

PerpetualPosition_OneHundredthBTCLong shows the application of big.NewInt for very small notional values, which is essential for accurate financial calculations.


47-51: Review of Perpetual Position Initialization

The initialization of PerpetualPosition_OneHundredthBTCShort is consistent with other short positions, using negative values correctly.


52-56: Review of Perpetual Position Initialization

PerpetualPosition_FourThousandthsBTCLong uses a very small notional value, and the use of big.NewInt helps prevent any rounding errors or precision loss.


57-61: Review of Perpetual Position Initialization

For PerpetualPosition_FourThousandthsBTCShort, the negative value representation is correct, and it's important that the function handling these values is robust against potential errors in sign handling.


62-66: Review of Perpetual Position Initialization

The larger notional value in PerpetualPosition_OneAndHalfBTCLong is managed using big.NewInt, which is crucial for handling such large numbers accurately.


67-71: Review of Perpetual Position Initialization

PerpetualPosition_OneTenthEthLong is initialized with a high precision value for ETH, which is necessary given the volatility and price movements in cryptocurrency markets.


72-76: Review of Perpetual Position Initialization

PerpetualPosition_OneTenthEthShort uses negative values to denote short positions, consistent with other declarations, ensuring that the handling of these values is uniform across the system.


77-81: Review of Perpetual Position Initialization

The use of math.MaxUint64 in PerpetualPosition_MaxUint64EthLong to represent a very large number is an interesting use case. Ensure that the big.NewInt constructor can handle this without any overflow or precision issues.


82-86: Review of Perpetual Position Initialization

For PerpetualPosition_MaxUint64EthShort, the negative representation of a very large number is crucial. It's important to validate that there are no overflow issues when big.NewInt is used in this context.


88-92: Review of Perpetual Position Initialization

PerpetualPosition_OneISOLong handles a very large notional value, which is appropriately managed using big.NewInt to ensure precision.


93-97: Review of Perpetual Position Initialization

PerpetualPosition_OneISO2Long demonstrates the use of big.NewInt for smaller notional values, maintaining consistency in handling numerical precision across different scales.


99-103: Review of Perpetual Position Initialization

PerpetualPosition_OneISOShort correctly uses a negative value to represent a short position, ensuring that the sign is handled correctly in calculations.


104-108: Review of Perpetual Position Initialization

PerpetualPosition_OneISO2Short continues the pattern of using big.NewInt for precise handling of small notional values in short positions.


111-114: Review of Asset Position Initialization

Usdc_Asset_0 is initialized with a zero value, which is straightforward but essential for certain financial calculations or placeholders.


115-118: Review of Asset Position Initialization

Usdc_Asset_1 represents a very small monetary value, and the use of big.NewInt ensures that even these small amounts are accurately represented in the system.


119-122: Review of Asset Position Initialization

Usdc_Asset_500 uses a moderate monetary value, and initializing this with big.NewInt is a good practice to maintain precision.


123-126: Review of Asset Position Initialization

The negative value in Short_Usdc_Asset_500 is handled correctly, representing a short asset position. It's important that the system processes these negative values accurately.


127-130: Review of Asset Position Initialization

Usdc_Asset_599 uses a slightly unusual value, which may be specific to certain test scenarios or edge cases. Ensure that this value is handled correctly in all relevant calculations.


131-134: Review of Asset Position Initialization

Usdc_Asset_660 is another example where big.NewInt is used to ensure precision in representing asset values.


135-138: Review of Asset Position Initialization

Short_Usdc_Asset_4_600 uses a significantly negative value to represent a short position. This must be handled carefully to avoid any issues with sign inversion or other computational errors.


139-142: Review of Asset Position Initialization

Short_Usdc_Asset_46_000 represents a larger short position, and the consistency in using big.NewInt for these values is crucial for accurate financial reporting and calculations.
[APPROVIDED]


143-146: Review of Asset Position Initialization

The negative value in Short_Usdc_Asset_9_900 is another instance where the system's ability to handle negative values accurately is critical for correct financial calculations.


147-150: Review of Asset Position Initialization

Usdc_Asset_10_000 uses a round number, which is often used in financial tests to simplify calculations or for benchmarking purposes.


155-158: Review of Asset Position Initialization

Usdc_Asset_10_200 is another instance of using big.NewInt for precise asset value representation, ensuring accuracy in financial calculations.


159-162: Review of Asset Position Initialization

The large value in Usdc_Asset_50_000 is handled using big.NewInt, which is essential for maintaining accuracy in high-value financial transactions or tests.


163-166: Review of Asset Position Initialization

Usdc_Asset_99_999 uses a high precision value that is likely important for testing edge cases or specific scenarios where exact values are crucial.


167-170: Review of Asset Position Initialization

Usdc_Asset_100_000 uses a significant round number, which is commonly used in financial simulations or tests to represent standard transaction sizes.


171-174: Review of Asset Position Initialization

Usdc_Asset_100_499 uses a very specific value, which might be used to test precision handling or rounding behaviors in financial systems.


175-178: Review of Asset Position Initialization

The very large value in Usdc_Asset_500_000 needs to be handled with high precision, and big.NewInt is the right choice for such scenarios.


179-182: Review of Asset Position Initialization

Long_Asset_1BTC is initialized with a value representing 1 BTC, which is a common unit in cryptocurrency tests. The use of big.NewInt ensures that this value is precisely managed.


183-186: Review of Asset Position Initialization

Short_Asset_1BTC shows the correct use of negative values for representing short positions in assets. This consistency is crucial for accurate financial simulations.


187-190: Review of Asset Position Initialization

Long_Asset_1ETH uses a significant value for ETH, which is necessary given the high value and volatility of ETH in financial markets.


191-194: Review of Asset Position Initialization

Short_Asset_1ETH correctly uses a negative value to represent a short position in ETH, ensuring that the system handles these scenarios accurately.

protocol/testutil/constants/subaccounts.go (12)

40-44: Review of Subaccount Perpetual Position Initialization

The initialization of Alice_Num0_1BTC_LONG_10_000USD uses big.NewInt correctly to handle large numbers. Ensure that the CreateSinglePerpetualPosition function is implemented to handle these parameters effectively.


53-57: Review of Subaccount Perpetual Position Initialization

The initialization of Alice_Num0_1ISO_LONG_10_000USD uses big.NewInt correctly to handle large numbers associated with an ISO currency. Ensure that the function is robust enough to handle these parameters without any issues.


80-84: Review of Subaccount Perpetual Position Initialization

Alice_Num1_1BTC_Short_100_000USD uses negative values to represent short positions, which is appropriate. Confirm that these values are handled correctly within the function.


93-97: Review of Subaccount Perpetual Position Initialization

Alice_Num1_1BTC_Long_500_000USD is initialized correctly using big.NewInt for representing a long position in BTC. This ensures precision in representing large cryptocurrency values.


120-124: Review of Subaccount Perpetual Position Initialization

Bob_Num0_1ISO_LONG_10_000USD uses big.NewInt to manage large numerical values associated with an ISO currency. This is crucial for accurate financial calculations and simulations.


133-137: Review of Subaccount Perpetual Position Initialization

Bob_Num0_1ISO2_LONG_10_000USD demonstrates the use of big.NewInt for smaller notional values, maintaining consistency in handling numerical precision across different scales.


146-150: Review of Subaccount Perpetual Position Initialization

Carl_Num0_100BTC_Short_10100USD uses a very large negative value to represent a significant short position in BTC. It's important to ensure that the system handles such large values accurately without any precision loss.


159-163: Review of Subaccount Perpetual Position Initialization

Carl_Num0_1BTC_Short uses big.NewInt to accurately represent a short position in BTC. This consistency is essential for correct financial calculations and risk management.


172-176: Review of Subaccount Perpetual Position Initialization

Carl_Num1_1BTC_Short continues the pattern of using big.NewInt for precise handling of short positions in BTC, ensuring accurate financial reporting and risk calculations.


180-187: Review of Subaccount Perpetual Position Initialization

Carl_Num0_1BTC_Short_49999USD uses a specific monetary value and a short BTC position, both of which are handled using big.NewInt to maintain precision. This is crucial for testing and simulations that require exact values.


191-198: Review of Subaccount Perpetual Position Initialization

Carl_Num0_1BTC_Short_50000USD is another example where both the monetary and BTC values are managed with high precision using big.NewInt, ensuring accuracy in financial transactions.


210-214: Review of Subaccount Perpetual Position Initialization

Carl_Num0_1BTC_Short_50499USD uses a very specific monetary value along with a short BTC position. The use of big.NewInt for both ensures that the values are handled precisely, which is essential for accurate financial reporting and compliance.

@jayy04 jayy04 merged commit 50ff9df into main Jun 26, 2024
18 checks passed
@jayy04 jayy04 deleted the jy/test-cleanup-2 branch June 26, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants