[CORE-829] add volatility_bounds_period to LiquidityTier proto and add VolatilityBounds proto

[tqin7]

Changelist

two commits

1. Add protos for variable margin fraction according to tech spec here
2. set VolatilityBoundsPeriod as part of LiquidityTier and validating that it's positive

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.

[linear]

CORE-829 add protos

[coderabbitai]

Walkthrough

The changes across various files in the codebase introduce a new concept of VolatilityBoundsPeriod to the perpetuals system. This involves adding a new field to the LiquidityTier structure, updating function signatures to accommodate this new field, and ensuring that it is properly validated, serialized, and deserialized. The updates also include modifications to tests and genesis configurations to reflect this new feature, which seems to be related to the recovery time for volatility bounds in the perpetuals market.

Changes

File Path	Change Summary
indexer/.../perpetual.ts proto/.../perpetual.proto	Introduced volatilityBoundsPeriod field and related functions for LiquidityTier and VolatilityBounds.
protocol/mocks/PerpetualsKeeper.go protocol/x/perpetuals/keeper/perpetual.go protocol/x/perpetuals/types/types.go	Modified function signatures to include volatilityBoundsPeriod parameter.
protocol/testing/... protocol/testutil/... protocol/x/perpetuals/... (various test files)	Added VolatilityBoundsPeriod field to test structures and updated test cases accordingly.
protocol/x/perpetuals/genesis.go protocol/x/perpetuals/module_test.go	Updated genesis-related functions and tests to include volatilityBoundsPeriod.
protocol/x/perpetuals/simulation/genesis.go	Added function to generate random VolatilityBoundsPeriod and updated state generation.
protocol/x/perpetuals/types/errors.go protocol/x/perpetuals/types/liquidity_tier.go	Added new error for non-positive VolatilityBoundsPeriod and validation check.

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 X ?

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• You can reply to a review comment from CodeRabbit.
• You can also chat with CodeRabbit bot directly around the specific lines of code or files in the PR by tagging @coderabbitai in a new comment.
• You can tag @coderabbitai in a PR comment and ask questions about the PR and the codebase.

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 a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @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.
• The JSON schema for the configuration file is available here.
• 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/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between dd168a0 and bffb295.

Files ignored due to filter (2)

• protocol/client/docs/swagger-ui/swagger.yaml
• protocol/x/perpetuals/types/perpetual.pb.go

Files selected for processing (2)

• indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (7 hunks)
• proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)

Additional comments: 14

indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (10)

• 1-1: The import of Duration and DurationSDKType is correctly added to support the new volatilityBoundsPeriod field in LiquidityTier.

• 213-222: The addition of the volatilityBoundsPeriod field to the LiquidityTier interface is consistent with the PR objectives and is correctly marked as optional.

• 269-288: The VolatilityBounds and VolatilityBoundsSDKType interfaces are correctly defined with min and max properties of type Long, aligning with the new proto message structure.

• 567-573: The createBaseLiquidityTier function is correctly updated to initialize the new volatilityBoundsPeriod field as undefined.

• 598-603: The encode function for LiquidityTier is correctly updated to conditionally encode the new volatilityBoundsPeriod field.

• 641-643: The decode function for LiquidityTier is correctly updated to handle the new volatilityBoundsPeriod field.

• 676-685: The encode function for VolatilityBounds is correctly implemented to handle the min and max fields.

• 688-711: The decode function for VolatilityBounds is correctly implemented to handle the min and max fields.

• 659-664: The fromPartial function for LiquidityTier is correctly updated to handle the new volatilityBoundsPeriod field.

• 714-719: The fromPartial function for VolatilityBounds is correctly implemented, allowing for partial object construction and ensuring TypeScript type safety.

proto/dydxprotocol/perpetuals/perpetual.proto (4)

• 5-5: The addition of the import statement for google/protobuf/duration.proto is necessary for the new Duration field type in LiquidityTier. Ensure that the corresponding library is present in the project dependencies.

• 107-114: The new field volatility_bounds_period in the LiquidityTier message is correctly defined with the google.protobuf.Duration type. Verify that the field number 7 is not already used in the LiquidityTier message to maintain backward compatibility.

• 117-123: The VolatilityBounds message is well-defined with min and max fields. Ensure that the new message type and its fields are integrated correctly in the codebase where the VolatilityBounds message is utilized.

• 117-124: The comments for the VolatilityBounds message provide a clear explanation of its purpose and how it should be used. This is important for maintainability and for future developers who will work with this code.

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 95c3aeb and c9424e9.

Files ignored due to filter (3)

• protocol/client/docs/swagger-ui/swagger.yaml
• protocol/scripts/genesis/sample_pregenesis.json
• protocol/x/perpetuals/types/perpetual.pb.go

Files selected for processing (18)

• indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (7 hunks)
• proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)
• protocol/mocks/PerpetualsKeeper.go (2 hunks)
• protocol/testing/genesis.sh (4 hunks)
• protocol/testutil/constants/genesis.go (1 hunks)
• protocol/testutil/constants/perpetuals.go (2 hunks)
• protocol/testutil/keeper/perpetuals.go (1 hunks)
• protocol/testutil/liquidity_tier/liquidity_tier.go (3 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 (10 hunks)
• protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
• protocol/x/perpetuals/keeper/perpetual_test.go (17 hunks)
• protocol/x/perpetuals/module_test.go (2 hunks)
• protocol/x/perpetuals/types/errors.go (1 hunks)
• protocol/x/perpetuals/types/liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/types/liquidity_tier_test.go (3 hunks)
• protocol/x/perpetuals/types/types.go (2 hunks)

Files skipped from review due to trivial changes (1)

• protocol/testing/genesis.sh

Additional comments: 56

indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (9)

• 1-1: The import of Duration and DurationSDKType is correctly added to support the new volatilityBoundsPeriod field in the LiquidityTier interface.

• 217-225: The volatilityBoundsPeriod field has been correctly added as an optional field to the LiquidityTier interface.

• 266-275: The volatilityBoundsPeriod field has been correctly added as an optional field to the LiquidityTierSDKType interface.

• 277-298: The new VolatilityBounds and VolatilityBoundsSDKType interfaces are correctly defined with min and max fields of type Long.

• 577-581: The createBaseLiquidityTier function is correctly updated to include the initialization of the volatilityBoundsPeriod field.

• 608-613: The encoding function for LiquidityTier is correctly updated to handle the volatilityBoundsPeriod field when it is not undefined.

• 648-653: The decoding function for LiquidityTier is correctly updated to decode the volatilityBoundsPeriod field.

• 669-673: The fromPartial function for LiquidityTier is correctly updated to include the volatilityBoundsPeriod field.

• 678-730: The encoding, decoding, and fromPartial functions for VolatilityBounds are correctly implemented, ensuring proper handling of the new interface.

proto/dydxprotocol/perpetuals/perpetual.proto (4)

• 5-5: The import of google/protobuf/duration.proto is correctly added to support the new volatility_bounds_period field of type google.protobuf.Duration.

• 113-116: The addition of the volatility_bounds_period field to the LiquidityTier message is consistent with the PR objectives, and the use of google.protobuf.Duration with gogoproto options nullable = false and stdduration = true is appropriate for representing a duration.

• 119-126: The new VolatilityBounds message with min and max fields is added as per the PR objectives. The use of uint64 for these fields is suitable for representing volatility bounds.

• 109-111: The base_position_notional field in the LiquidityTier message is marked as deprecated. This change is not mentioned in the PR objectives or the AI-generated summary, but it does not conflict with the new additions. Ensure that this deprecation is intentional and documented elsewhere if it is not part of the current PR's scope.

protocol/mocks/PerpetualsKeeper.go (1)

• 232-248: The addition of the volatilityBoundsPeriod parameter to the SetLiquidityTier function signature is consistent with the PR objectives to add a duration for volatility bounds recovery. Ensure that all calls to this function are updated to include the new parameter.

protocol/testutil/constants/genesis.go (1)

• 874-878: The addition of the "volatility_bounds_period" field with the value "1h" to the "Large-Cap" object in the GenesisState constant is consistent with the PR's objective. Ensure that the duration format "1h" is compatible with how durations are parsed and used throughout the application.

protocol/testutil/constants/perpetuals.go (3)

• 4-5: The import of the time package is correctly added to support the new VolatilityBoundsPeriod field of type time.Duration.

• 29-101: The VolatilityBoundsPeriod field has been consistently added to all LiquidityTier objects with a value of time.Hour.

• 26-104: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-103]

No further issues or changes are detected in the rest of the file. The addition of the VolatilityBoundsPeriod field is the only change and it aligns with the PR's objectives.

protocol/testutil/keeper/perpetuals.go (1)

• 176-182: The addition of l.VolatilityBoundsPeriod as a parameter to the SetLiquidityTier function call within the CreateTestLiquidityTiers function aligns with the PR's objective to add a volatility_bounds_period to the LiquidityTier proto. Ensure that the SetLiquidityTier function definition is updated to accept this new parameter and that all existing calls to this function are updated accordingly to prevent any potential state-breaking changes.

protocol/testutil/liquidity_tier/liquidity_tier.go (3)

• 4-5: The addition of the "time" package is necessary for the new time.Duration type used in the WithVolatilityBoundsPeriod function.

• 41-45: The WithVolatilityBoundsPeriod function is correctly implemented to set the VolatilityBoundsPeriod field of the LiquidityTier object.

• 63-63: The default value for VolatilityBoundsPeriod is sensibly set to time.Hour in the GenerateLiquidityTier function.

protocol/x/perpetuals/genesis.go (1)

• 26-31: The addition of elem.VolatilityBoundsPeriod to the SetLiquidityTier function call in the InitGenesis function is consistent with the PR's objective to add the volatility_bounds_period to the LiquidityTier proto. Ensure that the SetLiquidityTier function has been updated accordingly to accept this new parameter and that the data type of elem.VolatilityBoundsPeriod matches the expected type in the function signature.

protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (3)

• 30-36: The addition of msg.LiquidityTier.VolatilityBoundsPeriod to the SetLiquidityTier function call is consistent with the PR's objective to introduce a duration for volatility bounds recovery. Ensure that the SetLiquidityTier method in the Keeper interface and its implementation are updated to accept this new parameter and that it is handled appropriately within the method's logic.

• 33-33: Verify that the VolatilityBoundsPeriod field in msg.LiquidityTier is of type time.Duration as expected, and ensure that it is handled correctly in the SetLiquidityTier function. This may involve checking the types package where the LiquidityTier struct is defined.

• 30-36: Given that a new parameter has been added to the SetLiquidityTier function, consider whether this change is state-breaking, especially if the function is part of a public API or affects the application's state. If so, ensure that the PR is labeled correctly to reflect this.

protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (12)

• 5-5: The addition of the time package is appropriate for handling the VolatilityBoundsPeriod field which is of type time.Duration.

• 8-8: The addition of lib from github.com/dydxprotocol/v4-chain/protocol/lib is appropriate as it is used to reference lib.GovModuleAddress in the test cases.

• 24-24: The addition of the VolatilityBoundsPeriod field to the LiquidityTier struct in the test setup is consistent with the PR's objective to add this field.

• 40-40: The VolatilityBoundsPeriod field is correctly set in the test case for updating a liquidity tier.

• 53-53: The VolatilityBoundsPeriod field is correctly set in the test case for updating all parameters of a liquidity tier.

• 66-66: The VolatilityBoundsPeriod field is correctly set in the test case for creating a new liquidity tier.

• 79-79: The VolatilityBoundsPeriod field is correctly set in the test case for a failure scenario where the initial margin ppm exceeds the maximum value.

• 93-93: The VolatilityBoundsPeriod field is correctly set in the test case for a failure scenario where the maintenance fraction ppm exceeds the maximum value.

• 98-112: The addition of a new test case to validate that the VolatilityBoundsPeriod cannot be non-positive is a good practice to ensure the correctness of the input data.

• 122-122: The VolatilityBoundsPeriod field is correctly set in the test case for a failure scenario with an invalid authority.

• 136-136: The VolatilityBoundsPeriod field is correctly set in the test case for a failure scenario with an empty authority.

• 153-153: The VolatilityBoundsPeriod field is correctly passed to the SetLiquidityTier function in the test setup, aligning with the changes made to the function's signature.

protocol/x/perpetuals/keeper/perpetual.go (1)

• 1296-1302: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1296-1312]

The SetLiquidityTier function has been updated to include a new parameter volatilityBoundsPeriod. Ensure that all calls to this function throughout the codebase have been updated to include this new parameter. Additionally, verify that the VolatilityBoundsPeriod is being validated correctly within the LiquidityTier struct's validation method.

protocol/x/perpetuals/keeper/perpetual_test.go (7)

• 2794-2800: The addition of volatilityBoundsPeriod to the SetLiquidityTier function call aligns with the PR objective to introduce a duration for volatility bounds recovery in the LiquidityTier proto. Ensure that all other parts of the codebase that interact with LiquidityTier are updated to handle this new field appropriately.

• 2833-2839: The addition of volatilityBoundsPeriod to the SetLiquidityTier function call is consistently applied across test cases, which is good for maintaining the integrity of the test suite.

• 2883-2889: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [2883-2897]

The addition of volatilityBoundsPeriod to the SetLiquidityTier function call is consistently applied across test cases, which is good for maintaining the integrity of the test suite.

• 2910-2915: The addition of volatilityBoundsPeriod to the SetLiquidityTier function call is consistently applied across test cases, which is good for maintaining the integrity of the test suite.

• 2965-2971: The addition of volatilityBoundsPeriod to the SetLiquidityTier function call in the TestModifyLiquidityTier_Success test case is consistent with the PR objective to introduce a duration for volatility bounds recovery in the LiquidityTier proto.

• 3027-3032: The addition of volatilityBoundsPeriod to the SetLiquidityTier function call in the TestSetLiquidityTier_New_Failure test case is consistent with the PR objective to introduce a duration for volatility bounds recovery in the LiquidityTier proto.

• 3091-3095: The addition of volatilityBoundsPeriod to the SetLiquidityTier function call in the TestSetLiquidityTier_Existing_Failure test case is consistent with the PR objective to introduce a duration for volatility bounds recovery in the LiquidityTier proto.

protocol/x/perpetuals/module_test.go (3)

• 320-324: The addition of the volatility_bounds_period field in the JSON object within the TestAppModule_InitExportGenesis function aligns with the PR's objective to add this new field to the LiquidityTier proto. The value "100s" suggests that it is a duration string, which is consistent with the expected type for a period/duration field. Ensure that the rest of the codebase, especially the parsing logic for this field, correctly interprets this duration string format.

• 363-367: The volatility_bounds_period field is also correctly added to the expected JSON output in the TestAppModule_InitExportGenesis function. This change is consistent with the new field addition and should be reflected in the actual genesis export logic to ensure that the tests are valid.

• 320-327: While the test setup includes the new volatility_bounds_period field, it is crucial to verify that the actual genesis logic in the InitGenesis function and related areas of the codebase correctly handle this field. This includes parsing the duration string, validating the value, and correctly initializing the state with it. If the genesis logic is not part of this PR, ensure that it is implemented and tested elsewhere.

protocol/x/perpetuals/types/errors.go (1)

• 110-114: The addition of ErrVolatilityBoundsPeriodIsNonPositive with the error code 23 is consistent with the PR's objective to handle validation for the new VolatilityBoundsPeriod field. Ensure that the error code 23 is unique within the errors.go file and does not conflict with existing error codes.

protocol/x/perpetuals/types/liquidity_tier.go (1)

• 27-31: The addition of the validation check for VolatilityBoundsPeriod in the Validate method is correct and aligns with the PR's objective to ensure that the volatility_bounds_period is a positive value. This change is necessary for the new field to maintain data integrity and prevent invalid configurations.

protocol/x/perpetuals/types/liquidity_tier_test.go (3)

• 4-7: The addition of the time package import is necessary for the new VolatilityBoundsPeriod field of type time.Duration. This change is consistent with the PR's objectives.

• 49-62: The new test cases for the VolatilityBoundsPeriod field correctly validate against non-positive values, expecting the types.ErrVolatilityBoundsPeriodIsNonPositive error. This ensures that the VolatilityBoundsPeriod field is properly checked during validation.

• 69-72: The addition of the VolatilityBoundsPeriod field to the LiquidityTier struct in the test setup is correct and aligns with the updated LiquidityTier struct definition.

protocol/x/perpetuals/types/types.go (2)

• 5-5: The import of the time package is correctly added to support the new time.Duration type parameter in the SetLiquidityTier method.

• 93-99: The addition of the volatilityBoundsPeriod parameter to the SetLiquidityTier method is consistent with the PR objectives. Ensure that all implementations of the PerpetualsKeeper interface and calls to SetLiquidityTier are updated to handle this new parameter.

[coderabbitai]

Review Status

Actionable comments generated: 3

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 95c3aeb and fa8a390.

Files ignored due to filter (3)

• protocol/client/docs/swagger-ui/swagger.yaml
• protocol/scripts/genesis/sample_pregenesis.json
• protocol/x/perpetuals/types/perpetual.pb.go

Files selected for processing (25)

• indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (7 hunks)
• proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)
• protocol/mocks/PerpetualsKeeper.go (2 hunks)
• protocol/testing/e2e/gov/perpetuals_test.go (8 hunks)
• protocol/testing/genesis.sh (4 hunks)
• protocol/testutil/constants/genesis.go (1 hunks)
• protocol/testutil/constants/perpetuals.go (3 hunks)
• protocol/testutil/keeper/perpetuals.go (1 hunks)
• protocol/testutil/liquidity_tier/liquidity_tier.go (3 hunks)
• protocol/x/perpetuals/client/cli/query_perpetual_test.go (2 hunks)
• protocol/x/perpetuals/genesis.go (1 hunks)
• protocol/x/perpetuals/genesis_test.go (10 hunks)
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (10 hunks)
• protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
• protocol/x/perpetuals/keeper/perpetual_test.go (19 hunks)
• protocol/x/perpetuals/module_test.go (2 hunks)
• protocol/x/perpetuals/simulation/genesis.go (3 hunks)
• protocol/x/perpetuals/simulation/genesis_test.go (1 hunks)
• protocol/x/perpetuals/types/errors.go (1 hunks)
• protocol/x/perpetuals/types/genesis_test.go (12 hunks)
• protocol/x/perpetuals/types/liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/types/liquidity_tier_test.go (3 hunks)
• protocol/x/perpetuals/types/message_set_liquidity_tier_test.go (5 hunks)
• protocol/x/perpetuals/types/types.go (2 hunks)

Files skipped from review due to trivial changes (3)

• protocol/mocks/PerpetualsKeeper.go
• protocol/testing/genesis.sh
• protocol/testutil/keeper/perpetuals.go

Files skipped from review as they are similar to previous changes (14)

• indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts
• proto/dydxprotocol/perpetuals/perpetual.proto
• protocol/testutil/constants/genesis.go
• protocol/testutil/constants/perpetuals.go
• protocol/testutil/liquidity_tier/liquidity_tier.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.go
• protocol/x/perpetuals/module_test.go
• protocol/x/perpetuals/types/errors.go
• protocol/x/perpetuals/types/liquidity_tier.go
• protocol/x/perpetuals/types/liquidity_tier_test.go
• protocol/x/perpetuals/types/types.go

Additional comments: 26

protocol/testing/e2e/gov/perpetuals_test.go (5)

• 5-5: The addition of the time package import is appropriate for the new VolatilityBoundsPeriod field usage.

• 36-36: The addition of the VolatilityBoundsPeriod field to the TEST_LIQUIDITY_TIER struct aligns with the PR objectives.

• 360-360: The usage of the VolatilityBoundsPeriod field in test cases is consistent with the new struct definition and the PR's objectives.

Also applies to: 374-374, 389-389, 403-403, 417-417, 431-431, 445-445, 459-459

• 422-435: The addition of test cases to validate non-positive VolatilityBoundsPeriod values is a good practice to ensure robustness.

Also applies to: 436-448

• 36-36: The assignment of time.Hour to the VolatilityBoundsPeriod field in the TEST_LIQUIDITY_TIER struct and its usage in test cases is appropriate for testing the new functionality.

Also applies to: 360-360, 374-374, 389-389, 403-403, 417-417, 459-459

protocol/x/perpetuals/client/cli/query_perpetual_test.go (4)

• 5-11: The addition of the time package is appropriate for the new functionality related to VolatilityBoundsPeriod.

• 55-61: The addition of the VolatilityBoundsPeriod field to the LiquidityTier struct is consistent with the PR's objective and is correctly implemented with a dynamic value based on the loop index.

• 60-60: The use of nullify.Fill(&liquidityTier) is consistent with the existing codebase pattern and is correctly placed after the initialization of the LiquidityTier struct.

• 5-11: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [5-61]

The rest of the code in the provided snippet remains unchanged and is not directly relevant to the PR's objectives. The changes made are focused on the addition of the VolatilityBoundsPeriod field and are correctly implemented.

protocol/x/perpetuals/keeper/perpetual_test.go (4)

• 6-12: > Note: This review was outside the patches, and no patch overlapping with it was found. Original lines [15-17]

The addition of the volatilityBoundsPeriod field to the LiquidityTier structure is correctly implemented in the TestCreateLiquidityTier_Success function. The field is being set and validated as expected.

• 2978-2989: In the TestModifyLiquidityTier_Success function, the volatilityBoundsPeriod field is correctly added and validated. The test ensures that the modified liquidity tier has the expected values for all fields, including the new one.

• 3102-3103: The TestSetParams function does not include the new volatilityBoundsPeriod parameter. If this parameter is part of the Params structure, it should be included in the test cases to ensure that it is being set and validated correctly.

• 3082-3084: The TestIsPositionUpdatable function does not seem to be affected by the new volatilityBoundsPeriod field, as it is not part of the logic that determines if a position is updatable. Therefore, no changes are required here in relation to the new field.

protocol/x/perpetuals/simulation/genesis.go (3)

• 7-13: The addition of the time package is appropriate for the new functionality involving time durations.

• 107-111: The new function genVolatilityBoundsPeriod correctly implements the generation of a random duration for the volatility bounds period, aligning with the PR's objectives.

• 164-177: The RandomizedGenState function has been correctly updated to include the generation of the VolatilityBoundsPeriod for each LiquidityTier, in line with the PR's objectives.

protocol/x/perpetuals/simulation/genesis_test.go (1)

• 59-59: The addition of the assertion require.True(t, lt.VolatilityBoundsPeriod > 0) is consistent with the PR's objective to ensure that the VolatilityBoundsPeriod is positive. However, it is important to verify that VolatilityBoundsPeriod is initialized to a sensible default value before this assertion is made, and that it is of a numeric type, as the assertion implies. If VolatilityBoundsPeriod is not numeric, the comparison to 0 would not be valid.

protocol/x/perpetuals/types/genesis_test.go (5)

• 3-6: The addition of the time package import is appropriate for the use of time.Hour in the test cases.

• 38-42: The VolatilityBoundsPeriod field is correctly set to time.Hour in the LiquidityTier struct within the test cases, aligning with the PR's objectives.

• 338-397: The addition of test cases to validate the VolatilityBoundsPeriod when set to 0 or a negative value is a good practice to ensure the validation logic is working as expected.

• 366-366: The use of types.ErrVolatilityBoundsPeriodIsNonPositive in the test cases is correct and aligns with the PR's objective to handle cases where the VolatilityBoundsPeriod is non-positive.

• 335-400: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [3-400]

Overall, the changes in genesis_test.go are consistent with the PR's objectives and the AI-generated summary, and they correctly implement the new VolatilityBoundsPeriod field in the test cases.

protocol/x/perpetuals/types/message_set_liquidity_tier_test.go (4)

• 5-5: The addition of the time package import is appropriate for the use of time.Duration in the VolatilityBoundsPeriod field.

• 34-34: The VolatilityBoundsPeriod field is correctly added to the LiquidityTier struct and initialized with time.Hour in test cases to reflect a positive duration.

Also applies to: 53-53, 67-67, 81-81

• 86-113: The addition of test cases for the VolatilityBoundsPeriod field, including scenarios where it is zero and negative, is a good practice to ensure robust validation of the new field.

• 98-98: Verify that the error messages used in the test cases for zero and negative VolatilityBoundsPeriod values match the actual error messages produced by the system, which should correspond to the new error constant ErrVolatilityBoundsPeriodIsNonPositive.

Also applies to: 112-112

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 95c3aeb and c485b1c.

Files ignored due to filter (3)

• protocol/client/docs/swagger-ui/swagger.yaml
• protocol/scripts/genesis/sample_pregenesis.json
• protocol/x/perpetuals/types/perpetual.pb.go

Files selected for processing (25)

• indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (7 hunks)
• proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)
• protocol/mocks/PerpetualsKeeper.go (2 hunks)
• protocol/testing/e2e/gov/perpetuals_test.go (8 hunks)
• protocol/testing/genesis.sh (4 hunks)
• protocol/testutil/constants/genesis.go (1 hunks)
• protocol/testutil/constants/perpetuals.go (3 hunks)
• protocol/testutil/keeper/perpetuals.go (1 hunks)
• protocol/testutil/liquidity_tier/liquidity_tier.go (3 hunks)
• protocol/x/perpetuals/client/cli/query_perpetual_test.go (2 hunks)
• protocol/x/perpetuals/genesis.go (1 hunks)
• protocol/x/perpetuals/genesis_test.go (10 hunks)
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (10 hunks)
• protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
• protocol/x/perpetuals/keeper/perpetual_test.go (19 hunks)
• protocol/x/perpetuals/module_test.go (2 hunks)
• protocol/x/perpetuals/simulation/genesis.go (3 hunks)
• protocol/x/perpetuals/simulation/genesis_test.go (1 hunks)
• protocol/x/perpetuals/types/errors.go (1 hunks)
• protocol/x/perpetuals/types/genesis_test.go (12 hunks)
• protocol/x/perpetuals/types/liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/types/liquidity_tier_test.go (3 hunks)
• protocol/x/perpetuals/types/message_set_liquidity_tier_test.go (5 hunks)
• protocol/x/perpetuals/types/types.go (2 hunks)

Files skipped from review due to trivial changes (1)

• protocol/testutil/constants/genesis.go

Files skipped from review as they are similar to previous changes (21)

• proto/dydxprotocol/perpetuals/perpetual.proto
• protocol/mocks/PerpetualsKeeper.go
• protocol/testing/e2e/gov/perpetuals_test.go
• protocol/testing/genesis.sh
• protocol/testutil/keeper/perpetuals.go
• protocol/testutil/liquidity_tier/liquidity_tier.go
• protocol/x/perpetuals/client/cli/query_perpetual_test.go
• protocol/x/perpetuals/genesis.go
• protocol/x/perpetuals/genesis_test.go
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go
• protocol/x/perpetuals/keeper/perpetual.go
• protocol/x/perpetuals/keeper/perpetual_test.go
• protocol/x/perpetuals/module_test.go
• protocol/x/perpetuals/simulation/genesis.go
• protocol/x/perpetuals/simulation/genesis_test.go
• protocol/x/perpetuals/types/errors.go
• protocol/x/perpetuals/types/genesis_test.go
• protocol/x/perpetuals/types/liquidity_tier.go
• protocol/x/perpetuals/types/liquidity_tier_test.go
• protocol/x/perpetuals/types/message_set_liquidity_tier_test.go
• protocol/x/perpetuals/types/types.go

Additional comments: 17

indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (8)

• 219-225: The addition of the volatilityBoundsPeriod field to the LiquidityTier interface is consistent with the PR objectives. Ensure that the field is optional if it's not required for all liquidity tiers and that the type matches the expected type from the protobuf definition.

• 268-274: The addition of the volatility_bounds_period field to the LiquidityTierSDKType interface is consistent with the PR objectives. Ensure that the field is optional if it's not required for all liquidity tiers and that the type matches the expected type from the protobuf definition.

• 284-287: The VolatilityBounds interface has been added with min and max fields, which is consistent with the PR objectives. Ensure that the fields are correctly typed as Long to match the expected 64-bit integer type from the protobuf definition.

• 296-298: The VolatilityBoundsSDKType interface has been added with min and max fields, which is consistent with the PR objectives. Ensure that the fields are correctly typed as Long to match the expected 64-bit integer type from the protobuf definition.

• 577-581: The createBaseLiquidityTier function has been updated to include the initialization of the volatilityBoundsPeriod field. Ensure that the initialization is correct and if the field is optional, it should be initialized as undefined.

• 608-613: The LiquidityTier object encoding function has been updated to handle the volatilityBoundsPeriod field. Ensure that the encoding is done according to the protobuf specification and that the field is correctly handled when it is undefined.

• 651-653: The LiquidityTier object decoding function has been updated to handle the volatilityBoundsPeriod field. Ensure that the decoding is done according to the protobuf specification and that the field is correctly handled when it is not present in the input.

• 685-730: The VolatilityBounds object encoding, decoding, and partial creation functions have been added. Ensure that these functions are correctly implemented and handle the min and max fields according to the protobuf specification. Also, verify that the fromPartial function correctly handles undefined or null values for the min and max fields.

protocol/testutil/constants/perpetuals.go (4)

• 4-8: The import of the time package is correctly added to support the new VolatilityBoundsPeriod field of type time.Duration.

• 26-102: The VolatilityBoundsPeriod field has been added to the LiquidityTier struct and is consistently set to time.Hour for all tiers in the LiquidityTiers slice. This change aligns with the PR objectives to support variable margin fractions.

• 334-354: The VolatilityBoundsPeriod field is also correctly added to the LiquidityTiers slice within the Perpetuals_DefaultGenesisState. However, it's worth noting that the values for VolatilityBoundsPeriod vary here, with some tiers having different multipliers of time.Hour. This variation should be intentional and align with the business logic or configuration requirements.

• 26-104: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1-103]

Ensure that the addition of the VolatilityBoundsPeriod field to the LiquidityTier struct is handled in all parts of the codebase where LiquidityTier is used. This includes checking for proper initialization, serialization, and deserialization, as well as any logic that depends on the structure of LiquidityTier.

protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (5)

• 5-5: The import of time is correctly added to support the new VolatilityBoundsPeriod field.

• 8-8: The import of lib from github.com/dydxprotocol/v4-chain/protocol/lib is correctly added to support the usage of lib.GovModuleAddress in test cases.

• 24-24: The addition of the VolatilityBoundsPeriod field to the LiquidityTier struct in test cases is consistent with the PR objectives and is correctly implemented.

