-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add gov msg that sets vault quoting params #2017
Conversation
WalkthroughThe updates enhance the trading protocol by integrating new functionalities for managing vault quoting parameters and liquidation orders. Key interfaces and methods have been introduced to improve order tracking, parameter management, and system robustness. Encoding and decoding mechanisms facilitate smooth integration within the messaging framework, enriching the operational capabilities of the protocol. Changes
Poem
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 Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (7)
protocol/x/vault/keeper/vault_test.go (1)
163-164
: Add comments to explain the newquotingParams
field.The addition of the
quotingParams
field is a significant change. Adding comments will help future developers understand its purpose.+ // Vault quoting params. quotingParams *vaulttypes.QuotingParams
indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/order.ts (2)
709-724
: Ensure all fields are properly documented.The
StreamLiquidationOrder
interface is well-defined, but ensure that all fields have clear and concise documentation for maintainability.
730-745
: Ensure consistency betweenStreamLiquidationOrder
andStreamLiquidationOrderSDKType
.The
StreamLiquidationOrderSDKType
interface is consistent withStreamLiquidationOrder
. Ensure that any changes to one are reflected in the other.indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts (4)
400-409
: Ensure all fields are properly documented.The
StreamTakerOrder
interface is well-defined, but ensure that all fields have clear and concise documentation for maintainability.
416-425
: Ensure consistency betweenStreamTakerOrder
andStreamTakerOrderSDKType
.The
StreamTakerOrderSDKType
interface is consistent withStreamTakerOrder
. Ensure that any changes to one are reflected in the other.
432-450
: Ensure all fields are properly documented.The
StreamTakerOrderStatus
interface is well-defined, but ensure that all fields have clear and concise documentation for maintainability.
457-475
: Ensure consistency betweenStreamTakerOrderStatus
andStreamTakerOrderStatusSDKType
.The
StreamTakerOrderStatusSDKType
interface is consistent withStreamTakerOrderStatus
. Ensure that any changes to one are reflected in the other.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/vault/types/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (14)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/order.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (2 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
- protocol/app/msgs/all_msgs.go (1 hunks)
- protocol/app/msgs/internal_msgs.go (1 hunks)
- protocol/app/msgs/internal_msgs_test.go (1 hunks)
- protocol/lib/ante/internal_msg.go (1 hunks)
- protocol/testutil/constants/vault.go (1 hunks)
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params.go (1 hunks)
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go (1 hunks)
- protocol/x/vault/keeper/vault.go (2 hunks)
- protocol/x/vault/keeper/vault_test.go (3 hunks)
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 103-103: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 109-109: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (54)
protocol/testutil/constants/vault.go (1)
23-32
: Ensure the consistency of quoting parameters.The new
QuotingParams
variable is initialized with several fields. Ensure that these values are consistent with the expected quoting behavior and that they are correctly used in the application.Verification successful
QuotingParams values are consistent and correctly used.
The
QuotingParams
variable is initialized with valid values and is consistently used throughout the codebase. The validation methods ensure that the parameters are within expected ranges, and the test cases confirm the correct behavior ofQuotingParams
.
- Initialization:
protocol/testutil/constants/vault.go
- Validation:
protocol/x/vault/types/params.go
- Setting and Getting:
protocol/x/vault/keeper
- Test Cases:
protocol/x/vault/types/params_test.go
,protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go
,protocol/x/vault/keeper/orders_test.go
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of QuotingParams in the codebase. # Test: Search for the usage of QuotingParams. Expect: Correct usage and consistency with quoting behavior. rg --type go -A 5 $'QuotingParams'Length of output: 106115
Script:
#!/bin/bash # Description: Verify the detailed usage of QuotingParams in the codebase. # Search for the definition and initialization of QuotingParams. rg --type go -A 10 'QuotingParams =' protocol/testutil/constants/vault.go # Search for the setting of QuotingParams in the keeper package. rg --type go -A 10 'SetQuotingParams' protocol/x/vault/keeper # Search for the getting of QuotingParams in the keeper package. rg --type go -A 10 'GetQuotingParams' protocol/x/vault/keeper # Search for the validation of QuotingParams. rg --type go -A 10 'Validate' protocol/x/vault/types # Search for test cases involving QuotingParams. rg --type go -A 10 'QuotingParams' protocol/x/vault/types/params_test.go protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go protocol/x/vault/keeper/orders_test.goLength of output: 38680
protocol/x/vault/keeper/msg_server_set_vault_quoting_params.go (9)
1-1
: File header and package declaration.The file begins with the package declaration
keeper
, which is appropriate for the functionality provided.
3-11
: Import statements.The import statements include necessary packages for context handling, error management, governance types, and vault types. These imports are appropriate for the functionality provided.
13-13
: Function documentation.The function
SetVaultQuotingParams
is documented to set the quoting parameters of a specific vault. This documentation is clear and concise.
14-17
: Function signature.The function
SetVaultQuotingParams
accepts a context and a message of typeMsgSetVaultQuotingParams
, and returns a response or an error. This signature is appropriate for the functionality provided.
18-25
: Authority validation.The function checks if the authority is valid using
k.HasAuthority
. If not, it returns an error. This validation is crucial for security and correctness.
27-27
: Context unwrapping.The function unwraps the SDK context using
lib.UnwrapSDKContext
. This step is necessary for further operations within the function.
29-32
: Quoting parameters validation.The function validates the quoting parameters using
msg.QuotingParams.Validate()
. This validation ensures that the parameters are correct before proceeding.
34-37
: Setting quoting parameters.The function sets the quoting parameters for the specified vault using
k.Keeper.SetVaultQuotingParams
. Proper error handling is implemented to return an error if the operation fails.
39-39
: Function return.The function returns a
MsgSetVaultQuotingParamsResponse
if successful. This return type is appropriate for the functionality provided.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (4)
3-3
: Import new message types.The new message types
MsgSetVaultQuotingParams
andMsgSetVaultQuotingParamsResponse
are imported from./tx
. These imports are necessary for the new functionality.
12-14
: Add new method to Msg interface.The
setVaultQuotingParams
method is added to theMsg
interface to set quoting parameters for a specific vault. This method is necessary for the new functionality.
23-23
: Bind new method in constructor.The
setVaultQuotingParams
method is bound in the constructor of theMsgClientImpl
class. This binding ensures that the method can be called correctly within the class context.
38-42
: Implement new method in MsgClientImpl.The
setVaultQuotingParams
method is implemented in theMsgClientImpl
class. The method encodes the request, sends it via RPC, and decodes the response. This implementation is correct and follows best practices.protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go (7)
1-15
: LGTM! Imports and package declaration are appropriate.The imports and package declaration are well-organized and relevant to the test cases.
17-23
: LGTM! Test function and structure initialization.The test function and structure for test cases are well-defined.
24-30
: Success case: Vault Clob 0The test case for a successful scenario with Vault Clob 0 is well-defined.
31-37
: Success case: Vault Clob 1The test case for a successful scenario with Vault Clob 1 is well-defined.
38-45
: Failure case: Invalid AuthorityThe test case for a failure scenario with an invalid authority is well-defined.
46-61
: Failure case: Invalid Quoting ParamsThe test case for a failure scenario with invalid quoting parameters is well-defined.
64-87
: LGTM! Test execution and validation.The test execution and validation logic are well-implemented.
proto/dydxprotocol/vault/tx.proto (3)
22-24
: LGTM! RPC definition for SetVaultQuotingParams.The RPC definition for
SetVaultQuotingParams
is well-defined.
67-78
: LGTM! Message definition for MsgSetVaultQuotingParams.The message definition for
MsgSetVaultQuotingParams
is well-defined.
80-82
: LGTM! Message definition for MsgSetVaultQuotingParamsResponse.The message definition for
MsgSetVaultQuotingParamsResponse
is well-defined.protocol/lib/ante/internal_msg.go (2)
Line range hint
1-128
:
LGTM! Function definition and existing message types.The function definition and existing message types are well-defined.
130-130
: LGTM! Inclusion of MsgSetVaultQuotingParams.The inclusion of
MsgSetVaultQuotingParams
in theIsInternalMsg
function is well-implemented.protocol/x/vault/keeper/vault.go (2)
85-88
: LGTM! The added comments improve clarity.The detailed comments provide a clearer understanding of the decommissioning process.
109-111
: LGTM! The new step ensures quoting parameters are cleaned up.The addition of deleting vault quoting parameters enhances the completeness of the decommissioning process.
protocol/app/msgs/internal_msgs_test.go (1)
149-150
: LGTM! The new message paths enhance test coverage.The addition of message paths for setting vault quoting parameters improves the comprehensiveness of the tests.
protocol/app/msgs/internal_msgs.go (1)
191-192
: LGTM! The new message types enhance messaging capabilities.The addition of message types for setting vault quoting parameters improves the functionality of the vault module.
protocol/x/vault/keeper/vault_test.go (3)
166-186
: Ensure test case names accurately reflect their purpose.The test case names have been updated to include quoting params, which is good. Ensure that the names accurately reflect the purpose of each test case.
217-220
: Handle potential errors when setting vault quoting params.The code correctly checks if
quotingParams
is not nil before setting them. Ensure that any potential errors are handled appropriately.
233-238
: Verify default quoting params after decommissioning.The verification step to check that quoting params revert to their default state is a good addition. Ensure that this check is comprehensive.
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (7)
76-86
: Ensure comprehensive documentation for the new interface.The new
MsgSetVaultQuotingParams
interface is well-defined. Ensure that comprehensive documentation is provided to explain the purpose and usage of each field.
87-97
: Ensure comprehensive documentation for the SDK type.The
MsgSetVaultQuotingParamsSDKType
mirrors the structure ofMsgSetVaultQuotingParams
. Ensure that comprehensive documentation is provided to explain the purpose and usage of each field.
299-304
: Ensure comprehensive documentation for the new interface.The new
MsgSetVaultQuotingParams
interface is well-defined. Ensure that comprehensive documentation is provided to explain the purpose and usage of each field.
307-322
: Ensure comprehensive documentation for the encoding function.The
encode
function forMsgSetVaultQuotingParams
is well-implemented. Ensure that comprehensive documentation is provided to explain its usage.
324-352
: Ensure comprehensive documentation for the decoding function.The
decode
function forMsgSetVaultQuotingParams
is well-implemented. Ensure that comprehensive documentation is provided to explain its usage.
354-360
: Ensure comprehensive documentation for the fromPartial function.The
fromPartial
function forMsgSetVaultQuotingParams
is well-implemented. Ensure that comprehensive documentation is provided to explain its usage.
368-395
: Ensure comprehensive documentation for the encoding and decoding functions.The encoding and decoding functions for
MsgSetVaultQuotingParamsResponse
are well-implemented. Ensure that comprehensive documentation is provided to explain their usage.protocol/app/msgs/all_msgs.go (2)
248-248
: Ensure comprehensive documentation for the new message type.The new message type
"/dydxprotocol.vault.MsgSetVaultQuotingParams"
is a significant addition. Ensure that comprehensive documentation is provided to explain its purpose and usage.
249-249
: Ensure comprehensive documentation for the new message type.The new message type
"/dydxprotocol.vault.MsgSetVaultQuotingParamsResponse"
is a significant addition. Ensure that comprehensive documentation is provided to explain its purpose and usage.indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/order.ts (4)
1332-1340
: Ensure default values are appropriate.The function
createBaseStreamLiquidationOrder
initializes default values correctly. Ensure that these defaults are appropriate for all use cases.
1342-1365
: Ensure all fields are encoded properly.The function
StreamLiquidationOrder.encode
correctly serializes all fields. Ensure that any changes to the interface are reflected in the encoding logic.
1367-1403
: Ensure all fields are decoded properly.The function
StreamLiquidationOrder.decode
correctly deserializes all fields. Ensure that any changes to the interface are reflected in the decoding logic.
1405-1414
: Ensure all fields are handled properly infromPartial
.The function
StreamLiquidationOrder.fromPartial
correctly initializes all fields from a partial object. Ensure that any changes to the interface are reflected in this function.indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts (8)
1488-1494
: Ensure default values are appropriate.The function
createBaseStreamTakerOrder
initializes default values correctly. Ensure that these defaults are appropriate for all use cases.
1496-1511
: Ensure all fields are encoded properly.The function
StreamTakerOrder.encode
correctly serializes all fields. Ensure that any changes to the interface are reflected in the encoding logic.
1513-1541
: Ensure all fields are decoded properly.The function
StreamTakerOrder.decode
correctly deserializes all fields. Ensure that any changes to the interface are reflected in the decoding logic.
1543-1551
: Ensure all fields are handled properly infromPartial
.The function
StreamTakerOrder.fromPartial
correctly initializes all fields from a partial object. Ensure that any changes to the interface are reflected in this function.
1553-1558
: Ensure default values are appropriate.The function
createBaseStreamTakerOrderStatus
initializes default values correctly. Ensure that these defaults are appropriate for all use cases.
1561-1575
: Ensure all fields are encoded properly.The function
StreamTakerOrderStatus.encode
correctly serializes all fields. Ensure that any changes to the interface are reflected in the encoding logic.
1578-1606
: Ensure all fields are decoded properly.The function
StreamTakerOrderStatus.decode
correctly deserializes all fields. Ensure that any changes to the interface are reflected in the decoding logic.
1608-1615
: Ensure all fields are handled properly infromPartial
.The function
StreamTakerOrderStatus.fromPartial
correctly initializes all fields from a partial object. Ensure that any changes to the interface are reflected in this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- protocol/app/msgs/all_msgs.go (1 hunks)
- protocol/app/msgs/internal_msgs.go (1 hunks)
- protocol/app/msgs/internal_msgs_test.go (1 hunks)
- protocol/lib/ante/internal_msg.go (1 hunks)
Files skipped from review due to trivial changes (1)
- protocol/app/msgs/all_msgs.go
Files skipped from review as they are similar to previous changes (3)
- protocol/app/msgs/internal_msgs.go
- protocol/app/msgs/internal_msgs_test.go
- protocol/lib/ante/internal_msg.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/vault/types/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (12)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (2 hunks)
- proto/dydxprotocol/vault/tx.proto (2 hunks)
- protocol/app/msgs/all_msgs.go (1 hunks)
- protocol/app/msgs/internal_msgs.go (1 hunks)
- protocol/app/msgs/internal_msgs_test.go (1 hunks)
- protocol/lib/ante/internal_msg.go (1 hunks)
- protocol/testutil/constants/vault.go (1 hunks)
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params.go (1 hunks)
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go (1 hunks)
- protocol/x/vault/keeper/vault.go (2 hunks)
- protocol/x/vault/keeper/vault_test.go (3 hunks)
Files skipped from review as they are similar to previous changes (10)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts
- protocol/app/msgs/all_msgs.go
- protocol/app/msgs/internal_msgs.go
- protocol/app/msgs/internal_msgs_test.go
- protocol/lib/ante/internal_msg.go
- protocol/testutil/constants/vault.go
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params.go
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go
- protocol/x/vault/keeper/vault.go
- protocol/x/vault/keeper/vault_test.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts
[error] 103-103: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 109-109: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (5)
proto/dydxprotocol/vault/tx.proto (3)
22-24
: LGTM! Addition ofSetVaultQuotingParams
RPC method.The
SetVaultQuotingParams
RPC method is correctly defined and integrated within theMsg
service.
67-78
: LGTM! Addition ofMsgSetVaultQuotingParams
message.The
MsgSetVaultQuotingParams
message is correctly defined with appropriate fields and annotations.
80-82
: LGTM! Addition ofMsgSetVaultQuotingParamsResponse
message.The
MsgSetVaultQuotingParamsResponse
message is correctly defined.indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts (2)
78-86
: LGTM! Addition ofMsgSetVaultQuotingParams
interface.The
MsgSetVaultQuotingParams
interface is correctly defined with appropriate fields and annotations.
89-97
: LGTM! Addition ofMsgSetVaultQuotingParamsSDKType
interface.The
MsgSetVaultQuotingParamsSDKType
interface is correctly defined with appropriate fields and annotations.
QuotingParams: constants.QuotingParams, | ||
}, | ||
}, | ||
"Failure - Invalid Authority": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Add test case with empty authority?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/vault/keeper/msg_server_set_vault_quoting_params_test.go
@Mergifyio backport release/protocol/v6.x |
✅ Backports have been created
|
(cherry picked from commit 27a4923) # Conflicts: # indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.rpc.msg.ts # indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/tx.ts # proto/dydxprotocol/vault/tx.proto # protocol/app/msgs/all_msgs.go # protocol/app/msgs/internal_msgs.go # protocol/app/msgs/internal_msgs_test.go # protocol/lib/ante/internal_msg.go # protocol/x/vault/types/tx.pb.go
@Mergifyio backport release/protocol/v6.x |
✅ Backports have been created
|
(cherry picked from commit 27a4923)
Co-authored-by: Tian <[email protected]>
Changelist
add gov msg that sets vault quoting params
Test Plan
unit tests
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
setVaultQuotingParams
method to allow users to set quoting parameters for specific vaults.Bug Fixes
Tests
SetVaultQuotingParams
to ensure robust handling of quoting parameters.