[TRA-512] Add MsgCreateMarketPermissionless msg #1991
Conversation
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
WalkthroughThe recent modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Keeper
participant MarketSystem
User->>Keeper: CreateMarketPermissionless(msg)
Keeper->>MarketSystem: Check if hard cap reached
MarketSystem-->>Keeper: Hard cap status
alt If under cap
Keeper->>MarketSystem: Create market
MarketSystem-->>Keeper: Market created
else If over cap
Keeper-->>User: ErrMarketsHardCapReached
end
Keeper-->>User: MsgCreateMarketPermissionlessResponse
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/listing/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (8)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (3 hunks)
- proto/dydxprotocol/listing/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/x/listing/keeper/msg_create_market_permissionless.go (1 hunks)
- protocol/x/listing/types/errors.go (1 hunks)
- protocol/x/listing/types/expected_keepers.go (1 hunks)
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts
[error] 58-58: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 61-61: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (11)
protocol/x/listing/types/errors.go (1)
13-17: LGTM! The new error variable is registered correctly.The error message "listed markets hard cap reached" is clear and appropriate for its intended use.
protocol/x/listing/keeper/msg_create_market_permissionless.go (1)
1-40: LGTM! TheCreateMarketPermissionlessfunction is well-structured.The function includes proper context handling, market creation, and error handling. Ensure that the vault deposit logic is implemented as indicated by the TODO comment.
protocol/x/listing/types/expected_keepers.go (1)
53-53: LGTM! The new methodGetAllPerpetualsis correctly added to thePerpetualsKeeperinterface.The method signature and return type are appropriate for retrieving all perpetuals.
proto/dydxprotocol/listing/tx.proto (3)
6-7: Imports look good.The new imports for
gogoproto/gogo.protoanddydxprotocol/subaccounts/subaccount.protoare necessary for the new message types.
52-53: Message definition looks good.The
MsgCreateMarketPermissionlessResponsemessage is well-defined.However, consider using a type alias for the empty interface as suggested by static analysis tools.
- message MsgCreateMarketPermissionlessResponse {} + type MsgCreateMarketPermissionlessResponse = {};Likely invalid or redundant comment.
34-50: Message definition looks good.The
MsgCreateMarketPermissionlessmessage is well-defined with appropriate fields and options.However, ensure that the
quote_quantumsfield is used correctly throughout the codebase.Verification successful
The usage of
quote_quantumsis consistent and correct throughout the codebase.The field
quote_quantumsis appropriately used in message definitions, parameter settings, and event attributes, aligning with its intended purpose.
proto/dydxprotocol/vault/params.protoproto/dydxprotocol/vault/tx.protoprotocol/x/subaccounts/types/events.goprotocol/x/vault/types/tx.pb.goprotocol/x/vault/types/params.pb.goprotocol/x/vault/keeper/orders.goprotocol/x/rewards/keeper/keeper.goprotocol/x/listing/types/tx.pb.goprotocol/x/clob/types/mev_telemetry.goprotocol/x/clob/types/mev.pb.goprotocol/x/clob/types/events.goprotocol/testutil/constants/genesis.goprotocol/testing/containertest/preupgrade_genesis.jsonprotocol/scripts/genesis/sample_pregenesis.jsonprotocol/lib/metrics/constants.goprotocol/indexer/events/events.pb.goprotocol/lib/metrics/metric_keys.goprotocol/app/testdata/default_genesis_state.jsonprotocol/client/docs/swagger-ui/swagger.yamlproto/dydxprotocol/indexer/events/events.protoproto/dydxprotocol/clob/mev.protoScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `quote_quantums` field in the codebase. # Test: Search for the usage of `quote_quantums`. Expect: Consistent usage. rg --type proto -A 5 $'quote_quantums'Length of output: 67
Script:
#!/bin/bash # Description: Verify the usage of `quote_quantums` field in the codebase. # Find all .proto files and then search for `quote_quantums` within those files. fd -e proto -x rg 'quote_quantums' {}Length of output: 231
Script:
#!/bin/bash # Description: Verify the usage of `quote_quantums` field in the codebase. # Search for `quote_quantums` and include more lines of context around each match. rg 'quote_quantums' -A 10 -B 5Length of output: 61541
protocol/app/msgs/internal_msgs_test.go (1)
107-108: Test cases look good.The new test cases for
MsgCreateMarketPermissionlessandMsgCreateMarketPermissionlessResponseare correctly added and enhance the test coverage.indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (2)
34-55: Interface definitions look good.The
MsgCreateMarketPermissionlessandMsgCreateMarketPermissionlessSDKTypeinterfaces are well-defined with appropriate fields.
152-215: Encoding/decoding logic looks good.The encoding and decoding logic for
MsgCreateMarketPermissionlessis correctly implemented and follows best practices.protocol/app/msgs/internal_msgs.go (1)
149-150: New message types added successfully.The new message types
MsgCreateMarketPermissionlessandMsgCreateMarketPermissionlessResponseare correctly added to theInternalMsgSamplesDydxCustommap. Ensure that these message types are properly handled in the rest of the codebase.protocol/app/msgs/all_msgs.go (1)
202-203: New message types added successfully.The new message types
MsgCreateMarketPermissionlessandMsgCreateMarketPermissionlessResponseare correctly added to theAllTypeMessagesmap. Ensure that these message types are properly handled in the rest of the codebase.
| return nil, err | ||
| } | ||
|
|
||
| // TODO: vault deposit for PML |
There was a problem hiding this comment.
Reminder: Implement the vault deposit logic.
The TODO comment indicates that the vault deposit logic for PML is missing.
Do you want me to generate the implementation code or open a GitHub issue to track this task?
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)
- proto/dydxprotocol/listing/tx.proto (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/listing/tx.proto
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
protocol/x/listing/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (1)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (3 hunks)
Additional context used
Biome
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts
[error] 67-67: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
[error] 73-73: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Additional comments not posted (5)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.ts (5)
37-46: InterfaceMsgCreateMarketPermissionlesslooks good.The interface is well-defined with appropriate fields for market creation.
173-187: Encoding method forMsgCreateMarketPermissionlesslooks good.The method correctly handles the optional
subaccountIdfield.
189-217: Decoding method forMsgCreateMarketPermissionlesslooks good.The method correctly decodes the fields, including the optional
subaccountIdfield.
219-225:fromPartialmethod forMsgCreateMarketPermissionlesslooks good.The method correctly handles the optional
subaccountIdfield.
52-61: InterfaceMsgCreateMarketPermissionlessSDKTypelooks good.The interface is well-defined with appropriate fields for market creation.
| * MsgCreateMarketPermissionless response | ||
| */ | ||
|
|
||
| export interface MsgCreateMarketPermissionlessResponse {} |
There was a problem hiding this comment.
Consider using a type alias for the empty interface.
An empty interface is equivalent to {}. Use a type alias instead.
- export interface MsgCreateMarketPermissionlessResponse {}
+ export type MsgCreateMarketPermissionlessResponse = {};Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface MsgCreateMarketPermissionlessResponse {} | |
| export type MsgCreateMarketPermissionlessResponse = {}; |
Tools
Biome
[error] 67-67: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
| * MsgCreateMarketPermissionless response | ||
| */ | ||
|
|
||
| export interface MsgCreateMarketPermissionlessResponseSDKType {} |
There was a problem hiding this comment.
Consider using a type alias for the empty interface.
An empty interface is equivalent to {}. Use a type alias instead.
- export interface MsgCreateMarketPermissionlessResponseSDKType {}
+ export type MsgCreateMarketPermissionlessResponseSDKType = {};Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export interface MsgCreateMarketPermissionlessResponseSDKType {} | |
| export type MsgCreateMarketPermissionlessResponseSDKType = {}; |
Tools
Biome
[error] 73-73: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
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 (1)
protocol/x/listing/types/tx.pb.gois excluded by!**/*.pb.go
Files selected for processing (5)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (2 hunks)
- proto/dydxprotocol/listing/tx.proto (3 hunks)
- protocol/app/module/interface_registry.go (1 hunks)
- protocol/app/msgs/normal_msgs.go (2 hunks)
- protocol/app/msgs/normal_msgs_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- proto/dydxprotocol/listing/tx.proto
Additional comments not posted (8)
indexer/packages/v4-protos/src/codegen/dydxprotocol/listing/tx.rpc.msg.ts (3)
3-3: Import statement looks good.The new import statement for
MsgCreateMarketPermissionlessandMsgCreateMarketPermissionlessResponseis correctly added.
9-11: Interface method addition looks good.The new method
createMarketPermissionlessis correctly added to theMsginterface.
Line range hint
19-32: Class method implementation looks good.The new method
createMarketPermissionlessis correctly implemented in theMsgClientImplclass. It follows the same pattern as the existing methods.protocol/app/module/interface_registry.go (1)
98-100: New entry addition looks good.The new entry for
dydxprotocol.listing.MsgCreateMarketPermissionlessis correctly added to theCustomGetSignersmap.protocol/app/msgs/normal_msgs_test.go (1)
129-132: Test case addition looks good.The new entries for
MsgCreateMarketPermissionlessandMsgCreateMarketPermissionlessResponseare correctly added to theTestNormalMsgs_Keyfunction.protocol/app/msgs/normal_msgs.go (3)
21-21: Import statement looks good.The new import statement for the
listingpackage is correctly formatted and necessary for the new message types.
229-229: Registration ofMsgCreateMarketPermissionlesslooks good.The new message type
MsgCreateMarketPermissionlessis correctly registered and follows the existing pattern.
230-230: Registration ofMsgCreateMarketPermissionlessResponselooks good.The new message type
MsgCreateMarketPermissionlessResponseis correctly registered and follows the existing pattern.
| ctx := lib.UnwrapSDKContext(goCtx, types.ModuleName) | ||
|
|
||
| // Check if the number of listed markets is above the hard cap | ||
| numPerpetuals := len(k.PerpetualsKeeper.GetAllPerpetuals(ctx)) |
There was a problem hiding this comment.
Why check num perpetuals instead of num markets?
There was a problem hiding this comment.
No reason. Shouldn't really make a difference
Signed-off-by: Shrenuj Bansal <shrenuj@dydx.exchange>
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)
- protocol/testutil/constants/genesis.go (1 hunks)
- protocol/x/listing/keeper/msg_create_market_permissionless_test.go (1 hunks)
- protocol/x/listing/types/errors.go (1 hunks)
- protocol/x/listing/types/expected_keepers.go (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/x/listing/types/errors.go
- protocol/x/listing/types/expected_keepers.go
Additional comments not posted (8)
protocol/x/listing/keeper/msg_create_market_permissionless_test.go (6)
19-25: Good test structure.The test function is well-structured with multiple test cases covering different scenarios.
26-45: Comprehensive test cases.The test cases cover success and various failure scenarios, ensuring comprehensive testing.
3-17: Ensure all imports are necessary.Verify that all imported packages are necessary for the test file.
88-103: Ensure correct message creation and assertions.The test includes creating the message and asserting the expected errors. Verify that the message creation and assertions are correctly implemented and cover all scenarios.
60-86: Ensure correct market creation.The test includes creating a market and adding it to the market map. Verify that the market creation is necessary and correctly implemented.
Verification successful
Ensure correct market creation.
The test includes creating a market and adding it to the market map. The market creation appears to be correctly implemented as it follows the necessary steps to marshal metadata and create the market. The context provided shows that these functions are used appropriately within the test file.
- The
tickermetadata.MarshalDyDxfunction is used to marshal market metadata.- The
CreateMarketfunction is used to add the market to the market map.The market creation is necessary and correctly implemented for the test cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the market creation in the test file. # Test: Check for correct market creation and addition to the market map. Expect: Correct market creation. rg --type go 'tickermetadata.MarshalDyDx' -A 10 rg --type go 'CreateMarket' -A 5Length of output: 41381
48-58: Ensure correct test setup.The test setup includes initializing the test app and setting the hard cap. Verify that the setup is necessary and correctly implemented.
Verification successful
Test setup is correct.
The initialization of the test app and setting the hard cap are necessary and correctly implemented as part of the test setup.
- The use of
testapp.NewTestAppBuilderis consistent with other test files.- Setting the hard cap using
SetMarketsHardCapis a standard part of the setup for tests involving market limits.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test setup in the test file. # Test: Check for correct initialization of the test app and setting the hard cap. Expect: Correct setup. rg --type go 'testapp.NewTestAppBuilder' -A 10 rg --type go 'SetMarketsHardCap' -A 5Length of output: 217081
protocol/testutil/constants/genesis.go (2)
887-896: LGTM!The "Mid-Cap" entry is consistent with the other entries in the JSON structure and the values are appropriately formatted.
897-905: LGTM!The "Long-Tail" entry is consistent with the other entries in the JSON structure and the values are appropriately formatted.
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/msg_create_market_permissionless_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/x/listing/keeper/msg_create_market_permissionless_test.go
Changelist
Add implementation of MsgCreateMarketPermissionless message
Test Plan
TBA after some dep changes
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
Tests