Also applies to: 40-40, 53-53, 66-66, 79-79, 93-93, 121-121, 135-135, 152-152

• 98-111: The addition of the test case "Failure: volatility bounds period is non-positive" is a good practice to ensure the new validation logic for VolatilityBoundsPeriod is working as expected.

• 90-114: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [21-153]

The test cases are correctly updated to include the new VolatilityBoundsPeriod field, ensuring that the field is being tested in various scenarios.

[coderabbitai]

Review Status

Actionable comments generated: 5

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 95c3aeb and f4a6326.

Files ignored due to filter (3)

• protocol/client/docs/swagger-ui/swagger.yaml
• protocol/scripts/genesis/sample_pregenesis.json
• protocol/x/perpetuals/types/perpetual.pb.go

Files selected for processing (25)

• indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (7 hunks)
• proto/dydxprotocol/perpetuals/perpetual.proto (2 hunks)
• protocol/mocks/PerpetualsKeeper.go (2 hunks)
• protocol/testing/e2e/gov/perpetuals_test.go (8 hunks)
• protocol/testing/genesis.sh (6 hunks)
• protocol/testutil/constants/genesis.go (1 hunks)
• protocol/testutil/constants/perpetuals.go (3 hunks)
• protocol/testutil/keeper/perpetuals.go (1 hunks)
• protocol/testutil/liquidity_tier/liquidity_tier.go (3 hunks)
• protocol/x/perpetuals/client/cli/query_perpetual_test.go (2 hunks)
• protocol/x/perpetuals/genesis.go (1 hunks)
• protocol/x/perpetuals/genesis_test.go (10 hunks)
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (10 hunks)
• protocol/x/perpetuals/keeper/perpetual.go (2 hunks)
• protocol/x/perpetuals/keeper/perpetual_test.go (19 hunks)
• protocol/x/perpetuals/module_test.go (2 hunks)
• protocol/x/perpetuals/simulation/genesis.go (3 hunks)
• protocol/x/perpetuals/simulation/genesis_test.go (1 hunks)
• protocol/x/perpetuals/types/errors.go (1 hunks)
• protocol/x/perpetuals/types/genesis_test.go (12 hunks)
• protocol/x/perpetuals/types/liquidity_tier.go (1 hunks)
• protocol/x/perpetuals/types/liquidity_tier_test.go (3 hunks)
• protocol/x/perpetuals/types/message_set_liquidity_tier_test.go (5 hunks)
• protocol/x/perpetuals/types/types.go (2 hunks)

Additional comments: 84

indexer/packages/v4-protos/src/codegen/dydxprotocol/perpetuals/perpetual.ts (7)

• 1-4: The import of Duration and DurationSDKType is correctly added to support the new volatilityBoundsPeriod field in the LiquidityTier interface.

• 217-226: The addition of the volatilityBoundsPeriod field to the LiquidityTier interface is consistent with the PR's objective and is correctly marked as optional for backward compatibility.

• 277-298: The new VolatilityBounds and VolatilityBoundsSDKType interfaces are correctly defined with min and max fields of type Long, aligning with the PR's objective to introduce a new VolatilityBounds proto.

• 608-613: The encoding function for LiquidityTier has been updated to handle the new volatilityBoundsPeriod field, ensuring the message can be serialized correctly.

• 648-653: The decoding function for LiquidityTier has been updated to handle the new volatilityBoundsPeriod field, ensuring the message can be deserialized correctly.

• 669-673: The fromPartial function for LiquidityTier has been correctly updated to include the new volatilityBoundsPeriod field.

• 678-730: The creation of base instances and encoding/decoding functions for the new VolatilityBounds message is correctly implemented, which is necessary for the new proto's functionality.

proto/dydxprotocol/perpetuals/perpetual.proto (3)

• 5-5: The import of google/protobuf/duration.proto is correctly added to support the new volatility_bounds_period field in the LiquidityTier message.

• 113-116: The new volatility_bounds_period field in the LiquidityTier message is correctly defined with type google.protobuf.Duration and appropriate protobuf options to ensure it is not nullable and uses standard duration type.

• 119-126: The VolatilityBounds message is correctly introduced with min and max fields of type uint64, which is appropriate for representing notional values in the context of volatility bounds.

protocol/mocks/PerpetualsKeeper.go (1)

• 232-248: The changes to the SetLiquidityTier function signature and its mock implementation correctly add the volatilityBoundsPeriod parameter of type time.Duration. This aligns with the PR's objective to support volatility bounds in the perpetuals trading protocol. Ensure that all usages of this mock function in tests are updated to include the new parameter.

protocol/testing/e2e/gov/perpetuals_test.go (9)

• 5-5: The addition of the time package is necessary for the new VolatilityBoundsPeriod field of type time.Duration in the LiquidityTier struct.

• 36-36: The addition of the VolatilityBoundsPeriod field to the LiquidityTier struct aligns with the PR's objective to support variable margin fractions.

• 360-360: The VolatilityBoundsPeriod field is correctly used in test cases to simulate various scenarios, ensuring the new functionality is covered.

• 374-374: The VolatilityBoundsPeriod field is correctly used in test cases to simulate various scenarios, ensuring the new functionality is covered.

• 389-389: The VolatilityBoundsPeriod field is correctly used in test cases to simulate various scenarios, ensuring the new functionality is covered.

• 403-403: The VolatilityBoundsPeriod field is correctly used in test cases to simulate various scenarios, ensuring the new functionality is covered.

• 417-421: The addition of failure cases for VolatilityBoundsPeriod being zero or negative is a good practice to ensure robust error handling and validation in the system.

• 459-459: The VolatilityBoundsPeriod field is correctly used in test cases to simulate various scenarios, ensuring the new functionality is covered.

• 431-445: The addition of failure cases for VolatilityBoundsPeriod being zero or negative is a good practice to ensure robust error handling and validation in the system.

protocol/testing/genesis.sh (2)

• 131-131: The addition of the volatility_bounds_period field to the LiquidityTier configuration is consistent with the PR's objective to support variable margin fractions.

• 131-131: The volatility_bounds_period is set to a placeholder value of '1h' with a comment indicating a future update. Ensure that the finalized value is updated before deploying to production environments.

protocol/testutil/constants/genesis.go (1)

• 874-878: The addition of the volatility_bounds_period field to the liquidity_tiers object in the GenesisState constant is consistent with the PR's objective to support volatility bounds. Ensure that the consuming code that parses this JSON string correctly interprets the "1h" duration format and that any associated documentation or related configuration files are updated accordingly.

protocol/testutil/constants/perpetuals.go (2)

• 4-5: The addition of the time package is appropriate for supporting the new VolatilityBoundsPeriod field.

• 334-354: The VolatilityBoundsPeriod field has been added to the LiquidityTier struct instances within the Perpetuals_DefaultGenesisState.LiquidityTiers slice with varying values. This aligns with the PR's objective to support variable margin fractions. Verify that the genesis state logic correctly handles these new values.

protocol/testutil/keeper/perpetuals.go (1)

• 176-182: The addition of l.VolatilityBoundsPeriod to the SetLiquidityTier function call in CreateTestLiquidityTiers is consistent with the PR's objective to support variable margin fractions. Ensure that the SetLiquidityTier function signature has been updated to accept this new parameter and that all calls to this function are updated accordingly. Additionally, verify that the tests cover scenarios where VolatilityBoundsPeriod is set with various values, including edge cases.

protocol/testutil/liquidity_tier/liquidity_tier.go (3)

• 4-5: The addition of the time package import is appropriate for the new time-related functionality.

• 41-45: The implementation of WithVolatilityBoundsPeriod is consistent with the existing pattern of LtModifierOption functions and correctly sets the VolatilityBoundsPeriod field.

• 63-63: Setting the default value of VolatilityBoundsPeriod to time.Hour in the GenerateLiquidityTier function aligns with the PR's objective to provide a default period for volatility bounds.

protocol/x/perpetuals/client/cli/query_perpetual_test.go (2)

• 5-11: The addition of the time package import is appropriate for the new functionality introduced in the VolatilityBoundsPeriod field.

• 55-61: The VolatilityBoundsPeriod field is correctly added to the LiquidityTier struct and assigned a duration value. Ensure that the usage of time.Hour * time.Duration(i+1) aligns with the intended logic for setting the volatility bounds period.

protocol/x/perpetuals/genesis.go (1)

• 26-31: The addition of elem.VolatilityBoundsPeriod to the SetLiquidityTier function call within InitGenesis is consistent with the PR's objective to support volatility bounds. Ensure that the SetLiquidityTier function has been updated accordingly to accept this new parameter and that the LiquidityTier structure is properly extended to include this field. Additionally, verify that the VolatilityBoundsPeriod is being validated within the SetLiquidityTier function to ensure it meets any required constraints (e.g., being positive, within certain bounds, etc.).

protocol/x/perpetuals/genesis_test.go (5)

• 5-5: The addition of the time package is necessary for handling the time.Duration type used in the new volatilityBoundsPeriod field.

• 46-46: The addition of the volatilityBoundsPeriod field to the test structure is consistent with the PR's objective to support variable margin fractions.

• 57-57: The initialization of volatilityBoundsPeriod with time.Hour and other values in test cases is a good practice to test different scenarios and ensure that the new field behaves as expected.

Also applies to: 68-68, 79-79, 90-90, 101-101, 112-112, 123-123, 134-134, 145-145

• 106-123: The addition of test cases to check for zero and negative values of volatilityBoundsPeriod is a good practice to ensure that the validation logic for this new field is robust.

• 168-168: The inclusion of the VolatilityBoundsPeriod field in the LiquidityTier struct within the GenesisState struct aligns with the PR's objective to integrate the new field into the genesis state configuration.

protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1)

• 30-36: The hunk shows the addition of msg.LiquidityTier.VolatilityBoundsPeriod as a parameter to the SetLiquidityTier method call within the function body. However, the function signature in the hunk does not reflect the addition of the volatilityBoundsPeriod parameter as mentioned in the AI-generated summary. This discrepancy suggests that either the function signature change is not shown in the hunk or the change has not been made. Please verify if the function signature has been updated accordingly and ensure that all calls to this function are consistent with the new signature.

protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (7)

• 5-5: The addition of the "time" import is appropriate for handling the new VolatilityBoundsPeriod field.

• 24-24: The VolatilityBoundsPeriod field is correctly added to the LiquidityTier struct and is being utilized in test cases.

• 40-40: The VolatilityBoundsPeriod field is consistently set across various test cases, which is good for thorough testing of the new functionality.

Also applies to: 53-53, 66-66, 79-79, 93-93, 121-121, 135-135

• 98-111: The addition of a test case to validate the behavior when VolatilityBoundsPeriod is non-positive is a good practice for robust error handling.

• 152-152: The SetLiquidityTier function call is correctly updated to include the VolatilityBoundsPeriod parameter in the test setup.

• 90-114: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [27-138]

The test cases cover a range of scenarios, including successful updates and expected failures, which is indicative of comprehensive testing.

• 154-154: The test cases correctly verify the state of the LiquidityTier after the operation, which is crucial for ensuring state consistency.

protocol/x/perpetuals/keeper/perpetual.go (1)

• 1296-1302: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [1296-1312]

The changes from lines 1296 to 1312 introduce a new parameter volatilityBoundsPeriod to the SetLiquidityTier function. This update is consistent with the PR's objective to add support for volatility bounds in the perpetuals trading protocol. Ensure that all calls to this function are updated to include the new parameter where necessary.

protocol/x/perpetuals/keeper/perpetual_test.go (17)

• 6-12: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [6-22]

The addition of volatilityBoundsPeriod to the SetLiquidityTier function calls is noted. Ensure that this new field is validated for non-negative values, as negative durations are not logical for a period. Additionally, verify that this field is properly utilized throughout the codebase where relevant.

• 2794-2800: Ensure that the volatilityBoundsPeriod parameter is validated for non-negative values within the SetLiquidityTier function and that it is correctly used throughout the codebase.

• 2833-2839: As with previous instances, validate the volatilityBoundsPeriod parameter for non-negative values and ensure its proper usage within the codebase.

• 2858-2861: Continue to validate the volatilityBoundsPeriod parameter for non-negative values and confirm its correct implementation within the codebase.

