[OTE-200] OIMF protos and genesis values #1125
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe updates introduce the concept of open interest tracking and management in the perpetuals module of the dYdX protocol. New fields for open interest and its lower and upper caps in liquidity tiers have been added across various components, including data structures, serialization logic, and validation checks. These changes aim to enhance the protocol's functionality by enabling better management and monitoring of open interest in perpetual contracts, thus improving liquidity tier setup and overall market health. Changes
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
protocol/x/perpetuals/types/perpetual.pb.gois excluded by:!**/*.pb.go
Files selected for processing (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (11 hunks)
- proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)
Additional comments: 10
proto/dydxprotocol/perpetuals/perpetual.proto (2)
- 21-26: The addition of the
open_interestfield in thePerpetualmessage is correctly defined with a custom typeSerializableInt. This ensures that the open interest value, which is likely to be a large number, is handled appropriately in the system. The use of(gogoproto.nullable) = falseis also appropriate here, indicating that this field is mandatory and cannot be null.- 131-138: The fields
open_interest_lower_capandopen_interest_upper_capadded to theLiquidityTiermessage are correctly defined and follow the established pattern for similar fields. The comments provide clear explanations of the purpose and behavior of these fields, which is essential for maintainability and understanding of the code. It's good practice to include such detailed comments, especially when introducing new concepts like the Open Interest Margin Fraction (OIMF).indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (8)
- 72-74: The addition of the
openInterestfield to thePerpetualinterface is correctly implemented. The typeUint8Arrayis appropriate for representing serialized data, aligning with the use ofbytestype in the corresponding protobuf definition. This ensures consistency between the TypeScript and protobuf representations.- 87-89: Similarly, the
open_interestfield in thePerpetualSDKTypeinterface is correctly added, maintaining consistency with the protobuf definition and the non-SDK type interface. UsingUint8Arrayfor serialized data types is consistent and appropriate.- 289-301: The addition of
openInterestLowerCapandopenInterestUpperCapfields to theLiquidityTierinterface is correctly implemented. The use of theLongtype from thelonglibrary is appropriate for handling large integers, ensuring that the system can accurately represent and manipulate these values. The comments provide clear explanations, which is good for maintainability.- 345-357: The fields
open_interest_lower_capandopen_interest_upper_capin theLiquidityTierSDKTypeinterface are correctly added and use theLongtype for representing large integers. This consistency in type usage across the SDK and non-SDK type interfaces is crucial for maintaining a coherent type system within the TypeScript codebase.- 378-380: The serialization logic for the
openInterestfield in thePerpetualmessage encoding function is correctly implemented. Checking for non-zero length before encoding the field is a good practice, as it optimizes the serialization process by avoiding unnecessary operations for default or empty values.- 402-404: The decoding logic for the
openInterestfield in thePerpetualmessage decoding function correctly handles the field, ensuring that the serialized data is appropriately converted back into the TypeScript representation. This is crucial for the correct deserialization of messages received from external sources.- 691-698: The serialization logic for the
openInterestLowerCapandopenInterestUpperCapfields in theLiquidityTiermessage encoding function is correctly implemented. The use of!isZero()checks before encoding these fields is a good practice, optimizing the serialization process.- 735-742: The decoding logic for the
openInterestLowerCapandopenInterestUpperCapfields in theLiquidityTiermessage decoding function is correctly implemented, ensuring accurate deserialization of these fields. This is essential for the correct handling of messages that include these new fields.
jayy04
approved these changes
Mar 1, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
protocol/scripts/genesis/sample_pregenesis.jsonis excluded by:!**/*.json
Files selected for processing (15)
- protocol/mocks/PerpetualsKeeper.go (1 hunks)
- protocol/testing/genesis.sh (5 hunks)
- protocol/testutil/constants/perpetuals.go (3 hunks)
- protocol/testutil/keeper/perpetuals.go (1 hunks)
- protocol/testutil/perpetuals/perpetuals.go (1 hunks)
- protocol/x/perpetuals/genesis.go (1 hunks)
- protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1 hunks)
- protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (2 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (3 hunks)
- protocol/x/perpetuals/keeper/perpetual_test.go (16 hunks)
- protocol/x/perpetuals/module_test.go (3 hunks)
- protocol/x/perpetuals/simulation/genesis.go (1 hunks)
- protocol/x/perpetuals/types/errors.go (1 hunks)
- protocol/x/perpetuals/types/liquidity_tier.go (1 hunks)
- protocol/x/perpetuals/types/types.go (1 hunks)
Additional comments: 18
protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1)
- 33-34: The addition of
OpenInterestLowerCapandOpenInterestUpperCapto theLiquidityTierstruct in theSetLiquidityTierfunction is consistent with the PR's objective to manage open interest limits. Ensure that these new fields are validated to prevent invalid configurations, such as the lower cap being greater than the upper cap.protocol/x/perpetuals/genesis.go (1)
- 29-30: The addition of
OpenInterestLowerCapandOpenInterestUpperCapto theelemstruct in theInitGenesisfunction aligns with the PR's goal. It's crucial to ensure that these values are validated during the genesis initialization to prevent invalid configurations from being set at the network's start.protocol/x/perpetuals/types/types.go (1)
- 96-97: The addition of
openInterestLowerCapandopenInterestUpperCapfields to thePerpetualsKeeperinterface is a necessary update to support the new functionality. Ensure that all implementations of this interface are updated accordingly to handle these new fields.protocol/x/perpetuals/types/errors.go (1)
- 115-119: The introduction of
ErrOpenInterestLowerCapLargerThanUpperCapis a good practice to handle specific validation errors related to the new open interest cap fields. This will improve error handling and make debugging easier for developers.protocol/testutil/perpetuals/perpetuals.go (1)
- 70-70: The addition of the
OpenInterestfield with a default value ofdtypes.ZeroInt()in theGeneratePerpetualfunction is consistent with the PR's objectives. This ensures that the open interest is initialized properly in test scenarios. It's important to validate this field in real scenarios to prevent negative or unintended values.protocol/x/perpetuals/types/liquidity_tier.go (1)
- 27-34: The addition of a validation check in the
Validatemethod to ensure thatOpenInterestLowerCapis not greater thanOpenInterestUpperCapis crucial for maintaining data integrity. This validation prevents the creation of liquidity tiers with invalid open interest configurations, enhancing the robustness of the system.protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (1)
- 117-131: The addition of a new test case to verify the failure when
OpenInterestLowerCapis larger thanOpenInterestUpperCapis an excellent practice. This test ensures that the validation logic is correctly implemented and that invalid configurations are rejected as expected. It's important to cover all edge cases in testing to ensure the robustness of the system.protocol/x/perpetuals/simulation/genesis.go (1)
- 208-208: The addition of the
OpenInterestfield to thePerpetualstruct initialization withdtypes.ZeroInt()is consistent with the PR's objective to introduce new fields related to open interest in perpetual contracts. This change aligns with the described enhancements for tracking and managing open interest.However, it's important to ensure that the
OpenInterestfield is properly utilized in subsequent logic and that any necessary validations or operations related to open interest are implemented elsewhere in the codebase.protocol/testutil/keeper/perpetuals.go (1)
- 179-180: The addition of
OpenInterestLowerCapandOpenInterestUpperCapfields to theSetLiquidityTierfunction call in theCreateTestLiquidityTiersfunction is consistent with the PR's objective to introduce new fields related to open interest limits in liquidity tiers. This change aligns with the described enhancements for managing open interest limits through the Open Interest Margin Fraction.It's crucial to ensure that these new fields are properly validated and utilized in the logic that governs liquidity tiers and open interest management. Additionally, consider adding tests that specifically cover scenarios involving these new fields to ensure their correct behavior under various conditions.
protocol/mocks/PerpetualsKeeper.go (1)
- 248-264: The updated
SetLiquidityTierfunction now includesopenInterestLowerCapandopenInterestUpperCapparameters, which aligns with the PR's objective to introduce new fields related to open interest in perpetual contracts. This change enhances the mock's functionality by allowing it to simulate the setting of liquidity tiers with open interest limits. However, there are a few considerations:
- Correctness & Logic: The function correctly updates its signature to include the new parameters and returns the expected types (
perpetualstypes.LiquidityTieranderror). The mock's behavior is determined by theCalledmethod, which is standard for mocks generated bytestify/mock.- Maintainability: Including the new parameters directly in the function signature makes the function more transparent about its requirements. However, if the number of parameters grows significantly in the future, consider using a struct to encapsulate them, which can improve readability and maintainability.
- Error Handling: The mock function's error handling is consistent with the mocking framework's capabilities, allowing for flexible testing scenarios. Ensure that when using this mock in tests, both success and error cases are adequately covered to ensure robustness.
Overall, the changes are consistent with the PR's objectives and follow good practices for mock implementations. Ensure comprehensive test coverage to validate the behavior with the new parameters.
protocol/x/perpetuals/module_test.go (2)
- 319-320: The addition of the
open_interestfield in thePerpetualstructure during theExportGenesisfunction is correctly implemented. This field is crucial for tracking the total size of open long contracts. It's initialized to "0", which is appropriate for a genesis state. However, it's important to ensure that any updates to this field throughout the lifecycle of a perpetual contract are handled accurately and securely to maintain the integrity of the open interest tracking.- 331-332: The implementation of
open_interest_lower_capandopen_interest_upper_capfields in theLiquidityTierstructure within theExportGenesisfunction mirrors the initialization logic and is consistent with the PR's objectives. As with the initialization, ensure that the logic handling these fields throughout the system validates their values to prevent any inconsistencies or errors in the open interest limits.protocol/testutil/constants/perpetuals.go (4)
- 342-343: The initialization of
OpenInterestLowerCapandOpenInterestUpperCapfields with zero values for theLarge-Capliquidity tier in thePerpetuals_DefaultGenesisStateis correctly implemented. However, it's important to ensure that these values are appropriate for the intended use case and that any logic consuming these fields properly handles the case where both caps are set to zero, as this could imply no limits on open interest.- 351-352: The initialization of
OpenInterestLowerCapandOpenInterestUpperCapfields with specific values for theMid-Capliquidity tier in thePerpetuals_DefaultGenesisStateis correctly implemented. This demonstrates the flexibility in configuring different tiers with distinct open interest limits. Ensure that these values are validated elsewhere in the system to maintain the integrity of the open interest limits.- 360-361: The initialization of
OpenInterestLowerCapandOpenInterestUpperCapfields with specific values for theSmall-Capliquidity tier in thePerpetuals_DefaultGenesisStateis correctly implemented. As with theMid-Captier, ensure that these values are validated and that the system logic properly enforces these limits to prevent any potential issues related to open interest management.- 374-374: The initialization of the
OpenInterestfield with a zero value for perpetual objects in thePerpetuals_DefaultGenesisStateis correctly implemented. This is appropriate for a genesis state, indicating no open interest at the start. Ensure that updates to this field are handled accurately throughout the system to reflect changes in open interest correctly.protocol/x/perpetuals/keeper/perpetual.go (2)
- 74-74: The addition of the
OpenInterestfield with initialization usingdtypes.ZeroInt()is appropriate for representing the starting state of a new perpetual. Ensure that all functionalities and calculations that depend on or affect the open interest are updated to incorporate this new field.- 1378-1385: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1381-1395]
The addition of
openInterestLowerCapandopenInterestUpperCapparameters to theSetLiquidityTierfunction and their inclusion in theLiquidityTierstruct are correctly implemented. Ensure that these open interest limits are enforced in all relevant calculations and functionalities within the system to maintain the integrity of the Open Interest Margin Fraction limits.
Comment on lines
135
to
142
| dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[0].maintenance_fraction_ppm' -v '600000' # 60% of IM | ||
| dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[0].base_position_notional' -v '1000000000000' # 1_000_000 USDC | ||
| dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[0].impact_notional' -v '10000000000' # 10_000 USDC (500 USDC / 5%) | ||
| dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[0].open_interest_lower_cap' -v '0' # OIMF doesn't apply to Large-Cap | ||
| dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[0].open_interest_upper_cap' -v '0' # OIMF doesn't apply to Large-Cap | ||
|
|
||
| # Liquidity Tier: Mid-Cap | ||
| dasel put -t json -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[]' -v "{}" |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [128-173]
The changes from lines 128 to 173 introduce default values for open interest caps across different liquidity tiers (Large-Cap, Mid-Cap, Long-Tail, and Safety). It's crucial to ensure these values align with the project's financial models and risk assessments. Additionally, there's a repeated line (line 161) setting the open_interest_lower_cap for the Long-Tail tier, which seems like it should be setting the open_interest_upper_cap instead. This could be a typo that needs correction to avoid potential configuration errors in the genesis state.
- dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[2].open_interest_lower_cap' -v '20000000000000' # 20 million USDC
+ dasel put -t int -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[2].open_interest_upper_cap' -v '20000000000000' # 20 million USDC
Comment on lines
125
to
131
|
|
||
| # Update perpetuals module. | ||
| # Liquidity Tiers. | ||
| # TODO(OTE-208): Finalize default values for open interest caps. | ||
| dasel put -t json -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers' -v "[]" | ||
| # Liquidity Tier: Large-Cap | ||
| dasel put -t json -f "$GENESIS" '.app_state.perpetuals.liquidity_tiers.[]' -v "{}" |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-10]
The script header provides a good overview of its purpose but lacks detailed documentation on how to use it, especially for new contributors or external auditors. It would be beneficial to include a brief usage guide or examples, particularly for the edit_genesis function and other key operations within the script.
Consider adding a commented section at the top of the script with examples of how to run it and descriptions of the key functions and parameters it modifies. This will improve maintainability and ease of use for new developers or contributors.
Comment on lines
+285
to
+287
| "impact_notional":10000000000, | ||
| "open_interest_lower_cap":25000000000000, | ||
| "open_interest_upper_cap":50000000000000 |
There was a problem hiding this comment.
The addition of open_interest_lower_cap and open_interest_upper_cap fields in the LiquidityTier structure during the InitGenesis function is correctly implemented. These fields are essential for setting limits on the Open Interest Margin Fraction, aligning with the PR's objectives. However, ensure that these values are validated to prevent negative or illogical ranges which could lead to unexpected behavior in the system.
// Suggested validation (pseudo-code, implement according to project standards)
if tier.open_interest_lower_cap < 0 || tier.open_interest_upper_cap < 0 {
return errors.New("Open interest caps must be non-negative")
}
if tier.open_interest_lower_cap > tier.open_interest_upper_cap {
return errors.New("Open interest lower cap must not exceed upper cap")
}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
protocol/scripts/genesis/sample_pregenesis.jsonis excluded by:!**/*.json
Files selected for processing (2)
- protocol/testutil/constants/perpetuals.go (19 hunks)
- protocol/x/perpetuals/client/cli/query_perpetual_test.go (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/testutil/constants/perpetuals.go
Additional comments: 2
protocol/x/perpetuals/client/cli/query_perpetual_test.go (2)
- 91-91: Adding the
OpenInterestfield to thePerpetualstruct within the test setup function is consistent with the PR's objective of enhancing the protocol to track and manage open interest in perpetual contracts. This change ensures that the test environment reflects the updated data model and allows for testing related functionalities.
- Confirm that the addition of the
OpenInterestfield is accompanied by appropriate test cases that verify its behavior and integration with existing functionalities.- Consider adding validation checks within the test setup to ensure that
OpenInterestis initialized correctly and behaves as expected under various scenarios.
- 160-160: The use of
serializableIntCmpOptsin thecheckExpectedPerpfunction demonstrates a practical application of the custom comparison logic introduced earlier. This ensures that the comparison ofPerpetualobjects in tests accounts for the nuances ofSerializableIntfields, providing more accurate and meaningful test outcomes.
- Ensure that the custom comparison logic is applied consistently across all relevant test functions to maintain uniformity in how
SerializableIntfields are compared.- Review other test functions that involve
Perpetualobjects to identify any additional opportunities for applying this custom comparison logic.
Comment on lines
+27
to
+38
| var ( | ||
| serializableIntCmpOpts = cmp.Options{ | ||
| cmpopts.IgnoreFields(types.Perpetual{}, "FundingIndex"), // existing ignore option | ||
| cmp.FilterValues( | ||
| func(x, y dtypes.SerializableInt) bool { | ||
| // This will apply the custom comparer only to SerializableInt fields | ||
| return true // Apply this filter to all SerializableInt values | ||
| }, | ||
| cmp.Comparer(func(x, y dtypes.SerializableInt) bool { return x.Cmp(y) == 0 }), | ||
| ), | ||
| } | ||
| ) |
There was a problem hiding this comment.
The introduction of serializableIntCmpOpts is a good practice for customizing comparison logic in tests, especially for complex types like SerializableInt. This ensures that comparisons are meaningful and tailored to the specific needs of the type. However, it's important to ensure that this custom comparison logic is well-documented to aid future maintainers in understanding its purpose and functionality.
- Ensure that the custom comparison logic for
SerializableIntfields is documented, explaining why the default comparison was insufficient and how the custom logic addresses this. - Verify that all instances where
SerializableIntcomparisons are required in tests are using this new comparison option to maintain consistency.
Comment on lines
+233
to
+238
| cmpOptions := append( | ||
| serializableIntCmpOpts, | ||
| cmpopts.SortSlices(func(x, y types.Perpetual) bool { | ||
| return x.Params.Id > y.Params.Id | ||
| }), | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
The addition of cmpopts.SortSlices in the TestListPerpetual function's "Total" test case is a thoughtful approach to handling the comparison of slices that may not be in a consistent order. Sorting the slices by Params.Id before comparison ensures that the test is focused on the content of the slices rather than their order, which is particularly important for tests involving collections of objects.
- Verify that the sorting logic (
x.Params.Id > y.Params.Id) aligns with the expected order ofPerpetualobjects in the test scenarios. If the expected order is ascending, the comparison should be adjusted accordingly. - Consider applying similar sorting logic in other test cases or functions where the order of slices could impact the outcome of comparisons, to ensure consistency and reliability in test results.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
protocol/scripts/genesis/sample_pregenesis.jsonis excluded by:!**/*.json
Files selected for processing (4)
- protocol/testutil/constants/perpetuals.go (19 hunks)
- protocol/testutil/keeper/perpetuals.go (1 hunks)
- protocol/x/perpetuals/client/cli/query_perpetual_test.go (4 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- protocol/testutil/constants/perpetuals.go
- protocol/testutil/keeper/perpetuals.go
- protocol/x/perpetuals/client/cli/query_perpetual_test.go
Additional comments: 3
protocol/x/perpetuals/keeper/perpetual.go (3)
- 103-103: The addition of the
OpenInterestfield in theCreatePerpetualfunction is a logical step for tracking open interest in perpetual contracts. However, ensure that all necessary validations and checks are performed when manipulating this field in other parts of the codebase to maintain data integrity.- 1410-1411: Adding
openInterestLowerCapandopenInterestUpperCapparameters to theSetLiquidityTierfunction introduces a mechanism to set limits for the Open Interest Margin Fraction. This is a crucial feature for managing risk and ensuring the stability of the perpetual market. It's important to validate these parameters to ensure they are within reasonable bounds and thatopenInterestUpperCapis greater thanopenInterestLowerCap.- 100-106: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [6-6]
The usage of
math/randis noted. For operations requiring secure randomness, such as generating cryptographic keys or tokens, it's recommended to usecrypto/randinstead. However, if the usage here is for non-security-critical purposes, such as simulation or generating non-sensitive random values,math/randmay be acceptable. Please assess the context of its usage to determine if a switch tocrypto/randis necessary.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
protocol/scripts/genesis/sample_pregenesis.jsonis excluded by:!**/*.jsonprotocol/x/perpetuals/types/perpetual.pb.gois excluded by:!**/*.pb.go
Files selected for processing (18)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (11 hunks)
- proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)
- protocol/mocks/PerpetualsKeeper.go (1 hunks)
- protocol/testing/genesis.sh (5 hunks)
- protocol/testutil/constants/perpetuals.go (19 hunks)
- protocol/testutil/keeper/perpetuals.go (1 hunks)
- protocol/testutil/perpetuals/perpetuals.go (1 hunks)
- protocol/x/perpetuals/client/cli/query_perpetual_test.go (4 hunks)
- protocol/x/perpetuals/genesis.go (1 hunks)
- protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1 hunks)
- protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (2 hunks)
- protocol/x/perpetuals/keeper/perpetual.go (3 hunks)
- protocol/x/perpetuals/keeper/perpetual_test.go (16 hunks)
- protocol/x/perpetuals/module_test.go (3 hunks)
- protocol/x/perpetuals/simulation/genesis.go (1 hunks)
- protocol/x/perpetuals/types/errors.go (1 hunks)
- protocol/x/perpetuals/types/liquidity_tier.go (1 hunks)
- protocol/x/perpetuals/types/types.go (1 hunks)
Files skipped from review as they are similar to previous changes (16)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts
- proto/dydxprotocol/perpetuals/perpetual.proto
- protocol/mocks/PerpetualsKeeper.go
- protocol/testing/genesis.sh
- protocol/testutil/constants/perpetuals.go
- protocol/testutil/keeper/perpetuals.go
- protocol/testutil/perpetuals/perpetuals.go
- protocol/x/perpetuals/client/cli/query_perpetual_test.go
- protocol/x/perpetuals/genesis.go
- protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go
- protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go
- protocol/x/perpetuals/keeper/perpetual_test.go
- protocol/x/perpetuals/module_test.go
- protocol/x/perpetuals/types/errors.go
- protocol/x/perpetuals/types/liquidity_tier.go
- protocol/x/perpetuals/types/types.go
Additional comments: 4
protocol/x/perpetuals/simulation/genesis.go (2)
- 206-206: The addition of the
OpenInterestfield to thePerpetualstruct withdtypes.ZeroInt()as its value is consistent with the PR's objective to integrate Open Interest Margin Fraction (OIMF) parameters into the perpetuals protocol. This change is crucial for tracking the total size of open long contracts, which is a key metric for understanding market leverage and risk exposure.
- 203-209: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [9-9]
The use of
math/randthroughout this file is appropriate given its purpose in generating simulation data. In simulations, especially within a development or testing environment, the focus is on performance and reproducibility rather than cryptographic security. Therefore, the choice ofmath/randaligns with the requirements of generating randomized data for simulations in a blockchain protocol context.
protocol/x/perpetuals/keeper/perpetual.go (2)
- 103-103: The addition of the
OpenInterestfield in theCreatePerpetualfunction aligns with the PR's objective to track the total size of open long contracts. This change is crucial for understanding market leverage and risk exposure. Ensure that theOpenInterestfield is properly initialized and used throughout the system to maintain consistency and accuracy in tracking open interest levels.- 1407-1414: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1410-1424]
The introduction of
openInterestLowerCapandopenInterestUpperCapfields in theSetLiquidityTierfunction is a significant update that establishes boundaries for Open Interest Margin Fractions. This addition is essential for maintaining market stability and preventing excessive risk-taking. However, it's crucial to ensure that these new fields are validated correctly to prevent configuration errors, such as setting the lower cap higher than the upper cap, which could undermine the protocol's integrity. Additionally, consider adding checks to ensure that these values are used effectively in the system to enforce the intended constraints on open interest levels.
teddyding
changed the title
Eric-Warehime
pushed a commit
to skip-mev/v4-chain
that referenced
this pull request
Mar 12, 2024
* OIMF protos * add default genesis value, modify methods interface * fix genesis file * fix integration test * lint Signed-off-by: Eric <eric.warehime@gmail.com>
Eric-Warehime
added a commit
to skip-mev/v4-chain
that referenced
this pull request
Mar 12, 2024
commit d98f859 Author: Eric <eric.warehime@gmail.com> Date: Mon Mar 11 12:46:53 2024 -0700 Update sample pregenesis Signed-off-by: Eric <eric.warehime@gmail.com> commit 7f178fe Author: Mohammed Affan <affanmd@nyu.edu> Date: Mon Mar 11 13:46:08 2024 -0400 [OTE-209] Emit metrics gated through execution mode (dydxprotocol#1157) Signed-off-by: Eric <eric.warehime@gmail.com> commit 47e365d Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon Mar 11 13:43:16 2024 -0400 add aggregate comlink response code stats (dydxprotocol#1162) Signed-off-by: Eric <eric.warehime@gmail.com> commit 7774ad9 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Fri Mar 8 17:30:49 2024 -0500 [TRA-70] Add state migrations for isolated markets (dydxprotocol#1155) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com> commit 89c7405 Author: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Thu Mar 7 17:28:06 2024 -0500 [CT-517] E2E tests batch cancel (dydxprotocol#1149) * more end to end test * extraprint * more e2e test Signed-off-by: Eric <eric.warehime@gmail.com> commit 41a3a41 Author: Teddy Ding <teddy@dydx.exchange> Date: Thu Mar 7 15:42:30 2024 -0500 [OTE-200] OIMF protos (dydxprotocol#1125) * OIMF protos * add default genesis value, modify methods interface * fix genesis file * fix integration test * lint Signed-off-by: Eric <eric.warehime@gmail.com> commit 2a062b1 Author: Teddy Ding <teddy@dydx.exchange> Date: Thu Mar 7 15:24:15 2024 -0500 Add `v5` upgrade handler and set up container upgrade test (dydxprotocol#1153) * wip * update preupgrade_genesis * fix preupgrade_genesis.json * nit * setupUpgradeStoreLoaders for v5.0.0 Signed-off-by: Eric <eric.warehime@gmail.com> commit b7942b3 Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Thu Mar 7 13:43:48 2024 -0500 [CT-647] construct the initial orderbook snapshot (dydxprotocol#1147) * [CT-647] construct the initial orderbook snapshot * [CT-647] initialize new streams and send orderbook snapshot (dydxprotocol#1152) * [CT-647] initialize new streams and send orderbook snapshot * use sync once * comments Signed-off-by: Eric <eric.warehime@gmail.com> commit c67a3c6 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Thu Mar 7 12:40:37 2024 -0500 [TRA-84] Move SA module address transfers to use perpetual based SA accounts (dydxprotocol#1146) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange Signed-off-by: Eric <eric.warehime@gmail.com> commit dba23e0 Author: Mohammed Affan <affanmd@nyu.edu> Date: Thu Mar 7 10:34:11 2024 -0500 update readme link to point to the right page (dydxprotocol#1151) Signed-off-by: Eric <eric.warehime@gmail.com> commit b5870d5 Author: Tian <tian@dydx.exchange> Date: Wed Mar 6 16:43:04 2024 -0500 [TRA-86] scaffold x/vault (dydxprotocol#1148) * scaffold x/vault Signed-off-by: Eric <eric.warehime@gmail.com> commit 0eca041 Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Wed Mar 6 10:48:42 2024 -0500 [CT-652] add command line flag for full node streaming (dydxprotocol#1145) Signed-off-by: Eric <eric.warehime@gmail.com> commit b319cb8 Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Tue Mar 5 21:58:35 2024 -0500 [CT-646] stream offchain updates through stream manager (dydxprotocol#1138) * [CT-646] stream offchain updates through stream manager * comments * fix lint * get rid of finished * comments * comments Signed-off-by: Eric <eric.warehime@gmail.com> commit 1c54620 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Tue Mar 5 16:34:19 2024 -0500 [TRA-78] Add function to retrieve collateral pool addr for a subaccount (dydxprotocol#1142) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com> commit b8c1d62 Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Tue Mar 5 15:03:28 2024 -0500 [OTE-141] implement post /compliance/geoblock (dydxprotocol#1129) Signed-off-by: Eric <eric.warehime@gmail.com> commit ab8c570 Author: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Tue Mar 5 11:19:53 2024 -0500 Fix mock-gen dydxprotocol#1140 Signed-off-by: Eric <eric.warehime@gmail.com> commit 12506a1 Author: shrenujb <98204323+shrenujb@users.noreply.github.com> Date: Mon Mar 4 21:33:28 2024 -0500 [TRA-64] Use market specific insurance fund for cross or isolated markets (dydxprotocol#1132) Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange> Signed-off-by: Eric <eric.warehime@gmail.com> commit 929f09e Author: Jonathan Fung <121899091+jonfung-dydx@users.noreply.github.com> Date: Mon Mar 4 13:48:04 2024 -0800 [CT-514] Clob `MsgBatchCancel` functionality (dydxprotocol#1110) * wip implementation * use new cometbft * Revert "use new cometbft" This reverts commit e5b8a03. * go mod tidy * basic e2e test * more msgBatchCancels in code * repeated fixed32 -> uint32 * remove debug prints * update cometbft replace go.mod sha * one more debug print * typo * regen indexer protos * update comment on proto * proto comment changes * extract stateful validation into own fn * pr format comments * clean up test file * new return type with success and failure Signed-off-by: Eric <eric.warehime@gmail.com> commit 41de83e Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon Mar 4 12:22:16 2024 -0500 add index to address read replica lag (dydxprotocol#1137) Signed-off-by: Eric <eric.warehime@gmail.com> commit 735d9a8 Author: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon Mar 4 11:56:59 2024 -0500 rename (dydxprotocol#1136) Signed-off-by: Eric <eric.warehime@gmail.com> commit 86617dd Author: jayy04 <103467857+jayy04@users.noreply.github.com> Date: Mon Mar 4 10:43:31 2024 -0500 [CT-644] instantiate grpc stream manager (dydxprotocol#1134) * [CT-644] instantiate grpc stream manager * update type * update channel type Signed-off-by: Eric <eric.warehime@gmail.com> commit 32afd64 Author: Eric <eric.warehime@gmail.com> Date: Mon Mar 11 12:41:06 2024 -0700 Update go version in Dockerfile Signed-off-by: Eric <eric.warehime@gmail.com> commit ba27204 Author: Eric <eric.warehime@gmail.com> Date: Fri Mar 8 09:44:04 2024 -0800 Add slinky utils, use that to convert between market and currency pair commit 667a804 Author: Eric <eric.warehime@gmail.com> Date: Wed Mar 6 20:43:40 2024 -0800 Update error messages commit d53292c Author: Eric <eric.warehime@gmail.com> Date: Wed Mar 6 20:16:01 2024 -0800 Update docstrings, rename OracleClient commit daad125 Author: Eric <eric.warehime@gmail.com> Date: Mon Mar 4 10:51:23 2024 -0800 VoteExtension slinky logic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changelist
Create OIMF protos according to tech spec
Test Plan
N/A
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.