[TRA-566] Add listing vault deposit params proto and msg #2120
Conversation
Signed-off-by: Shrenuj Bansal <[email protected]>
WalkthroughRecent changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MsgClient
participant Keeper
User->>MsgClient: SetVaultDepositParams(request)
MsgClient->>Keeper: SetListingVaultDepositParams(request)
Keeper-->>MsgClient: Response(confirm)
MsgClient-->>User: Confirmation
sequenceDiagram
participant User
participant Keeper
User->>Keeper: QueryVaultDepositParams()
Keeper-->>User: Return vault deposit parameters
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
protocol/x/listing/types/params.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/query.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (16)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (5 hunks)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
- proto/dydxprotocol/listing/params.proto (1 hunks)
- proto/dydxprotocol/listing/query.proto (2 hunks)
- proto/dydxprotocol/listing/tx.proto (4 hunks)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go (1 hunks)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/keeper/listing.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (2)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
- protocol/x/listing/types/keys.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts
[error] 24-24: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 27-27: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts
[error] 102-102: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 108-108: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (29)
indexer/packages/v4-protos/src/codegen/google/bundle.ts (2)
1-6: Verify the necessity of new imports.Ensure that the newly added imports (
_122,_123,_124,_125,_126,_127) are required for the functionality provided by this module. If any of these are unused, consider removing them to keep the code clean.
8-14: Ensure compatibility with existing code.The changes to the
apiandprotobufobjects involve updating the spread of imports. Verify that these changes do not break any existing code that relies on the previous structure of these objects.Run the following script to check for usage of
apiandprotobufin the codebase:protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go (1)
8-15: Ensure proper error handling.The function
ListingVaultDepositParamsretrieves parameters and returns them. Ensure thatGetListingVaultDepositParamshandles errors correctly and that any potential errors are managed appropriately in this function.If
GetListingVaultDepositParamscan return an error, consider handling it here to ensure robustness.proto/dydxprotocol/listing/query.proto (2)
4-5: Verify the necessity of imports.Ensure that the imported files
gogoproto/gogo.protoanddydxprotocol/listing/params.protoare necessary for the new message definitions. If any are unused, consider removing them.
21-27: Review new message definitions for completeness.The new messages
QueryListingVaultDepositParamsandQueryListingVaultDepositParamsResponseshould be complete and correctly defined. Ensure that theparamsfield inQueryListingVaultDepositParamsResponsecorrectly referencesListingVaultDepositParamsand that the nullable option is appropriate.proto/dydxprotocol/listing/params.proto (1)
8-25: Ensure correct use of custom types and options.The fields
new_vault_deposit_amountandmain_vault_deposit_amountuse a custom typeSerializableInt. Verify that this type is correctly implemented and that the nullable option is appropriate. Additionally, ensure thatnum_blocks_to_lock_sharesis correctly defined as auint32.protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params_test.go (1)
13-29: Ensure test coverage is comprehensive.The test
TestQueryListingVaultDepositParamschecks the retrieval of vault deposit parameters. Ensure that it covers all necessary scenarios, including edge cases. Consider adding more tests if needed to cover different parameter values or error scenarios.protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.go (1)
12-34: FunctionSetListingVaultDepositParamsis well-structured.The function correctly handles context, authority verification, and setting of parameters. The use of
errorsmod.Wrapffor error handling is appropriate, and the logic aligns with the intended functionality. Ensure that all necessary tests are in place to validate this functionality.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (1)
14-14: New methodsetListingVaultDepositParamsis correctly implemented.The method follows the established pattern for RPC methods, ensuring consistency and proper functionality. The binding in the constructor is correctly done to maintain context.
Also applies to: 38-42
protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1)
16-73: Test coverage forSetListingVaultDepositParamsis comprehensive.The tests effectively cover both successful and failure scenarios, including invalid authority and empty authority cases. The use of table-driven tests enhances readability and maintainability.
proto/dydxprotocol/listing/tx.proto (1)
22-24: New RPC method and message types are well-defined.The
SetListingVaultDepositParamsRPC method and its associated message types are clearly defined, with appropriate comments explaining their purpose. Ensure that these changes are reflected in the corresponding service implementations.Also applies to: 65-79
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1)
5-89: InterfaceListingVaultDepositParamsand associated functions are well-implemented.The interface and its SDK type are clearly defined, and the encoding/decoding functions are implemented following protobuf conventions. The use of
DeepPartialinfromPartialis a good practice for handling optional fields.protocol/x/listing/keeper/listing.go (2)
152-161: LGTM!The
SetListingVaultDepositParamsfunction correctly marshals and stores the listing vault deposit parameters in the key-value store. The implementation is straightforward and adheres to best practices.
163-171: LGTM!The
GetListingVaultDepositParamsfunction correctly retrieves and unmarshals the listing vault deposit parameters from the key-value store. The implementation is clear and efficient.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (6)
30-32: LGTM!The
QueryListingVaultDepositParamsResponseinterface is correctly defined with an optionalparamsfield.
35-37: LGTM!The
QueryListingVaultDepositParamsResponseSDKTypeinterface is correctly defined with an optionalparamsfield.
118-120: LGTM!The
createBaseQueryListingVaultDepositParamsfunction correctly returns an empty object, consistent with theQueryListingVaultDepositParamsinterface.
122-149: LGTM!The
QueryListingVaultDepositParamsobject correctly implements encoding, decoding, and fromPartial methods for handling the type.
152-156: LGTM!The
createBaseQueryListingVaultDepositParamsResponsefunction correctly initializes a response object with an undefinedparamsfield.
158-193: LGTM!The
QueryListingVaultDepositParamsResponseobject correctly implements encoding, decoding, and fromPartial methods for handling the type.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (6)
80-85: LGTM!The
MsgSetListingVaultDepositParamsinterface is correctly defined withauthorityand optionalparamsfields.
91-96: LGTM!The
MsgSetListingVaultDepositParamsSDKTypeinterface is correctly defined withauthorityand optionalparamsfields.
298-303: LGTM!The
createBaseMsgSetListingVaultDepositParamsfunction correctly initializes a message object with default values.
305-349: LGTM!The
MsgSetListingVaultDepositParamsobject correctly implements encoding, decoding, and fromPartial methods for handling the type.
353-355: LGTM!The
createBaseMsgSetListingVaultDepositParamsResponsefunction correctly returns an empty object, consistent with theMsgSetListingVaultDepositParamsResponseinterface.
357-383: LGTM!The
MsgSetListingVaultDepositParamsResponseobject correctly implements encoding, decoding, and fromPartial methods for handling the type.indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (3)
63-65: LGTM!The new imports for
listing/params,listing/query, andlisting/txare correctly added to support the new functionality.
284-287: LGTM!The namespace export for
listingis correctly updated to include the new imports, reflecting the expanded functionality.
289-380: LGTM!The updates to other namespace exports are consistent with the restructuring and enhance the modularity of the module.
| export interface QueryListingVaultDepositParams {} | ||
| /** Queries the listing vault deposit params */ | ||
|
|
||
| export interface QueryListingVaultDepositParamsSDKType {} |
There was a problem hiding this comment.
Consider using a type alias instead of an empty interface.
An empty interface is equivalent to {}. Using a type alias can make the code clearer.
// Replace the empty interface with a type alias
export type QueryListingVaultDepositParamsSDKType = {};Tools
Biome
[error] 27-27: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
| * MsgSetListingVaultDepositParams response | ||
| */ | ||
|
|
||
| export interface MsgSetListingVaultDepositParamsResponseSDKType {} |
There was a problem hiding this comment.
Consider using a type alias instead of an empty interface.
An empty interface is equivalent to {}. Using a type alias can make the code clearer.
// Replace the empty interface with a type alias
export type MsgSetListingVaultDepositParamsResponseSDKType = {};Tools
Biome
[error] 108-108: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (3)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (1)
14-14: Document new method in the interface.Consider adding a brief comment to describe the purpose and usage of the
setListingVaultDepositParamsmethod in theMsginterface.+ /** Sets parameters for a listing vault deposit */ setListingVaultDepositParams(request: MsgSetListingVaultDepositParams): Promise<MsgSetListingVaultDepositParamsResponse>;proto/dydxprotocol/listing/tx.proto (1)
22-24: Ensure consistency in proto comments.The comment for
SetListingVaultDepositParamsis clear. Ensure that similar comments are maintained for all RPC methods for consistency.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1)
5-14: Ensure documentation for interface fields.The
ListingVaultDepositParamsinterface fields are documented. Ensure that these comments are maintained and updated as the interface evolves.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
protocol/x/listing/types/params.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/query.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (16)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (5 hunks)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
- proto/dydxprotocol/listing/params.proto (1 hunks)
- proto/dydxprotocol/listing/query.proto (2 hunks)
- proto/dydxprotocol/listing/tx.proto (4 hunks)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go (1 hunks)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/keeper/listing.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (2)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
- protocol/x/listing/types/keys.go
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts
[error] 24-24: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 27-27: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts
[error] 102-102: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 108-108: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (17)
indexer/packages/v4-protos/src/codegen/google/bundle.ts (1)
1-14: Imports and namespace restructuring approved.The changes to the import statements and the restructuring of the
_127and removal of_121are appropriate adjustments.protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go (1)
8-15: FunctionListingVaultDepositParamsimplementation approved.The implementation correctly retrieves and returns the listing vault deposit parameters. The use of
GetListingVaultDepositParamsensures encapsulation of the parameter retrieval logic.proto/dydxprotocol/listing/query.proto (1)
Line range hint
4-26: New message types for querying vault deposit parameters approved.The
QueryListingVaultDepositParamsandQueryListingVaultDepositParamsResponsemessage types are well-defined. The use of(gogoproto.nullable) = falsefor theparamsfield is appropriate to ensure non-nullability.proto/dydxprotocol/listing/params.proto (1)
1-25: Definition ofListingVaultDepositParamsmessage approved.The message structure is well-designed, using
byteswith a custom type for deposit amounts to ensure precision. Theuint32for the lockup period is suitable, and the gogoproto options are correctly applied.protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params_test.go (1)
13-29: Test forListingVaultDepositParamsquery approved.The test effectively verifies the functionality of querying listing vault deposit parameters. The setup and assertions using
require.NoErrorandrequire.Equalensure comprehensive validation.protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.go (1)
12-34: Ensure comprehensive error handling.The function correctly checks for authority and handles errors when setting vault deposit parameters. Ensure that any additional errors from
SetListingVaultDepositParamsare appropriately logged or handled upstream.Consider verifying that all potential errors from
SetListingVaultDepositParamsare covered in the calling code.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (2)
3-3: Ensure imports are necessary and used.The import statement includes
MsgSetListingVaultDepositParamsandMsgSetListingVaultDepositParamsResponse. Ensure these are used consistently across the file.
38-42: LGTM! Ensure method integration.The
setListingVaultDepositParamsmethod is correctly implemented. Ensure that it is integrated and tested within the broader application context.protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1)
16-73: LGTM! Comprehensive test coverage.The test cases cover both success and failure scenarios effectively. Ensure that any future changes to the
SetListingVaultDepositParamsfunction are reflected in these tests.proto/dydxprotocol/listing/tx.proto (1)
65-75: LGTM! Proto message definitions are clear.The message definitions for
MsgSetListingVaultDepositParamsandMsgSetListingVaultDepositParamsResponseare well-defined. Ensure these are consistently used in the application.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1)
36-89: LGTM! Encoding and decoding logic is sound.The encoding and decoding methods for
ListingVaultDepositParamsare correctly implemented. Ensure these methods are tested for various input scenarios.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (2)
122-125: Ensure encoding and decoding functions are correctly implemented.The
encodeanddecodefunctions forQueryListingVaultDepositParamsare implemented correctly, but ensure that they are tested thoroughly to handle edge cases, such as unexpected input formats.Ensure that unit tests cover various scenarios for encoding and decoding, including invalid inputs.
158-165: Verify encoding and decoding logic for response types.The
encodeanddecodefunctions forQueryListingVaultDepositParamsResponsehandle optional parameters. Ensure that these functions are tested to confirm they handle cases whereparamsis undefined or malformed.Review and expand unit tests to cover scenarios where
paramsmay be missing or incorrectly formatted.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (2)
305-316: Check encoding logic for authority and params.The
encodefunction forMsgSetListingVaultDepositParamsserializesauthorityandparams. Ensure that these fields are correctly handled, especially whenparamsis undefined.Review and expand unit tests to ensure proper serialization of optional fields.
357-360: Ensure decoding logic handles empty responses.The
decodefunction forMsgSetListingVaultDepositParamsResponseshould correctly handle cases where the response is empty. Confirm that this behavior is tested.Verify that unit tests cover scenarios with empty responses to ensure robustness.
indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (2)
63-69: Verify import paths for listing module.The import paths for the listing module have been updated. Ensure that these paths are correct and that all necessary files are included.
Check that the updated import paths match the actual file structure and that no files are missing.
Line range hint
179-287: Ensure namespace restructuring maintains functionality.The restructuring of namespaces using the spread operator should maintain existing functionality. Verify that all components are correctly included and accessible.
Review the usage of these namespaces in the codebase to confirm that no functionality is broken due to restructuring.
| @@ -18,6 +19,22 @@ export interface QueryMarketsHardCapResponseSDKType { | |||
| /** Response type indicating the hard cap on listed markets */ | |||
| hard_cap: number; | |||
| } | |||
| /** Queries the listing vault deposit params */ | |||
|
|
|||
| export interface QueryListingVaultDepositParams {} | |||
There was a problem hiding this comment.
Avoid using empty interfaces.
The QueryListingVaultDepositParams and QueryListingVaultDepositParamsSDKType interfaces are currently empty, which is equivalent to using {}. Consider using a type alias instead to improve clarity.
Replace the interfaces with type aliases:
export type QueryListingVaultDepositParams = {};
export type QueryListingVaultDepositParamsSDKType = {};Also applies to: 27-27
Tools
Biome
[error] 24-24: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
| * MsgSetListingVaultDepositParams response | ||
| */ | ||
|
|
||
| export interface MsgSetListingVaultDepositParamsResponse {} |
There was a problem hiding this comment.
Avoid using empty interfaces.
The MsgSetListingVaultDepositParamsResponse and MsgSetListingVaultDepositParamsResponseSDKType interfaces are empty. Consider using a type alias instead for clarity.
Replace the interfaces with type aliases:
export type MsgSetListingVaultDepositParamsResponse = {};
export type MsgSetListingVaultDepositParamsResponseSDKType = {};Also applies to: 108-108
Tools
Biome
[error] 102-102: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (12)
indexer/packages/v4-protos/src/codegen/google/bundle.ts (1)
1-6: Reorganize imports for clarity.The import statements have been rearranged, and a new import
_127has been added. Ensure that these changes are necessary and maintain the intended functionality.Consider grouping related imports together for better readability.
protocol/x/listing/types/keys.go (1)
17-18: Document the purpose ofListingVaultDepositParamsKey.The new constant
ListingVaultDepositParamsKeyis added for retrieving listing vault deposit parameters. Ensure that its usage is well-documented across the codebase to facilitate understanding and maintenance.Consider adding comments or documentation where this key is used to clarify its role and importance.
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (4)
5-14: Ensure Consistent Documentation.The interface
ListingVaultDepositParamshas comments for each field. Ensure these comments are consistent and clear across similar interfaces for better maintainability.
17-26: Ensure Consistent Naming Conventions.The interface
ListingVaultDepositParamsSDKTypeuses snake_case for field names, which is typical for SDK types. Ensure this naming convention is consistently applied across all SDK interfaces.
28-34: Consider Adding Type Annotations for Function Return Values.While TypeScript can infer return types, explicitly annotating the return type of
createBaseListingVaultDepositParamscan improve readability and maintainability.-function createBaseListingVaultDepositParams(): ListingVaultDepositParams { +function createBaseListingVaultDepositParams(): ListingVaultDepositParams {
83-89: Ensure Consistency infromPartialImplementation.The
fromPartialfunction provides flexibility in constructing objects. Ensure that similar functions across the codebase follow a consistent pattern for initializing default values.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (3)
30-37: Ensure Consistent Optional Field Handling.In
QueryListingVaultDepositParamsResponseandQueryListingVaultDepositParamsResponseSDKType, theparamsfield is optional. Ensure that this is consistently handled in both encoding and decoding functions.
118-120: Consider Adding Type Annotations for Function Return Values.Explicitly annotating the return type of
createBaseQueryListingVaultDepositParamscan improve readability and maintainability.-function createBaseQueryListingVaultDepositParams(): QueryListingVaultDepositParams { +function createBaseQueryListingVaultDepositParams(): QueryListingVaultDepositParams {
152-156: Consider Adding Type Annotations for Function Return Values.Explicitly annotating the return type of
createBaseQueryListingVaultDepositParamsResponsecan improve readability and maintainability.-function createBaseQueryListingVaultDepositParamsResponse(): QueryListingVaultDepositParamsResponse { +function createBaseQueryListingVaultDepositParamsResponse(): QueryListingVaultDepositParamsResponse {indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (2)
80-95: Ensure Consistent Documentation and Naming.The interfaces
MsgSetListingVaultDepositParamsandMsgSetListingVaultDepositParamsSDKTypehave comments and field names that should be consistent with other similar interfaces. Ensure uniformity in documentation and naming conventions.
298-303: Consider Adding Type Annotations for Function Return Values.Explicitly annotating the return type of
createBaseMsgSetListingVaultDepositParamscan improve readability and maintainability.-function createBaseMsgSetListingVaultDepositParams(): MsgSetListingVaultDepositParams { +function createBaseMsgSetListingVaultDepositParams(): MsgSetListingVaultDepositParams {indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (1)
63-69: Ensure Consistent Import Ordering.The imports for the
listingmodule have been added. Ensure that the ordering of imports is consistent with the project's style guide to maintain readability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
protocol/x/listing/types/params.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/query.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (16)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (5 hunks)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/google/bundle.ts (1 hunks)
- proto/dydxprotocol/listing/params.proto (1 hunks)
- proto/dydxprotocol/listing/query.proto (2 hunks)
- proto/dydxprotocol/listing/tx.proto (4 hunks)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go (1 hunks)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/keeper/listing.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (1)
- indexer/packages/v4-protos/src/codegen/gogoproto/bundle.ts
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts
[error] 24-24: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 27-27: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts
[error] 102-102: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 108-108: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (16)
indexer/packages/v4-protos/src/codegen/google/bundle.ts (1)
8-14: Verify the impact of adding_127toprotobuf.The addition of
_127to theprotobufobject suggests new functionality or dependencies. Verify that this change aligns with the intended enhancements and does not introduce any breaking changes.Run the following script to check for usages of
_127within the codebase to ensure compatibility:Verification successful
Addition of
_127toprotobufis consistent with expected enhancements. The_127module introduces standard functionality for handlingAnytype protocol buffer messages, which aligns with typical protobuf usage patterns and should not introduce breaking changes.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usages of `_127` in the codebase. # Test: Search for usages of `_127`. Expect: Compatible changes. rg --type ts '_127'Length of output: 183
Script:
#!/bin/bash # Description: Inspect the contents of the `_127` module to understand its impact. # Locate the file that defines `_127`. fd 'any' --type f --exec catLength of output: 9227
proto/dydxprotocol/listing/query.proto (1)
21-26: Ensure alignment with service definitions.The new message types
QueryListingVaultDepositParamsandQueryListingVaultDepositParamsResponseare added for querying vault deposit parameters. Ensure these align with the service definitions and are correctly integrated into the gRPC service.Check that these messages are correctly used in service definitions and implementations.
proto/dydxprotocol/listing/params.proto (1)
8-25: Verify custom type usage inListingVaultDepositParams.The message
ListingVaultDepositParamsuses a custom typeSerializableIntfor deposit amounts. Ensure that this custom type is correctly defined and integrated within the codebase.Run the following script to verify the definition and usage of
SerializableInt:Verification successful
Custom type
SerializableIntis correctly integrated.The
SerializableInttype is well-defined and extensively used across the codebase, including its integration in theListingVaultDepositParamsmessage. The type is defined inprotocol/dtypes/serializable_int.goand includes necessary methods for serialization and other operations. No issues found with its usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of `SerializableInt`. # Test: Search for the definition and usage of `SerializableInt`. Expect: Correct integration. rg 'SerializableInt'Length of output: 23875
protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params_test.go (1)
13-30: Good test coverage for querying vault deposit parameters.The test function
TestQueryListingVaultDepositParamsis well-structured and effectively verifies the setting and retrieval of vault deposit parameters. Consider adding edge cases or additional scenarios if applicable.protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.go (1)
12-34: Well-implemented authority check and error handling.The function
SetListingVaultDepositParamscorrectly checks for valid authority and handles errors appropriately. Ensure that theHasAuthoritymethod is thoroughly tested to prevent unauthorized access.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (1)
14-14: New methodsetListingVaultDepositParamsadded successfully.The addition of the
setListingVaultDepositParamsmethod in both theMsginterface andMsgClientImplclass is well-executed. Ensure that this new functionality is covered by tests to verify its correct operation.Also applies to: 23-23, 38-42
protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1)
16-73: Comprehensive test coverage for setting vault deposit parameters.The test function
TestSetListingVaultDepositParamseffectively covers various scenarios, including success and failure cases. The use of table-driven tests is commendable for its clarity and maintainability.proto/dydxprotocol/listing/tx.proto (1)
22-24: New RPC method and message types defined successfully.The
SetListingVaultDepositParamsRPC method and its associated message types are well-defined and documented. Ensure that these additions are integrated into the overall system and tested for compatibility.Also applies to: 65-75
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (2)
36-51: Verify Encoding Logic.The
encodefunction forListingVaultDepositParamsappears correct, but ensure that all fields are being encoded as expected, especially when dealing with binary data likeUint8Array.
53-81: Verify Decoding Logic.The
decodefunction forListingVaultDepositParamsshould correctly handle all possible input scenarios. Verify that it handles unexpected or malformed input gracefully.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.ts (2)
158-165: Verify Encoding Logic for Optional Fields.The
encodefunction forQueryListingVaultDepositParamsResponseshould correctly handle the optionalparamsfield. Verify that it encodes this field only when it is defined.
167-187: Verify Decoding Logic for Optional Fields.The
decodefunction forQueryListingVaultDepositParamsResponseshould correctly handle the optionalparamsfield. Verify that it decodes this field correctly when present.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (2)
305-316: Verify Encoding Logic for Optional Fields.The
encodefunction forMsgSetListingVaultDepositParamsshould correctly handle the optionalparamsfield. Verify that it encodes this field only when it is defined.
318-342: Verify Decoding Logic for Optional Fields.The
decodefunction forMsgSetListingVaultDepositParamsshould correctly handle the optionalparamsfield. Verify that it decodes this field correctly when present.indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (2)
Line range hint
179-287: Verify Export Consistency.The exports for various modules, including
affiliates,assets, andlisting, have been updated. Verify that these changes maintain consistency with the rest of the codebase and do not introduce any breaking changes.
289-377: Verify Export Consistency.The exports for modules like
perpetuals,prices, andvaulthave been updated. Ensure these changes are consistent with the rest of the codebase and verify that they do not introduce any breaking changes.
| func (k Keeper) ListingVaultDepositParams( | ||
| ctx sdk.Context, | ||
| req *types.QueryListingVaultDepositParams, | ||
| ) (*types.QueryListingVaultDepositParamsResponse, error) { | ||
| params := k.GetListingVaultDepositParams(ctx) | ||
| return &types.QueryListingVaultDepositParamsResponse{ | ||
| Params: params, | ||
| }, nil |
There was a problem hiding this comment.
Ensure comprehensive error handling.
The function ListingVaultDepositParams retrieves and returns parameters without any error handling. Consider adding error handling to manage potential issues during parameter retrieval.
Implement error handling to ensure robustness:
func (k Keeper) ListingVaultDepositParams(
ctx sdk.Context,
req *types.QueryListingVaultDepositParams,
) (*types.QueryListingVaultDepositParamsResponse, error) {
params, err := k.GetListingVaultDepositParams(ctx)
if err != nil {
return nil, err
}
return &types.QueryListingVaultDepositParamsResponse{
Params: params,
}, nil
}
protocol/x/listing/keeper/listing.go
Outdated
| // Function to set listing vault deposit params in module store | ||
| func (k Keeper) SetListingVaultDepositParams( | ||
| ctx sdk.Context, | ||
| params types.ListingVaultDepositParams, | ||
| ) error { | ||
| store := ctx.KVStore(k.storeKey) | ||
| key := []byte(types.ListingVaultDepositParamsKey) | ||
| store.Set(key, k.cdc.MustMarshal(¶ms)) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Ensure Proper Error Handling in SetListingVaultDepositParams.
The function SetListingVaultDepositParams sets parameters in the store but does not handle potential errors from MustMarshal. Consider adding error handling to prevent unexpected failures.
func (k Keeper) SetListingVaultDepositParams(
ctx sdk.Context,
params types.ListingVaultDepositParams,
) error {
store := ctx.KVStore(k.storeKey)
key := []byte(types.ListingVaultDepositParamsKey)
bz, err := k.cdc.Marshal(¶ms)
if err != nil {
return err
}
store.Set(key, bz)
return nil
}| // Function to get listing vault deposit params from module store | ||
| func (k Keeper) GetListingVaultDepositParams( | ||
| ctx sdk.Context, | ||
| ) (vaultDepositParams types.ListingVaultDepositParams) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| b := store.Get([]byte(types.ListingVaultDepositParamsKey)) | ||
| k.cdc.MustUnmarshal(b, &vaultDepositParams) | ||
| return vaultDepositParams | ||
| } |
There was a problem hiding this comment.
Handle Potential Nil Values in GetListingVaultDepositParams.
The function GetListingVaultDepositParams does not check if the retrieved bytes are nil before unmarshalling. Consider adding a check to handle cases where the key might not exist.
func (k Keeper) GetListingVaultDepositParams(
ctx sdk.Context,
) (vaultDepositParams types.ListingVaultDepositParams, err error) {
store := ctx.KVStore(k.storeKey)
b := store.Get([]byte(types.ListingVaultDepositParamsKey))
if b == nil {
return vaultDepositParams, errors.New("listing vault deposit params not found")
}
err = k.cdc.Unmarshal(b, &vaultDepositParams)
return vaultDepositParams, err
}| export interface QueryListingVaultDepositParams {} | ||
| /** Queries the listing vault deposit params */ | ||
|
|
||
| export interface QueryListingVaultDepositParamsSDKType {} |
There was a problem hiding this comment.
Use Type Alias Instead of Empty Interface.
The interfaces QueryListingVaultDepositParams and QueryListingVaultDepositParamsSDKType are empty. Consider using a type alias instead to improve clarity and reduce unnecessary complexity.
export type QueryListingVaultDepositParams = {};
export type QueryListingVaultDepositParamsSDKType = {};Tools
Biome
[error] 24-24: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 27-27: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
| export interface MsgSetListingVaultDepositParamsResponse {} | ||
| /** | ||
| * MsgSetListingVaultDepositParamsResponse defines the | ||
| * MsgSetListingVaultDepositParams response | ||
| */ | ||
|
|
||
| export interface MsgSetListingVaultDepositParamsResponseSDKType {} |
There was a problem hiding this comment.
Use Type Alias Instead of Empty Interface.
The interfaces MsgSetListingVaultDepositParamsResponse and MsgSetListingVaultDepositParamsResponseSDKType are empty. Consider using a type alias instead to improve clarity and reduce unnecessary complexity.
export type MsgSetListingVaultDepositParamsResponse = {};
export type MsgSetListingVaultDepositParamsResponseSDKType = {};Tools
Biome
[error] 102-102: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 108-108: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- proto/dydxprotocol/listing/query.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)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/listing/query.proto
Additional comments not posted (3)
protocol/app/msgs/internal_msgs_test.go (1)
107-108: Test coverage expanded with new message paths.The addition of
"/dydxprotocol.listing.MsgSetListingVaultDepositParams"and"/dydxprotocol.listing.MsgSetListingVaultDepositParamsResponse"to the test case enhances the test coverage for the new functionalities related to listing vault deposit parameters. Ensure that these new message types are correctly implemented and integrated within the system.protocol/app/msgs/internal_msgs.go (1)
149-150: New message types added for listing vault deposit parameters.The inclusion of
"/dydxprotocol.listing.MsgSetListingVaultDepositParams"and"/dydxprotocol.listing.MsgSetListingVaultDepositParamsResponse"in the internal message samples is consistent with the PR objectives. These additions enhance the protocol's functionality regarding vault deposit parameters.protocol/app/msgs/all_msgs.go (1)
204-205: Integration of new message types for listing vault deposit parameters.The addition of
"/dydxprotocol.listing.MsgSetListingVaultDepositParams"and"/dydxprotocol.listing.MsgSetListingVaultDepositParamsResponse"to the list of all messages aligns with the PR's goal of enhancing the protocol's capabilities. This integration appears well-executed and maintains consistency with the existing message types.
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- protocol/app/msgs/internal_msgs_test.go (1 hunks)
- protocol/lib/ante/internal_msg.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/app/msgs/internal_msgs_test.go
Additional comments not posted (1)
protocol/lib/ante/internal_msg.go (1)
100-100: LGTM! Verify the definition and usage of*listing.MsgSetListingVaultDepositParams.The addition of
*listing.MsgSetListingVaultDepositParamsto the list of internal messages is consistent with the existing structure.Ensure that
MsgSetListingVaultDepositParamsis correctly defined in thelistingpackage and used appropriately throughout the codebase.Run the following script to verify the definition and usage:
Verification successful
Verification Successful:
MsgSetListingVaultDepositParamsis correctly defined and used.The
MsgSetListingVaultDepositParamsis well-integrated into the codebase, with its definition and usage consistent across relevant files. This confirms that its addition to theIsInternalMsgfunction is appropriate and does not introduce any issues.
- Defined in:
protocol/x/listing/types/tx.pb.go- Used in:
protocol/x/listing/keeper/msg_set_listing_vault_deposit_params.goand associated test files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition and usage of `MsgSetListingVaultDepositParams`. # Test: Search for the definition. Expect: A valid definition in the listing package. rg --type go 'type MsgSetListingVaultDepositParams' -A 5 # Test: Search for the usage. Expect: Occurrences where this message is used. rg --type go 'MsgSetListingVaultDepositParams' -A 5Length of output: 22101
| option go_package = "github.com/dydxprotocol/v4-chain/protocol/x/listing/types"; | ||
|
|
||
| // ListingVaultDepositParams represents the params for PML megavault deposits | ||
| message ListingVaultDepositParams { |
There was a problem hiding this comment.
should we just call it ListingParams? in the future, there might be additional params we can add to this too
There was a problem hiding this comment.
ListingParams is a bit ambiguous since it can also refer to params associated with a single listing.
I think this is fine for now. We can alter the name in the future if its really needed
| // Function to set listing vault deposit params in module store | ||
| func (k Keeper) SetListingVaultDepositParams( | ||
| ctx sdk.Context, | ||
| params types.ListingVaultDepositParams, |
There was a problem hiding this comment.
can we add validation on this? e.g. ensure that deposit_amount is non-negative
There was a problem hiding this comment.
Shouldn't really matter since this is gov controlled.
Also the transfer to vault functions in x/vault should already have this validation, I believe, which is the more important place
There was a problem hiding this comment.
usually it's nice to have validation even on gov controlled params and don't think we should depend on validation in another module
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
protocol/x/listing/types/params.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/query.pb.gois excluded by!**/*.pb.goprotocol/x/listing/types/query.pb.gw.gois excluded by!**/*.pb.gw.go
Files selected for processing (9)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (4 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/lcd.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.lcd.ts (1 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.rpc.Query.ts (2 hunks)
- proto/dydxprotocol/listing/params.proto (1 hunks)
- proto/dydxprotocol/listing/query.proto (1 hunks)
- protocol/x/listing/client/cli/query.go (1 hunks)
- protocol/x/listing/client/cli/query_params.go (1 hunks)
Files skipped from review due to trivial changes (1)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/params.ts
Files skipped from review as they are similar to previous changes (2)
- proto/dydxprotocol/listing/params.proto
- proto/dydxprotocol/listing/query.proto
Additional comments not posted (6)
protocol/x/listing/client/cli/query.go (1)
23-24: LGTM! The new command addition is well-integrated.The addition of
CmdQueryListingVaultDepositParams()to theGetQueryCmdfunction expands the CLI functionality appropriately.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.lcd.ts (1)
1-22: LGTM! The LCD query client is correctly implemented.The
LCDQueryClientclass is well-structured, and thelistingVaultDepositParamsfunction correctly queries the endpoint.protocol/x/listing/client/cli/query_params.go (1)
12-33: LGTM! The CLI command for querying vault deposit parameters is well-implemented.The
CmdQueryListingVaultDepositParamsfunction is correctly structured and integrates well with the Cosmos SDK client context.indexer/packages/v4-protos/src/codegen/dydxprotocol/lcd.ts (1)
33-35: LGTM! The addition of thelistingproperty is consistent with existing patterns.The new
listingproperty in thecreateLCDClientfunction enhances the client's capabilities by adding support for listing-related queries.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/query.rpc.Query.ts (1)
12-12: LGTM! ThelistingVaultDepositParamsmethod is a valuable addition.The new method enhances the gRPC querier service by allowing queries for listing vault deposit parameters. The implementation is consistent with existing patterns.
Also applies to: 20-20, 29-33, 44-45
indexer/packages/v4-protos/src/codegen/dydxprotocol/bundle.ts (1)
63-171: LGTM! The restructuring of imports and exports enhances modularity and clarity.The changes improve code organization and expand functionality, aligning with the objective of enhancing the protocol's capabilities.
Also applies to: 180-382
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go (1 hunks)
Files skipped from review due to trivial changes (1)
- protocol/x/listing/keeper/grpc_query_listing_vault_deposit_params.go
Signed-off-by: Shrenuj Bansal <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
protocol/x/listing/types/params.go (1)
14-27: Validation logic is correctly implemented.The
Validatemethod effectively checks for negative values and uses the new error constants to provide specific feedback. Consider adding comments to explain the significance of the parameters, such as whyNumBlocksToLockSharesdefaults to 30 days.func (p ListingVaultDepositParams) Validate() error { + // Ensure deposit amounts are non-negative and blocks to lock shares is positive.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- protocol/x/listing/keeper/listing.go (1 hunks)
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go (1 hunks)
- protocol/x/listing/types/errors.go (1 hunks)
- protocol/x/listing/types/params.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/x/listing/keeper/listing.go
- protocol/x/listing/keeper/msg_set_listing_vault_deposit_params_test.go
Additional comments not posted (2)
protocol/x/listing/types/errors.go (1)
24-35: New error constants are correctly defined.The new error constants
ErrInvalidDepositAmountandErrInvalidNumBlocksToLockSharesenhance the error handling by providing specific feedback for vault deposit parameters. The error codes are sequential and consistent with existing definitions.protocol/x/listing/types/params.go (1)
5-11: Default parameters are well-defined.The
DefaultParamsfunction sets sensible defaults for vault deposit parameters, ensuring a baseline configuration.
Changelist
Add protos for new market listing vault deposit parameters
Add msg and query for setting/getting above proto
Test Plan
Added tests
Author/Reviewer Checklist
state-breakinglabel.indexer-postgres-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests