[CORE-689] - CLI commands for x/clob params #699
Conversation
WalkthroughThe changes introduce new functionalities to query block rate limit configuration, liquidations configuration, and equity tier limit configuration. This is achieved through the addition of new RPC methods, CLI commands, and corresponding tests. The changes span across multiple files, including protobuf definitions, client-side CLI commands, server-side gRPC handlers, and their respective test files. Changes
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: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- protocol/x/clob/types/query.pb.go
- protocol/x/clob/types/query.pb.gw.go
Files selected for processing (16)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.lcd.ts (3 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.rpc.Query.ts (5 hunks)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts (3 hunks)
- proto/dydxprotocol/clob/query.proto (3 hunks)
- protocol/mocks/QueryClient.go (2 hunks)
- protocol/x/clob/client/cli/query.go (1 hunks)
- protocol/x/clob/client/cli/query_block_rate_limit_configuration.go (1 hunks)
- protocol/x/clob/client/cli/query_block_rate_limit_configuration_test.go (1 hunks)
- protocol/x/clob/client/cli/query_equity_tier_limit_config.go (1 hunks)
- protocol/x/clob/client/cli/query_equity_tier_limit_config_test.go (1 hunks)
- protocol/x/clob/client/cli/query_liquidations_configuration.go (1 hunks)
- protocol/x/clob/client/cli/query_liquidations_configuration_test.go (1 hunks)
- protocol/x/clob/keeper/grpc_query_block_rate_limit_configuration.go (1 hunks)
- protocol/x/clob/keeper/grpc_query_block_rate_limit_configuration_test.go (1 hunks)
- protocol/x/clob/keeper/grpc_query_liquidations_configuration.go (1 hunks)
- protocol/x/clob/keeper/grpc_query_liquidations_configuration_test.go (1 hunks)
Files skipped from review due to trivial changes (5)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.ts
- protocol/x/clob/client/cli/query.go
- protocol/x/clob/client/cli/query_block_rate_limit_configuration_test.go
- protocol/x/clob/keeper/grpc_query_block_rate_limit_configuration_test.go
- protocol/x/clob/keeper/grpc_query_liquidations_configuration_test.go
Additional comments: 19
protocol/x/clob/client/cli/query_liquidations_configuration.go (1)
- 1-34: The new hunk introduces a new CLI command
get-liquidations-configto query the liquidations configuration. It uses theQueryLiquidationsConfigurationRequestmessage to make the query and prints the response. The error handling is done correctly, and the code is clean and maintainable. However, it's important to ensure that theLiquidationsConfigurationfunction in theQueryClientinterface is implemented correctly and returns the expected results.protocol/x/clob/client/cli/query_block_rate_limit_configuration.go (1)
- 1-34: The code looks good overall. It correctly sets up a new CLI command to query the block rate limit configuration. The command uses the
QueryClientto make the request and handles any errors that might occur. The result is then printed using thePrintProtofunction. The command is also correctly added to the Cobra command tree with theAddQueryFlagsToCmdfunction. However, it's important to ensure that theBlockRateLimitConfigurationfunction in theQueryClientis implemented correctly and returns the expected results.protocol/x/clob/client/cli/query_equity_tier_limit_config.go (1)
- 1-34: The new hunk introduces a new CLI command
get-equity-tier-limit-configto query the equity tier limit configuration. The command uses theEquityTierLimitConfigurationfunction from theQueryClientinterface to make the query and prints the result using thePrintProtofunction from theclientCtx. The error handling is done correctly, and the function signature is consistent with the other CLI commands in the codebase. The code is modular, maintainable, and follows best practices.protocol/x/clob/client/cli/query_liquidations_configuration_test.go (1)
- 15-32: The test
TestCmdGetLiquidationsConfigurationis well written and covers the main functionality of theCmdGetLiquidationsConfigurationcommand. It checks that the command executes without errors and that the output is correctly unmarshalled into aQueryLiquidationsConfigurationResponseobject. It also verifies that theLiquidationsConfigfield of the response is not nil and matches the expected default value. This test ensures that the command is working as expected and returning the correct configuration.protocol/x/clob/keeper/grpc_query_liquidations_configuration.go (1)
- 1-25: The function
LiquidationsConfigurationis well implemented. It checks if the request isniland returns an error if it is, which is good for error handling. It then retrieves the liquidations configuration using theGetLiquidationsConfigmethod and returns it in the response. The use ofUnwrapSDKContextis correct to convert the gRPC context to the SDK context. The error message in line 17 is clear and informative.protocol/x/clob/keeper/grpc_query_block_rate_limit_configuration.go (1)
- 11-25: The function
BlockRateLimitConfigurationis well implemented. It checks if the request isniland returns an error if it is, which is a good practice for error handling. It then retrieves the block rate limit configuration and returns it in the response. The function is simple, clear, and follows the best practices for gRPC handlers.proto/dydxprotocol/clob/query.proto (3)
7-10: The new imports for
block_rate_limit_config.protoandliquidations_config.protoare added to support the new RPC methods for querying block rate limit and liquidations configurations. Ensure these proto files exist and are correctly defined.46-57: Two new RPC methods
BlockRateLimitConfigurationandLiquidationsConfigurationare added to the service. These methods are used to query the block rate limit and liquidations configurations respectively. The HTTP GET endpoints for these methods are also defined. Ensure these endpoints are correctly integrated into the server routing.134-153: New request and response message types are defined for the new RPC methods. The request messages are empty as no parameters are required for these queries. The response messages contain the queried configurations as non-nullable fields. Ensure that the server always returns a valid configuration in the response to avoid null value errors.
indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.lcd.ts (3)
3-3: The import statement has been extended to include new types related to block rate limit configuration and liquidations configuration. Ensure that these types are correctly defined and used in the rest of the code.
16-17: Two new methods
blockRateLimitConfigurationandliquidationsConfigurationare being bound to the class instance. Ensure that these methods are correctly defined and used in the rest of the code.50-63: Two new methods
blockRateLimitConfigurationandliquidationsConfigurationhave been added to theLCDQueryClientclass. These methods are used to query block rate limit configuration and liquidations configuration respectively. They construct the API endpoint and make a GET request to that endpoint using thereqobject. They return a promise that resolves to aQueryBlockRateLimitConfigurationResponseSDKTypeandQueryLiquidationsConfigurationResponseSDKTypeobject respectively. Ensure that these methods are correctly used in the rest of the code.protocol/mocks/QueryClient.go (2)
149-177: The new mock function
BlockRateLimitConfigurationis added correctly. It follows the same pattern as the other mock functions in theQueryClientstruct. It takes a context, aQueryBlockRateLimitConfigurationRequestobject, and a variadic parameter ofgrpc.CallOption. It returns aQueryBlockRateLimitConfigurationResponseobject and an error. The function uses the testify mock package'sCalledmethod to simulate a function call and return aCallobject. TheCallobject'sGetmethod is used to retrieve the return values of the function call. The function handles the case where the return value is a function and calls it with the provided arguments. It also handles the case where the return value is not a function and directly assigns it to the return variable. The function then returns the response object and error.299-327: The new mock function
LiquidationsConfigurationis added correctly. It follows the same pattern as the other mock functions in theQueryClientstruct. It takes a context, aQueryLiquidationsConfigurationRequestobject, and a variadic parameter ofgrpc.CallOption. It returns aQueryLiquidationsConfigurationResponseobject and an error. The function uses the testify mock package'sCalledmethod to simulate a function call and return aCallobject. TheCallobject'sGetmethod is used to retrieve the return values of the function call. The function handles the case where the return value is a function and calls it with the provided arguments. It also handles the case where the return value is not a function and directly assigns it to the return variable. The function then returns the response object and error.indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.rpc.Query.ts (5)
4-4: The import statement has been extended to include new request and response types for querying block rate limit configuration and liquidations configuration. This change is necessary to support the new query methods added to the
Queryinterface andQueryClientImplclass.22-27: New methods
blockRateLimitConfigurationandliquidationsConfigurationhave been added to theQueryinterface. These methods take optional request parameters and return promises that resolve to the corresponding response types. These changes are in line with the existing structure of the interface and provide additional functionality for querying block rate limit and liquidations configurations.39-40: The
blockRateLimitConfigurationandliquidationsConfigurationmethods are bound to thethiscontext in the constructor of theQueryClientImplclass. This is a standard practice in JavaScript to ensure that the methods have the correctthiscontext when they are called.75-85: The
blockRateLimitConfigurationandliquidationsConfigurationmethods are implemented in theQueryClientImplclass. These methods encode the request parameters, send a gRPC request using therpcinstance, and return a promise that resolves to the decoded response. The implementation of these methods is consistent with the other methods in the class and follows best practices for handling gRPC requests and responses.110-117: The
blockRateLimitConfigurationandliquidationsConfigurationmethods are added to theQueryClientextension returned by thecreateRpcQueryExtensionfunction. These methods call the corresponding methods on thequeryServiceinstance, which is an instance ofQueryClientImpl. This allows the extendedQueryClientto use the new query methods.
| func TestCmdGetEquityTierLimitConfig(t *testing.T) { | ||
| net, _ := networkWithClobPairObjects(t, 2) | ||
| ctx := net.Validators[0].ClientCtx | ||
| common := []string{ | ||
| fmt.Sprintf("--%s=json", tmcli.OutputFlag), | ||
| } | ||
|
|
||
| out, err := clitestutil.ExecTestCLICmd(ctx, cli.CmdGetEquityTierLimitConfig(), common) | ||
| require.NoError(t, err) | ||
| var resp types.QueryEquityTierLimitConfigurationResponse | ||
| require.NoError(t, net.Config.Codec.UnmarshalJSON(out.Bytes(), &resp)) | ||
| require.NotNil(t, resp.EquityTierLimitConfig) | ||
| require.Equal( | ||
| t, | ||
| emptyEquityTierLimitConfig, | ||
| resp.EquityTierLimitConfig, | ||
| ) | ||
| } |
There was a problem hiding this comment.
The test TestCmdGetEquityTierLimitConfig is checking if the CmdGetEquityTierLimitConfig command returns the expected emptyEquityTierLimitConfig. However, it does not test the case where the configuration is not empty. It would be beneficial to add a test case where the configuration has some values to ensure that the command works correctly in all scenarios.
func TestCmdGetEquityTierLimitConfig(t *testing.T) {
net, _ := networkWithClobPairObjects(t, 2)
ctx := net.Validators[0].ClientCtx
common := []string{
fmt.Sprintf("--%s=json", tmcli.OutputFlag),
}
out, err := clitestutil.ExecTestCLICmd(ctx, cli.CmdGetEquityTierLimitConfig(), common)
require.NoError(t, err)
var resp types.QueryEquityTierLimitConfigurationResponse
require.NoError(t, net.Config.Codec.UnmarshalJSON(out.Bytes(), &resp))
require.NotNil(t, resp.EquityTierLimitConfig)
require.Equal(
t,
emptyEquityTierLimitConfig,
resp.EquityTierLimitConfig,
)
+ // Add a test case for non-empty configuration
+ nonEmptyConfig := types.EquityTierLimitConfiguration{
+ ShortTermOrderEquityTiers: []types.EquityTierLimit{{Tier: 1, Limit: 100}},
+ StatefulOrderEquityTiers: []types.EquityTierLimit{{Tier: 2, Limit: 200}},
+ }
+ // Set the configuration in the network
+ // ...
+ // Execute the command again
+ out, err = clitestutil.ExecTestCLICmd(ctx, cli.CmdGetEquityTierLimitConfig(), common)
+ require.NoError(t, err)
+ require.NoError(t, net.Config.Codec.UnmarshalJSON(out.Bytes(), &resp))
+ require.NotNil(t, resp.EquityTierLimitConfig)
+ require.Equal(
+ t,
+ nonEmptyConfig,
+ resp.EquityTierLimitConfig,
+ )
}Committable suggestion (Beta)
| func TestCmdGetEquityTierLimitConfig(t *testing.T) { | |
| net, _ := networkWithClobPairObjects(t, 2) | |
| ctx := net.Validators[0].ClientCtx | |
| common := []string{ | |
| fmt.Sprintf("--%s=json", tmcli.OutputFlag), | |
| } | |
| out, err := clitestutil.ExecTestCLICmd(ctx, cli.CmdGetEquityTierLimitConfig(), common) | |
| require.NoError(t, err) | |
| var resp types.QueryEquityTierLimitConfigurationResponse | |
| require.NoError(t, net.Config.Codec.UnmarshalJSON(out.Bytes(), &resp)) | |
| require.NotNil(t, resp.EquityTierLimitConfig) | |
| require.Equal( | |
| t, | |
| emptyEquityTierLimitConfig, | |
| resp.EquityTierLimitConfig, | |
| ) | |
| } | |
| func TestCmdGetEquityTierLimitConfig(t *testing.T) { | |
| net, _ := networkWithClobPairObjects(t, 2) | |
| ctx := net.Validators[0].ClientCtx | |
| common := []string{ | |
| fmt.Sprintf("--%s=json", tmcli.OutputFlag), | |
| } | |
| out, err := clitestutil.ExecTestCLICmd(ctx, cli.CmdGetEquityTierLimitConfig(), common) | |
| require.NoError(t, err) | |
| var resp types.QueryEquityTierLimitConfigurationResponse | |
| require.NoError(t, net.Config.Codec.UnmarshalJSON(out.Bytes(), &resp)) | |
| require.NotNil(t, resp.EquityTierLimitConfig) | |
| require.Equal( | |
| t, | |
| emptyEquityTierLimitConfig, | |
| resp.EquityTierLimitConfig, | |
| ) | |
| // Add a test case for non-empty configuration | |
| nonEmptyConfig := types.EquityTierLimitConfiguration{ | |
| ShortTermOrderEquityTiers: []types.EquityTierLimit{{Tier: 1, Limit: 100}}, | |
| StatefulOrderEquityTiers: []types.EquityTierLimit{{Tier: 2, Limit: 200}}, | |
| } | |
| // Set the configuration in the network | |
| // ... | |
| // Execute the command again | |
| out, err = clitestutil.ExecTestCLICmd(ctx, cli.CmdGetEquityTierLimitConfig(), common) | |
| require.NoError(t, err) | |
| require.NoError(t, net.Config.Codec.UnmarshalJSON(out.Bytes(), &resp)) | |
| require.NotNil(t, resp.EquityTierLimitConfig) | |
| require.Equal( | |
| t, | |
| nonEmptyConfig, | |
| resp.EquityTierLimitConfig, | |
| ) | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- protocol/x/clob/module_test.go (1 hunks)
Additional comments: 1
protocol/x/clob/module_test.go (1)
- 277-282: The number of commands and their names have been updated correctly to reflect the new functionality added to the
clobmodule. Ensure that these new commands (get-block-rate-limit-config,get-equity-tier-limit-config,get-liquidations-config) are implemented correctly and their functionality is tested elsewhere in the codebase.
proto/dydxprotocol/clob/query.proto
Outdated
| // Queries LiquidationsConfiguration. | ||
| rpc LiquidationsConfiguration(QueryLiquidationsConfigurationRequest) | ||
| returns (QueryLiquidationsConfigurationResponse) { | ||
| option (google.api.http).get = "/dydxprotocol/clob/liquidations"; |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- protocol/x/clob/types/query.pb.go
- protocol/x/clob/types/query.pb.gw.go
Files selected for processing (2)
- indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.lcd.ts (3 hunks)
- proto/dydxprotocol/clob/query.proto (3 hunks)
Additional comments: 6
proto/dydxprotocol/clob/query.proto (3)
7-10: The new imports for
block_rate_limit_config.protoandliquidations_config.protoare necessary for the newly added RPC methods. Ensure that these proto files exist and are correctly defined.47-57: Two new RPC methods
BlockRateLimitConfigurationandLiquidationsConfigurationhave been added. These methods are used to query the block rate limit configuration and liquidations configuration respectively. The HTTP GET paths for these methods are correctly defined.134-153: New request and response message types have been defined for the newly added RPC methods. The request messages are empty as no parameters are required for these queries. The response messages contain the respective configurations as non-nullable fields. This is acceptable as long as the configurations are guaranteed to exist in the system.
indexer/packages/v4-protos/src/codegen/dydxprotocol/clob/query.lcd.ts (3)
3-3: The import statement has been updated to include new types related to block rate limit configuration and liquidations configuration. This is in line with the changes made in the rest of the file.
16-17: The constructor of
LCDQueryClienthas been updated to bind the new methodsblockRateLimitConfigurationandliquidationsConfigurationto the instance. This is necessary to ensure that the methods have the correctthiscontext when called.50-63: Two new methods have been added to the
LCDQueryClientclass:blockRateLimitConfigurationandliquidationsConfiguration. These methods construct the respective API endpoints and make GET requests to them using thereqobject. They return promises that resolve to the respective response types. The methods are correctly implemented and follow the same pattern as the existing methods in the class.
(cherry picked from commit f1c7a9d)
Co-authored-by: Crystal Lemire <[email protected]>
Changelist
Added cli query support for liquidations config, block rate limit config, equity tier limit config.
Test Plan
standard gRPC and CLI tests. Manual test results below.
Author/Reviewer Checklist
state-breakinglabel.PrepareProposalorProcessProposal, manually add the labelproposal-breaking.feature:[feature-name].backport/[branch-name].refactor,chore,bug.CLI commands on locahost docker: