[CORE-841] Add VolatilityBounds store #891
Conversation
…'s a positive duration
…, and initialize volatility bounds when creating a perpetual
WalkthroughThe overall change introduces a new concept of volatility bounds period to the perpetuals module of the dYdX protocol. This includes adding a new field to track the duration for volatility bounds recovery in liquidity tiers, updating validation logic, and modifying test cases to accommodate the new field. The changes span across various files, including protobuf definitions, keeper functions, test utilities, and genesis state configurations, reflecting a comprehensive update to the protocol's handling of volatility bounds. 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 X ? TipsChat with CodeRabbit Bot (
|
![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: 3
Configuration used: CodeRabbit UI
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 (27)
- 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 (5 hunks)
- protocol/x/perpetuals/keeper/perpetual_test.go (23 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/keys.go (1 hunks)
- protocol/x/perpetuals/types/keys_test.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/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
Additional comments: 74
proto/dydxprotocol/perpetuals/perpetual.proto (4)
5-5: The import of
google/protobuf/duration.protois correctly added to support the newDurationfield type.113-116: The addition of the
volatility_bounds_periodfield to theLiquidityTiermessage with the appropriategogoprotooptions is correctly implemented.119-126: The new
VolatilityBoundsmessage withminandmaxfields is added as per the PR objectives and is correctly defined.109-127: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [5-126]
Please ensure that the new
VolatilityBoundsmessage and thevolatility_bounds_periodfield in theLiquidityTiermessage are covered by appropriate tests to validate their behavior and integration with the rest of the system.protocol/mocks/PerpetualsKeeper.go (2)
11-12: The addition of the "time" package import is appropriate for the new
volatilityBoundsPeriodparameter type.232-245: The
SetLiquidityTierfunction signature has been correctly updated to include thevolatilityBoundsPeriodparameter. Ensure that all calls to this function are updated to pass the new parameter where this mock is used.Verification successful
The
SetLiquidityTierfunction signature has been correctly updated to include thevolatilityBoundsPeriodparameter, and the calls to this function in the test files have been updated accordingly to pass the new parameter. No issues were found in the calls that were checked.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to SetLiquidityTier that do not include the new volatilityBoundsPeriod parameter. ast-grep --lang go --pattern $'$_.$_.SetLiquidityTier($_, $_, $_, $_, $_, $_)'Length of output: 79
Script:
#!/bin/bash # Search for calls to SetLiquidityTier with any number of parameters to ensure they have been updated. ast-grep --lang go --pattern $'$_.$_.SetLiquidityTier($$$)'Length of output: 7335
protocol/testing/e2e/gov/perpetuals_test.go (4)
5-5: The addition of the
timepackage import is appropriate for the use oftime.Durationin theLiquidityTierstruct.36-36: The
VolatilityBoundsPeriodfield is correctly added to theLiquidityTierstruct with a default value oftime.Hour.360-360: The
VolatilityBoundsPeriodfield is consistently used across test cases, ensuring that the new functionality is covered by the tests.Also applies to: 374-374, 389-389, 403-403, 417-417, 431-431, 445-445, 459-459
- 422-434: The test cases for zero and negative
VolatilityBoundsPeriodvalues are crucial for ensuring that invalid inputs are handled correctly.Also applies to: 436-448
protocol/testing/genesis.sh (2)
1547-1547: The
volatility_bounds_periodis set to '1h' with a TODO comment to use a research-finalized value. Ensure that this value is updated with the finalized value from research before deploying to production to avoid using placeholder configurations.1547-1547: The reference to CORE-838 in the TODO comment suggests that there is a pending task to update the
volatility_bounds_period. Verify that this task is tracked and addressed in the project management system to ensure that the script reflects the finalized research values.protocol/testutil/constants/perpetuals.go (4)
4-5: The import of the
timepackage is necessary for the newVolatilityBoundsPeriodfield that usestime.Duration. This change is consistent with the PR's objective to handle time-based volatility bounds.29-101: The
VolatilityBoundsPeriodfield has been added to allLiquidityTierobjects with a value oftime.Hour. Confirm that this uniform value is intended for all tiers, as different tiers might require different volatility bounds periods.337-353: The
VolatilityBoundsPeriodfield in thePerpetuals_DefaultGenesisStatehas different values for different tiers, which suggests that the volatility bounds period can vary by tier. Ensure that theLiquidityTiersdefinitions above also reflect this variability if required.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
VolatilityBoundsPeriodfield is accompanied by appropriate tests to verify its behavior within the system. This is crucial even though the test changes are not part of the provided hunks.protocol/testutil/keeper/perpetuals.go (1)
- 176-182: The addition of
l.VolatilityBoundsPeriodto theSetLiquidityTierfunction call is consistent with the PR's objective to manage volatility bounds. Ensure that all other calls toSetLiquidityTieracross the codebase have been updated to include this new parameter where necessary.Verification successful
The verification process confirms that the
SetLiquidityTierfunction signature has been updated to include theVolatilityBoundsPeriodparameter, and the function's usage across the codebase has been adjusted accordingly. No issues were found with the changes related to this function.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to `SetLiquidityTier` that do not include the new `VolatilityBoundsPeriod` parameter. ast-grep --lang go --pattern $'$_.$SetLiquidityTier($_, $_, $_, $_)'Length of output: 69
Script:
#!/bin/bash # Verify calls to `SetLiquidityTier` with the updated parameter list, including `VolatilityBoundsPeriod`. ast-grep --lang go --pattern $'$_.$SetLiquidityTier($_, $_, $_, $_, $_)' # Verify the function signature of `SetLiquidityTier` to ensure it includes the new `VolatilityBoundsPeriod` parameter. ast-grep --lang go --pattern $'func ($_ $_) SetLiquidityTier($_, $_, $_, $_, $_) $_'Length of output: 159
Script:
#!/bin/bash # Search for the `SetLiquidityTier` function definition to capture the current signature. ast-grep --lang go --pattern $'func ($_ $_) SetLiquidityTier($$$) $_' # Search for any file that contains the `SetLiquidityTier` function to examine its usage in context. rg 'SetLiquidityTier' --files-with-matchesLength of output: 9478
protocol/testutil/liquidity_tier/liquidity_tier.go (4)
4-5: The addition of the
timepackage import is appropriate for the new functionality that involvestime.Duration.41-45: The
WithVolatilityBoundsPeriodfunction is correctly implemented to allow setting theVolatilityBoundsPeriodfield in theLiquidityTierobject.63-63: The default value for
VolatilityBoundsPeriodis correctly set totime.Hourin theGenerateLiquidityTierfunction.38-49: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [41-63]
Please ensure that the new functionality introduced by
WithVolatilityBoundsPeriodand the default value inGenerateLiquidityTierare covered by appropriate tests to maintain code quality and prevent regressions.protocol/x/perpetuals/client/cli/query_perpetual_test.go (4)
5-11: The addition of the
timepackage import is appropriate for the new functionality introduced.55-61: The
VolatilityBoundsPeriodfield is correctly added and initialized within theLiquidityTierstruct. The use oftime.Hour * time.Duration(i+1)ensures that eachLiquidityTierhas a distinct and non-zero duration for the volatility bounds period, which aligns with the PR's objective to manage volatility bounds.60-60: Ensure that the
nullify.Fillfunction is updated to handle the newVolatilityBoundsPeriodfield if necessary. This function is used to set zero values for struct fields, and it should be verified that it does not unintentionally reset the newly addedVolatilityBoundsPeriodafter it has been initialized.Verification successful
Let's generate a new shell script to search for the
nullify.Fillfunction definition using ripgrep.
Let's generate a new shell script to view the implementation of the
nullify.Fillfunction.
Based on the implementation of the
nullify.Fillfunction, it does not reset fields to their zero values but ensures that nil pointers and empty slices or structs are initialized. Since theVolatilityBoundsPeriodfield is a non-pointer value type,nullify.Fillwould not affect its value once it's set. Therefore, there should be no issue with theVolatilityBoundsPeriodfield being reset after initialization in the test setup function.* 5-11: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [5-61]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that nullify.Fill function correctly handles the new VolatilityBoundsPeriod field. ast-grep --lang go --pattern $'func ($_ *LiquidityTier) Fill() { $$$ }'Length of output: 74
Script:
#!/bin/bash # Search for the definition of the nullify.Fill function rg --type go 'func Fill'Length of output: 168
Script:
#!/bin/bash # View the implementation of the nullify.Fill function rg --type go --context 10 'func Fill' protocol/testutil/nullify/nullify.goLength of output: 755
Please ensure that the new
VolatilityBoundsPeriodfield is covered by appropriate tests to verify its behavior within the perpetuals module, as per the PR's objective. This includes testing the validation logic, serialization/deserialization, and any business logic that relies on this new field.protocol/x/perpetuals/genesis.go (3)
- 26-31: The addition of
elem.VolatilityBoundsPeriodto theSetLiquidityTierfunction call in theInitGenesisfunction is consistent with the PR's objective to manage volatility bounds. Ensure that theSetLiquidityTierfunction signature has been updated to accept this new parameter and that the parameter is being handled correctly within the function.Verification successful
The updated script successfully located the
SetLiquidityTierfunction within theperpetual.gofile, and the output confirms that the function signature has been updated to include thevolatilityBoundsPeriodparameter of typetime.Duration. This aligns with the PR's objective to manage volatility bounds within the perpetuals module.The function also appears to correctly construct a
LiquidityTierstruct with theVolatilityBoundsPeriodfield and validates it, which suggests that the new parameter is being handled appropriately within the function.Based on the information provided by the script output, the addition of the
VolatilityBoundsPeriodparameter to theSetLiquidityTierfunction is verified to be consistent with the PR's objectives and the function's implementation appears to be correct.* 26-31: Given the PR description's emphasis on testing the changes, ensure that the test suite has been updated to cover the new `VolatilityBoundsPeriod` parameter in the `SetLiquidityTier` function call. This includes unit tests for the `InitGenesis` function and any other tests that involve liquidity tiers.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `SetLiquidityTier` function signature includes the new `VolatilityBoundsPeriod` parameter. ast-grep --lang go --pattern $'func (k Keeper) SetLiquidityTier($_, $_, $_, $_, $_, $_, VolatilityBoundsPeriod $_) $_ { $$$ }'Length of output: 127
Script:
#!/bin/bash # Search for the `SetLiquidityTier` function to verify its signature includes the new `VolatilityBoundsPeriod` parameter. ast-grep --lang go --pattern $'func (k Keeper) SetLiquidityTier($$$) $_ { $$$ }'Length of output: 3492
Verification successful
The output from the executed script indicates that the
VolatilityBoundsPeriodparameter has been included in various test files related to theSetLiquidityTierfunction. The parameter is present in unit tests within files such asperpetuals_test.go,msg_server_set_liquidity_tier_test.go,perpetual_test.go, andmessage_set_liquidity_tier_test.go. These tests include positive, zero, and negative values for theVolatilityBoundsPeriod, which suggests that the new functionality is being tested for different scenarios, including edge cases.Based on this information, it appears that the test suite has been updated to cover the new
VolatilityBoundsPeriodparameter as part of the changes made in the PR.Reply:
The test suite has been updated to include the new
VolatilityBoundsPeriodparameter in various test scenarios across the codebase, ensuring that the new functionality is properly tested.* 26-31: Confirm that the `LiquidityTier` struct has been updated to include the new `volatilityBoundsPeriod` field as mentioned in the AI-generated summary. This field should be properly documented and validated within the struct's methods.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test files related to the `SetLiquidityTier` function to ensure they include the new parameter. rg 'SetLiquidityTier' --type=go --files-with-matches | xargs rg 'VolatilityBoundsPeriod'Length of output: 4267
Verification successful
The output from the shell script confirms that the
LiquidityTierstruct does indeed include a field namedVolatilityBoundsPeriod, as described in the AI-generated summary. This field is of typetime.Durationand is documented with comments explaining its purpose and effect.Based on this information, the initial concern raised in the review comment has been addressed, and the
LiquidityTierstruct is updated to include the newVolatilityBoundsPeriodfield as expected.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the `LiquidityTier` struct includes the new `volatilityBoundsPeriod` field. ast-grep --lang go --pattern $'type LiquidityTier struct { $$$ volatilityBoundsPeriod $_ $$$ }'Length of output: 96
Script:
#!/bin/bash # Broaden the search to find any fields related to volatility bounds in the `LiquidityTier` struct. ast-grep --lang go --pattern $'type LiquidityTier struct { $$$ volatilityBounds $_ $$$ }'Length of output: 90
Script:
#!/bin/bash # Output the entire structure of the `LiquidityTier` struct to verify its fields. ast-grep --lang go --pattern $'type LiquidityTier struct { $$$ }'Length of output: 3476
protocol/x/perpetuals/genesis_test.go (6)
5-5: The addition of the
timepackage import is appropriate to support the newVolatilityBoundsPeriodfield of typetime.Duration.46-46: The addition of the
volatilityBoundsPeriodfield to thetestCasestruct aligns with the PR's objectives to manage volatility bounds.57-57: Initialization of
volatilityBoundsPeriodwithtime.Hourin test cases is correct and tests the new functionality as expected.112-112: Setting
volatilityBoundsPeriodto zero in a test case is a valid test to ensure that the validation logic catches zero as an invalid value.123-123: Setting
volatilityBoundsPeriodto a negative value in a test case is a valid test to ensure that the validation logic catches negative values as invalid.168-168: The
VolatilityBoundsPeriodfield is correctly added to theLiquidityTierstruct within theGenesisStateinitialization in the test setup, which is consistent with the PR's objectives.protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier.go (1)
- 30-36: The addition of
msg.LiquidityTier.VolatilityBoundsPeriodto theSetLiquidityTierfunction is consistent with the PR's objective to manage volatility bounds. Ensure that the new parameter is properly validated and that any necessary logic to handle the volatility bounds period is implemented within theSetLiquidityTiermethod of theKeeper.protocol/x/perpetuals/keeper/msg_server_set_liquidity_tier_test.go (4)
5-11: The addition of the
timepackage is appropriate as it is used within the test cases to set theVolatilityBoundsPeriodfield.24-25: The new
VolatilityBoundsPeriodfield is correctly initialized using thetimepackage to represent a duration.98-110: The new test case "Failure: volatility bounds period is non-positive" correctly tests for invalid configuration of the
VolatilityBoundsPeriodfield, ensuring that it cannot be set to a non-positive duration.149-155: The call to
SetLiquidityTierwithin the test setup correctly includes theVolatilityBoundsPeriodfield, aligning with the changes made to theLiquidityTierstruct.protocol/x/perpetuals/keeper/perpetual.go (4)
5-5: The addition of the "math" package is appropriate for the use of
math.MaxUint64in the volatility bounds initialization.1118-1142: The implementation of
GetVolatilityBoundsandSetVolatilityBoundsfunctions follows the existing code style and conventions, and they are correctly placed within the file structure.75-83: The
SetVolatilityBoundsfunction initializes the volatility bounds withMax: 0andMin: math.MaxUint64, which sets the bounds to their extreme possible values. Ensure this is the intended behavior and not a logic error where default operational values were expected.75-83: The addition of volatility bounds initialization in the
CreatePerpetualfunction aligns with the PR's objective to manage volatility bounds for perpetuals.protocol/x/perpetuals/keeper/perpetual_test.go (5)
2800-2801: The addition of
VolatilityBoundsPeriodto theLiquidityTierstruct needs to be handled correctly in all places whereLiquidityTieris used. Ensure that the new field is properly initialized, validated, and used wherever necessary.2839-2839: Ensure that the
VolatilityBoundsPeriodis validated to prevent non-positive values, which could lead to unexpected behavior.3022-3026: The test case should verify that the
VolatilityBoundsPeriodis correctly modified and persisted in the state.3069-3077: The test case should verify that setting a non-positive
VolatilityBoundsPeriodresults in an appropriate error.3236-3308: The new test case
TestGetSetVolatilityBoundsshould ensure that the getter and setter forVolatilityBoundswork as expected, including handling non-existent perpetual IDs correctly.protocol/x/perpetuals/simulation/genesis.go (4)
10-10: The addition of the
timepackage is appropriate for the new functionality introduced.107-111: The
genVolatilityBoundsPeriodfunction correctly generates a random duration for the volatility bounds period.167-174: The integration of
genVolatilityBoundsPeriodwithinRandomizedGenStateto set theVolatilityBoundsPeriodforLiquidityTierinstances is correctly implemented.104-115: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [104-175]
No issues detected with the rest of the code in the provided hunk. It appears to be consistent with the existing simulation logic and 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 a good practice to ensure that theVolatilityBoundsPeriodis always positive in the generated genesis state. This aligns with the PR's objective to introduce volatility bounds and validates the new functionality within the test.protocol/x/perpetuals/types/errors.go (1)
- 110-118: The addition of
ErrVolatilityBoundsPeriodIsNonPositiveandErrVolatilityBoundsDoesNotExistwith error codes 23 and 24 respectively, are consistent with the PR's objective to handle volatility bounds. Ensure that these error codes are unique within the module and that the error messages are clear and descriptive.protocol/x/perpetuals/types/genesis_test.go (4)
6-6: The addition of the
timepackage import is appropriate for the use oftime.Durationin theVolatilityBoundsPeriodfield.41-41: The
VolatilityBoundsPeriodfield is consistently set totime.Houracross various test cases, which is a sensible default for testing.Also applies to: 79-79, 117-117, 147-147, 177-177, 207-207, 237-237, 267-267, 297-297, 327-327
- 338-366: New test cases have been added to validate the
VolatilityBoundsPeriodfield against non-positive values, which is a good practice to ensure robustness.Also applies to: 368-396
- 366-366: Verify that the error
types.ErrVolatilityBoundsPeriodIsNonPositiveis properly defined and implemented in thetypespackage.Also applies to: 396-396
Verification successful
The error
types.ErrVolatilityBoundsPeriodIsNonPositiveis indeed defined in theerrors.gofile within thetypespackage of theprotocol/x/perpetualsmodule. It is also referenced in test cases in bothliquidity_tier_test.goandgenesis_test.go, which suggests that it is implemented and used for testing purposes as expected.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the definition of ErrVolatilityBoundsPeriodIsNonPositive in the types package. rg 'ErrVolatilityBoundsPeriodIsNonPositive' protocol/x/perpetuals/types/Length of output: 730
protocol/x/perpetuals/types/keys.go (1)
- 30-31: The addition of
VolatilityBoundsKeyPrefixconstant is consistent with the PR's objective to manage volatility bounds. Ensure that this new key prefix does not conflict with any existing key prefixes and is used consistently throughout the codebase.Verification successful
The
VolatilityBoundsKeyPrefixhas been verified to be unique and is being used correctly in the codebase to manage volatility bounds data. There are no conflicts with existing keys, and the usage inperpetual.gois consistent with the PR's objectives.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the new key prefix to ensure it's not conflicting with existing keys. rg 'VolBounds:' --files-with-matches # Search for the usage of the new key prefix to ensure it's being used consistently. rg 'VolatilityBoundsKeyPrefix' --files-with-matchesLength of output: 284
Script:
#!/bin/bash # Check if 'VolBounds:' is unique and not part of any other key. rg 'VolBounds:' --files-with-matches | xargs rg 'VolBounds:' -C 3 # Verify the usage of 'VolatilityBoundsKeyPrefix' in `perpetual.go`. rg 'VolatilityBoundsKeyPrefix' protocol/x/perpetuals/keeper/perpetual.go -C 3Length of output: 1806
protocol/x/perpetuals/types/keys_test.go (1)
- 20-20: The addition of the test assertion for
VolatilityBoundsKeyPrefixis correct and aligns with the PR's objective to introduce volatility bounds within the perpetuals module.protocol/x/perpetuals/types/liquidity_tier.go (2)
27-29: The addition of the validation check for
VolatilityBoundsPeriodin theValidatemethod is correct and ensures that the period is a positive value. This is a necessary check for time-based fields to prevent invalid configurations.24-32: Please ensure that the new validation logic for
VolatilityBoundsPeriodis covered by unit tests to verify that it correctly rejects non-positive values.protocol/x/perpetuals/types/liquidity_tier_test.go (6)
7-7: The import of the
timepackage is necessary for the newVolatilityBoundsPeriodfield of typetime.Duration.18-62: The test cases for
VolatilityBoundsPeriodare well-structured and cover the expected scenarios, including positive, zero, and negative values for theVolatilityBoundsPeriodfield.72-72: The
VolatilityBoundsPeriodfield is correctly set in theLiquidityTierstruct within the test cases.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 existing test logic appears to remain intact and functional with the addition of the
VolatilityBoundsPeriodfield.
15-62: It's important to ensure that all new logic introduced by the PR is covered by tests. The added test cases in
liquidity_tier_test.goare a good start, but a comprehensive test plan should be in place to validate all aspects of the new functionality.18-62: The addition of the
VolatilityBoundsPeriodfield to theLiquidityTierstruct could potentially be a breaking change. It's important to verify that all parts of the codebase that use this struct have been updated accordingly.protocol/x/perpetuals/types/message_set_liquidity_tier_test.go (4)
5-5: The addition of the
timepackage import is appropriate for handling the newVolatilityBoundsPeriodfield.34-34: The
VolatilityBoundsPeriodfield is correctly set to a positive duration (time.Hour) in the test cases.Also applies to: 53-53, 67-67, 81-81
86-112: The new test cases for validating the
VolatilityBoundsPeriodfield are correctly checking for non-positive values and providing clear error messages.78-116: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [2-116]
The test cases added for the
MsgSetLiquidityTier_ValidateBasicfunction appear to cover the new functionality related to theVolatilityBoundsPeriodfield adequately.protocol/x/perpetuals/types/types.go (2)
93-99: The addition of the
volatilityBoundsPeriodparameter to theSetLiquidityTiermethod in thePerpetualsKeeperinterface is consistent with the PR's objectives to manage volatility bounds. Ensure that all implementations of this interface and calls to this method are updated to include the newvolatilityBoundsPeriodparameter.93-99: Please ensure that the test cases for the
SetLiquidityTiermethod are updated to include the newvolatilityBoundsPeriodparameter to cover the new functionality.
Changelist
add perpetuals store for volatility bounds, getter and setter methods, and initialize volatility bounds when creating a perpetual
Test Plan
[Describe how this PR was tested (if applicable)]
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.