• 2873-2876: Again, ensure the volatilityBoundsPeriod parameter is validated for non-negative values and that it is correctly used throughout the codebase.

• 2884-2890: Validate the volatilityBoundsPeriod parameter for non-negative values and ensure its correct usage within the codebase.

• 2893-2896: Continue to validate the volatilityBoundsPeriod parameter for non-negative values and confirm its correct implementation within the codebase.

• 2902-2905: Ensure the volatilityBoundsPeriod parameter is validated for non-negative values and that it is correctly used throughout the codebase.

• 2911-2914: As with previous instances, validate the volatilityBoundsPeriod parameter for non-negative values and ensure its proper usage within the codebase.

• 2947-2950: Continue to validate the volatilityBoundsPeriod parameter for non-negative values and confirm its correct implementation within the codebase.

• 2966-2969: Ensure the volatilityBoundsPeriod parameter is validated for non-negative values and that it is correctly used throughout the codebase.

• 2978-2989: Validate the volatilityBoundsPeriod parameter for non-negative values and ensure its correct usage within the codebase.

• 3016-3022: As with previous instances, validate the volatilityBoundsPeriod parameter for non-negative values and ensure its proper usage within the codebase.

• 3033-3036: Continue to validate the volatilityBoundsPeriod parameter for non-negative values and confirm its correct implementation within the codebase.

• 3042-3045: Ensure the volatilityBoundsPeriod parameter is validated for non-negative values and that it is correctly used throughout the codebase.

• 3051-3054: Validate the volatilityBoundsPeriod parameter for non-negative values and ensure its correct usage within the codebase.

• 3060-3063: As with previous instances, validate the volatilityBoundsPeriod parameter for non-negative values and ensure its proper usage within the codebase.

protocol/x/perpetuals/module_test.go (2)

• 320-325: The addition of the volatility_bounds_period field to the LiquidityTier in the test JSON object aligns with the PR's objective to support variable margin fractions. This change is consistent with the PR's description and the AI-generated summary.

• 363-367: Similarly, the addition of the volatility_bounds_period field to another LiquidityTier in the expected genesis JSON object is consistent with the PR's objective and the AI-generated summary. This ensures that the tests will check for the presence of the new field in the genesis state.

protocol/x/perpetuals/simulation/genesis.go (3)

• 7-13: The addition of the time package import is appropriate for the new functionality that deals with time durations.

• 107-111: The implementation of genVolatilityBoundsPeriod correctly generates a random duration for the volatility bounds period.

• 164-177: The changes to RandomizedGenState to include the volatilityBoundsPeriod in the LiquidityTier struct are consistent with the PR's objectives to support variable margin fractions.

protocol/x/perpetuals/simulation/genesis_test.go (1)

• 59-59: The addition of the assertion require.True(t, lt.VolatilityBoundsPeriod > 0) is a correct implementation to ensure that the VolatilityBoundsPeriod is positive, aligning with the PR's objective to introduce validation for the new volatility_bounds_period field.

protocol/x/perpetuals/types/errors.go (1)

• 110-114: The addition of the ErrVolatilityBoundsPeriodIsNonPositive error constant is correctly implemented with a unique error code and a descriptive message. This aligns with the PR's objective to handle non-positive values for the volatility_bounds_period.

protocol/x/perpetuals/types/genesis_test.go (3)

• 6-6: The addition of the "time" package is necessary for the new VolatilityBoundsPeriod field of type time.Duration.

• 41-41: The addition of the VolatilityBoundsPeriod field to the LiquidityTier struct in test cases aligns with the PR's objectives to support variable margin fractions.

Also applies to: 79-79, 117-117, 147-147, 177-177, 207-207, 237-237, 267-267, 297-297, 327-327

• 338-397: The addition of test cases to validate the VolatilityBoundsPeriod when set to zero or a negative value is a good practice to ensure the new field is properly validated and error handling is correct.

protocol/x/perpetuals/types/liquidity_tier.go (1)

• 27-29: The addition of the validation check for VolatilityBoundsPeriod ensures that it is positive, which aligns with the PR's objective to handle volatility bounds correctly. This is a critical check to prevent invalid configurations that could lead to undefined behavior in the system.

protocol/x/perpetuals/types/liquidity_tier_test.go (4)

• 4-10: The addition of the time package is necessary for handling the new VolatilityBoundsPeriod field in test cases.

• 15-62: The new test cases for VolatilityBoundsPeriod correctly validate the expected behavior for zero and negative values, ensuring that the VolatilityBoundsPeriod is positive as per the PR objectives.

• 69-75: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [65-75]

The VolatilityBoundsPeriod field has been correctly integrated into the LiquidityTier struct for validation within the test function.

• 69-75: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [65-75]

The test function TestLiquidityTierValidate properly asserts the new error ErrVolatilityBoundsPeriodIsNonPositive when VolatilityBoundsPeriod is zero or negative, aligning with the PR's error handling objectives.

protocol/x/perpetuals/types/message_set_liquidity_tier_test.go (4)

• 5-5: The addition of the time package import is appropriate for the new VolatilityBoundsPeriod field of type time.Duration.

• 34-34: The VolatilityBoundsPeriod field is correctly added to the LiquidityTier struct and is being initialized with time.Hour in the test cases, which is a valid duration for testing purposes.

Also applies to: 53-53, 67-67, 81-81

• 86-113: New test cases have been added to validate the VolatilityBoundsPeriod field, ensuring that it cannot be zero or negative, which aligns with the PR's objective to enforce a positive duration for this field.

• 31-35: The existing test cases have been correctly updated to include the VolatilityBoundsPeriod field, ensuring that the tests remain valid after the structural changes to the LiquidityTier.

Also applies to: 50-54, 64-68, 78-82

protocol/x/perpetuals/types/types.go (2)

• 5-5: The import of the time package is correctly added to support the new time.Duration type in the PerpetualsKeeper interface.

• 93-99: The PerpetualsKeeper interface has been updated with a new parameter volatilityBoundsPeriod. Ensure that all implementations of this interface are updated to handle the new parameter.

[roy-dydx]

What's you have here looks good. Can you add logic to the v3.0.0 upgrade handler changing the volatilityBoundsPeriod of existing perps to some non-zero value? (with a similar TODO as the genesis stuff to change values to something specific in the future)